As far as I can tell AutoLocation isn't about koha picking a library branch automatically from your login IP address as much as it's a security feature to prevent you from logging into another branch from outside an IP range. As such, I feel the preference is either misnamed, or the description is incorrect.. either way it needs some clarification adding.
Do you have an idea for a good name? RestrictStaffAccessByIP?
Adding some Cc's for opinions
I like RestrictStaffAccessByIP The other ones I thought of are basically variations of the same name - RestrictAccessByIP - AccessRestrictionByIP (very similar to/based on SessionRestrictionByIP, maybe too similar?) - StaffAllowByIP (based on SelfCheckAllowByIPRanges) - ControlAccessByIP - IPStaffAccessControl - IPStaffAccessRestriction If you change it, can you also change the sentence to "[Don't require|Require] staff to log in from a computer in the IP address range specified by their library (if any)" (instead of .... [No|Yes]) ?
I also like RestrictStaffAccessByIP - does what it says! Or, if we want to go for something a little bit shorter: ControlAccessByIP from among Caroline's suggestions.
Maybe "StaffRestrictAccessByIP" in case we add "OPACRestrictAccessByIP"
Created attachment 166975 [details] [review] Bug 26176: Rename AutoLocation to StaffLoginRestrictBranchByIP This patch sets AutoLocation to be called StaffLoginRestrictBranchByIP. The new name is chosen to reflect the new pref StaffLoginBranchBasedOnIP. Also this patch corrects the order of sysprefs in installer file. To test: Follow test plans on bug 36665 and bug 35890 and confirm that the preferences continue to work as expected Confirm the descriptions of the prefs in the staff interface match the behaviors expected
Created attachment 166976 [details] [review] Bug 26176: (follow-up) Prevent random failures from too long branchcode
Created attachment 167019 [details] [review] Bug 26176: Rename AutoLocation to StaffLoginRestrictBranchByIP This patch sets AutoLocation to be called StaffLoginRestrictBranchByIP. The new name is chosen to reflect the new pref StaffLoginBranchBasedOnIP. Also this patch corrects the order of sysprefs in installer file. To test: Follow test plans on bug 36665 and bug 35890 and confirm that the preferences continue to work as expected Confirm the descriptions of the prefs in the staff interface match the behaviors expected
Created attachment 167020 [details] [review] Bug 26176: (follow-up) Prevent random failures from too long branchcode
Created attachment 167072 [details] [review] Bug 26176: Rename AutoLocation to StaffLoginRestrictBranchByIP This patch sets AutoLocation to be called StaffLoginRestrictBranchByIP. The new name is chosen to reflect the new pref StaffLoginBranchBasedOnIP. Also this patch corrects the order of sysprefs in installer file. To test: Follow test plans on bug 36665 and bug 35890 and confirm that the preferences continue to work as expected Confirm the descriptions of the prefs in the staff interface match the behaviors expected Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I had trouble with the rebase (had to apply --reject). Second patch seems not needed, the changes are already in "Bug 36908: Sort branches based on branchcode"
(In reply to Jonathan Druart from comment #11) > I had trouble with the rebase (had to apply --reject). Second patch seems > not needed, the changes are already in "Bug 36908: Sort branches based on > branchcode" Yeah, I picked that patch back on the other bug when rebasing as that's when the problem was introduced
Created attachment 167091 [details] [review] Bug 26176: Rename AutoLocation to StaffLoginRestrictBranchByIP This patch sets AutoLocation to be called StaffLoginRestrictBranchByIP. The new name is chosen to reflect the new pref StaffLoginBranchBasedOnIP. Also this patch corrects the order of sysprefs in installer file. To test: Follow test plans on bug 36665 and bug 35890 and confirm that the preferences continue to work as expected Confirm the descriptions of the prefs in the staff interface match the behaviors expected Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 167092 [details] [review] Bug 26176: (QA follow-up) Branch -> Library Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 167093 [details] [review] Bug 26176: (QA follow-up) Also rename StaffLoginBranchBasedOnIP This patch also renames StaffLoginBranchBasedOnIP to StaffLoginLibraryBasedOnIP to be consistent with our terminology guidelines Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
(In reply to Martin Renvoize from comment #15) > Created attachment 167093 [details] [review] [review] > Bug 26176: (QA follow-up) Also rename StaffLoginBranchBasedOnIP > > This patch also renames StaffLoginBranchBasedOnIP to > StaffLoginLibraryBasedOnIP to be consistent with our terminology > guidelines > > Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> I would suggest to use this pattern: $dbh->do(q{ UPDATE }) == 1 && say_success(q{Updated!});
Created attachment 167094 [details] [review] Bug 26176: (follow-up) Fix one more occurrence
Created attachment 167095 [details] [review] Bug 26176: (follow-up) Fix one more occurrence
Created attachment 167096 [details] [review] Bug 26176: (follow-up) Fix one more occurrence
Created attachment 167114 [details] [review] Bug 26176: (follow-up) Use suggested patturn in update
Pushed for 24.05! Well done everyone, thank you!
Created attachment 167162 [details] [review] Bug 26176: (follow-up) Replace branch with library in syspref description
Arg copy/paste error in "Also rename StaffLoginBranchBasedOnIP" patch : UPDATE systempreferences SET variable = "StaffLoginRestrictLibraryByIP" WHERE variable = "StaffLoginLibraryBasedOnIP" It is: UPDATE systempreferences SET variable = "StaffLoginLibraryBasedOnIP" WHERE variable = "StaffLoginBranchBasedOnIP"
(In reply to Fridolin Somers from comment #24) > Arg copy/paste error in "Also rename StaffLoginBranchBasedOnIP" patch : > > UPDATE systempreferences SET variable = "StaffLoginRestrictLibraryByIP" > WHERE variable = "StaffLoginLibraryBasedOnIP" > > It is: > > UPDATE systempreferences SET variable = "StaffLoginLibraryBasedOnIP" > WHERE variable = "StaffLoginBranchBasedOnIP" Not good. :( We'll need a new bug report and a follow-up.
On it
Waiting for Bug 36986 to backport
Pushed to 23.11.x for 23.11.06
not backporting to 23.05.x unless requested
Documented by Caroline alongside Bug 36665.