1) Import or create a new record. 2) Examine the marcxml of that record in the database - the fields are in the correct numerical order. 3) Edit the record - no need to change any data, just save it. 4) Examine the marcxml in the database - the 999 field is now right after leader and before 001. This is due to TransformHtmlToMarc in C4/Biblio.pm, specifically the $param eq 'biblionumber' case, which pushes the param in the array first, and then the sub uses $record->append_field() Perhaps the array should be sorted by field tag before appending the fields? AFAIK, this doesn't really show up anywhere in Koha, but if you ever edit the records via a script using MARC::Record, new fields cannot be easily inserted into correct position.
I've noticed this as well recently. I was wondering if it was intentional or not...
Is this really a regression? If it is, could you tell me a version which was behaving as expected?
*** This bug has been marked as a duplicate of bug 21881 ***
(In reply to Jonathan Druart from comment #3) > > *** This bug has been marked as a duplicate of bug 21881 *** I think maybe 21881 should've been marked as a duplicate of this bug? Should we move the discussion over to 21881?
(In reply to Jonathan Druart from comment #2) > Is this really a regression? If it is, could you tell me a version which was > behaving as expected? I'd say it's a regression. As for a version which was behaving as expected... I'd have to double-check, but I think it was OK in Koha 3.20 but not Koha 17.05.
*** Bug 21881 has been marked as a duplicate of this bug. ***
(In reply to David Cook from comment #4) > (In reply to Jonathan Druart from comment #3) > > > > *** This bug has been marked as a duplicate of bug 21881 *** > > I think maybe 21881 should've been marked as a duplicate of this bug? Should > we move the discussion over to 21881? Sorry, I meant to do the reverse.
Created attachment 83365 [details] 999 should not be in the first field
(In reply to David Cook from comment #5) > (In reply to Jonathan Druart from comment #2) > > Is this really a regression? If it is, could you tell me a version which was > > behaving as expected? > > I'd say it's a regression. As for a version which was behaving as > expected... I'd have to double-check, but I think it was OK in Koha 3.20 but > not Koha 17.05. I have written a quick test to confirm that: it seems that 3.20 behaved as it already.
Created attachment 100469 [details] [review] Bug 21708: Sort MARC fields in TransformHtmlToMarc Instead of pushing 999 in the front :)
Still adding small test and benchmarking
Record with 25 fields: 0.000038 sec
(In reply to paxed from comment #0) > This is due to TransformHtmlToMarc in C4/Biblio.pm, specifically the $param > eq 'biblionumber' case, which pushes the param in the array first, and then > the sub uses $record->append_field() > > Perhaps the array should be sorted by field tag before appending the fields? Yes, I saw that too :)
Created attachment 100556 [details] [review] Bug 24544: Add script to insert persistent identifiers
Comment on attachment 100556 [details] [review] Bug 24544: Add script to insert persistent identifiers Wrong report
Created attachment 100557 [details] [review] Bug 21708: Add a few tests in TransformHtmlToMarc.t Test plan: Run t/db_dependent/Biblio/TransformHtmlToMarc.t without the module change patch. Verify that the test fails on the order of fields. Go to test plan of the second patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 100558 [details] [review] Bug 21708: Sort MARC fields in TransformHtmlToMarc Instead of pushing 999 in the front :) Test plan: Do not yet apply this patch. Edit a record. Save. And open MARC view/view plain on opac-detail. Verify that the biblionumber field (MARC21 999) follows leader. Apply this patch. Restart Plack. Edit a record. Save. And open MARC view/view plain on opac-detail. Verify that the biblionumber field is in right position. Run t/db_dependent/Biblio/TransformHtmlToMarc.t. Should pass now. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 101334 [details] [review] Bug 21708: Add a few tests in TransformHtmlToMarc.t Test plan: Run t/db_dependent/Biblio/TransformHtmlToMarc.t without the module change patch. Verify that the test fails on the order of fields. Go to test plan of the second patch. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Test fails
Created attachment 101335 [details] [review] Bug 21708: Sort MARC fields in TransformHtmlToMarc Instead of pushing 999 in the front :) Test plan: Do not yet apply this patch. Edit a record. Save. And open MARC view/view plain on opac-detail. Verify that the biblionumber field (MARC21 999) follows leader. Apply this patch. Restart Plack. Edit a record. Save. And open MARC view/view plain on opac-detail. Verify that the biblionumber field is in right position. Run t/db_dependent/Biblio/TransformHtmlToMarc.t. Should pass now. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Test pass, correct tag order.
Created attachment 101377 [details] [review] Bug 21708: Add a few tests in TransformHtmlToMarc.t Test plan: Run t/db_dependent/Biblio/TransformHtmlToMarc.t without the module change patch. Verify that the test fails on the order of fields. Go to test plan of the second patch. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Test fails Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 101378 [details] [review] Bug 21708: Sort MARC fields in TransformHtmlToMarc Instead of pushing 999 in the front :) Test plan: Do not yet apply this patch. Edit a record. Save. And open MARC view/view plain on opac-detail. Verify that the biblionumber field (MARC21 999) follows leader. Apply this patch. Restart Plack. Edit a record. Save. And open MARC view/view plain on opac-detail. Verify that the biblionumber field is in right position. Run t/db_dependent/Biblio/TransformHtmlToMarc.t. Should pass now. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Test pass, correct tag order. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Thx Bernardo and Katrin
Nice work everyone! Pushed to master for 20.05
backported to 19.11.x for 19.11.05
backported to 19.05.x for 19.05.10
Minor behaviour patch will not be backported to 18.11.x series.