Bug 37562

Summary: Duplicate patron check when user cannot see patron leads to a blank popup
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: PatronsAssignee: Nick Clemens (kidclamp) <nick>
Status: Pushed to oldstable --- QA Contact: Emily Lamancusa (emlam) <emily.lamancusa>
Severity: normal    
Priority: P5 - low CC: emily.lamancusa, fridolin.somers, gmcharlt, kyle.m.hall, lucas
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00,24.05.06
Circulation function:
Attachments: Bug 37562: Add patron title to duplicate patron warning and don't show popup if cannot see patron
Bug 37562: Add patron title to duplicate patron warning and don't show popup if cannot see patron
Bug 37562: Add patron title to duplicate patron warning and don't show popup if cannot see patron
Bug 37562: (QA follow-up) Remove FIXME

Description Nick Clemens (kidclamp) 2024-08-02 17:42:11 UTC
To recreate:
1 - Edit a user to grant catalogue and all borrower permissions except 'view_borrower_infos_from_any_libraries'
2 - Find a patron from a different library and note surname and firstname
3 - Login as the patron above
4 - Enter a new patron with the same surname and firstname
5 - See the 'Duplicate patron' warning
6 - Click to view the patron
7 - No info is listed
Comment 1 Nick Clemens (kidclamp) 2024-08-02 17:47:14 UTC
Created attachment 170011 [details] [review]
Bug 37562: Add patron title to duplicate patron warning and don't show popup if cannot see patron

This patch passes the suspected duplicate to the template and uses patron title to display very brief info.
If the user cannot view the patron there is no longer a link tot he brief popup and they will only see
'A patron from library X"

There is a FIXME asking if we should use search_limited - I believe we should check all branches, so the staff can ask the patron if they
have an exising account in a consortium depending on rules about multiple cards

To test:
 1 - Edit a user to grant catalogue and all borrower permissions except 'view_borrower_infos_from_any_libraries'
 2 - Find a patron from a different library and note surname and firstname
 3 - Login as the patron above
 4 - Enter a new patron with the same surname and firstname
 5 - See the 'Duplicate patron' warning
 6 - Click to view the patron
 7 - No info is listed
 8 - Apply patch
 9 - Reload and resubmit - or fill out form again
10 - Note that you see 'A patron from library XXX' and no popup link
11 - Add view_borrower_infos_from_any_libraries to the staff
12 - Repeat the duplication and confirm the warning now has patron name and the popup link is visible and works
Comment 2 Sam Lau 2024-08-02 18:33:29 UTC
Created attachment 170020 [details] [review]
Bug 37562: Add patron title to duplicate patron warning and don't show popup if cannot see patron

This patch passes the suspected duplicate to the template and uses patron title to display very brief info.
If the user cannot view the patron there is no longer a link tot he brief popup and they will only see
'A patron from library X"

There is a FIXME asking if we should use search_limited - I believe we should check all branches, so the staff can ask the patron if they
have an exising account in a consortium depending on rules about multiple cards

To test:
 1 - Edit a user to grant catalogue and all borrower permissions except 'view_borrower_infos_from_any_libraries'
 2 - Find a patron from a different library and note surname and firstname
 3 - Login as the patron above
 4 - Enter a new patron with the same surname and firstname
 5 - See the 'Duplicate patron' warning
 6 - Click to view the patron
 7 - No info is listed
 8 - Apply patch
 9 - Reload and resubmit - or fill out form again
10 - Note that you see 'A patron from library XXX' and no popup link
11 - Add view_borrower_infos_from_any_libraries to the staff
12 - Repeat the duplication and confirm the warning now has patron name and the popup link is visible and works

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 3 Emily Lamancusa (emlam) 2024-08-14 15:53:07 UTC
Created attachment 170323 [details] [review]
Bug 37562: Add patron title to duplicate patron warning and don't show popup if cannot see patron

This patch passes the suspected duplicate to the template and uses patron title to display very brief info.
If the user cannot view the patron there is no longer a link tot he brief popup and they will only see
'A patron from library X"

There is a FIXME asking if we should use search_limited - I believe we should check all branches, so the staff can ask the patron if they
have an exising account in a consortium depending on rules about multiple cards

To test:
 1 - Edit a user to grant catalogue and all borrower permissions except 'view_borrower_infos_from_any_libraries'
 2 - Find a patron from a different library and note surname and firstname
 3 - Login as the patron above
 4 - Enter a new patron with the same surname and firstname
 5 - See the 'Duplicate patron' warning
 6 - Click to view the patron
 7 - No info is listed
 8 - Apply patch
 9 - Reload and resubmit - or fill out form again
10 - Note that you see 'A patron from library XXX' and no popup link
11 - Add view_borrower_infos_from_any_libraries to the staff
12 - Repeat the duplication and confirm the warning now has patron name and the popup link is visible and works

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 4 Emily Lamancusa (emlam) 2024-08-14 15:59:09 UTC
Created attachment 170324 [details] [review]
Bug 37562: (QA follow-up) Remove FIXME
Comment 5 Emily Lamancusa (emlam) 2024-08-14 16:12:27 UTC
This looks good! I agree with your reasoning on keeping the full search - changing that behavior now would likely be seen as a regression to a consortium whose data privacy rules are stricter than their card reciprocity rules. At this point it makes sense to remove that FIXME, since the commit message justifies keeping the existing behavior. For the rest, the patch works nicely and the QA script is happy. Passing QA
Comment 6 Katrin Fischer 2024-08-16 15:37:32 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 7 Lucas Gass (lukeg) 2024-11-18 18:48:04 UTC
Backported to 24.05.x for upcoming 24.05.06
Comment 8 Fridolin Somers 2024-12-06 09:21:33 UTC
Does not apply easy on 23.11.x
Please provide rebased patches if you can :)