https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40594
--- Comment #6 from Martin Renvoize (ashimema) <[email protected]> --- 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. _______________________________________________ Koha-bugs mailing list -- [email protected] To unsubscribe send an email to [email protected] website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
