[Bug 40594] New: Allow batch record modification to create multiple items on existing bibs
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594 Bug ID: 40594 Summary: Allow batch record modification to create multiple items on existing bibs Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Cataloging Assignee: koha-bugs@lists.koha-community.org Reporter: pedro.amorim@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org CC: m.de.rooy@rijksmuseum.nl Created attachment 185115 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185115&action=edit MMT sample multiple mapped items Bug 26869 only considered one item to be added per library modification execution, if more than one item is mapped, it fails with an FK constraint. I don't know the reason behind it, but the original test plan stated: test with more than one occurrence of 952$a -> confirm 'FK constraint' error is shown, no modification made I don't think there's any reason why this restriction should exist. The attached Marc Modification Template sample should work perfectly fine and add new - multiple - items to biblio records, as mapped. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594 Pedro Amorim <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris.rowlands6@nhs.net, | |martin.renvoize@openfifth.c | |o.uk, | |pedro.amorim@openfifth.co.u | |k Depends on| |26869 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26869 [Bug 26869] Enable batch record modification to create items on existing bibs -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594 Pedro Amorim <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594 --- Comment #1 from Pedro Amorim <pedro.amorim@openfifth.co.uk> --- Created attachment 185116 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185116&action=edit Bug 40594: Consider multiple items mapped in MMT Test plan, clean k-t-d: Before test, pick a record and confirm it has no items, for this test we're using biblio 79: <staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=79 1) Add a new MARC modification template, visit: <staff_url>/cgi-bin/koha/tools/marc_modification_templates.pl 2) Click 'New template', enter a name and click 'Submit' 3) Add multiple items mapping, use the attached Marc Modification template sample as reference 4) Do a batch record modification, visit: <staff_url>/cgi-bin/koha/tools/batch_record_modification.pl 5) Select the tab 'Enter a list of record numbers' and enter '79' 6) On the template, select the template previously created. Click 'continue' 7) Click 'Modify selected records'. Click 'View detail of the enqueued job'. Notice success message is shown with no errors 8) Check the biblio, visit: <staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=79 9) Notice it now has however how many new items you added to the mapping Additional testing notes from bug 26869: test with 952 but with other subfields, without subfield 'a' -> confirm 'FK constraint' error is shown, no modification made test with 952$a too long e.g. 'aaaaaaaa' -> confirm 'too long' error is shown test with no 952 mapping -> confirm things work as intended test with 952 mapping plus other tags -> confirm things work as intended prove koha/t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t prove koha/t/db_dependent/Koha/BackgroundJob/BatchUpdateBiblio.t -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594 Esther Melander <esther.melander@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |esther.melander@bywatersolu | |tions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594 hebah@bywatersolutions.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hebah@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594 Bernard <bernard.scaife@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bernard.scaife@openfifth.co | |.uk --- Comment #2 from Bernard <bernard.scaife@openfifth.co.uk> --- Test plan in #1 followed All records were successfully modified. Further tests: a) test with 952 but with other subfields, without subfield 'a' -> confirm 'FK constraint' error is shown, no modification made Removed 952$a from one of 3 items. It successfully loaded the other 2 but no error is shown relating to "FK constraint". FAIL b) test with 952$a too long e.g. 'aaaaaaaa' -> confirm 'too long' error is shown Made one 952$a 'aaaaaaaaaaaaaa' It successfully loaded the other 2 but no error is shown relating to "too long". FAIL c) test with no 952 mapping -> confirm things work as intended Made template with only 264a field to be added. Works fine. PASS d) test with 952 mapping plus other tags -> confirm things work as intended Template with 952 and 856 - works fine PASS -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |pedro.amorim@openfifth.co.u |ity.org |k --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- It looks like Bernard tested and identified an issue in comment#2, but didn't set this to FQA. Updating status. Patch still applies and passes QA checks. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Status|Needs Signoff |Failed QA --- Comment #4 from David Nind <david@davidnind.com> --- As per comment #3 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #185116|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594 --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 205633 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205633&action=edit Bug 40594: Consider multiple items mapped in MMT Test plan, clean k-t-d: Before test, pick a record and confirm it has no items, for this test we're using biblio 79: <staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=79 1) Add a new MARC modification template, visit: <staff_url>/cgi-bin/koha/tools/marc_modification_templates.pl 2) Click 'New template', enter a name and click 'Submit' 3) Add multiple items mapping, use the attached Marc Modification template sample as reference 4) Do a batch record modification, visit: <staff_url>/cgi-bin/koha/tools/batch_record_modification.pl 5) Select the tab 'Enter a list of record numbers' and enter '79' 6) On the template, select the template previously created. Click 'continue' 7) Click 'Modify selected records'. Click 'View detail of the enqueued job'. Notice success message is shown with no errors 8) Check the biblio, visit: <staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=79 9) Notice it now has however how many new items you added to the mapping Additional testing notes from bug 26869: test with 952 but with other subfields, without subfield 'a' -> confirm 'FK constraint' error is shown, no modification made test with 952$a too long e.g. 'aaaaaaaa' -> confirm 'too long' error is shown test with no 952 mapping -> confirm things work as intended test with 952 mapping plus other tags -> confirm things work as intended prove koha/t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t prove koha/t/db_dependent/Koha/BackgroundJob/BatchUpdateBiblio.t Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594 --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 205634 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205634&action=edit Bug 40594: (QA follow-up) Report and skip individually invalid item fields Bernard's QA on the previous patch found that when several items are mapped in the same MARC Modification Template and one of them is missing a mandatory subfield (homebranch/holdingbranch/itemtype), the malformed item was silently created (or silently dropped) with no error surfaced anywhere in the job report. can_add_item_from_marc_record() only guarantees that *at least one* occurrence of the item field is fully mapped, so it lets a batch through even when some of the mapped items are individually incomplete. C4::Items::AddItemBatchFromMarc() already returns a list of per-item errors for the ones it rejects, but BatchUpdateBiblio.pm was discarding that return value entirely, and CheckItemPreSave() does not flag a mandatory subfield that is missing outright (only one that is present but invalid), so an item field missing e.g. subfield 'a' was never rejected before reaching Koha::Item->store. Add strip_invalid_item_fields(), which walks each item field on the record before it is handed to AddItemBatchFromMarc, removes any occurrence missing a mandatory subfield, and returns an error entry for each one removed (same shape as C4::Items::_repack_item_errors). Combine those with AddItemBatchFromMarc's own returned errors and surface them all as an 'item_not_added' message on the job report, so partially-invalid templates no longer fail silently while still allowing the well-formed items in the same batch to be created. Self-review follow-up, folded into this patch rather than left as a separate one since they complete what this commit already set out to do: * The 'item_not_added' message was never actually rendered anywhere - batch_biblio_record_modification.inc had no matching SWITCH case, so the job report showed an empty alert box for it. Add a CASE listing each item's error_information. * strip_invalid_item_fields() numbers errors by the item's original position in the record, but AddItemBatchFromMarc numbers its own errors by position within the *already-stripped* record it receives. Combining both lists as-is could report the same item_sequence for two different items, or attribute an AddItemBatchFromMarc error to the wrong item, whenever a stripped item isn't the last one mapped. strip_invalid_item_fields() now also returns the retained items' original sequence numbers, and process() uses that to translate AddItemBatchFromMarc's item_sequence back to the item's real, original position before merging the two error lists. * can_add_item_from_marc_record() and strip_invalid_item_fields() each independently looked up the same three MarcSubfieldStructures for every record processed. Factored the lookup out into _mandatory_item_mss(), computed once per job and passed in, since the mapping only depends on system preferences, not on the record. Test plan: 1) prove t/db_dependent/Koha/BackgroundJob/BatchUpdateBiblio.t 2) Repeat the manual test plan from comment #2, mapping 3 items where one is missing its homebranch/holdingbranch subfields -> confirm the 2 well-formed items are created and the job report now shows an 'item_not_added' error message identifying the skipped one. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org