Bug 6813 - Search for duplicates across orders, suggestions and catalog
Summary: Search for duplicates across orders, suggestions and catalog
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-30 08:54 UTC by Savitra Sirohi
Modified: 2019-05-04 11:29 UTC (History)
12 users (show)

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


Attachments
Acquistions duplicate search across orders, suggestions and catalog (16.43 KB, patch)
2012-06-08 11:17 UTC, Amit Gupta
Details | Diff | Splinter Review
Bug 6813: Followup: indentation + simplify conditions (18.39 KB, patch)
2012-08-10 10:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6813: Acquistions duplicate search across orders, suggestions and catalog (14.61 KB, patch)
2012-08-10 12:20 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 6813: Acquistions duplicate search across orders, suggestions and catalog (14.61 KB, patch)
2012-08-10 12:51 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6813: Followup: indentation + simplify conditions (14.60 KB, patch)
2012-08-10 13:11 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6813: Followup: indentation + simplify conditions (14.60 KB, patch)
2012-08-10 13:27 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6813: Followup: indentation + simplify conditions (14.60 KB, patch)
2012-08-10 14:48 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6813: Followup: indentation + simplify conditions (14.60 KB, patch)
2012-08-10 15:07 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6813: Followup: indentation + simplify conditions (14.60 KB, patch)
2012-08-10 15:08 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6813: Acquistions duplicate search across orders, suggestions and catalog (14.66 KB, patch)
2012-08-10 15:21 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6813: Followup: indentation + simplify conditions (14.60 KB, patch)
2012-08-10 15:21 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 6813: Followup: Fix HTML4 coding, new wording, warning on empty search terms (6.83 KB, patch)
2012-09-23 14:45 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6813: Acquistions duplicate search across orders, suggestions and catalog (14.71 KB, patch)
2012-10-05 14:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6813: Followup: indentation + simplify conditions (14.64 KB, patch)
2012-10-05 14:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6813: Followup: Fix HTML4 coding, new wording, warning on empty search terms (6.88 KB, patch)
2012-10-05 14:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 6813: Followup: FIX tabs and replace all occurrences of duplicate search (16.05 KB, patch)
2012-11-07 15:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
[Signed off] Bug 6813: Followup: FIX tabs and replace all occurrences of duplicate search (16.12 KB, patch)
2012-12-01 00:11 UTC, Melia Meggs
Details | Diff | Splinter Review
Bug 6813 follow-up, fix POD (732 bytes, patch)
2012-12-11 19:33 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 6813: Acquistions duplicate search across orders, suggestions and catalog (15.36 KB, patch)
2013-03-01 10:27 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 6813: Followup: indentation + simplify conditions (14.64 KB, patch)
2013-03-01 10:27 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 6813: Followup: Fix HTML4 coding, new wording, warning on empty search terms (6.88 KB, patch)
2013-03-01 10:28 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 6813: Followup: FIX tabs and replace all occurrences of duplicate search (16.65 KB, patch)
2013-03-01 10:28 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 Savitra Sirohi 2011-08-30 08:54:52 UTC
A new duplicate search tool in the acquisitions module that will allow search based on ISBN, title and/or author. Results will be across suggestions, orders and catalog.

This tool will allow staff to check for duplicates before creating a new suggestion or order.
Comment 1 Nicole C. Engard 2012-03-06 23:39:34 UTC
Is anyone working on this? This would be very handy.

Nicole
Comment 2 Amit Gupta 2012-06-08 11:17:30 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2012-07-23 15:07:13 UTC
(early) QA Comments :

1/ The commit message notices "PATCH 3/3". Where are the 2 first commits ? I think this one is the unique one :)

2/ The indentation is 4 spaces to separate blocks. It is not the case in this patch (8 or 0 in several cases, the average is good but it is not what is expected :-) ).

3/  if ( cond ) {} else { //some instructions }
should be replaced with
    unless ( cond ) { //some instructions }
Comment 4 Marc Véron 2012-08-10 09:03:23 UTC
Great enhancement (that b.t.w. was urgently asked by our librarians a couple of days ago).

I tested the patch. It behaves as expected, and I would like to sign-off...
Comment 5 Jonathan Druart 2012-08-10 10:29:46 UTC Comment hidden (obsolete)
Comment 6 Marc Véron 2012-08-10 12:20:44 UTC Comment hidden (obsolete)
Comment 7 Marc Véron 2012-08-10 12:24:43 UTC
Followed "Bugs with more than one patch"  
on http://wiki.koha-community.org/wiki/Git_bz_configuration

...but certainly did something wrong. Will try again..
Comment 8 Marc Véron 2012-08-10 12:51:33 UTC Comment hidden (obsolete)
Comment 9 Marc Véron 2012-08-10 13:11:24 UTC Comment hidden (obsolete)
Comment 10 Marc Véron 2012-08-10 13:27:14 UTC Comment hidden (obsolete)
Comment 11 Marc Véron 2012-08-10 14:48:05 UTC Comment hidden (obsolete)
Comment 12 Marc Véron 2012-08-10 15:07:45 UTC Comment hidden (obsolete)
Comment 13 Marc Véron 2012-08-10 15:08:21 UTC Comment hidden (obsolete)
Comment 14 Marc Véron 2012-08-10 15:21:34 UTC Comment hidden (obsolete)
Comment 15 Marc Véron 2012-08-10 15:21:44 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2012-09-14 14:14:24 UTC
QA Comments:

I already made QA on it.

I fixed my QA remarks and the patch I proposed was signoff by Marc Véron.

So I mark it as Passed QA.
Comment 17 Paul Poulain 2012-09-17 16:04:43 UTC
QA comment:
 * the template does not respect HTML4 coding guidelines = there are UCfirst on every word, please switch to lowercase
 * have you tested on a large database. MySQL will not optimize SELECT ... FROM biblio WHERE title LIKE '%something%', and performances will be very poor. I think it should be specified on the home "duplicatesearch" page
 * the name "duplicate search" sounds strange to me. Shouldn't it be something like "multi-source search" or something like that ? (Search for duplicate ?) being not english native, I won't be affirmative, but Nicole, if you could confirm...
 * when you reach the page, you get a: 
Duplicate Search
No Result
that is irrelevant.

failing QA (i'll accept this patch after feature freeze, no side effect risk, that's only new code)
Comment 18 Marc Véron 2012-09-23 14:45:48 UTC Comment hidden (obsolete)
Comment 19 Marc Véron 2012-09-23 14:48:40 UTC
I suppose the changes I made need a new signoff...

Oh, and in my previous comment I mistaked with the wording, it is "Duplicate finder" (Google: 1'710'000)

Marc
Comment 20 Kyle M Hall 2012-10-05 14:41:40 UTC Comment hidden (obsolete)
Comment 21 Kyle M Hall 2012-10-05 14:41:58 UTC Comment hidden (obsolete)
Comment 22 Kyle M Hall 2012-10-05 14:42:22 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2012-11-07 15:32:32 UTC Comment hidden (obsolete)
Comment 24 Melia Meggs 2012-12-01 00:11:34 UTC Comment hidden (obsolete)
Comment 25 Melia Meggs 2012-12-01 00:13:14 UTC
I applied all patches and followed the test plan.  Duplicate Finder search works as advertised.
Comment 26 Jonathan Druart 2012-12-03 08:22:27 UTC
Need QA for the last patch.
Comment 27 Paul Poulain 2012-12-11 19:33:29 UTC
QA comment:
 * passes koha-qa.pl, except for a POD mistake revealed in C4/Suggestions.pm, I attach a follow-up fixing this problem
 * the patch work as expected, no side effect or any problem spotted.

However, I won't pass QA this patch immediately because I've 2 questions = why is this tool in the acquisition module ? It is not specifically related to acquisition ? I think it's something that looks like the /cataloguing/addbooks.pl, that searches in the catalog and the reservoir. Shouldn't we replace the cataloguing/addbooks.pl by this new script (on which we would add a "reservoir" tab) ? Once the librarian has done his search, we could have, instead of just "New record" (to catalogate a new biblio), a "New record" and a "New suggestion" button.

That would improve consistency imo.

I won't fail QA for now, waiting for patch authors opinion.
That will require more effort, but I think it's worth it, because it will both improve Koha functionally, usability, and code quality.
Comment 28 Paul Poulain 2012-12-11 19:33:44 UTC Comment hidden (obsolete)
Comment 29 Katrin Fischer 2012-12-27 10:47:56 UTC
I agree with Paul that this tool could be better integrated in existing workflows. Currently you can find duplicates, but you can't create orders or perform any other actions from the tool.

But better integration would mean a lot more work/rewrite of the feature and it still might be useful for libraries as it is now (small steps...)

Some things I noted:

1) FindBiblios only searches on biblio.title instead of using the search engine.

2) I am not sure about the permissions for the new feature:
+ flagsrequired   => { acquisition => 'group_manage', acquisition => 'order_manage', acquisition => 'order_receive' },
I think as no critical information is shown in the search results we could maybe  be a bit less strict here. Anyway a check for permissions should be added to the link in the navigation toolbar, so you can only see the duplicate finder if you are allowed to use it.

3) On the suggestion result list, the status is shown as in the database and not translatable. For example: ASKED

4) The patches are also missing unit tests for the new routines FindSuggestions and FindBiblios.

As these patches are quite old (June 2012), I am not sure if 4) is a failed QA - leaving this decision to Jared.
Comment 30 Jared Camins-Esakov 2012-12-28 12:32:54 UTC
(In reply to comment #29)
> 4) The patches are also missing unit tests for the new routines
> FindSuggestions and FindBiblios.
> 
> As these patches are quite old (June 2012), I am not sure if 4) is a failed
> QA - leaving this decision to Jared.

I will accept this patch without unit tests, though it will not make me happy.
Comment 31 Marcel de Rooy 2013-03-01 09:53:37 UTC
QA: Looking now.
Comment 32 Marcel de Rooy 2013-03-01 10:07:18 UTC
Comment on attachment 12714 [details] [review]
[SIGNED-OFF] Bug 6813: Acquistions duplicate search across orders, suggestions and catalog

Obsoleting the first patch for merge conflict. Have resolved it and will send new one after applying the rest with git-bz :)
Comment 33 Marcel de Rooy 2013-03-01 10:11:34 UTC
Comment on attachment 14009 [details] [review]
Bug 6813 follow-up, fix POD

Obsoleting this one too. Failed, will amend the last patch for this edit.
Comment 34 Marcel de Rooy 2013-03-01 10:27:13 UTC
QA Comment:

Amit or Savitra,
Some comments have been given already. So I will not repeat them all.
But I like the idea of your new tool!
FindBiblios works with sql statements. That is a risk for performance.
I think (like Paul) that this tool should not be in Acquisitions. Could you place it under .. Tools ?

Please send a followup and I will continue on that.
Comment 35 Marcel de Rooy 2013-03-01 10:27:51 UTC
Created attachment 15793 [details] [review]
Bug 6813: Acquistions duplicate search across orders, suggestions and catalog

To Test:
Create a an suggestion, order and catalog record for a certain title. Use the duplicate search tool available in the acquisitions menu (left navigation bar) to search and find the suggestion, order and catalog record matching the search term.

Enter one or more of title, author and ISBN in the search criteria.

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 36 Marcel de Rooy 2013-03-01 10:27:58 UTC
Created attachment 15794 [details] [review]
Bug 6813: Followup: indentation + simplify conditions

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 37 Marcel de Rooy 2013-03-01 10:28:02 UTC
Created attachment 15795 [details] [review]
Bug 6813: Followup: Fix HTML4 coding, new wording, warning on empty search terms

The aim of this followup is to fix issues from Comment #17:

- HTML4 encoding UCfirst fixed
- Warning regardig large databases
- Added javascript with warning if search terms are empty
- Removed not used fields from search form
- Changed wording from "Duplicate search" (Google: 15'000) to "Find duplicates" (Google: 576'000)

To Test:
Create a an suggestion, order and catalog record for a certain title. Use the duplicate search tool available in the acquisitions menu (left navigation bar) to search and find the suggestion, order and catalog record matching the search term.

Enter one or more of title, author and ISBN in the search criteria.

Marc

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 38 Marcel de Rooy 2013-03-01 10:28:07 UTC
Created attachment 15796 [details] [review]
Bug 6813: Followup: FIX tabs and replace all occurrences of duplicate search

- tabs was broken
- 'duplicate search' have to be replaced with 'duplicate finder'
- adds datatable instead of tablesorter

Signed-off-by: Melia Meggs <melia@bywatersolutions.com>

Incorporated Paul's POD fix for Suggestions.pm in here (marcelr 010313)
Comment 39 Paul Poulain 2013-03-21 16:58:30 UTC
We don't have feedback from savitra or Amit, so failing QA
Comment 40 Katrin Fischer 2019-05-04 11:29:12 UTC
It looks like this got really close at one point, but was then abandoned. I am not sure if any of the existing code here from 2013 could be reused. Resetting to NEW.