Bug 34188 - Require library selection when logging in
Summary: Require library selection when logging in
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Sam Lau
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks: 35259
  Show dependency treegraph
 
Reported: 2023-07-03 15:26 UTC by Sam Lau
Modified: 2023-12-08 16:30 UTC (History)
7 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/771
Text to go in the release notes:
When the new system preference `ForceLibrarySelection` is activated, staff users will have to choose a library when logging into the staff interface.
Version(s) released in:
23.11.00
Circulation function:


Attachments
Bug 34188: Require Library Branch Selection when Logging in (6.07 KB, patch)
2023-07-11 19:33 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 34188: Require Library Branch Selection when Logging in (6.09 KB, patch)
2023-07-11 19:39 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 34188: Require Library Branch Selection when Logging in (6.17 KB, patch)
2023-07-12 09:50 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 34188: Require Library Branch Selection when Logging in (5.99 KB, patch)
2023-07-17 15:51 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 34188: Require Library Branch Selection when Logging in (6.00 KB, patch)
2023-07-18 13:24 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 34188: (follow-up) Rename pref to 'ForceLibrarySelection' and tidy (3.74 KB, patch)
2023-10-17 19:34 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
k (3.79 KB, patch)
2023-11-04 23:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34188: Require Library Branch Selection when Logging in (6.06 KB, patch)
2023-11-04 23:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34188: (follow-up) Rename pref to 'ForceLibrarySelection' and tidy (3.79 KB, patch)
2023-11-04 23:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34188: (QA follow-up) Fix different versions of system preference name: Force(d)LibrarySelection (2.44 KB, patch)
2023-11-04 23:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34188: (QA follow-up) Add standard validation to login form (1.29 KB, patch)
2023-11-04 23:55 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Lau 2023-07-03 15:26:19 UTC
Some staff members move between locations and can occasionally log into the wrong location. This can impact circulation rules, transit items, etc when those staff members are working. This proposed development would require staff to choose their library when logging in.

This functionality would be a system preference where a library system can enable or disable the forced library selection. If enabled, users on the staff side would see a blank in the top line of the library dropdown and be forced to select a library from the list. Users who do not select a branch are returned to the login screen as Koha would not allow the default branch to be used as a fallback.
Comment 1 Sam Lau 2023-07-11 19:33:44 UTC
Created attachment 153337 [details] [review]
Bug 34188: Require Library Branch Selection when Logging in

This patch adds the ability to force staff to select a library when logging into the staff interface. This is done via a new system preference: 'ForcedLibrarySelection'

To test:
1) Apply patch, restart_all, and updatedatabase
2) Log out of the staff interface. Notice the login form looks the same and the "Library:" dropdown has 'My Library" selected as default. Log back into the staff interface.
3) In Administration, search for the system preference "ForcedLibrarySelection". Set it to 'Force' and press save.
4) Log out of the staff interface. Notice that this time, the "Library:" dropdown is required and has a blank selection as the default.
5) Fill in the username and password but do not select a library. Click 'Log in'
6) Notice you cannot log in and are asked to 'Please select an item in the list'
7) Select a library from the drop down and click 'Log in'
8) Notice the login was successful and you were logged in to the library you selected.9) Try loggin in with some other libraries to verify it works as expected
10) Sign-off :)
Comment 2 Sam Lau 2023-07-11 19:39:58 UTC
Created attachment 153338 [details] [review]
Bug 34188: Require Library Branch Selection when Logging in

This patch adds the ability to force staff to select a library when logging into the staff interface. This is done via a new system preference: 'ForcedLibrarySelection'

To test:
1) Apply patch, restart_all, and updatedatabase
2) Log out of the staff interface. Notice the login form looks the same and the "Library:" dropdown has 'My Library" selected as default. Log back into the staff interface.
3) In Administration, search for the system preference "ForcedLibrarySelection". Set it to 'Force' and press save.
4) Log out of the staff interface. Notice that this time, the "Library:" dropdown is required and has a blank selection as the default.
5) Fill in the username and password but do not select a library. Click 'Log in'
6) Notice you cannot log in and are asked to 'Please select an item in the list'
7) Select a library from the drop down and click 'Log in'
8) Notice the login was successful and you were logged in to the library you selected.9) Try loggin in with some other libraries to verify it works as expected
10) Sign-off :)
Comment 3 Matt Blenkinsop 2023-07-12 09:50:57 UTC
Created attachment 153348 [details] [review]
Bug 34188: Require Library Branch Selection when Logging in

This patch adds the ability to force staff to select a library when logging into the staff interface. This is done via a new system preference: 'ForcedLibrarySelection'

To test:
1) Apply patch, restart_all, and updatedatabase
2) Log out of the staff interface. Notice the login form looks the same and the "Library:" dropdown has 'My Library" selected as default. Log back into the staff interface.
3) In Administration, search for the system preference "ForcedLibrarySelection". Set it to 'Force' and press save.
4) Log out of the staff interface. Notice that this time, the "Library:" dropdown is required and has a blank selection as the default.
5) Fill in the username and password but do not select a library. Click 'Log in'
6) Notice you cannot log in and are asked to 'Please select an item in the list'
7) Select a library from the drop down and click 'Log in'
8) Notice the login was successful and you were logged in to the library you selected.9) Try loggin in with some other libraries to verify it works as expected
10) Sign-off :)

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 4 Katrin Fischer 2023-07-16 12:06:36 UTC
Hi Sam, 

congrats on your patch :)

Some notes:

1) The patch description should always describe the fix, not the problem. For example here:

Add system preference to make library required on login

2) Always run the QA test tools:

 FAIL	installer/data/mysql/atomicupdate/bug_34188.pl
   FAIL	  file permissions
		File must have the exec flag
 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt
   FAIL	  filters
		missing_filter at line 121 (            <label for="branch" class="[% libraryClass %]">Library:</label>)
		missing_filter at line 122 (            <select name="branch" id="branch" class="input" tabindex="3" [% libraryRequired %]>)
   FAIL	  tt_valid
		lines 122

There is a script in misc/devel that can help you fix the filters quickly: add_missing_filters.pl

The TT valid points to some template issue, double check your changes for this one.

Overall this looks good - close!
Comment 5 Sam Lau 2023-07-17 15:51:44 UTC
Created attachment 153563 [details] [review]
Bug 34188: Require Library Branch Selection when Logging in

This patch adds the ability to force staff to select a library when logging into the staff interface. This is done via a new system preference: 'ForcedLibrarySelection'

To test:
1) Apply patch, restart_all, and updatedatabase
2) Log out of the staff interface. Notice the login form looks the same and the "Library:" dropdown has 'My Library" selected as default. Log back into the staff interface.
3) In Administration, search for the system preference "ForcedLibrarySelection". Set it to 'Force' and press save.
4) Log out of the staff interface. Notice that this time, the "Library:" dropdown is required and has a blank selection as the default.
5) Fill in the username and password but do not select a library. Click 'Log in'
6) Notice you cannot log in and are asked to 'Please select an item in the list'
7) Select a library from the drop down and click 'Log in'
8) Notice the login was successful and you were logged in to the library you selected.
9) Try loggin in with some other libraries to verify it works as expected
10) Sign-off :)
Comment 6 Owen Leonard 2023-07-18 13:24:12 UTC
Created attachment 153605 [details] [review]
Bug 34188: Require Library Branch Selection when Logging in

This patch adds the ability to force staff to select a library when
logging into the staff interface. This is done via a new system
preference: 'ForcedLibrarySelection'

To test:
1) Apply patch, restart_all, and updatedatabase
2) Log out of the staff interface. Notice the login form looks the same
   and the "Library:" dropdown has 'My Library" selected as default. Log
   back into the staff interface.
3) In Administration, search for the system preference
   "ForcedLibrarySelection". Set it to 'Force' and press save.
4) Log out of the staff interface. Notice that this time, the "Library:"
   dropdown is required and has a blank selection as the default.
5) Fill in the username and password but do not select a library. Click
   'Log in'
6) Notice you cannot log in and are asked to 'Please select an item in
   the list'
7) Select a library from the drop down and click 'Log in'
8) Notice the login was successful and you were logged in to the library
   you selected.
9) Try loggin in with some other libraries to verify it works as
   expected
10) Sign-off :)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 7 Jonathan Druart 2023-09-01 12:38:54 UTC
1. Why Forced* instead of Force*?
2. You are not enforcing the condition server-side, shouldn't we?
Comment 8 Martin Renvoize (ashimema) 2023-09-18 12:06:09 UTC
Agreed on the serverside requirement.. we should really check the pref there too rather than do the standard auto-selection.
Comment 9 Martin Renvoize (ashimema) 2023-09-18 12:06:40 UTC
I love the work here though.. we do with clientside with jQuery all the time.. it would be nice to have a switch instead.
Comment 10 Nick Clemens (kidclamp) 2023-09-28 16:24:23 UTC
(In reply to Martin Renvoize from comment #8)
> Agreed on the serverside requirement.. we should really check the pref there
> too rather than do the standard auto-selection.

(In reply to Jonathan Druart from comment #7)
> 2. You are not enforcing the condition server-side, shouldn't we?

I think we did talk about this initially - your library won't be removed from the list, so is still a valid selection. If you are going to far as to trick the script to sign you in to your library, I think you have been forced to make a selection :-)

We could add a check, but it would need to be in Auth.pm, and is a bigger change that need more tests. I don't see this one so much as a security feature, but a convenience feature to ensure libraries don't accidentally sign in to the wrong branch
Comment 11 Nick Clemens (kidclamp) 2023-10-17 19:34:31 UTC
Created attachment 157285 [details] [review]
Bug 34188: (follow-up) Rename pref to 'ForceLibrarySelection' and tidy
Comment 12 Katrin Fischer 2023-11-04 23:55:13 UTC
Created attachment 158408 [details] [review]
k
Comment 13 Katrin Fischer 2023-11-04 23:55:34 UTC
Created attachment 158409 [details] [review]
Bug 34188: Require Library Branch Selection when Logging in

This patch adds the ability to force staff to select a library when
logging into the staff interface. This is done via a new system
preference: 'ForcedLibrarySelection'

To test:
1) Apply patch, restart_all, and updatedatabase
2) Log out of the staff interface. Notice the login form looks the same
   and the "Library:" dropdown has 'My Library" selected as default. Log
   back into the staff interface.
3) In Administration, search for the system preference
   "ForcedLibrarySelection". Set it to 'Force' and press save.
4) Log out of the staff interface. Notice that this time, the "Library:"
   dropdown is required and has a blank selection as the default.
5) Fill in the username and password but do not select a library. Click
   'Log in'
6) Notice you cannot log in and are asked to 'Please select an item in
   the list'
7) Select a library from the drop down and click 'Log in'
8) Notice the login was successful and you were logged in to the library
   you selected.
9) Try loggin in with some other libraries to verify it works as
   expected
10) Sign-off :)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2023-11-04 23:55:36 UTC
Created attachment 158410 [details] [review]
Bug 34188: (follow-up) Rename pref to 'ForceLibrarySelection' and tidy

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Katrin Fischer 2023-11-04 23:55:40 UTC
Created attachment 158411 [details] [review]
Bug 34188: (QA follow-up) Fix different versions of system preference name: Force(d)LibrarySelection

There were references to ForceLibrarySelection and ForceLibrarySelection (notice
the additional d) in the patch.

Standardized to: ForceLibrarySelection

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Katrin Fischer 2023-11-04 23:55:43 UTC
Created attachment 158412 [details] [review]
Bug 34188: (QA follow-up) Add standard validation to login form

This changes how the required appears. Now it will display 'our'
text for requires fields instead of using the browser default.
Comment 17 Tomás Cohen Arazi (tcohen) 2023-11-08 20:42:37 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 18 Fridolin Somers 2023-11-11 01:29:42 UTC
Enhancement not pushed to 23.05.x