Bug 13021 - DataTables server-side processing - guarantor search
Summary: DataTables server-side processing - guarantor search
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 12648
Blocks: 12633 13879 14044
  Show dependency treegraph
 
Reported: 2014-10-02 07:29 UTC by Jonathan Druart
Modified: 2017-06-14 22:05 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 13021: guarantor search - prepare existing code to be reused (1.49 KB, patch)
2014-10-02 08:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13021: guarantor search - main patch (17.19 KB, patch)
2014-10-02 08:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13021: guarantor search - guarantor_search.tt is useless (5.68 KB, patch)
2014-10-02 08:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13021: guarantor search - prepare existing code to be reused (1.49 KB, patch)
2014-10-11 14:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13021: guarantor search - main patch (20.09 KB, patch)
2014-10-11 14:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13021: guarantor search - guarantor_search.tt is useless (5.68 KB, patch)
2014-10-11 14:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13021: guarantor search - main patch (20.10 KB, patch)
2015-01-15 10:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13021: guarantor search - guarantor_search.tt is useless (5.68 KB, patch)
2015-01-15 10:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13021: guarantor search - guarantor_search.tt is useless (5.74 KB, patch)
2015-01-15 23:08 UTC, the.invinnysible.one
Details | Diff | Splinter Review
Bug 13021: guarantor search - prepare existing code to be reused (1.55 KB, patch)
2015-01-16 08:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13021: guarantor search - main patch (20.16 KB, patch)
2015-01-16 08:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13021: guarantor search - guarantor_search.tt is useless (5.74 KB, patch)
2015-01-16 08:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13021: Reintroduce the name column for guarantor search (1.94 KB, patch)
2015-01-19 10:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13021: Fix conflict with bug 8096 (1.44 KB, patch)
2015-01-30 14:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 13021: guarantor search - prepare existing code to be reused (1.58 KB, patch)
2015-01-30 14:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 13021: guarantor search - main patch (20.24 KB, patch)
2015-01-30 14:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 13021: guarantor search - guarantor_search.tt is useless (5.82 KB, patch)
2015-01-30 14:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 13021: Reintroduce the name column for guarantor search (1.98 KB, patch)
2015-01-30 14:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 13021: Fix conflict with bug 8096 (1.50 KB, patch)
2015-01-30 14:44 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2014-10-02 07:29:25 UTC
Bug 12648 refactores some code for the patron search (link patrons to a basket and an order, in the acquisition module).

This code can be reuse for the guarantor search.
Comment 1 Jonathan Druart 2014-10-02 07:30:05 UTC
This will remove 1 call to C4::Members::Search (see bug 12633).
Comment 2 Jonathan Druart 2014-10-02 08:27:28 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2014-10-02 08:27:36 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2014-10-02 08:27:40 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2014-10-11 14:10:49 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2014-10-11 14:10:56 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2014-10-11 14:11:01 UTC Comment hidden (obsolete)
Comment 8 Frédéric Demians 2014-11-05 07:32:25 UTC
I can't apply your patches, and get this Git error:

fatal: sha1 information is lacking or useless (acqui/add_user_search.pl).
Comment 9 Jonathan Druart 2014-11-05 09:57:44 UTC
(In reply to Frédéric Demians from comment #8)
> I can't apply your patches, and get this Git error:
> 
> fatal: sha1 information is lacking or useless (acqui/add_user_search.pl).

This patch depends on bug 12648, try to apply it before this one.
Comment 10 Jonathan Druart 2015-01-15 10:17:11 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2015-01-15 10:17:15 UTC Comment hidden (obsolete)
Comment 12 the.invinnysible.one 2015-01-15 23:08:19 UTC Comment hidden (obsolete)
Comment 13 the.invinnysible.one 2015-01-15 23:09:30 UTC
I think everything looks okay. Signing off.
Comment 14 Jonathan Druart 2015-01-16 08:50:22 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2015-01-16 08:50:26 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2015-01-16 08:50:29 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2015-01-16 08:51:42 UTC
(In reply to the.invinnysible.one from comment #13)
> I think everything looks okay. Signing off.

Thanks Morag!
I added you signed off to the 3 patches.
Comment 18 Kyle M Hall 2015-01-16 16:45:11 UTC
The patron name column appears to be missing from the search results table!
Comment 19 Jonathan Druart 2015-01-19 10:48:39 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2015-01-19 10:49:11 UTC
(In reply to Kyle M Hall from comment #18)
> The patron name column appears to be missing from the search results table!

Thanks for catching that!
The last patch reintroduced the name column.
Comment 21 Kyle M Hall 2015-01-30 13:04:49 UTC
I'm afraid I must report that with the last followup, the search never returns any results! If I remove the last patch I once again get results, but of course the name column is missing without it.
Comment 22 Jonathan Druart 2015-01-30 14:41:37 UTC Comment hidden (obsolete)
Comment 23 Kyle M Hall 2015-01-30 14:44:38 UTC
Created attachment 35628 [details] [review]
[PASSED QA] Bug 13021: guarantor search - prepare existing code to be reused

The add_user_search tt file will be reuse in next commits, this commit
just moves it in a "common" directory.

Signed-off-by: Morag Hills <the.invinnysible.one@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 24 Kyle M Hall 2015-01-30 14:44:47 UTC
Created attachment 35629 [details] [review]
[PASSED QA] Bug 13021: guarantor search - main patch

This patch is the main patch.
The "common" template is improved to allow different type of picking:
"add" or "select".
The first one appends a patron to a list, the second one selects the
patron and close the result search window.

The members/guarantor_search.pl has completly changed but is quite the
same file as acqui/add_user_search.pl. Both should exist: they don't
belong to the same module (acqui vs members), the picking type is
different (add vs select) and the columns are not the same.

The changes in the common template are very powerful, it's now possible
to list the column we want! This will be very useful for further
reusability.

Before this patch, all patrons mathing the pattern were return. Now only
the first 20 are (depends on the DataTables selected value).

For QA: This patch introduces a new template plugin "To", for now it
permits to convert a perl structure to json. In the idea, it could
permit to convert foo to bar too.

Test plan:
1/ Verify there is no regression in the guarantor search. When the
selection has been done, all data from the guarantor should fill the
form in the "main address" section.
Note that the request is done when the search input in not empty and the
user stop to write for 1 sec.
2/ Verify there is no regression on the 2 other pages where this patron
search is used: link a patron to an order and to a basket (in the
acquisition module).

Signed-off-by: Morag Hills <the.invinnysible.one@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 25 Kyle M Hall 2015-01-30 14:44:50 UTC
Created attachment 35630 [details] [review]
[PASSED QA] Bug 13021: guarantor search - guarantor_search.tt is useless

The guarantor_search.tt is now useless, no script uses it.
It can be removed safely.

Signed-off-by: Morag Hills <the.invinnysible.one@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 26 Kyle M Hall 2015-01-30 14:44:52 UTC
Created attachment 35631 [details] [review]
[PASSED QA] Bug 13021: Reintroduce the name column for guarantor search

Previous patch removed the name column, it's a regression.
This patch reintroduces it.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 27 Kyle M Hall 2015-01-30 14:44:55 UTC Comment hidden (obsolete)
Comment 28 Tomás Cohen Arazi 2015-03-13 13:45:05 UTC
Jonathan, is it possible that the latest patch doesn't belong here?
Comment 29 Jonathan Druart 2015-03-13 16:31:16 UTC
(In reply to Tomás Cohen Arazi from comment #28)
> Jonathan, is it possible that the latest patch doesn't belong here?

Yep, same as bug 12648 comment 76:

  Created attachment 36810 [details] [review] [review]
    Bug 12648: Fix conflict with bug 8096
Comment 30 Tomás Cohen Arazi 2015-03-19 18:05:00 UTC
Patches pushed to master.

Good job Jonathan!
Comment 31 Jonathan Druart 2015-03-20 13:12:35 UTC
Regression found, will be fixed soon on bug 13879.