In C4::Search::FindDuplicate, when biblio has no ISBN, the duplicate search adds : $query .= " and itemtype=$result->{itemtype}". This should be only when itemtype is defined in biblio and not in items.
Created attachment 14535 [details] [review] Proposed patch See commit comment
Created attachment 14557 [details] [review] [SIGNED-OFF] Bug 9377 - itemtype in duplicate biblio search In C4::Search::FindDuplicate, when biblio has no ISBN, the duplicate search adds : $query .= " and itemtype=$result->{itemtype}". This should be only when itemtype is defined in biblio and not in items. Test plan : On a database with itemtype mapped in biblioitems and "item-level_itypes" syspref on "biblio record" : - Go to a biblio details page - Click on "Edit as new (duplicate)" - If ISBN is defined, remove it - Click on save => a duplicate is detected - Change biblio item type - Click on save => no duplicate detected - Set "item-level_itypes" syspref to "specific record" - Come back to biblio details page - Click on "Edit as new (duplicate)" - If ISBN is defined, remove it - Click on save => a duplicate is detected - Change biblio item type => a duplicate is detected Signed-off-by: Marc Veron <veron@veron.ch> Followed test plan. Behaves as expected.
QA: Looking now.
Created attachment 15798 [details] [review] Bug 9377 - itemtype in duplicate biblio search In C4::Search::FindDuplicate, when biblio has no ISBN, the duplicate search adds : $query .= " and itemtype=$result->{itemtype}". This should be only when itemtype is defined in biblio and not in items. Test plan : On a database with itemtype mapped in biblioitems and "item-level_itypes" syspref on "biblio record" : - Go to a biblio details page - Click on "Edit as new (duplicate)" - If ISBN is defined, remove it - Click on save => a duplicate is detected - Change biblio item type - Click on save => no duplicate detected - Set "item-level_itypes" syspref to "specific record" - Come back to biblio details page - Click on "Edit as new (duplicate)" - If ISBN is defined, remove it - Click on save => a duplicate is detected - Change biblio item type => a duplicate is detected Signed-off-by: Marc Veron <veron@veron.ch> Followed test plan. Behaves as expected. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
QA Comment: Tested. Works. Had some funny things when editing as new while changing frameworks, but apparently not related to your patch. Somewhat out of scope: But I am wondering if I save a new biblio with same title or ISBN and different itemtype (with item level biblio), if I would still like to have the warning? For the rest: Code looks good to me. Passed QA
Why is this patch needed? Would it not be better to simply remove the itemtype check altogether, if it does not work properly?
(In reply to comment #6) > Why is this patch needed? Would it not be better to simply remove the > itemtype check altogether, if it does not work properly? I agree if it is accepted by community. A false duplicate on title and author must be very unusual (maybe book and movie), and better show duplicate message too often than not enough.
(In reply to comment #7) > I agree if it is accepted by community. > A false duplicate on title and author must be very unusual (maybe book and > movie), and better show duplicate message too often than not enough. +1 from me Please send an adjusted patch or followup. I will be happy to sign off if needed.. Please change status accordingly.
+1 from me too.
Created attachment 15942 [details] [review] Proposed patch (remove itemtype) Rewritten patch that simply removes item type from find duplicate. See commit message
Created attachment 15964 [details] [review] Bug 9377 - itemtype in duplicate biblio search In C4::Search::FindDuplicate, when biblio has no ISBN, the duplicate search adds : $query .= " and itemtype=$result->{itemtype}". This is wrong when itemtype is defined in items. This patch simply removes the itemtype from dublicate search. Test plan : - Go to a biblio details page - Click on "Edit as new (duplicate)" - If ISBN is defined, remove it - Click on save => a duplicate is detected - Change biblio item type and save => a duplicate is detected Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested with no isbn, other itemtype. Before the patch Koha just saves the record, now it gives a duplicate warning. Great. Signed off
QA Comment: Code looks even better :) Passed QA
This patch has been pushed to master.
Pushed to 3.10.x and 3.8.x will be in 3.10.4 and 3.8.11