Bug 30063 - Make the main patron search use the /patrons REST API route
Summary: Make the main patron search use the /patrons REST API route
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL: https://gitlab.com/joubu/Koha/-/tree/...
Keywords:
: 26865 29125 (view as bug list)
Depends on: 30094
Blocks: 30416 30576 30607 30622 30639 31229 30404 30405 30420 30455 30485 31530
  Show dependency treegraph
 
Reported: 2022-02-09 14:37 UTC by Jonathan Druart
Modified: 2022-09-07 20:43 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 30063: Number of checkouts (8.99 KB, patch)
2022-02-15 10:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Overdues count (5.37 KB, patch)
2022-02-15 10:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Make the main patron search use the REST API (50.02 KB, patch)
2022-02-15 10:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Remove svc/members/search (9.70 KB, patch)
2022-02-15 10:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Remove occurrences of C4::Utils::DataTables::Members (33.45 KB, patch)
2022-02-15 10:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Fix address format (5.75 KB, patch)
2022-02-15 10:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Add kind of POD for the include file (3.14 KB, patch)
2022-02-15 10:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Display search description for main search only (2.80 KB, patch)
2022-02-15 10:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Fix table settings (1.87 KB, patch)
2022-03-29 08:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Sort by "Name" firstname,surname (7.72 KB, patch)
2022-03-30 10:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Number of checkouts (6.99 KB, patch)
2022-03-31 12:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Overdues count (5.67 KB, patch)
2022-03-31 12:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Make the main patron search use the REST API (52.46 KB, patch)
2022-03-31 12:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Remove svc/members/search (9.84 KB, patch)
2022-03-31 12:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Remove occurrences of C4::Utils::DataTables::Members (33.59 KB, patch)
2022-03-31 12:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Fix address format (5.94 KB, patch)
2022-03-31 12:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Add kind of POD for the include file (3.29 KB, patch)
2022-03-31 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Display search description for main search only (2.95 KB, patch)
2022-03-31 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Fix selenium tests (2.94 KB, patch)
2022-03-31 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Fix sticky (1.17 KB, patch)
2022-03-31 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Add selenium tests (9.28 KB, patch)
2022-03-31 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Be more flexible for filters (5.07 KB, patch)
2022-03-31 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Fix table settings (2.02 KB, patch)
2022-03-31 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Sort by "Name" firstname,surname (8.65 KB, patch)
2022-03-31 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Restore back behaviour (3.51 KB, patch)
2022-03-31 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Fix start with (1.45 KB, patch)
2022-03-31 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Fix permission for GET /patrons (1.03 KB, patch)
2022-03-31 12:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Sort by "Name" surname:firstname (2.54 KB, patch)
2022-03-31 12:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Fix QA issues (1.86 KB, patch)
2022-03-31 12:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30063: Number of checkouts (7.04 KB, patch)
2022-03-31 13:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Overdues count (5.73 KB, patch)
2022-03-31 13:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Make the main patron search use the REST API (52.51 KB, patch)
2022-03-31 13:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Remove svc/members/search (9.90 KB, patch)
2022-03-31 13:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Remove occurrences of C4::Utils::DataTables::Members (33.65 KB, patch)
2022-03-31 13:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Fix address format (6.00 KB, patch)
2022-03-31 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Add kind of POD for the include file (3.35 KB, patch)
2022-03-31 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Display search description for main search only (3.01 KB, patch)
2022-03-31 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Fix selenium tests (3.00 KB, patch)
2022-03-31 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Fix sticky (1.23 KB, patch)
2022-03-31 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Add selenium tests (9.34 KB, patch)
2022-03-31 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Be more flexible for filters (5.13 KB, patch)
2022-03-31 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Fix table settings (2.08 KB, patch)
2022-03-31 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Sort by "Name" firstname,surname (8.70 KB, patch)
2022-03-31 13:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Restore back behaviour (3.56 KB, patch)
2022-03-31 13:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Fix start with (1.51 KB, patch)
2022-03-31 13:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Fix permission for GET /patrons (1.09 KB, patch)
2022-03-31 13:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Sort by "Name" surname:firstname (2.60 KB, patch)
2022-03-31 13:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: Fix QA issues (1.92 KB, patch)
2022-03-31 13:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30063: (follow-up) Fix missing TT filters (1.19 KB, patch)
2022-04-05 11:26 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2022-02-09 14:37:39 UTC

    
Comment 1 Jonathan Druart 2022-02-15 10:31:11 UTC
Created attachment 130608 [details] [review]
Bug 30063: Number of checkouts
Comment 2 Jonathan Druart 2022-02-15 10:31:15 UTC
Created attachment 130609 [details] [review]
Bug 30063: Overdues count
Comment 3 Jonathan Druart 2022-02-15 10:31:19 UTC
Created attachment 130610 [details] [review]
Bug 30063: Make the main patron search use the REST API

To test:
* Length menu (PatronsPerPage)
* Query description
* Highlight of the current library

* sticky header - Does not work (?)

If the table does not show when you submit the filter form, make sure
you regenerated the compiled CSS.
Comment 4 Jonathan Druart 2022-02-15 10:31:23 UTC
Created attachment 130611 [details] [review]
Bug 30063: Remove svc/members/search
Comment 5 Jonathan Druart 2022-02-15 10:31:27 UTC
Created attachment 130612 [details] [review]
Bug 30063: Remove occurrences of C4::Utils::DataTables::Members
Comment 6 Jonathan Druart 2022-02-15 10:31:30 UTC
Created attachment 130613 [details] [review]
Bug 30063: Fix address format
Comment 7 Jonathan Druart 2022-02-15 10:31:34 UTC
Created attachment 130614 [details] [review]
Bug 30063: Add kind of POD for the include file
Comment 8 Jonathan Druart 2022-02-15 10:31:38 UTC
Created attachment 130615 [details] [review]
Bug 30063: Display search description for main search only
Comment 9 Jonathan Druart 2022-02-15 11:30:13 UTC
Please use the remote branch for testing, it contains all the tree.
Comment 10 Fridolin Somers 2022-03-10 02:47:56 UTC
Merge conflict on koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
Because of Bug 29136 pushed
Comment 11 Fridolin Somers 2022-03-10 02:58:29 UTC
In patrons search page /cgi-bin/koha/members/members-home.pl it removes :
Browse by last name: A B C D ...

And when searching I get no results table o_O
I see the API call in network tought.
Comment 12 Jonathan Druart 2022-03-10 08:12:26 UTC
(In reply to Fridolin Somers from comment #10)
> Merge conflict on koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
> Because of Bug 29136 pushed

Remote branch rebased.

(In reply to Fridolin Somers from comment #11)
> In patrons search page /cgi-bin/koha/members/members-home.pl it removes :
> Browse by last name: A B C D ...

I am seeing it.

> And when searching I get no results table o_O
> I see the API call in network tought.

See the commit message: "If the table does not show when you submit the filter form, make sure
you regenerated the compiled CSS."

You need to `yarn build css` and refresh your browser's cache.
Comment 13 Fridolin Somers 2022-03-10 21:50:59 UTC
Ahhh thanks, it works.
Looks great :D

Some feedbacks :

1)
When arriving in patrons home page without searching : /cgi-bin/koha/members/members-home.pl
We see the header with 'Select all' 'Clear all' ...
This fells odd.
We may hide it like in current master.

2)
Why not keeping 'Browse by last name: A B ...' on top of header 'Select all' 'Clear all' ... ?

3)
Perform a search with some text in left form.
Click on a letter of 'Browse by last name'.
Note that search text is not removed in left form like in current master.

We may also clear form in page header, but keeping it is the current behavior.
Comment 14 Jonathan Druart 2022-03-15 11:26:33 UTC
(In reply to Fridolin Somers from comment #13)
> Ahhh thanks, it works.
> Looks great :D
> 
> Some feedbacks :
> 
> 1)
> When arriving in patrons home page without searching :
> /cgi-bin/koha/members/members-home.pl
> We see the header with 'Select all' 'Clear all' ...
> This fells odd.
> We may hide it like in current master.

I didn't manage to hide it. When it's hidden the "sticky" behaviour is broken (see patch "Fix sticky")

> 2)
> Why not keeping 'Browse by last name: A B ...' on top of header 'Select all'
> 'Clear all' ... ?

Because I had to rebuild a bit how the "blocks" are ordered. The header with actions (select/clear all, add to list, merge) is only available from the main patron search, when the "browse by" is available for all patron searches. It was easier to then in the "browse by" code into the include file.
We can certainly change the order if there is a real need, but it will make the code a bit more complex then.

> 3)
> Perform a search with some text in left form.
> Click on a letter of 'Browse by last name'.
> Note that search text is not removed in left form like in current master.

Don't you think this behaviour is better?

> We may also clear form in page header, but keeping it is the current
> behavior.

When? I wouldn't modify another form, that would feel weird.
Comment 15 Fridolin Somers 2022-03-16 00:18:35 UTC
1) 2)
OK, this is minor.

3)
OK we will see if other feed-backs see it.
Keeping input terms allows to rerun search, it can be a feature not a bug ;)

I sign off.
Comment 16 Jonathan Druart 2022-03-16 07:03:45 UTC
(In reply to Fridolin Somers from comment #15)
> 1) 2)
> OK, this is minor.
> 
> 3)
> OK we will see if other feed-backs see it.
> Keeping input terms allows to rerun search, it can be a feature not a bug ;)
> 
> I sign off.

Thanks Frido! Does it mean you signoff on the whole tree? This bug depends on several others that are not signed off yet :)
Comment 17 Fridolin Somers 2022-03-17 07:03:42 UTC
Yep looks strong enough to sign.
Comment 18 Fridolin Somers 2022-03-17 08:13:59 UTC
Ahhh the tree contains several bugs not yet signed, I revert
Comment 19 Jonathan Druart 2022-03-17 14:48:40 UTC
*** Bug 29125 has been marked as a duplicate of this bug. ***
Comment 20 Jonathan Druart 2022-03-17 15:25:16 UTC
*** Bug 26865 has been marked as a duplicate of this bug. ***
Comment 21 Jonathan Druart 2022-03-29 08:29:00 UTC
Created attachment 132387 [details] [review]
Bug 30063: Fix table settings
Comment 22 Arthur Suzuki 2022-03-29 11:34:34 UTC
patch doesn't apply on master, although all dependencies do apply properly :)
<pre>
132387 - Bug 30063: Fix table settings

Application de  Bug 30063: Fix table settings
Utilisation de l'information de l'index pour reconstruire un arbre de base...
M	koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt
Retour à un patch de la base et fusion à 3 points...
Fusion automatique de koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt
CONFLIT (contenu) : Conflit de fusion dans koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt
error: Échec d'intégration des modifications.
l'application de la rustine a échoué à 0001 Bug 30063: Fix table settings
astuce: Utilisez 'git am --show-current-patch=diff' pour visualiser le patch en échec
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-30063-Fix-table-settings-7voPZp.patch
</pre>
Comment 23 Séverine Queune 2022-03-29 13:39:32 UTC
This is a first feedback about the member-home.pl page only, tested using the branch available on Joubu's GitLab.

1) About Frido's point 3, I completely agree with the new behaviour that keeps the search performs in the form and then filters the results. As an example, at BULAC, we have plenty of homonyms specifically for Asiatic short names that can be used as firstname or family name and it can be really difficult sometimes to find someone. Having a new way to refine the search is really useful.

2) Sorting by 'Name' isn't logical for a user like me : the table display surname then firstname but sorting uses firstname then surname

3) Some special signs need to be escaped to be search, like % or _
Probably not related to this patch, but searching for % searches for not NULL datas. On 'Notes' filter column e.g., it bring some unexpected results displaying patrons without any circulation notes just because the SQL field is not NULL.

4) I had some strange behaviours using the column filters or general search : many times, the search only applied when I kind of 'refresh' the table by sorting it (see https://snipboard.io/6EGyV8.jpg) To reproduce : enter your search very quickly in the search box or filter box ; see that many results are not supposed to be here.

5) Hiding a column in table setting introduces a bug : the filter doesn't search the data in the selected column but the data of the column just on the left : see screenshots
 - https://snipboard.io/JAdjup.jpg : column 'Card' hidden, filter 'Date' search for the name 
 - https://snipboard.io/1gb9Kq.jpg : column 'Card' hidden, filter 'Name' search for the card 
 - https://snipboard.io/RKB4ip.jpg : column 'Card' hidden, filter 'Checkouts' search for the expiration date 
 - https://snipboard.io/DBX5Ve.jpg :  column 'Name' hidden, filter 'Date' search for the name 
You can also notice on these screenshots that there is some wrong with filter for it's not supposed to exists for the 'Checkout' and 'Actions' columns, and there is no more filter for 'Notes'.

6) It's not possible to got back to the form search if the results bring a unique result : the form reloaded automatically and bring us back to the patron account. This is not the behaviour in master and I feel uncomfortable with that change.

7) On side search form, 'Search type' = 'Starts with' doesn't work for none of the 'Search field' I tried (cardnumber, address, firstname, surname).

8) The patch doesn't work for users with only 'edit_borrowers' or 'view_borrower_infos_from_any_libraries' permissions : the full 'borrowers' permission seems to be needed to display results

All the other points (Length menu, query description, highlight of the current library) are ok expect for length menu displaying two times '20 results' but I'll fill another bug as it's already existing in master and 20.11.11).

I'll feedback about the other tables in the dedicated bugs.
Comment 24 Jonathan Druart 2022-03-30 10:23:26 UTC
Created attachment 132552 [details] [review]
Bug 30063: Sort by "Name" firstname,surname

As it's how it's displayed
Comment 25 Jonathan Druart 2022-03-30 11:42:57 UTC
(In reply to Séverine Queune from comment #23)
> This is a first feedback about the member-home.pl page only, tested using
> the branch available on Joubu's GitLab.
> 
> 1) About Frido's point 3, I completely agree with the new behaviour that
> keeps the search performs in the form and then filters the results. As an
> example, at BULAC, we have plenty of homonyms specifically for Asiatic short
> names that can be used as firstname or family name and it can be really
> difficult sometimes to find someone. Having a new way to refine the search
> is really useful.

Thanks for the feedback!

> 2) Sorting by 'Name' isn't logical for a user like me : the table display
> surname then firstname but sorting uses firstname then surname

Fixed!
Bug 30063: Sort by "Name" firstname,surname

> 3) Some special signs need to be escaped to be search, like % or _
> Probably not related to this patch, but searching for % searches for not
> NULL datas. On 'Notes' filter column e.g., it bring some unexpected results
> displaying patrons without any circulation notes just because the SQL field
> is not NULL.

Bug 30393.

> 4) I had some strange behaviours using the column filters or general search
> : many times, the search only applied when I kind of 'refresh' the table by
> sorting it (see https://snipboard.io/6EGyV8.jpg) To reproduce : enter your
> search very quickly in the search box or filter box ; see that many results
> are not supposed to be here.

Bug 30394.

> 5) Hiding a column in table setting introduces a bug : the filter doesn't
> search the data in the selected column but the data of the column just on
> the left : see screenshots
>  - https://snipboard.io/JAdjup.jpg : column 'Card' hidden, filter 'Date'
> search for the name 
>  - https://snipboard.io/1gb9Kq.jpg : column 'Card' hidden, filter 'Name'
> search for the card 
>  - https://snipboard.io/RKB4ip.jpg : column 'Card' hidden, filter
> 'Checkouts' search for the expiration date 
>  - https://snipboard.io/DBX5Ve.jpg :  column 'Name' hidden, filter 'Date'
> search for the name 
> You can also notice on these screenshots that there is some wrong with
> filter for it's not supposed to exists for the 'Checkout' and 'Actions'
> columns, and there is no more filter for 'Notes'.

Opened bug 30406.

> 6) It's not possible to got back to the form search if the results bring a
> unique result : the form reloaded automatically and bring us back to the
> patron account. This is not the behaviour in master and I feel uncomfortable
> with that change.

Not sure it's fixed, I am getting an error on Firefox but it's working on Chromium.
Bug 30063: Restore back behaviour

> 7) On side search form, 'Search type' = 'Starts with' doesn't work for none
> of the 'Search field' I tried (cardnumber, address, firstname, surname).

Fixed!
Bug 30063: Fix start with

> 8) The patch doesn't work for users with only 'edit_borrowers' or
> 'view_borrower_infos_from_any_libraries' permissions : the full 'borrowers'
> permission seems to be needed to display results

Fixed!
Bug 30063: Fix permission for GET /patrons

> All the other points (Length menu, query description, highlight of the
> current library) are ok expect for length menu displaying two times '20
> results' but I'll fill another bug as it's already existing in master and
> 20.11.11).
> 
> I'll feedback about the other tables in the dedicated bugs.

Thanks a lot!
Comment 26 Séverine Queune 2022-03-30 13:52:05 UTC
Thanks a lot Jonathan !

I fail the patch once again as the sorting by surname then firstname doesn't work.

I've got the same problem on point 6 : it works with Chromium but not Firefox. Weird...

All the other points related to this specific bugs are fixed :)
Comment 27 Jonathan Druart 2022-03-30 15:35:05 UTC
(In reply to Jonathan Druart from comment #25)
> (In reply to Séverine Queune from comment #23)
> > 2) Sorting by 'Name' isn't logical for a user like me : the table display
> > surname then firstname but sorting uses firstname then surname
> 
> Fixed!
> Bug 30063: Sort by "Name" firstname,surname

Replaced by
Bug 30063: Sort by "Name" surname:firstname
Comment 28 Jonathan Druart 2022-03-30 15:45:04 UTC
(In reply to Séverine Queune from comment #26)
> Thanks a lot Jonathan !
> 
> I fail the patch once again as the sorting by surname then firstname doesn't
> work.
> 
> I've got the same problem on point 6 : it works with Chromium but not
> Firefox. Weird...
> 
> All the other points related to this specific bugs are fixed :)

Should be fixed now!
Comment 29 Séverine Queune 2022-03-31 11:41:23 UTC
Tested using Joubu's branch, everything seems fixed !
Comment 30 Jonathan Druart 2022-03-31 12:46:14 UTC
Created attachment 132731 [details] [review]
Bug 30063: Number of checkouts

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 31 Jonathan Druart 2022-03-31 12:46:22 UTC
Created attachment 132732 [details] [review]
Bug 30063: Overdues count

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 32 Jonathan Druart 2022-03-31 12:46:29 UTC
Created attachment 132733 [details] [review]
Bug 30063: Make the main patron search use the REST API

To test:
* Length menu (PatronsPerPage)
* Query description
* Highlight of the current library

* sticky header - Does not work (?)

If the table does not show when you submit the filter form, make sure
you regenerated the compiled CSS.

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 33 Jonathan Druart 2022-03-31 12:46:39 UTC
Created attachment 132734 [details] [review]
Bug 30063: Remove svc/members/search

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 34 Jonathan Druart 2022-03-31 12:46:48 UTC
Created attachment 132735 [details] [review]
Bug 30063: Remove occurrences of C4::Utils::DataTables::Members

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 35 Jonathan Druart 2022-03-31 12:46:57 UTC
Created attachment 132736 [details] [review]
Bug 30063: Fix address format

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 36 Jonathan Druart 2022-03-31 12:47:03 UTC
Created attachment 132737 [details] [review]
Bug 30063: Add kind of POD for the include file

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 37 Jonathan Druart 2022-03-31 12:47:09 UTC
Created attachment 132738 [details] [review]
Bug 30063: Display search description for main search only

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 38 Jonathan Druart 2022-03-31 12:47:17 UTC
Created attachment 132739 [details] [review]
Bug 30063: Fix selenium tests

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 39 Jonathan Druart 2022-03-31 12:47:23 UTC
Created attachment 132740 [details] [review]
Bug 30063: Fix sticky

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 40 Jonathan Druart 2022-03-31 12:47:29 UTC
Created attachment 132741 [details] [review]
Bug 30063: Add selenium tests

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 41 Jonathan Druart 2022-03-31 12:47:34 UTC
Created attachment 132742 [details] [review]
Bug 30063: Be more flexible for filters

On later follow-ups (ERM) we need to filter columns that contain AVs,
and so be more flexible. Here we are expecting a _id and _str keys we
are gonna use to build the select's options

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 42 Jonathan Druart 2022-03-31 12:47:40 UTC
Created attachment 132743 [details] [review]
Bug 30063: Fix table settings

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 43 Jonathan Druart 2022-03-31 12:47:47 UTC
Created attachment 132744 [details] [review]
Bug 30063: Sort by "Name" firstname,surname

As it's how it's displayed

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 44 Jonathan Druart 2022-03-31 12:47:53 UTC
Created attachment 132745 [details] [review]
Bug 30063: Restore back behaviour

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 45 Jonathan Druart 2022-03-31 12:47:59 UTC
Created attachment 132746 [details] [review]
Bug 30063: Fix start with

It was doing "end with"...

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 46 Jonathan Druart 2022-03-31 12:48:06 UTC
Created attachment 132747 [details] [review]
Bug 30063: Fix permission for GET /patrons

edit_borrowers should be enough, we don't need the whole borrowers
module flag

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 47 Jonathan Druart 2022-03-31 12:48:15 UTC
Created attachment 132748 [details] [review]
Bug 30063: Sort by "Name" surname:firstname

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 48 Jonathan Druart 2022-03-31 12:48:22 UTC
Created attachment 132749 [details] [review]
Bug 30063: Fix QA issues

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Comment 49 Martin Renvoize 2022-03-31 13:12:24 UTC
I've spoken to Severine and she tells me she's already highlighted one issue but we both agree it can be handled as a distinct bug.. I'll report it and add screenshots of before and after to highlight the issue

Otherwise, this is great again.
Comment 50 Martin Renvoize 2022-03-31 13:21:25 UTC
Created attachment 132759 [details] [review]
Bug 30063: Number of checkouts

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 51 Martin Renvoize 2022-03-31 13:21:36 UTC
Created attachment 132760 [details] [review]
Bug 30063: Overdues count

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 52 Martin Renvoize 2022-03-31 13:21:43 UTC
Created attachment 132761 [details] [review]
Bug 30063: Make the main patron search use the REST API

To test:
* Length menu (PatronsPerPage)
* Query description
* Highlight of the current library

* sticky header - Does not work (?)

If the table does not show when you submit the filter form, make sure
you regenerated the compiled CSS.

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 53 Martin Renvoize 2022-03-31 13:21:50 UTC
Created attachment 132762 [details] [review]
Bug 30063: Remove svc/members/search

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 54 Martin Renvoize 2022-03-31 13:21:56 UTC
Created attachment 132763 [details] [review]
Bug 30063: Remove occurrences of C4::Utils::DataTables::Members

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 55 Martin Renvoize 2022-03-31 13:22:03 UTC
Created attachment 132764 [details] [review]
Bug 30063: Fix address format

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 56 Martin Renvoize 2022-03-31 13:22:11 UTC
Created attachment 132765 [details] [review]
Bug 30063: Add kind of POD for the include file

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 57 Martin Renvoize 2022-03-31 13:22:18 UTC
Created attachment 132766 [details] [review]
Bug 30063: Display search description for main search only

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 58 Martin Renvoize 2022-03-31 13:22:24 UTC
Created attachment 132767 [details] [review]
Bug 30063: Fix selenium tests

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 59 Martin Renvoize 2022-03-31 13:22:30 UTC
Created attachment 132768 [details] [review]
Bug 30063: Fix sticky

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 60 Martin Renvoize 2022-03-31 13:22:38 UTC
Created attachment 132769 [details] [review]
Bug 30063: Add selenium tests

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 61 Martin Renvoize 2022-03-31 13:22:47 UTC
Created attachment 132770 [details] [review]
Bug 30063: Be more flexible for filters

On later follow-ups (ERM) we need to filter columns that contain AVs,
and so be more flexible. Here we are expecting a _id and _str keys we
are gonna use to build the select's options

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 62 Martin Renvoize 2022-03-31 13:22:54 UTC
Created attachment 132771 [details] [review]
Bug 30063: Fix table settings

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 63 Martin Renvoize 2022-03-31 13:23:01 UTC
Created attachment 132772 [details] [review]
Bug 30063: Sort by "Name" firstname,surname

As it's how it's displayed

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 64 Martin Renvoize 2022-03-31 13:23:07 UTC
Created attachment 132773 [details] [review]
Bug 30063: Restore back behaviour

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 65 Martin Renvoize 2022-03-31 13:23:16 UTC
Created attachment 132774 [details] [review]
Bug 30063: Fix start with

It was doing "end with"...

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 66 Martin Renvoize 2022-03-31 13:23:24 UTC
Created attachment 132775 [details] [review]
Bug 30063: Fix permission for GET /patrons

edit_borrowers should be enough, we don't need the whole borrowers
module flag

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 67 Martin Renvoize 2022-03-31 13:23:30 UTC
Created attachment 132776 [details] [review]
Bug 30063: Sort by "Name" surname:firstname

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 68 Martin Renvoize 2022-03-31 13:23:35 UTC
Created attachment 132777 [details] [review]
Bug 30063: Fix QA issues

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 69 Martin Renvoize 2022-03-31 14:10:28 UTC
OK, I can't find any hard regressions.. minor UX changes and tweeks yes, but not real breakage.

This gives us some great foundations to continue improving upon.. I'd like to see things like the 'overdues' alias for 'get_overdues' actually progress to just replace the 'get_overdues' method instead.. and I'd love to see that same method turned into a relationship at the dbic level to benefit from dbic query handling.

But.. generally I'm more than happy to PQA on this...  I'll add bugs to record my other thougths.
Comment 70 Fridolin Somers 2022-04-04 07:49:03 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 71 Jonathan Druart 2022-04-05 09:53:42 UTC
Thanks to everybody involved in this huge patch series!
Comment 72 Jonathan Druart 2022-04-05 11:26:29 UTC
Created attachment 132958 [details] [review]
Bug 30063: (follow-up) Fix missing TT filters
Comment 73 Fridolin Somers 2022-04-05 15:08:23 UTC
> Bug 30063: (follow-up) Fix missing TT filters
Was also missing :
  $("#[% sticky_header | html %]").show();
I've added my commit :
c636b39a89 Bug 30063: Fix missing TT filters
Then yours :
5fdfed1ee5 Bug 30063: (follow-up) Fix missing TT filters
Comment 74 Martin Renvoize 2022-04-22 15:36:07 UTC
patrons_search.t selenium test is failing and I'm not entirely sure why :(
Comment 75 Jonathan Druart 2022-04-27 10:50:13 UTC
(In reply to Martin Renvoize from comment #74)
> patrons_search.t selenium test is failing and I'm not entirely sure why :(

Fixed on bug 30626.