[Bug 6657] New: Making 999 visible in framework duplicates 999 entry and affects index
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Bug #: 6657 Summary: Making 999 visible in framework duplicates 999 entry and affects index Classification: Unclassified Change sponsored?: --- Product: Koha Version: rel_3_6 Platform: All OS/Version: All Status: NEW Severity: major Priority: P5 Component: Cataloging AssignedTo: gmcharlt@gmail.com ReportedBy: jwagner@ptfs.com QAContact: koha-bugs@lists.koha-community.org CC: ian.walls@bywatersolutions.com The 999 field is usually hidden in the frameworks; the 999$c is where Koha stores the biblionumber for the title. If someone edits the framework to make any field of the 999 visible, then any edit to any field of the MARC record results in a duplicate 999 with the biblionumber repeated. This can cause the record to appear twice in the zebra index, and can also make OPAC suppression not work. I have verified this behavior in current development head and several other versions. To replicate, export a title record and check its 999 field. There should only be one. For example: =999 \\$c219$d219 or <datafield tag="999" ind1=" " ind2=" "> <subfield code="c">219</subfield> <subfield code="d">219</subfield> </datafield> Then edit the framework to make any field of the 999 visible, and edit that same record. Make any change to the title, such as adding a 5xx note. Export the record again and check the 999. There will now be two of them, repeating the biblionumber: =999 \\$c219$d219 =999 \\$c219 or <datafield tag="999" ind1=" " ind2=" "> <subfield code="c">219</subfield> <subfield code="d">219</subfield> </datafield> <datafield tag="999" ind1=" " ind2=" "> <subfield code="c">219</subfield> </datafield> Changing the framework field back to hidden and forcing the MARC record back through zebraqueue does not fix the problem. The only fix we've found is to manually remove the extra 999 at the table level, then rebuild indexes. The first solution for the problem is to not set the 999 field as visible in any framework, but since you can't stop people from being adventurous, we also developed a code safeguard. It has not yet been backported from our current development head to the version in our public git repository; when it is, I'll add a link. In the meantime, here are the details of that commit. If the framework allows the biblionumber (999$c) field to be displayed and edited within the MARC editor, then this results in an additional 999$c value being added to the MARC record. This is because the code already pulled the biblionumber from the biblio table. This patch removes the addition of the extra 999$c field, although this field shouldn't really be editable in the first place. diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 2e28f3f..3f61519 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -1858,13 +1858,13 @@ sub TransformHtmlToMarc { my $record = MARC::Record->new(); my $i=0; my @fields; + my ( $biblionumbertagfield, $biblionumbertagsubfield ) = + &GetMarcFromKohaField( "biblio.biblionumber", '' ); while ($params->[$i]){ # browse all CGI params my $param = $params->[$i]; my $newfield=0; # if we are on biblionumber, store it in the MARC::Record (it may not be in the edited fields) if ($param eq 'biblionumber') { - my ( $biblionumbertagfield, $biblionumbertagsubfield ) = - &GetMarcFromKohaField( "biblio.biblionumber", '' ); if ($biblionumbertagfield < 10) { $newfield = MARC::Field->new( $biblionumbertagfield, @@ -1903,6 +1903,12 @@ sub TransformHtmlToMarc { } else { while(defined $params->[$j] && $params->[$j] =~ /_code_/){ # browse all it's subfield my $inner_param = $params->[$j]; + my $biblionumbertagsubfield_string = "_code_" . $biblionumbertagsubfield . "_"; + if (($tag eq $biblionumbertagfield) && + ($inner_param =~ /$biblionumbertagsubfield_string/)) { + $j += 2; + next; + } if ($newfield){ -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Alen Vodopijevec <alen@irb.hr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alen@irb.hr --- Comment #1 from Alen Vodopijevec <alen@irb.hr> --- I can confirm persistence of this bug in 3.10.02 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Elaine Bradtke <eb@efdss.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |eb@efdss.org --- Comment #2 from Elaine Bradtke <eb@efdss.org> --- in 3.10.02 the problem of multiple 999c fields and OPAC record suppression persists. However, the 999 subfields in our framework are all hidden. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|3.6 |master -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |liz@catalyst.net.nz --- Comment #3 from Liz Rea <liz@catalyst.net.nz> --- This still happens in 3.12 as well. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Tom Misilo <misilot@fit.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |misilot@fit.edu -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Barton Chittenden <barton@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |barton@bywatersolutions.com --- Comment #4 from Barton Chittenden <barton@bywatersolutions.com> --- This is still occurring in 3.18.06 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 42667 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42667&action=edit Bug 6657: Prevent biblionumber to be duplicated If the biblionumber field is displayed in the framework, on editing a biblio the field/subfield will be duplicated. To prevent that this patch adds a check when building the field list. Test plan: 1/ map biblio.biblionumber with 999$c 2/ Display 999$c in a framework 3/ Edit a biblio using this framework 4/ Save => The field should not have been duplicated 5/ map biblio.biblionumber with 001 6/ Display 001 in a framework 7/ Edit a biblio using this framework 8/ Save => The field should not have been duplicated -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Assignee|gmcharlt@gmail.com |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- No change to get tests here, this code is too messy. -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #42667|0 |1 is obsolete| | --- Comment #7 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 44199 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44199&action=edit [SIGNED-OFF] Bug 6657: Prevent biblionumber to be duplicated If the biblionumber field is displayed in the framework, on editing a biblio the field/subfield will be duplicated. To prevent that this patch adds a check when building the field list. Test plan: 1/ map biblio.biblionumber with 999$c 2/ Display 999$c in a framework 3/ Edit a biblio using this framework 4/ Save => The field should not have been duplicated 5/ map biblio.biblionumber with 001 6/ Display 001 in a framework 7/ Edit a biblio using this framework 8/ Save => The field should not have been duplicated Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Works as described, aldo removes duplicate values. No errors -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Would it be possible to unit test this? -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Katrin Fischer from comment #8)
Would it be possible to unit test this?
Yes, it would, but it's kind of mental torture to provide a test, the subroutine is not tested at all. -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- The patch works as described, but I notice that the biblioitemnumber still duplicates. I think it would be good to fix that as well. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44199|0 |1 is obsolete| | --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 44869 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44869&action=edit [PASSED QA] Bug 6657: Prevent biblionumber to be duplicated If the biblionumber field is displayed in the framework, on editing a biblio the field/subfield will be duplicated. To prevent that this patch adds a check when building the field list. Test plan: 1/ map biblio.biblionumber with 999$c 2/ Display 999$c in a framework 3/ Edit a biblio using this framework 4/ Save => The field should not have been duplicated 5/ map biblio.biblionumber with 001 6/ Display 001 in a framework 7/ Edit a biblio using this framework 8/ Save => The field should not have been duplicated Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Works as described, aldo removes duplicate values. No errors Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Status|Passed QA |Pushed to Master --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr --- Comment #13 from Frédéric Demians <frederic@tamil.fr> --- This patch has been pushed to 3.20.x, will be in 3.20.6. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #14 from Liz Rea <liz@catalyst.net.nz> --- Pushed to 3.18.13, and released. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #15 from Frédéric Demians <frederic@tamil.fr> --- This patch has a side effect on authorities creation (tested in 3.20.x). The authorities.pl run foreever, and adding a new authority is impossible. -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Stable |ASSIGNED -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #16 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 46517 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46517&action=edit Bug 6657: Follow-up to fix error on authority creation Bug 6657 modified the way C4::Biblio::TransformHtmlToMarc operates in order to solve an issue occuring during biblio record cataloguing. But this function is also used by authorities cataloguing, and the code in this case is irrelevante. This followup allows to distinguish for which kind of record TransformHtmlToMarc is called: biblio/authority. A bug appears in authority creation without this patch in some circunstances: when authid is linked to 001 field. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44869|0 |1 is obsolete| | -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Comment on attachment 46517 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46517 Bug 6657: Follow-up to fix error on authority creation Review of attachment 46517: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=6657&attachment=46517) ----------------------------------------------------------------- ::: C4/Biblio.pm @@ +2541,5 @@
if ( $tag < 10 ) { # no code for theses fields # in MARC editor, 000 contains the leader. + if ( $isbiblio && $tag == $biblionumbertagfield ) { + $i += 4;
Could you explain this line please? -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=15358 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #18 from Frédéric Demians <frederic@tamil.fr> --- (In reply to Jonathan Druart from comment #17)
Comment on attachment 46517 [details] [review] Bug 6657: Follow-up to fix error on authority creation
Review of attachment 46517 [details] [review]: -----------------------------------------------------------------
::: C4/Biblio.pm @@ +2541,5 @@
if ( $tag < 10 ) { # no code for theses fields # in MARC editor, 000 contains the leader. + if ( $isbiblio && $tag == $biblionumbertagfield ) { + $i += 4;
Could you explain this line please?
In @params array, for a control field (tag < 10), there is 4 entries. For example: tag_001_indicator1_816115 tag_001_indicator2_816115 tag_001_code_00_816115_723352 tag_001_subfield_00_816115_723352 So $i += 4 is done before next in order to move to the next field in the @params array. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Looking at koha-tmpl/intranet-tmpl/prog/en/includes/merge-record.inc, it seems that this sequence could be different. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #20 from Frédéric Demians <frederic@tamil.fr> --- OK. In any case, $i has to be increment before next. Otherwise the loop on the @params array never ends: that's what I get now on 3.20/3.22/master branches. I will just increment by 1, relying on this line to positionne $i on the array good element: } elsif ( $param =~ /^tag_(\d*)_indicator1_/ ) -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #46517|0 |1 is obsolete| | --- Comment #21 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 46536 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46536&action=edit Bug 6657: Follow-up to fix error on authority creation Bug 6657 modified the way C4::Biblio::TransformHtmlToMarc operates in order to solve an issue occuring during biblio record cataloguing. But this function is also used by authorities cataloguing, and the code in this case is irrelevante. This followup allows to distinguish for which kind of record TransformHtmlToMarc is called: biblio/authority. A bug appears in authority creation without this patch in some circunstances: when authid is linked to 001 field. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #46536|0 |1 is obsolete| | --- Comment #22 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 46537 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46537&action=edit Bug 6657: Follow-up to fix error on authority creation Bug 6657 modified the way C4::Biblio::TransformHtmlToMarc operates in order to solve an issue occuring during biblio record cataloguing. But this function is also used by authorities cataloguing, and the code in this case is irrelevante. This followup allows to distinguish for which kind of record TransformHtmlToMarc is called: biblio/authority. A bug appears in authority creation without this patch in some circunstances: when authid is linked to 001 field. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #23 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- What about setting biblionumbertagsubfield and biblionumbertagfield to an empty string at the beginning of the subroutine if we are processing an authority record? It would avoid to add lines to this code... -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #24 from Frédéric Demians <frederic@tamil.fr> --- No problem if you prefer. But do you agree with the principle of changing TransformMarcHtmlToMarc() signature, ie adding a second parameter to distinguish biblio/authority record? -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #25 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 46538 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46538&action=edit Bug 6657: Add tests for TransformHtmlToMarc -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #26 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Frédéric Demians from comment #24)
No problem if you prefer. But do you agree with the principle of changing TransformMarcHtmlToMarc() signature, ie adding a second parameter to distinguish biblio/authority record?
No, I hate the idea :) But I don't see any other quick and easy solution. -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #27 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #25)
Created attachment 46538 [details] [review] Bug 6657: Add tests for TransformHtmlToMarc
Regarding these tests, your patch is wrong. But the tests may be wrong too :) -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #46537|0 |1 is obsolete| | --- Comment #28 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 46542 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46542&action=edit Bug 6657: Follow-up to fix error on authority creation Bug 6657 modified the way C4::Biblio::TransformHtmlToMarc operates in order to solve an issue occuring during biblio record cataloguing. But this function is also used by authorities cataloguing, and the code in this case is irrelevante. This followup allows to distinguish for which kind of record TransformHtmlToMarc is called: biblio/authority. A bug appears in authority creation without this patch in some circunstances: when authid is linked to 001 field. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #46538|0 |1 is obsolete| | --- Comment #29 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 46543 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46543&action=edit Bug 6657: Add tests for TransformHtmlToMarc Signed-off-by: Frédéric Demians <f.demians@tamil.fr> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #30 from Julian Maurice <julian.maurice@biblibre.com> --- What about replacing the 'while' loop by a good old 'for' loop ? This way, 'next' would increment the counter correctly. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #31 from Frédéric Demians <frederic@tamil.fr> ---
Regarding these tests, your patch is wrong. But the tests may be wrong too :)
+1 for UT! I've signed-off your patch, which works with amending the call to TransformHtmlToMarc (2nd parameter was missing). -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #32 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 46544 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46544&action=edit Bug 6657: counter-patch Replace while by for -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #46544|0 |1 is obsolete| | --- Comment #33 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 46545 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46545&action=edit Bug 6657: counter-patch Replace while by for Signed-off-by: Frédéric Demians <f.demians@tamil.fr> -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #46545|0 |1 is obsolete| | --- Comment #34 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 46547 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46547&action=edit Bug 6657: TransformHtmlToMarc - rewrite the loop This rewrites the while loop into a for loop, so $i still gets incremented when we call next -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com --- Comment #35 from Julian Maurice <julian.maurice@biblibre.com> --- It seems that the "while -> for" rewrite is not enough alone, so I rewrite my patch to fit well with the others -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #46547|0 |1 is obsolete| | --- Comment #36 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 46568 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46568&action=edit Bug 6657: TransformHtmlToMarc - rewrite the loop This rewrites the while loop into a for loop, so $i still gets incremented when we call next -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #46568|0 |1 is obsolete| | --- Comment #37 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 46569 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46569&action=edit Bug 6657: TransformHtmlToMarc - rewrite the loop This rewrites the while loop into a for loop, so $i still gets incremented when we call next Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Make sense. Add readability. Infinite loop no more possible. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff --- Comment #38 from Frédéric Demians <frederic@tamil.fr> --- I finally reset this patch status to 'Needs Signoff' since the 1st patch of the set isn't yet signed. Maybe the importance of this patch should be upgraded to Critical. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |critical --- Comment #39 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
This patch has a side effect on authorities creation (tested in 3.20.x). The >authorities.pl run foreever, and adding a new authority is impossible.
Hi Frederic, I agree with critical - I have changed the severity. For next time - I think it's better to handle these things on a separate bug report linked to the one which caused the problem. Then the title can reflect the urgency better and it's easer to navigate the discussion. As we use the bug titles for the release notes it also helps to better communicate a fixed problem. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #40 from Frédéric Demians <frederic@tamil.fr> --- (In reply to Katrin Fischer from comment #39)
This patch has a side effect on authorities creation (tested in 3.20.x). The >authorities.pl run foreever, and adding a new authority is impossible.
Hi Frederic, I agree with critical - I have changed the severity.
For next time - I think it's better to handle these things on a separate bug report linked to the one which caused the problem. Then the title can reflect the urgency better and it's easer to navigate the discussion. As we use the bug titles for the release notes it also helps to better communicate a fixed problem.
Katrin, I agree that a new bug report would have been better. It's still not too late. I can create a new bug and attach the 3 patches if you may want. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #41 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Frederic, I am ok with both - leaving it here or separating it out. -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Heather Braum <hbraum@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hbraum@nekls.org -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |15572 Referenced Bugs: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15572 [Bug 15572] Authority creation fails when authid is linked to 001 field -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #46542|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #46543|0 |1 is obsolete| | -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #46569|0 |1 is obsolete| | -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6657 --- Comment #42 from Frédéric Demians <frederic@tamil.fr> --- (In reply to Katrin Fischer from comment #41)
Hi Frederic, I am ok with both - leaving it here or separating it out.
I've closed this patch, since it's already pushed in master, and attached the set of patch to new bug 15572. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org