Description
Savitra Sirohi
2011-08-30 08:54:52 UTC
Is anyone working on this? This would be very handy. Nicole Created attachment 9982 [details] [review] Acquistions duplicate search across orders, suggestions and catalog 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. (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 } 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... Created attachment 11527 [details] [review] Bug 6813: Followup: indentation + simplify conditions Created attachment 11531 [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. 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.. Created attachment 11532 [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. Created attachment 11533 [details] [review] [SIGNED-OFF] Bug 6813: Followup: indentation + simplify conditions Signed-off-by: Marc Veron <veron@veron.ch> Created attachment 11534 [details] [review] [SIGNED-OFF] Bug 6813: Followup: indentation + simplify conditions Signed-off-by: Marc Veron <veron@veron.ch> Created attachment 11537 [details] [review] [SIGNED-OFF] Bug 6813: Followup: indentation + simplify conditions Signed-off-by: Marc Veron <veron@veron.ch> Created attachment 11538 [details] [review] [SIGNED-OFF] Bug 6813: Followup: indentation + simplify conditions Signed-off-by: Marc Veron <veron@veron.ch> Created attachment 11539 [details] [review] [SIGNED-OFF] Bug 6813: Followup: indentation + simplify conditions Signed-off-by: Marc Veron <veron@veron.ch> Created attachment 11541 [details] [review] [SIGNED-OFF] 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> Created attachment 11542 [details] [review] [SIGNED-OFF] Bug 6813: Followup: indentation + simplify conditions Signed-off-by: Marc Veron <veron@veron.ch> 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. 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) Created attachment 12456 [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 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 Created attachment 12714 [details] [review] [SIGNED-OFF] 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> Created attachment 12715 [details] [review] [SIGNED-OFF] Bug 6813: Followup: indentation + simplify conditions Signed-off-by: Marc Veron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 12716 [details] [review] [SIGNED-OFF] 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> Created attachment 13286 [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 Created attachment 13806 [details] [review] [Signed off] 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> I applied all patches and followed the test plan. Duplicate Finder search works as advertised. Need QA for the last patch. 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. Created attachment 14009 [details] [review] Bug 6813 follow-up, fix POD 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. (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. QA: Looking now. 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 on attachment 14009 [details] [review] Bug 6813 follow-up, fix POD Obsoleting this one too. Failed, will amend the last patch for this edit. 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. 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> 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> 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> 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) We don't have feedback from savitra or Amit, so failing QA 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. *** This bug has been marked as a duplicate of bug 14973 *** |