Bug 9377 - itemtype in duplicate biblio search
Summary: itemtype in duplicate biblio search
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin SOMERS
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-11 09:04 UTC by Fridolin SOMERS
Modified: 2014-05-26 21:04 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed patch (1.75 KB, patch)
2013-01-11 09:54 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9377 - itemtype in duplicate biblio search (1.88 KB, patch)
2013-01-14 08:53 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 9377 - itemtype in duplicate biblio search (1.93 KB, patch)
2013-03-01 11:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Proposed patch (remove itemtype) (1.23 KB, patch)
2013-03-07 16:43 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 9377 - itemtype in duplicate biblio search (1.34 KB, patch)
2013-03-08 07:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin SOMERS 2013-01-11 09:04:01 UTC
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.
Comment 1 Fridolin SOMERS 2013-01-11 09:54:57 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2013-01-14 08:53:08 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2013-03-01 10:37:46 UTC
QA: Looking now.
Comment 4 Marcel de Rooy 2013-03-01 11:00:31 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2013-03-01 11:03:40 UTC
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
Comment 6 Jared Camins-Esakov 2013-03-02 17:04:24 UTC
Why is this patch needed? Would it not be better to simply remove the itemtype check altogether, if it does not work properly?
Comment 7 Fridolin SOMERS 2013-03-04 13:39:34 UTC
(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.
Comment 8 Marcel de Rooy 2013-03-06 12:22:14 UTC
(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.
Comment 9 Katrin Fischer 2013-03-06 13:40:32 UTC
+1 from me too.
Comment 10 Fridolin SOMERS 2013-03-07 16:43:05 UTC Comment hidden (obsolete)
Comment 11 Marcel de Rooy 2013-03-08 07:45:28 UTC
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>
Comment 12 Marcel de Rooy 2013-03-08 07:46:24 UTC
Tested with no isbn, other itemtype. Before the patch Koha just saves the record, now it gives a duplicate warning. Great.

Signed off
Comment 13 Marcel de Rooy 2013-03-08 07:48:32 UTC
QA Comment: Code looks even better :) Passed QA
Comment 14 Jared Camins-Esakov 2013-03-11 12:09:12 UTC
This patch has been pushed to master.
Comment 15 Chris Cormack 2013-03-16 20:26:36 UTC
Pushed to 3.10.x and 3.8.x will be in 3.10.4 and 3.8.11