[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

Fridolin SOMERS  changed:

   What|Removed |Added

 CC||fridolin.som...@biblibre.co
   ||m

--- Comment #16 from Fridolin SOMERS  ---
Pushed to 17.05.x for v17.05.08

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

Nick Clemens  changed:

   What|Removed |Added

 Status|Pushed to Master|Pushed to Stable

--- Comment #15 from Nick Clemens  ---
Awesome work all, pushed to stable for 17.11.02

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

Jonathan Druart  changed:

   What|Removed |Added

 Status|Passed QA   |Pushed to Master

--- Comment #14 from Jonathan Druart  
---
Pushed to master for 18.05, thanks to everybody involved!

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

Josef Moravec  changed:

   What|Removed |Added

 CC||josef.mora...@gmail.com
 Status|Signed Off  |Passed QA
 QA Contact|testo...@bugs.koha-communit |josef.mora...@gmail.com
   |y.org   |

--- Comment #13 from Josef Moravec  ---
Looks good

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

Josef Moravec  changed:

   What|Removed |Added

  Attachment #69637|0   |1
is obsolete||
  Attachment #69638|0   |1
is obsolete||

--- Comment #11 from Josef Moravec  ---
Created attachment 70104
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70104=edit
Bug 18458: Add a subtest in Merge.t

This test illustrates the problem we have if you run it without the
second patch. And it serves to demonstrate that we resolved the
situation if you run it after the second patch.

Test plan:
[1] Without the second patch: The last subtest should fail.

Signed-off-by: Marcel de Rooy 

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

--- Comment #12 from Josef Moravec  ---
Created attachment 70105
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70105=edit
Bug 18458: Merging authority record incorrectly orders subfields

While doing a merge, if a subfield(s) precedes the controlled subfields
(like $i before $a in 7XX, which comes before $a -- rare but will
become more and more usual) after merging will be moved to the end.
This is not right.

The patch (with AuthorityMergeMode == loose) make merge consider the
subfields order: all subfields which originally were found before
the first controlled subfield (e.g. $i before $a in 7XX / MARC 21)
will remain in the front, the rest of not controlled subfields that
should remain in the field will come after the subfields copied
from authority rec.

As a bonus, $9 will be placed at the end.

To test:
0) Have AuthorityMergeMode == loose;
1) Have some field in bibliorecord, controlled by an authority, with
extra subfield(s) (i.e. not present in authority rec.) placed at the
beginning of the field;
2) Open (not necessarily edit) and save the connected authority;
3) See that the extra subfieds were moved to the end of the field
   (and $9 is in the front);
4) Apply the patch;
5) Reorder subfields in biblio field;
6) Open (not necessarily edit) and save the connected authority;
7) See that the order has been conserved, additionally $9 the last
   subfield in the field.

Signed-off-by: Marcel de Rooy 
Amended:
Moved field creation to its original place. Changed $9 handling. Simplified the
following add_subfields for loop. Edited comments.
Restored the append_fields_ordered call (see comment6).

With this patch, the Merge.t test now passes.

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

Marcel de Rooy  changed:

   What|Removed |Added

  Attachment #69345|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

--- Comment #9 from Marcel de Rooy  ---
Created attachment 69637
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=69637=edit
Bug 18458: (Follow-up) Add a subtest in Merge.t

This test illustrates the problem we have if you run it without the
second patch. And it serves to demonstrate that we resolved the
situation if you run it after the second patch.

Test plan:
[1] Without the second patch: The last subtest should fail.

Signed-off-by: Marcel de Rooy 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

--- Comment #10 from Marcel de Rooy  ---
Created attachment 69638
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=69638=edit
Bug 18458 - Merging authority record incorrectly orders subfields

While doing a merge, if a subfield(s) precedes the controlled subfields
(like $i before $a in 7XX, which comes before $a -- rare but will
become more and more usual) after merging will be moved to the end.
This is not right.

The patch (with AuthorityMergeMode == loose) make merge consider the
subfields order: all subfields which originally were found before
the first controlled subfield (e.g. $i before $a in 7XX / MARC 21)
will remain in the front, the rest of not controlled subfields that
should remain in the field will come after the subfields copied
from authority rec.

As a bonus, $9 will be placed at the end.

To test:
0) Have AuthorityMergeMode == loose;
1) Have some field in bibliorecord, controlled by an authority, with
extra subfield(s) (i.e. not present in authority rec.) placed at the
beginning of the field;
2) Open (not necessarily edit) and save the connected authority;
3) See that the extra subfieds were moved to the end of the field
   (and $9 is in the front);
4) Apply the patch;
5) Reorder subfields in biblio field;
6) Open (not necessarily edit) and save the connected authority;
7) See that the order has been conserved, additionally $9 the last
   subfield in the field.

Signed-off-by: Marcel de Rooy 
Amended:
Moved field creation to its original place. Changed $9 handling. Simplified the
following add_subfields for loop. Edited comments.
Restored the append_fields_ordered call (see comment6).

With this patch, the Merge.t test now passes.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

--- Comment #8 from Marcel de Rooy  ---
Current approach was:
$9 Controlled fields Old extra fields
The controlled fields form a block; $9 should be moved to the end.
Your approach is:
Extra-prefix Controlled block Extra-postfix $9
With our current Koha records $9 will be the first one as long as it has not
been saved with this patch. So we need to avoid $9 in this loop (amended).
Note that since $i from your 7XX example has been moved backwards, it will not
automatically move to the front anymore. But new 7XXs with $i $a will be okay.
Adding a subtest in Merge.t

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

Marcel de Rooy  changed:

   What|Removed |Added

 Status|Failed QA   |Signed Off

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

Marcel de Rooy  changed:

   What|Removed |Added

   Patch complexity|--- |Small patch
   Assignee|gmcha...@gmail.com  |janus...@gmail.com

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

--- Comment #7 from Marcel de Rooy  ---
Working here again now

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

--- Comment #6 from Janusz Kaczmarek  ---
(In reply to Marcel de Rooy from comment #5)
> Janusz,
> Thx again for this patch.
> I am not so sure though if your code really makes sub merge easier (it is
> hard enough already).
> Couldnt we do the following in order to keep the order intact:
> 
> Copy all fields in original order including $9.
> Overwrite the authority controlled fields. If $overwrite, delete a subfield
> that is not controlled (only skip $9). [A hypothetical question: what about
> repeated subfields?]
> Update $9 finally.
> 
> Another thing:
> -append_fields_ordered( $marcrecord, $field_to );
> +$marcrecord->insert_fields_ordered($field_to);
> This is a regression. Look at the comments in append_fields_ordered. You
> change the order of fields here.
> 
> Finally, we need some tests to support your changes. Have a look at
> t/db_dependent/Authority/Merge.t :)

Hi Marcel,

Thank you for your comments.  From the bottom up:

You are right--but I have never done it (the t/ story) :(

append_fields_ordered -- you are right, I mixed it with append_fields -- and
this is what I wouldn't like here, hence the correction that turns out to be
regression. 

Hmmm, and what does it mean indeed "controlled" at all?  Because even with the
current approach we do not "clear" obviously controlled subfields that had been
introduced by mistake by a cataloger.  (E.g. if there is $a-$d in personal name
authority record, but a cataloger by mistake put additional $c by hand, $c will
not disappear after a "refreshing" merge; it disappears only if it was $a-$c
first in authority and then becomes $a-$d).  But anyway, it is definitely
better now than it was before your corrections.  But--wouldn't we need
something like: C4/Heading/MARC21.pm (which is used for linking) to clearly
state what is controlled?--I now, it is hardcoded, but MARC itself is hardcoded
too, i.e. title is in 245 $a (resp. 200 $a) etc.

Again, I feel like my proposal is more error-proof.  I mean: controlled
subfields form always a coherent block (do they?).  So, if a cataloger puts, by
mistake, an uncontrolled subfield between controlled subfields (which is not
very rare in standard Koha editor)--it will go to the end after my merge (which
was most probably his/hers intention).  Only if he/she puts an uncontrolled
introductory subfield at the beginning, it will remain there.

And, if $overwrite, it is no sense at all to copy the original field--it seems
to me it even more complicated than my approach, that I like ;)  And even
worse: with your proposal there is no simple way to cope with potentially
repeated subdields (e.g. $x) that would be subject to changed. 

And as to $9--yes, we like to have it at the very end (since it it totally non
standard and since it blemish the Labeled MARC view! :) 

So, to sum up: I would willingly return to append_fields_ordered, but if I
could ask for suggestions on how to prepare t/, would be great. 

I would appreciate your comments.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

Marcel de Rooy  changed:

   What|Removed |Added

 Status|Needs Signoff   |Failed QA

--- Comment #5 from Marcel de Rooy  ---
Janusz,
Thx again for this patch.
I am not so sure though if your code really makes sub merge easier (it is hard
enough already).
Couldnt we do the following in order to keep the order intact:

Copy all fields in original order including $9.
Overwrite the authority controlled fields. If $overwrite, delete a subfield
that is not controlled (only skip $9). [A hypothetical question: what about
repeated subfields?]
Update $9 finally.

Another thing:
-append_fields_ordered( $marcrecord, $field_to );
+$marcrecord->insert_fields_ordered($field_to);
This is a regression. Look at the comments in append_fields_ordered. You change
the order of fields here.

Finally, we need some tests to support your changes. Have a look at
t/db_dependent/Authority/Merge.t :)

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

Janusz Kaczmarek  changed:

   What|Removed |Added

 Status|NEW |Needs Signoff

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

--- Comment #4 from Janusz Kaczmarek  ---
Created attachment 69345
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=69345=edit
Bug 18458 - Merging authority record incorrectly orders  subfields

While doing a merge, if a subfield(s) precedes the controlled subfields
(like $i before $a in 7XX, which comes before $a -- rare but will
become more and more usual) after merging will be moved to the end.
This is not right.

The patch (with AuthorityMergeMode == loose) make merge consider the
subfields order: all subfields which originally were found before
the first controlled subfield (e.g. $i before $a in 7XX / MARC 21)
will remain in the front, the rest of not controlled subfields that
should remain in the field will come after the subfields copied
from authority rec.

As a bonus, $9 will be placed at the end.

To test:
0) Have AuthorityMergeMode == loose;
1) Have some field in bibliorecord, controlled by an authority, with
extra subfield(s) (i.e. not present in authority rec.) placed at the
beginning of the field;
2) Open (not necessarily edit) and save the connected authority;
3) See that the extra subfieds were moved to the end of the field
   (and $9 is in the front);
4) Apply the patch;
5) Reorder subfields in biblio field;
6) Open (not necessarily edit) and save the connected authority;
7) See that the order has been conserved, additionally $9 the last
   subfield in the field.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

Janusz Kaczmarek  changed:

   What|Removed |Added

 CC||janus...@gmail.com

--- Comment #3 from Janusz Kaczmarek  ---
I would add to this: if a subfield precedes the controlled subfields (like $i
in 7XX, which comes before $a -- rare but will become more and more usual)
after merging will be moved to the end.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

Patrick Robitaille  changed:

   What|Removed |Added

 CC||patrick.robitaille@collecto
   ||.ca

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

Marjorie Barry-Vila  changed:

   What|Removed |Added

 CC||marjorie.barry-v...@ccsr.qc
   ||.ca

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

--- Comment #2 from Marcel de Rooy  ---
(In reply to Nick Clemens from comment #0)
> When merging two authorities and updating the linked biblios
> MARC::Field->new() is called  with the tagnumber, indicators, and the new
> subfield $9
> 
> This causes this subfield to be the first in the field as a whole and is
> inconsistent with usualy ordering

The old merge routine did actually do the same.
Also note that we normally have the $9 before the a..z subfields in the editor.

This does not mean that we should do it differently of course.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

--- Comment #1 from Marcel de Rooy  ---
Will have a look.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 18458] Merging authority record incorrectly orders subfields

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18458

Nick Clemens  changed:

   What|Removed |Added

 CC||m.de.r...@rijksmuseum.nl

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/