Bug 9811 introduces a new search way for patrons. Some code have been refactored and the server-side option of DataTables is now used. Bug 12648 (basket users) and bug 13021 (guarantors) uses the same way to search patrons. But in 3 other places, the former C4::Members::Search subroutine is still used (and in some cases, a manual pagination...): - budget users and owner - serial recipients - patron cards The 2 expected goals are 1/ Use the same centralized way to search for patrons and 2/ to get rid of the last calls to the C4::Members::Search (and so SQLHelper).
Created attachment 37104 [details] [review] Bug 13891: DataTables server-side processing - budget users This first patch remove the previous way to search for users to link to budgets. Test plan: 1/ Edit or create a fund 2/ Edit the owner of this fund a select a patron 3/ Add 1+ users to the user lists of this fund
Created attachment 37105 [details] [review] Bug 13891: DataTables server-side processing - Serial recipients This second patch removes the previous way to search for serial recipients. Test plan: 1/ Edit the routing list for a subscription (serials/routing.pl?subscriptionid=X) 2/ Add 1+ recipients 3/ Confirm there is no regression on the add/delete/search actions QA note: This search does not use a clean way to interact with the window opener. Indeed the opener is reloaded to display the new item. This patch does not change this behavior, but note the trick in the template (common/patron_search.tt) to wait for the opener in order not to get a JS error. This is also used by the next patch (patron card).
Created attachment 37106 [details] [review] Bug 13891: DataTables server-side processing - patron cards This third patch removes the previous way to search for patron cards users. Test plan: 1/ Add items to a patron card batch (patroncards/edit-batch.pl?op=edit&element_id=X) 2/ Add 1+ patrons to the batch 3/ Confirm there is no regression
Created attachment 37107 [details] [review] Bug 13891: DataTables server-side processing - Fix regressions This patch fixes the regressions introduced by the previous patches. If you have tested all in once, you didn't see them. It introduces library, category and "first letter" filters. Test plan: 1/ On all pages impacted by previous patches + new order empty (link patron to an order) + guarantor search 2/ Add / Select patron to the list 3/ Use the filters 4/ Confirm there is no regression
6 files deleted, 400 lines of code removed but new features added :) Please test!
Patch does not apply: Applying: Bug 13891: DataTables server-side processing - budget users Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/common/patron_search.tt Removing koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudget_user_search.tt Removing admin/aqbudget_user_search.pl Applying: Bug 13891: DataTables server-side processing - Serial recipients Applying: Bug 13891: DataTables server-side processing - patron cards Applying: Bug 13891: DataTables server-side processing - Fix regressions fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/common/patron_search.tt). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 13891: DataTables server-side processing - Fix regressions
(In reply to Marc Véron from comment #6) Sorry Marc, I forgot to add the dependency on bug 13879.
I have the following problem (tested with adding owner and user to fund and with routing list, but I thing it will appear at all places): 1) Do a search in the pop-up window 2) The names in the result table are linked to patron's detail page. If you click on a name, patron's details appear inside this pop-up window, no way back. I would expect the names to have no detail link at all OR the link on the names to behave like the 'Select' link.
(In reply to Marc Véron from comment #8) > I have the following problem (tested with adding owner and user to fund and > with routing list, but I thing it will appear at all places): > > 1) Do a search in the pop-up window > 2) The names in the result table are linked to patron's detail page. If you > click on a name, patron's details appear inside this pop-up window, no way > back. > > I would expect the names to have no detail link at all OR the link on the > names to behave like the 'Select' link. I think the links already existed before this patch set. I am not sure removing them is a good idea. What did you mean with "behave like the 'Select' link"? Open in a new window?
Created attachment 37180 [details] No links on patron's names (behaviour without patches) Hi Jonathan, Without patches applied the names are not linked. With "Behave like the select link" I mean that if a link is on the name, it should select this patron for the fund (or whatever the pop-up was opened for). But I think it is better to have no links at all on the names, as it was without patch. See screenshot in attachment.
(In reply to Marc Véron from comment #10) > Created attachment 37180 [details] > No links on patron's names (behaviour without patches) > > Hi Jonathan, > > Without patches applied the names are not linked. On the users and owner of a budget, but it is on the order users search :) So we need to have the same behavior everywhere. > With "Behave like the select link" I mean that if a link is on the name, it > should select this patron for the fund (or whatever the pop-up was opened > for). > > But I think it is better to have no links at all on the names, as it was > without patch. I don't have any objection to remove the link but I would prefer to wait for the QA step before removing it. If it's your only concern, you can add it as a tester note and let the QAer and RM take the decision. I will submit a followup if they agree with you. Note that it is possible to have a link in a page and not in another one, but it would be a weird behavior.
Created attachment 37184 [details] [review] [Signed-off] Bug 13891: DataTables server-side processing - budget users This first patch remove the previous way to search for users to link to budgets. Test plan: 1/ Edit or create a fund 2/ Edit the owner of this fund a select a patron 3/ Add 1+ users to the user lists of this fund Tested together with other patches. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 37185 [details] [review] [Signed-off] Bug 13891: DataTables server-side processing - Serial recipients This second patch removes the previous way to search for serial recipients. Test plan: 1/ Edit the routing list for a subscription (serials/routing.pl?subscriptionid=X) 2/ Add 1+ recipients 3/ Confirm there is no regression on the add/delete/search actions QA note: This search does not use a clean way to interact with the window opener. Indeed the opener is reloaded to display the new item. This patch does not change this behavior, but note the trick in the template (common/patron_search.tt) to wait for the opener in order not to get a JS error. This is also used by the next patch (patron card). Tested together with other patches. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 37186 [details] [review] [Signed-off] Bug 13891: DataTables server-side processing - patron cards This third patch removes the previous way to search for patron cards users. Test plan: 1/ Add items to a patron card batch (patroncards/edit-batch.pl?op=edit&element_id=X) 2/ Add 1+ patrons to the batch 3/ Confirm there is no regression Tested together with other patches. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 37187 [details] [review] [Signed-off] Bug 13891: DataTables server-side processing - Fix regressions This patch fixes the regressions introduced by the previous patches. If you have tested all in once, you didn't see them. It introduces library, category and "first letter" filters. Test plan: 1/ On all pages impacted by previous patches + new order empty (link patron to an order) + guarantor search 2/ Add / Select patron to the list 3/ Use the filters 4/ Confirm there is no regression Tested together with other patches. Signed-off-by: Marc Véron <veron@veron.ch>
QA Note, it appears that serials/add_user_search takes into account the search text field when browsing by last name. Original version doesn't have name browse. Could be considered a bug or a feature depending on your point of view. This behavior does no occur on the "patron cards" patron search.
> This behavior does no occur on the "patron cards" patron search. correct, it *does* occur.
Created attachment 37334 [details] [review] [PASSED QA] Bug 13891: DataTables server-side processing - budget users This first patch remove the previous way to search for users to link to budgets. Test plan: 1/ Edit or create a fund 2/ Edit the owner of this fund a select a patron 3/ Add 1+ users to the user lists of this fund Tested together with other patches. Signed-off-by: Marc Veron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 37335 [details] [review] [PASSED QA] Bug 13891: DataTables server-side processing - Serial recipients This second patch removes the previous way to search for serial recipients. Test plan: 1/ Edit the routing list for a subscription (serials/routing.pl?subscriptionid=X) 2/ Add 1+ recipients 3/ Confirm there is no regression on the add/delete/search actions QA note: This search does not use a clean way to interact with the window opener. Indeed the opener is reloaded to display the new item. This patch does not change this behavior, but note the trick in the template (common/patron_search.tt) to wait for the opener in order not to get a JS error. This is also used by the next patch (patron card). Tested together with other patches. Signed-off-by: Marc Veron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 37336 [details] [review] [PASSED QA] Bug 13891: DataTables server-side processing - patron cards This third patch removes the previous way to search for patron cards users. Test plan: 1/ Add items to a patron card batch (patroncards/edit-batch.pl?op=edit&element_id=X) 2/ Add 1+ patrons to the batch 3/ Confirm there is no regression Tested together with other patches. Signed-off-by: Marc Veron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 37337 [details] [review] [PASSED QA] Bug 13891: DataTables server-side processing - Fix regressions This patch fixes the regressions introduced by the previous patches. If you have tested all in once, you didn't see them. It introduces library, category and "first letter" filters. Test plan: 1/ On all pages impacted by previous patches + new order empty (link patron to an order) + guarantor search 2/ Add / Select patron to the list 3/ Use the filters 4/ Confirm there is no regression Tested together with other patches. Signed-off-by: Marc Veron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Patches pushed to master. Good job Jonathan!
Is it possible to port this back to 3.18?
(In reply to Barton Chittenden from comment #23) > Is it possible to port this back to 3.18? I don't think it's a good idea. The patches don't apply and the dependency tree is quite important (most of them are not in 3.18 yet).
*** Bug 6513 has been marked as a duplicate of this bug. ***