Bug 30624 - Add a permission to control the ability to change the logged in library
Summary: Add a permission to control the ability to change the logged in library
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Lucas Gass
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-04-26 20:18 UTC by Lucas Gass
Modified: 2023-06-13 16:07 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/685
Text to go in the release notes:
This enhancement adds a new `loggedinlibrary` permission to allow or prevent staff members to set the library where they log in.
Version(s) released in:
23.05.00


Attachments
Bug 30624: Add select_loggedin_library permission (10.55 KB, patch)
2022-06-13 23:16 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: Add select_loggedin_library permission (11.45 KB, patch)
2022-06-14 20:33 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: DB Update to add permission (1.09 KB, patch)
2022-07-14 15:58 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: Add select_loggedin_library permission (11.49 KB, patch)
2022-07-29 20:58 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 30624: DB Update to add permission (1.13 KB, patch)
2022-07-29 20:58 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 30624: Add select_loggedin_library permission (11.45 KB, patch)
2022-08-29 15:56 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: DB Update to add permission (1.13 KB, patch)
2022-08-29 15:56 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: (follow-up) Remove IndependentBraches check on auth.tt (3.63 KB, patch)
2022-08-29 16:02 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: Add select_loggedin_library permission (11.96 KB, patch)
2022-09-28 21:51 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: DB Update to add permission (1.13 KB, patch)
2022-09-28 21:51 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: (follow-up) Remove IndependentBraches check on auth.tt (4.56 KB, patch)
2022-09-28 21:52 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: Add select_loggedin_library permission (11.95 KB, patch)
2022-10-20 16:57 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: DB Update to add permission (1.13 KB, patch)
2022-10-20 16:57 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: (follow-up) Remove IndependentBraches check on auth.tt (4.56 KB, patch)
2022-10-20 16:57 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: Add loggedinlibrary permission and DB update (6.94 KB, patch)
2022-12-13 21:00 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: Unit tests (4.69 KB, patch)
2022-12-13 22:27 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: Add loggedinlibrary permission and DB update (10.65 KB, patch)
2022-12-14 17:35 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: Add loggedinlibrary permission and DB update (11.53 KB, patch)
2022-12-14 21:41 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: Add loggedinlibrary permission and DB update (11.53 KB, patch)
2022-12-14 21:44 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: Unit tests (2.98 KB, patch)
2022-12-14 21:54 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: Add loggedinlibrary permission and DB update (11.57 KB, patch)
2023-01-04 22:21 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: Unit tests (3.03 KB, patch)
2023-01-04 22:21 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: Adjust number of tests (690 bytes, patch)
2023-01-24 16:52 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: (QA follow-up) Use C4:Context to get permssions (2.22 KB, patch)
2023-02-28 17:06 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30624: Add loggedinlibrary permission and DB update (11.61 KB, patch)
2023-03-04 13:38 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 30624: Unit tests (2.84 KB, patch)
2023-03-04 13:38 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 30624: Adjust number of tests (745 bytes, patch)
2023-03-04 13:38 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 30624: (QA follow-up) Use C4:Context to get permissions (2.28 KB, patch)
2023-03-04 13:38 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 30624: Make DB update idempotent (1.02 KB, patch)
2023-03-13 12:18 UTC, Lucas Gass
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass 2022-04-26 20:18:14 UTC
Bug 29887 added the system preference IndependentBranchesLoggedInLibrary. This bug intends to remove that system preference and replace it with a permission: 'select_loggedin_library'
Comment 1 Lucas Gass 2022-06-13 23:16:33 UTC
Created attachment 136055 [details] [review]
Bug 30624: Add select_loggedin_library permission

To test:
1. Apply patch, updatedatabase, restart_all
2. Have a user with superlibrarian privileges ( User1 )
3. Have a user who has staff access and circulate privileges but is not a super librarian. ( User2 ) Make note of this users home library
4. Turn on the system preference 'CircSidebar'.

-MAIN log in ( auth.tt )
5. As User1, go to the main login screen and try logging in. You should be able to log in AND you should be able to properly chnage your branch BEFORE logging in.
6. As User2, to to the main login screnn amd try logging in. You should be able to but if you try and switch your libraray to anything beside the user's home branch it will not work. You will be logged in at your home branch.
7. For User2, set the new permission 'select_loggedin_library' on.
8. Now you should be able to successfully switch libraries before log in.
9. Turn the 'select_loggedin_library' back off for User2.

-AFTER log in-
10. With User1, click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
11. With User2, click on your name/branch in the top right. If you have 'UseCirculationDesks' on, you should see 'Set desk', otherwise you should see nothing.
12. Repeat step 7.
13. NOw if you click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
14. Repeat Step 9.

-CircSideBar-
15. With 'CircSideBar' turned on, go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup) with User1. You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
16. Try with User2 and you will not see a 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set desk' link.
17. Repeat step 7.
18. For with User2 you go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup). You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
19. Repeat step 9.

-Set library page-
20. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User1. You will see a dropdown for 'Set library'. Make sure you can change your library successfully.
21. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. You should NOT see see a dropdown for 'Set library'.
22. Repeat step 7.
23. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. Now you should see a dropdown for 'Set library'.
Comment 2 Lucas Gass 2022-06-14 13:41:53 UTC
Realized this needs a few tweaks before it is ready for testing. Setting to assigned.
Comment 3 Lucas Gass 2022-06-14 20:33:12 UTC
Created attachment 136076 [details] [review]
Bug 30624: Add select_loggedin_library permission

To test:
1. Apply patch, updatedatabase, restart_all
2. Have a user with superlibrarian privileges ( User1 )
3. Have a user who has staff access and circulate privileges but is not a super librarian. ( User2 ) Make note of this users home library
4. Turn on the system preference 'CircSidebar'.

-MAIN log in ( auth.tt )
5. As User1, go to the main login screen and try logging in. You should be able to log in AND you should be able to properly chnage your branch BEFORE logging in.
6. As User2, to to the main login screnn amd try logging in. You should be able to but if you try and switch your libraray to anything beside the user's home branch it will not work. You will be logged in at your home branch.
7. For User2, set the new permission 'select_loggedin_library' on.
8. Now you should be able to successfully switch libraries before log in.
9. Turn the 'select_loggedin_library' back off for User2.

-AFTER log in-
10. With User1, click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
11. With User2, click on your name/branch in the top right. If you have 'UseCirculationDesks' on, you should see 'Set desk', otherwise you should see nothing.
12. Repeat step 7.
13. NOw if you click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
14. Repeat Step 9.

-CircSideBar-
15. With 'CircSideBar' turned on, go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup) with User1. You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
16. Try with User2 and you will not see a 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set desk' link.
17. Repeat step 7.
18. For with User2 you go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup). You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
19. Repeat step 9.

-Set library page-
20. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User1. You will see a dropdown for 'Set library'. Make sure you can change your library successfully.
21. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. You should NOT see see a dropdown for 'Set library'.
22. Repeat step 7.
23. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. Now you should see a dropdown for 'Set library'.
Comment 4 Katrin Fischer 2022-06-14 21:08:42 UTC
Hi Lucas, i believe the plan changed - could you please update the bug description?
Comment 5 Lucas Gass 2022-06-14 21:22:32 UTC
This patch attempts to decouple the ability to change branches from the system preference IndependentBranches.

Instead of having to turn on IndependentBranches, which does a bunch of other things, libraries can now use a permission (select_loggedin_library) to control who can change branches.
Comment 6 AspenCat Team 2022-07-12 20:15:54 UTC
Lucas demonstrated the test plan and I'm signing off on the work - Bob Bennhoff
Comment 7 Lucas Gass 2022-07-14 15:58:44 UTC
Created attachment 137733 [details] [review]
Bug 30624: DB Update to add permission
Comment 8 ByWater Sandboxes 2022-07-29 20:58:35 UTC
Created attachment 138377 [details] [review]
Bug 30624: Add select_loggedin_library permission

To test:
1. Apply patch, updatedatabase, restart_all
2. Have a user with superlibrarian privileges ( User1 )
3. Have a user who has staff access and circulate privileges but is not a super librarian. ( User2 ) Make note of this users home library
4. Turn on the system preference 'CircSidebar'.

-MAIN log in ( auth.tt )
5. As User1, go to the main login screen and try logging in. You should be able to log in AND you should be able to properly chnage your branch BEFORE logging in.
6. As User2, to to the main login screnn amd try logging in. You should be able to but if you try and switch your libraray to anything beside the user's home branch it will not work. You will be logged in at your home branch.
7. For User2, set the new permission 'select_loggedin_library' on.
8. Now you should be able to successfully switch libraries before log in.
9. Turn the 'select_loggedin_library' back off for User2.

-AFTER log in-
10. With User1, click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
11. With User2, click on your name/branch in the top right. If you have 'UseCirculationDesks' on, you should see 'Set desk', otherwise you should see nothing.
12. Repeat step 7.
13. NOw if you click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
14. Repeat Step 9.

-CircSideBar-
15. With 'CircSideBar' turned on, go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup) with User1. You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
16. Try with User2 and you will not see a 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set desk' link.
17. Repeat step 7.
18. For with User2 you go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup). You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
19. Repeat step 9.

-Set library page-
20. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User1. You will see a dropdown for 'Set library'. Make sure you can change your library successfully.
21. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. You should NOT see see a dropdown for 'Set library'.
22. Repeat step 7.
23. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. Now you should see a dropdown for 'Set library'.

Signed-off-by: Andrew <andrewfh@gmail.com>
Comment 9 ByWater Sandboxes 2022-07-29 20:58:39 UTC
Created attachment 138378 [details] [review]
Bug 30624: DB Update to add permission

Signed-off-by: Andrew <andrewfh@gmail.com>
Comment 10 Fridolin Somers 2022-08-13 08:25:13 UTC
We must also impact the staff authentication page no ?
Comment 11 Lucas Gass 2022-08-15 13:45:42 UTC
(In reply to Fridolin Somers from comment #10)
> We must also impact the staff authentication page no ?

Unless I am mistaken by the question, that is handled in the first patch. ( Auth.pm )
Comment 12 Fridolin Somers 2022-08-16 05:28:17 UTC
I mean in authentication TT.
Actually there is only a UNLESS IndependentBranches :

https://git.koha-community.org/Koha-community/Koha/src/commit/fc78953951837f6fe7a3e9d66ac497dcb5511f53/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt#L91
Comment 13 Lucas Gass 2022-08-29 15:56:52 UTC
Created attachment 139961 [details] [review]
Bug 30624: Add select_loggedin_library permission

To test:
1. Apply patch, updatedatabase, restart_all
2. Have a user with superlibrarian privileges ( User1 )
3. Have a user who has staff access and circulate privileges but is not a super librarian. ( User2 ) Make note of this users home library
4. Turn on the system preference 'CircSidebar'.

-MAIN log in ( auth.tt )
5. As User1, go to the main login screen and try logging in. You should be able to log in AND you should be able to properly chnage your branch BEFORE logging in.
6. As User2, to to the main login screnn amd try logging in. You should be able to but if you try and switch your libraray to anything beside the user's home branch it will not work. You will be logged in at your home branch.
7. For User2, set the new permission 'select_loggedin_library' on.
8. Now you should be able to successfully switch libraries before log in.
9. Turn the 'select_loggedin_library' back off for User2.

-AFTER log in-
10. With User1, click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
11. With User2, click on your name/branch in the top right. If you have 'UseCirculationDesks' on, you should see 'Set desk', otherwise you should see nothing.
12. Repeat step 7.
13. NOw if you click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
14. Repeat Step 9.

-CircSideBar-
15. With 'CircSideBar' turned on, go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup) with User1. You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
16. Try with User2 and you will not see a 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set desk' link.
17. Repeat step 7.
18. For with User2 you go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup). You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
19. Repeat step 9.

-Set library page-
20. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User1. You will see a dropdown for 'Set library'. Make sure you can change your library successfully.
21. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. You should NOT see see a dropdown for 'Set library'.
22. Repeat step 7.
23. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. Now you should see a dropdown for 'Set library'.

Signed-off-by: Andrew <andrewfh@gmail.com>
Comment 14 Lucas Gass 2022-08-29 15:56:56 UTC
Created attachment 139962 [details] [review]
Bug 30624: DB Update to add permission

Signed-off-by: Andrew <andrewfh@gmail.com>
Comment 15 Lucas Gass 2022-08-29 15:57:24 UTC
rebased for current master
Comment 16 Lucas Gass 2022-08-29 16:00:38 UTC
(In reply to Fridolin Somers from comment #12)
> I mean in authentication TT.
> Actually there is only a UNLESS IndependentBranches :
> 
> https://git.koha-community.org/Koha-community/Koha/src/commit/
> fc78953951837f6fe7a3e9d66ac497dcb5511f53/koha-tmpl/intranet-tmpl/prog/en/
> modules/auth.tt#L91

Ah, yes! Since that will be handled by a permission we no longer need this check, the dropdown should always show.
Comment 17 Lucas Gass 2022-08-29 16:02:35 UTC
Created attachment 139963 [details] [review]
Bug 30624: (follow-up) Remove IndependentBraches check on auth.tt
Comment 18 Lucas Gass 2022-09-26 19:22:52 UTC
No longer applies to current master, rebase coming.
Comment 19 Lucas Gass 2022-09-28 21:51:53 UTC
Created attachment 141069 [details] [review]
Bug 30624: Add select_loggedin_library permission

To test:
1. Apply patch, updatedatabase, restart_all
2. Have a user with superlibrarian privileges ( User1 )
3. Have a user who has staff access and circulate privileges but is not a super librarian. ( User2 ) Make note of this users home library
4. Turn on the system preference 'CircSidebar'.

-MAIN log in ( auth.tt )
5. As User1, go to the main login screen and try logging in. You should be able to log in AND you should be able to properly chnage your branch BEFORE logging in.
6. As User2, to to the main login screnn amd try logging in. You should be able to but if you try and switch your libraray to anything beside the user's home branch it will not work. You will be logged in at your home branch.
7. For User2, set the new permission 'select_loggedin_library' on.
8. Now you should be able to successfully switch libraries before log in.
9. Turn the 'select_loggedin_library' back off for User2.

-AFTER log in-
10. With User1, click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
11. With User2, click on your name/branch in the top right. If you have 'UseCirculationDesks' on, you should see 'Set desk', otherwise you should see nothing.
12. Repeat step 7.
13. NOw if you click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
14. Repeat Step 9.

-CircSideBar-
15. With 'CircSideBar' turned on, go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup) with User1. You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
16. Try with User2 and you will not see a 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set desk' link.
17. Repeat step 7.
18. For with User2 you go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup). You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
19. Repeat step 9.

-Set library page-
20. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User1. You will see a dropdown for 'Set library'. Make sure you can change your library successfully.
21. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. You should NOT see see a dropdown for 'Set library'.
22. Repeat step 7.
23. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. Now you should see a dropdown for 'Set library'.

Signed-off-by: Andrew <andrewfh@gmail.com>
Comment 20 Lucas Gass 2022-09-28 21:51:58 UTC
Created attachment 141070 [details] [review]
Bug 30624: DB Update to add permission

Signed-off-by: Andrew <andrewfh@gmail.com>
Comment 21 Lucas Gass 2022-09-28 21:52:04 UTC
Created attachment 141071 [details] [review]
Bug 30624: (follow-up) Remove IndependentBraches check on auth.tt
Comment 22 Katrin Fischer 2022-10-16 15:02:40 UTC
There is a change on set-library.tt causing a conflict that I am not sure enough about to resolve. Can you please rebase?
Comment 23 Lucas Gass 2022-10-20 16:57:25 UTC
Created attachment 142269 [details] [review]
Bug 30624: Add select_loggedin_library permission

To test:
1. Apply patch, updatedatabase, restart_all
2. Have a user with superlibrarian privileges ( User1 )
3. Have a user who has staff access and circulate privileges but is not a super librarian. ( User2 ) Make note of this users home library
4. Turn on the system preference 'CircSidebar'.

-MAIN log in ( auth.tt )
5. As User1, go to the main login screen and try logging in. You should be able to log in AND you should be able to properly chnage your branch BEFORE logging in.
6. As User2, to to the main login screnn amd try logging in. You should be able to but if you try and switch your libraray to anything beside the user's home branch it will not work. You will be logged in at your home branch.
7. For User2, set the new permission 'select_loggedin_library' on.
8. Now you should be able to successfully switch libraries before log in.
9. Turn the 'select_loggedin_library' back off for User2.

-AFTER log in-
10. With User1, click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
11. With User2, click on your name/branch in the top right. If you have 'UseCirculationDesks' on, you should see 'Set desk', otherwise you should see nothing.
12. Repeat step 7.
13. NOw if you click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
14. Repeat Step 9.

-CircSideBar-
15. With 'CircSideBar' turned on, go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup) with User1. You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
16. Try with User2 and you will not see a 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set desk' link.
17. Repeat step 7.
18. For with User2 you go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup). You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
19. Repeat step 9.

-Set library page-
20. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User1. You will see a dropdown for 'Set library'. Make sure you can change your library successfully.
21. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. You should NOT see see a dropdown for 'Set library'.
22. Repeat step 7.
23. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. Now you should see a dropdown for 'Set library'.

Signed-off-by: Andrew <andrewfh@gmail.com>
Comment 24 Lucas Gass 2022-10-20 16:57:33 UTC
Created attachment 142270 [details] [review]
Bug 30624: DB Update to add permission

Signed-off-by: Andrew <andrewfh@gmail.com>
Comment 25 Lucas Gass 2022-10-20 16:57:38 UTC
Created attachment 142271 [details] [review]
Bug 30624: (follow-up) Remove IndependentBraches check on auth.tt
Comment 26 Lucas Gass 2022-10-20 16:58:12 UTC
Rebased for current master
Comment 27 Katrin Fischer 2022-10-27 19:06:39 UTC
Back here.
Comment 28 Katrin Fischer 2022-10-27 19:41:53 UTC
There are several remaining issues here :(

1) Database update

The database update takes care of adding the permission, but it doesn't take care to update the existing staff accounts. With this patch as is people would loose the ability to change libraries on update which would be a major behavior change.

As you remove IndependentBranches checks, we should factor this in as well. I guess if IndependentBranches is used, don't update non-superlibrarians. If it isn't used, update non-superlibrarians with the new permission.

2) Unit test

The change to Auth.pm requires a unit test

3) The permission check in Auth.pm doesn't look quite right... maybe instead of $flags->{superlibrarian} $patron->is_superlibrarian should be used? 

4) Why do you remove the check in koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt in auth.tt without replacing it with a permission check?

5) Permission

I am not sure the permission is a good fit for the "Administration" module. So far, we use this for access to the different parts of the administration start page only. It feels more at the same level as staff_access maybe?

6) Login page staff

The page still offers the library pull down, as of course, the permissions can be different by user. But maybe we should give a warning if someone who can't do so tries to log in with another library? (non-blocker)

7) Translatability

I believe this construct is problematic or all TT variables would show up in translations:

+[% SET page_title = "" %]
+[% IF ( CAN_user_parameters_select_loggedin_library ) %]
+    [% IF Koha.Preference('UseCirculationDesks') %]
+        [% page_title = "Set library and desk" %]
+    [% ELSE %]
+        [% page_title = "Set library" %]
+    [% END %]
+[% ELSE %]
+    [% IF Koha.Preference('UseCirculationDesks') %]
+        [% page_title = "Set desk"%]
+    [% END %]
+[% END %]
Comment 29 Lucas Gass 2022-10-28 14:41:17 UTC
> 5) Permission
> 
> I am not sure the permission is a good fit for the "Administration" module.
> So far, we use this for access to the different parts of the administration
> start page only. It feels more at the same level as staff_access maybe?

Thanks for taking the time to look through this Cait! To clarify, do you think 'select_loggedin_library' subpermisson should go under "Allow staff members to modify permissions, usernames, and passwords for other staff members (staffaccess)"? 

As I started playing around with this I noticed another possible bug, Bug 32036.
Comment 30 Katrin Fischer 2022-10-28 14:58:43 UTC
Hi Lucas, I think you need at least 2 sub-permissions then, that's why it's not working. Like:  new thing X and remaining.
What I meant is that I see it more as a top level or maybe as a sub-permission of a new module as it doesn't fit with the rest... but I am not sure how to phrase it.
Comment 31 Lucas Gass 2022-12-13 21:00:39 UTC
Created attachment 144557 [details] [review]
Bug 30624: Add loggedinlibrary permission and DB update
Comment 32 Lucas Gass 2022-12-13 22:27:16 UTC
Created attachment 144559 [details] [review]
Bug 30624: Unit tests

To test:
1. Apply patch, updatedatabase, restart_all
2. Have a user with superlibrarian privileges ( User1 )
3. Have a user who has staff access and circulate privileges but is not a super librarian. ( User2 ) Make note of this users home library
4. Turn on the system preference 'CircSidebar'.

-MAIN log in ( auth.tt )
5. As User1, go to the main login screen and try logging in. You should be able to log in AND you should be able to properly chnage your branch BEFORE logging in.
6. As User2, to to the main login screnn amd try logging in. You should be able to but if you try and switch your libraray to anything beside the user's home branch it will not work. You will be logged in at your home branch.
7. For User2, set the new top level permission 'Allow staff to change logged in library (loggedinlibrary).
8. Now you should be able to successfully switch libraries before log in.
9. Turn the 'loggedinlibrary' permission back off for User2.

-AFTER log in-
10. With User1, click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
11. With User2, click on your name/branch in the top right. If you have 'UseCirculationDesks' on, you should see 'Set desk', otherwise you should see nothing.
12. Repeat step 7.
13. NOw if you click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
14. Repeat Step 9.

-CircSideBar-
15. With 'CircSideBar' turned on, go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup) with User1. You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
16. Try with User2 and you will not see a 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set desk' link.
17. Repeat step 7.
18. For with User2 you go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup). You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
19. Repeat step 9.

-Set library page-
20. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User1. You will see a dropdown for 'Set library'. Make sure you can change your library successfully.
21. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. You should NOT see see a dropdown for 'Set library'.
22. Repeat step 7.
23. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. Now you should see a dropdown for 'Set library'.
Comment 33 Lucas Gass 2022-12-14 17:35:05 UTC
Created attachment 144584 [details] [review]
Bug 30624: Add loggedinlibrary permission and DB update

To test:
1. Apply patch, updatedatabase, restart_all
2. Have a user with superlibrarian privileges ( User1 )
3. Have a user who has staff access and circulate privileges but is not a super librarian. ( User2 ) Make note of this users home library
4. Turn on the system preference 'CircSidebar'.

-MAIN log in ( auth.tt )
5. As User1, go to the main login screen and try logging in. You should be able to log in AND you should be able to properly chnage your branch BEFORE logging in.
6. As User2, to to the main login screnn amd try logging in. You should be able to but if you try and switch your libraray to anything beside the user's home branch it will not work. You will be logged in at your home branch.
7. For User2, set the new top level permission 'Allow staff to change logged in library (loggedinlibrary).
8. Now you should be able to successfully switch libraries before log in.
9. Turn the 'loggedinlibrary' permission back off for User2.

-AFTER log in-
10. With User1, click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
11. With User2, click on your name/branch in the top right. If you have 'UseCirculationDesks' on, you should see 'Set desk', otherwise you should see nothing.
12. Repeat step 7.
13. NOw if you click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
14. Repeat Step 9.

-CircSideBar-
15. With 'CircSideBar' turned on, go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup) with User1. You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
16. Try with User2 and you will not see a 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set desk' link.
17. Repeat step 7.
18. For with User2 you go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup). You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
19. Repeat step 9.

-Set library page-
20. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User1. You will see a dropdown for 'Set library'. Make sure you can change your library successfully.
21. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. You should NOT see see a dropdown for 'Set library'.
22. Repeat step 7.
23. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. Now you should see a dropdown for 'Set library'.
Comment 34 Lucas Gass 2022-12-14 21:41:54 UTC
Created attachment 144594 [details] [review]
Bug 30624: Add loggedinlibrary permission and DB update

To test:
1. Apply patch, updatedatabase, restart_all
2. Have a user with superlibrarian privileges ( User1 )
3. Have a user who has staff access and circulate privileges but is not a super librarian. ( User2 ) Make note of this users home library
4. Turn on the system preference 'CircSidebar'.

-MAIN log in ( auth.tt )
5. As User1, go to the main login screen and try logging in. You should be able to log in AND you should be able to properly chnage your branch BEFORE logging in.
6. As User2, to to the main login screnn amd try logging in. You should be able to but if you try and switch your libraray to anything beside the user's home branch it will not work. You will be logged in at your home branch.
7. For User2, set the new top level permission 'Allow staff to change logged in library (loggedinlibrary).
8. Now you should be able to successfully switch libraries before log in.
9. Turn the 'loggedinlibrary' permission back off for User2.

-AFTER log in-
10. With User1, click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
11. With User2, click on your name/branch in the top right. If you have 'UseCirculationDesks' on, you should see 'Set desk', otherwise you should see nothing.
12. Repeat step 7.
13. NOw if you click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
14. Repeat Step 9.

-CircSideBar-
15. With 'CircSideBar' turned on, go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup) with User1. You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
16. Try with User2 and you will not see a 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set desk' link.
17. Repeat step 7.
18. For with User2 you go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup). You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
19. Repeat step 9.

-Set library page-
20. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User1. You will see a dropdown for 'Set library'. Make sure you can change your library successfully.
21. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. You should NOT see see a dropdown for 'Set library'.
22. Repeat step 7.
23. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. Now you should see a dropdown for 'Set library'.
Comment 35 Lucas Gass 2022-12-14 21:44:30 UTC
Created attachment 144595 [details] [review]
Bug 30624: Add loggedinlibrary permission and DB update

To test:
1. Apply patch, updatedatabase, restart_all
2. Have a user with superlibrarian privileges ( User1 )
3. Have a user who has staff access and circulate privileges but is not a super librarian. ( User2 ) Make note of this users home library
4. Turn on the system preference 'CircSidebar'.

-MAIN log in ( auth.tt )
5. As User1, go to the main login screen and try logging in. You should be able to log in AND you should be able to properly chnage your branch BEFORE logging in.
6. As User2, to to the main login screnn amd try logging in. You should be able to but if you try and switch your libraray to anything beside the user's home branch it will not work. You will be logged in at your home branch.
7. For User2, set the new top level permission 'Allow staff to change logged in library (loggedinlibrary).
8. Now you should be able to successfully switch libraries before log in.
9. Turn the 'loggedinlibrary' permission back off for User2.

-AFTER log in-
10. With User1, click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
11. With User2, click on your name/branch in the top right. If you have 'UseCirculationDesks' on, you should see 'Set desk', otherwise you should see nothing.
12. Repeat step 7.
13. NOw if you click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
14. Repeat Step 9.

-CircSideBar-
15. With 'CircSideBar' turned on, go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup) with User1. You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
16. Try with User2 and you will not see a 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set desk' link.
17. Repeat step 7.
18. For with User2 you go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup). You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
19. Repeat step 9.

-Set library page-
20. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User1. You will see a dropdown for 'Set library'. Make sure you can change your library successfully.
21. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. You should NOT see see a dropdown for 'Set library'.
22. Repeat step 7.
23. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. Now you should see a dropdown for 'Set library'.
Comment 36 Lucas Gass 2022-12-14 21:54:01 UTC
Created attachment 144596 [details] [review]
Bug 30624: Unit tests
Comment 37 Lucas Gass 2023-01-04 22:21:08 UTC
Created attachment 145048 [details] [review]
Bug 30624: Add loggedinlibrary permission and DB update

To test:
1. Apply patch, updatedatabase, restart_all
2. Have a user with superlibrarian privileges ( User1 )
3. Have a user who has staff access and circulate privileges but is not a super librarian. ( User2 ) Make note of this users home library
4. Turn on the system preference 'CircSidebar'.

-MAIN log in ( auth.tt )
5. As User1, go to the main login screen and try logging in. You should be able to log in AND you should be able to properly chnage your branch BEFORE logging in.
6. As User2, to to the main login screnn amd try logging in. You should be able to but if you try and switch your libraray to anything beside the user's home branch it will not work. You will be logged in at your home branch.
7. For User2, set the new top level permission 'Allow staff to change logged in library (loggedinlibrary).
8. Now you should be able to successfully switch libraries before log in.
9. Turn the 'loggedinlibrary' permission back off for User2.

-AFTER log in-
10. With User1, click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
11. With User2, click on your name/branch in the top right. If you have 'UseCirculationDesks' on, you should see 'Set desk', otherwise you should see nothing.
12. Repeat step 7.
13. NOw if you click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
14. Repeat Step 9.

-CircSideBar-
15. With 'CircSideBar' turned on, go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup) with User1. You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
16. Try with User2 and you will not see a 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set desk' link.
17. Repeat step 7.
18. For with User2 you go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup). You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
19. Repeat step 9.

-Set library page-
20. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User1. You will see a dropdown for 'Set library'. Make sure you can change your library successfully.
21. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. You should NOT see see a dropdown for 'Set library'.
22. Repeat step 7.
23. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. Now you should see a dropdown for 'Set library'.

Signed-off by: Bob Bennhoff/AspenCat Team
Comment 38 Lucas Gass 2023-01-04 22:21:13 UTC
Created attachment 145049 [details] [review]
Bug 30624: Unit tests

Signed-off by: Bob Bennhoff/AspenCat Team
Comment 39 Lucas Gass 2023-01-24 16:52:21 UTC
Created attachment 145631 [details] [review]
Bug 30624: Adjust number of tests
Comment 40 Katrin Fischer 2023-02-27 21:17:14 UTC
1) We have a mix now of $flags->{superlibrarian} == 1 and $patron->is_superlibrarian

I remember that is_superlibrarian was created in order to fix and avoid a lot of false/wrong permission checks for superlibrarian in the codebase. I am not sure if the flags will work correctly, maybe someone else could weigh in? Should we be consistent or doesn't it matter?

2) Database update

UPDATE borrowers SET flags = flags + (1<<29) WHERE flags & 4

Can you explain that one to me? I have written stuff like that before, but it's not coming back to me right now. I had expected something like "where flags != 0 and IS NOT NULL"?

3) Changing home library

The staff user can still change their own home branch and circumvent the limitation that way. Should we make the library read only when the permission is missing?

These are mostly questions, hope we can move this forward!
Comment 41 Lucas Gass 2023-02-28 17:02:59 UTC
(In reply to Katrin Fischer from comment #40)
> 1) We have a mix now of $flags->{superlibrarian} == 1 and
> $patron->is_superlibrarian
> 
> I remember that is_superlibrarian was created in order to fix and avoid a
> lot of false/wrong permission checks for superlibrarian in the codebase. I
> am not sure if the flags will work correctly, maybe someone else could weigh
> in? Should we be consistent or doesn't it matter?

The inconsistency is set-library.pl. Instead of building a patron object so that we can use $patron->is_superlibrarian I think we can use C4::Context for the check.

> 2) Database update
> 
> UPDATE borrowers SET flags = flags + (1<<29) WHERE flags & 4
> 
> Can you explain that one to me? I have written stuff like that before, but
> it's not coming back to me right now. I had expected something like "where
> flags != 0 and IS NOT NULL"?

Using 1<<29 since borrower.flags is stored in bits. It's the bitwise operator '<<'. (right shift) I used in order to preserve other permissions set in borrower.flag. It takes the original value and adds (1<<29).

> 3) Changing home library
> 
> The staff user can still change their own home branch and circumvent the
> limitation that way. Should we make the library read only when the
> permission is missing?
Good point. Do you think that is within the scope of this bug?
Comment 42 Lucas Gass 2023-02-28 17:06:46 UTC
Created attachment 147509 [details] [review]
Bug 30624: (QA follow-up) Use C4:Context to get permssions
Comment 43 Lucas Gass 2023-02-28 17:07:56 UTC
In an effoft to keep this moving I am removing the 'FQA' status since I answered the questions asked and provided a follow-up.
Comment 44 Katrin Fischer 2023-02-28 17:08:30 UTC
 
> > 2) Database update
> > 
> > UPDATE borrowers SET flags = flags + (1<<29) WHERE flags & 4
> > 
> > Can you explain that one to me? I have written stuff like that before, but
> > it's not coming back to me right now. I had expected something like "where
> > flags != 0 and IS NOT NULL"?
> 
> Using 1<<29 since borrower.flags is stored in bits. It's the bitwise
> operator '<<'. (right shift) I used in order to preserve other permissions
> set in borrower.flag. It takes the original value and adds (1<<29).

I understood that part, I was wondering about the flags & 4 in the where condition :)
> 
> > 3) Changing home library
> > 
> > The staff user can still change their own home branch and circumvent the
> > limitation that way. Should we make the library read only when the
> > permission is missing?
> Good point. Do you think that is within the scope of this bug?

I am torn, I think it would make it feature complete, but also ok to handle as a follow-up. What do you think?
Comment 45 Lucas Gass 2023-02-28 17:32:41 UTC
> 2) Database update
> 
> UPDATE borrowers SET flags = flags + (1<<29) WHERE flags & 4
> 
> Can you explain that one to me? I have written stuff like that before, but
> it's not coming back to me right now. I had expected something like "where
> flags != 0 and IS NOT NULL"?

Because I want to set the new loggedinlibrary permission for patrons that have the staff access permission when IndependentBranches is turned on. 

Now that I think about it what you say would work too "WHERE flags != 0 and IS NOT NULL". 

Happy to change it if you'd like!
Comment 46 Lucas Gass 2023-03-02 14:15:02 UTC
(In reply to Lucas Gass from comment #45)
> > 2) Database update
> > 
> > UPDATE borrowers SET flags = flags + (1<<29) WHERE flags & 4
> > 
> > Can you explain that one to me? I have written stuff like that before, but
> > it's not coming back to me right now. I had expected something like "where
> > flags != 0 and IS NOT NULL"?
> 
> Because I want to set the new loggedinlibrary permission for patrons that
> have the staff access permission when IndependentBranches is turned on. 
> 
> Now that I think about it what you say would work too "WHERE flags != 0 and
> IS NOT NULL". 
> 
> Happy to change it if you'd like!

Nick brought up a good point. We should specifically check for the staff access (catalogue) permission. Some API accounts may have other permissions but not staff access.
Comment 47 Lucas Gass 2023-03-02 14:54:43 UTC
> > > 3) Changing home library
> > > 
> > > The staff user can still change their own home branch and circumvent the
> > > limitation that way. Should we make the library read only when the
> > > permission is missing?

We probably need to limit which branches we pass to the template. If someone is acting nefariously they could potentially use the browser dev tools to remove a readonly attribute.
Comment 48 Lucas Gass 2023-03-02 17:39:07 UTC
> > 3) Changing home library
> > 
> > The staff user can still change their own home branch and circumvent the
> > limitation that way. Should we make the library read only when the
> > permission is missing?

Cait, I discussed this a bit more with Nick. We don't think we need any additional work here. There is a the 'view_borrower_infos_from_any_libraries' subpermission to take care of this. Do you agree?
Comment 49 Katrin Fischer 2023-03-02 22:07:33 UTC
(In reply to Lucas Gass from comment #48)
> > > 3) Changing home library
> > > 
> > > The staff user can still change their own home branch and circumvent the
> > > limitation that way. Should we make the library read only when the
> > > permission is missing?
> 
> Cait, I discussed this a bit more with Nick. We don't think we need any
> additional work here. There is a the
> 'view_borrower_infos_from_any_libraries' subpermission to take care of this.
> Do you agree?

I think we could make it a separate bug, but I don't think that 'view_borrower_infos_from_any_libraries covers it, unless it removes the ability to reset my own (staff accounts) homebranch?
Comment 50 Lucas Gass 2023-03-03 13:17:40 UTC
> I think we could make it a separate bug, but I don't think that
> 'view_borrower_infos_from_any_libraries covers it, unless it removes the
> ability to reset my own (staff accounts) homebranch?

It does. Without the view_borrower_infos_from_any_libraries a staff member cannot change their own branch or anyone else's.
Comment 51 Katrin Fischer 2023-03-03 13:58:53 UTC
(In reply to Lucas Gass from comment #50)
> > I think we could make it a separate bug, but I don't think that
> > 'view_borrower_infos_from_any_libraries covers it, unless it removes the
> > ability to reset my own (staff accounts) homebranch?
> 
> It does. Without the view_borrower_infos_from_any_libraries a staff member
> cannot change their own branch or anyone else's.

Interesting! I didn't know that. I wonder how we could advertise that better somehow. Separate issue, but something to think about. I would have seen it more as part of this permission. Maybe both should have the effect? (not blocking, just thinking loudly about possible next steps)
Comment 52 Katrin Fischer 2023-03-04 13:37:21 UTC
(In reply to Lucas Gass from comment #46)
> (In reply to Lucas Gass from comment #45)
> 
> Nick brought up a good point. We should specifically check for the staff
> access (catalogue) permission. Some API accounts may have other permissions
> but not staff access.

Makes sense, thanks!
Comment 53 Katrin Fischer 2023-03-04 13:38:13 UTC
Created attachment 147753 [details] [review]
Bug 30624: Add loggedinlibrary permission and DB update

To test:
1. Apply patch, updatedatabase, restart_all
2. Have a user with superlibrarian privileges ( User1 )
3. Have a user who has staff access and circulate privileges but is not a super librarian. ( User2 ) Make note of this users home library
4. Turn on the system preference 'CircSidebar'.

-MAIN log in ( auth.tt )
5. As User1, go to the main login screen and try logging in. You should be able to log in AND you should be able to properly chnage your branch BEFORE logging in.
6. As User2, to to the main login screnn amd try logging in. You should be able to but if you try and switch your libraray to anything beside the user's home branch it will not work. You will be logged in at your home branch.
7. For User2, set the new top level permission 'Allow staff to change logged in library (loggedinlibrary).
8. Now you should be able to successfully switch libraries before log in.
9. Turn the 'loggedinlibrary' permission back off for User2.

-AFTER log in-
10. With User1, click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
11. With User2, click on your name/branch in the top right. If you have 'UseCirculationDesks' on, you should see 'Set desk', otherwise you should see nothing.
12. Repeat step 7.
13. NOw if you click on your name/branch in the top right, you should see the the link 'Set library' at the top. If you turn on 'UseCirculationDesks' the link will be 'Set library and desk'.
14. Repeat Step 9.

-CircSideBar-
15. With 'CircSideBar' turned on, go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup) with User1. You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
16. Try with User2 and you will not see a 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set desk' link.
17. Repeat step 7.
18. For with User2 you go to any ciculation page (Holds queue, Holds to pull, Holds awaiting pickup). You will see the 'Set library' link. If 'UseCirculationDesks' is on you will see a 'Set library and desk'.
19. Repeat step 9.

-Set library page-
20. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User1. You will see a dropdown for 'Set library'. Make sure you can change your library successfully.
21. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. You should NOT see see a dropdown for 'Set library'.
22. Repeat step 7.
23. Go to the set library page (http://localhost:8081/cgi-bin/koha/circ/set-library.pl) with User2. Now you should see a dropdown for 'Set library'.

Signed-off by: Bob Bennhoff/AspenCat Team

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 54 Katrin Fischer 2023-03-04 13:38:17 UTC
Created attachment 147754 [details] [review]
Bug 30624: Unit tests

Signed-off by: Bob Bennhoff/AspenCat Team

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 55 Katrin Fischer 2023-03-04 13:38:20 UTC
Created attachment 147755 [details] [review]
Bug 30624: Adjust number of tests

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 56 Katrin Fischer 2023-03-04 13:38:23 UTC
Created attachment 147756 [details] [review]
Bug 30624: (QA follow-up) Use C4:Context to get permissions

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 57 Tomás Cohen Arazi 2023-03-10 13:05:47 UTC
I see this error when trying to run the atomicupdate:

ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Out of range value for column 'flags' at row 80 at /kohadevbox/koha/C4/Installer.pm line 739
Comment 58 Lucas Gass 2023-03-10 13:22:08 UTC
(In reply to Tomás Cohen Arazi from comment #57)
> I see this error when trying to run the atomicupdate:
> 
> ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Out of range
> value for column 'flags' at row 80 at /kohadevbox/koha/C4/Installer.pm line
> 739

I don't recreate this. Can you tell me how you are?
Comment 59 Lucas Gass 2023-03-10 13:42:31 UTC
(In reply to Lucas Gass from comment #58)
> (In reply to Tomás Cohen Arazi from comment #57)
> > I see this error when trying to run the atomicupdate:
> > 
> > ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Out of range
> > value for column 'flags' at row 80 at /kohadevbox/koha/C4/Installer.pm line
> > 739
> 
> I don't recreate this. Can you tell me how you are?

I see, when permissions are set a certain way the binary becomes too big too fit in the flags column.
Comment 60 Lucas Gass 2023-03-10 13:43:00 UTC
too big*
Comment 61 Katrin Fischer 2023-03-10 13:52:52 UTC
Hm. What's the best fix here? Should we extend the borrowers.flags (and deletedborrowers and borrower_modifications) on a separate bug?

Ultimately we should move away from the bit structure, but maybe we could do a "quick" fix and not push a permission rewrite on this one.
Comment 62 Lucas Gass 2023-03-13 12:18:03 UTC
Created attachment 148116 [details] [review]
Bug 30624: Make DB update idempotent
Comment 63 Lucas Gass 2023-03-13 12:19:15 UTC
(In reply to Katrin Fischer from comment #61)
> Hm. What's the best fix here? Should we extend the borrowers.flags (and
> deletedborrowers and borrower_modifications) on a separate bug?
> 
> Ultimately we should move away from the bit structure, but maybe we could do
> a "quick" fix and not push a permission rewrite on this one.

I think the best fix for this bug is to make the DB update idempotent. I have added a follow-up to do so.
Comment 64 Tomás Cohen Arazi 2023-03-13 19:05:31 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 65 Matt Blenkinsop 2023-03-14 10:39:13 UTC
Enhancement - not backporting to 22.11.x

Nice work everyone!