When merging 2 records (/cgi-bin/koha/cataloguing/merge.pl), the destination record is build using the fields and subfields checked in source records. When a field is checked, the javascript code searches in destination record a field with a greater tag number to insert new field before. When the new field tag number is greater than all existing field tag numbers, the field is not added.
Created attachment 19963 [details] [review] Bug 10648 - In records merge greatest field can not be added See commit message
(In reply to Fridolyn SOMERS from comment #1) > Created attachment 19963 [details] [review] [review] > Bug 10648 - In records merge greatest field can not be added This patch is based on master + Bug 10650 because this bug moves impacted code.
Created attachment 19965 [details] [review] [3.12.x] Bug 10648 - In records merge greatest field can not be added Same patch for 3.12.x and lower.
Applying: Bug 10648 - In records merge greatest field can not be added Using index info to reconstruct a base tree... M cataloguing/merge.pl M koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/merge.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/merge.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/merge.tt Auto-merging cataloguing/merge.pl CONFLICT (content): Merge conflict in cataloguing/merge.pl Patch failed at 0001 Bug 10648 - In records merge greatest field can not be added The copy of the patch that failed is found in: /home/christopher/git/koha/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".
Created attachment 21176 [details] [review] Bug 10648 - In records merge greatest field can not be added Rebased master
Created attachment 21177 [details] [review] [3.12.x] Bug 10648 - In records merge greatest field can not be added Rebased 3.12.x
I wonder if "I'm just a bot" has also tested patch for 3.12.x on master ?
Main patch applies on master ;-) I'm testing. Mathieu
Created attachment 21205 [details] [review] [PATCH][SIGNED OFF] Bug 10648 - In records merge greatest field can not be added It works, I sign off patch for master. I can't sign off the patch made for 3.12.x Mathieu
Both patches apply to their versions, but running out of time to continue here now.
Created attachment 21838 [details] [review] [PASSED QA][3.12.x] Bug 10648 - In records merge greatest field can not be added When merging 2 records (/cgi-bin/koha/cataloguing/merge.pl), the destination record is build using the fields and subfields checked in source records. When a field is checked, the javascript code searches in destination record a field with a greater tag number to insert new field before. When the new field tag number is greater than all existing field tag numbers, the field is not added. This patch corrects this by adding at end if no greater field tag number exists. Also adds a sort of fields by tag number because all merge code is based on this. Test plan : - Add to a framework a repeatable field with the greates non existing tag number. For example 998. - Edit to records with this framework and add them a value in this tag. - Put those records is a list - Go to this list and check the two records - Click on "Merge selected" - Click on next - Go to second source record - Click on the greatest tag number. for example 998. => The field is added at the end of destination record Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests and QA script pass. Fixed some tabs. Could reproduce the behaviour by defining a 999$z subfield. After the patch it works as expected.
Patch for master still needs QA, will do that later.
Created attachment 22697 [details] [review] Bug 10648 - In records merge greatest field can not be added When merging 2 records (/cgi-bin/koha/cataloguing/merge.pl), the destination record is build using the fields and subfields checked in source records. When a field is checked, the javascript code searches in destination record a field with a greater tag number to insert new field before. When the new field tag number is greater than all existing field tag numbers, the field is not added. This patch corrects this by adding at end if no greater field tag number exists. Also adds a sort of fields by tag number because all mergo code is based on this. Test plan : - Add to a framework a repeatable field with the greater non existing tag number. For example 998. - Edit 2 records with this framework and add them a value in this tag. - Put those records is a list - Go to this list and check the two records - Click on "Merge selected" - Click on next - Go to second source record - Click on the greatest tag number. for example 998. => The field is added at the end of destination record Signed-off-by:Mathieu Saby <mathieu.saby@univ-rennes2.fr> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
QA Comment: Good catch. No string changes. Looks good to me. No complaints from qa tools. Passed QA
Comment on attachment 22697 [details] [review] Bug 10648 - In records merge greatest field can not be added Review of attachment 22697 [details] [review]: ----------------------------------------------------------------- ::: Koha/Util/MARC.pm @@ +40,4 @@ > my @array; > my @fields = $record->fields(); > > + foreach my $field ( sort { $a->tag() <=> $b->tag() } @fields) { This is actually a problem: in MARC21, at least, fields are not always meant to be sorted in numeric tag order. For example, for the 6XX fields for subject headings, you could have a 651 (geographic term) followed by a 650 (topical term). That would signify that the book was more about the geographic area expressed in the 651 than about the topic expressed in the 650. As another example, 5XX are not always meant to be sorted in numeric order. For the purpose of the merge interface, it's sufficient that new tags be inserted after the first field with a numerically lesser tag, or at the end of the record as a fallback -- but it shouldn't reorder existing fields in the target record.
Because of the sorting issue mentioned in my previous comment, my inclination is to push the JavaScript part of the patch, but not the change to Koha/Util/MARC.pm. That way, the bug gets fixed, but there's no unexpected reordering of fields in the destination record. Fridolin: comments on this approach?
(Note : idem in unimarc. see 210 http://multimedia.bnf.fr/unimarcb_trad/B210-6-2012.pdf : the first occurence of the 210 field have a special meaning) Mathieu
(in fact it is not exactly the same case, but resorting all fields, even in Unimarc, may not be a good idea)
(In reply to Galen Charlton from comment #16) > Because of the sorting issue mentioned in my previous comment, my > inclination is to push the JavaScript part of the patch, but not the change > to Koha/Util/MARC.pm. That way, the bug gets fixed, but there's no > unexpected reordering of fields in the destination record. > > Fridolin: comments on this approach? I agree. It would be perfect. Do you wan me to create a new patch ?
(In reply to Fridolin SOMERS from comment #19) > (In reply to Galen Charlton from comment #16) > > Because of the sorting issue mentioned in my previous comment, my > > inclination is to push the JavaScript part of the patch, but not the change > > to Koha/Util/MARC.pm. That way, the bug gets fixed, but there's no > > unexpected reordering of fields in the destination record. > > > > Fridolin: comments on this approach? > I agree. It would be perfect. > Do you wan me to create a new patch ? Yes, please.
Created attachment 23877 [details] [review] Bug 10648 - In records merge greatest field can not be added When merging 2 records (/cgi-bin/koha/cataloguing/merge.pl), the destination record is build using the fields and subfields checked in source records. When a field is checked, the javascript code searches in destination record a field with a greater tag number to insert new field before. When the new field tag number is greater than all existing field tag numbers, the field is not added. This patch corrects this by adding at end if no greater field tag number exists. Also adds a sort of fields by tag number because all mergo code is based on this. Test plan : - Add to a framework a repeatable field with the greater non existing tag number. For example 998. - Edit 2 records with this framework and add them a value in this tag. - Put those records is a list - Go to this list and check the two records - Click on "Merge selected" - Click on next - Go to second source record - Click on the greatest tag number. for example 998. => The field is added at the end of destination record
Here is new patch. Please re-signoff.
Patch tested with a sandbox, by Nick Clemens <nick@quecheelibrary.org>
Interesting: Status is Signed off, but all patches are obsolete. Changing status to Failed QA. Author or signer: Please correct.
Created attachment 29581 [details] [review] Bug 10648 - In records merge greatest field can not be added When merging 2 records (/cgi-bin/koha/cataloguing/merge.pl), the destination record is build using the fields and subfields checked in source records. When a field is checked, the javascript code searches in destination record a field with a greater tag number to insert new field before. When the new field tag number is greater than all existing field tag numbers, the field is not added. This patch corrects this by adding at end if no greater field tag number exists. Also adds a sort of fields by tag number because all mergo code is based on this. Test plan : - Add to a framework a repeatable field with the greater non existing tag number. For example 998. - Edit 2 records with this framework and add them a value in this tag. - Put those records is a list - Go to this list and check the two records - Click on "Merge selected" - Click on next - Go to second source record - Click on the greatest tag number. for example 998. => The field is added at the end of destination record Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
(In reply to M. de Rooy from comment #24) > Interesting: Status is Signed off, but all patches are obsolete. > Changing status to Failed QA. > Author or signer: Please correct. The git-bz on sandboxes were not up-to-date, they were not able to sent patches. I added the signoff line on the last patch.
Created attachment 29843 [details] [review] [PASSED QA] Bug 10648 - In records merge greatest field can not be added When merging 2 records (/cgi-bin/koha/cataloguing/merge.pl), the destination record is build using the fields and subfields checked in source records. When a field is checked, the javascript code searches in destination record a field with a greater tag number to insert new field before. When the new field tag number is greater than all existing field tag numbers, the field is not added. This patch corrects this by adding at end if no greater field tag number exists. Also adds a sort of fields by tag number because all mergo code is based on this. Test plan : - Add to a framework a repeatable field with the greater non existing tag number. For example 998. - Edit 2 records with this framework and add them a value in this tag. - Put those records is a list - Go to this list and check the two records - Click on "Merge selected" - Click on next - Go to second source record - Click on the greatest tag number. for example 998. => The field is added at the end of destination record Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Patch pushed to master. Thanks Fridolin!
Pushed to 3.16.x for inclusion in 3.16.3.