[Bug 36908] New: Clarify and standardize the behavior of AutoLocation/ StaffLoginBranchBasedOnIP system preferences
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Bug ID: 36908 Summary: Clarify and standardize the behavior of AutoLocation/ StaffLoginBranchBasedOnIP system preferences Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Authentication Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: dpavlin@rot13.org The existing AutoLocation system preference (to be renamed in bug 26176) and the new system preference StaffLoginBranchBasedOnIP both attempt to assign the correct branch for the user during login based on the current IP If multiple branches match the IP, or if the user specifies a branch during login the behavior can verify. We should ensure the behavior is consistent and clarify how it works in the syspref descriptions -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 --- Comment #1 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 166966 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166966&action=edit Bug 36908: Expand, clarify, and tidy tests for AutoLocation -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 --- Comment #2 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 166967 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166967&action=edit Bug 36908: Additional unit tests to identify flaw when two branches have same IP This could be considered a configuration flaw, but when: StaffLoginBranchBasedOnIP enabled and not AutoLocation or AutoLocation enabledand no IP set in user's branch AND two branches have the same IP set the user can be logged in randomly to one of the matching branches. These test often pass, but will also randomly fail Easier to verify with a one liner demonstrating current code: perl -e 'use Koha::Libraries; use List::MoreUtils qw(uniq); my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search->as_list }; my $branchcode="CPL"; warn Data::Dumper::Dumper( uniq( $branchcode, keys %$branches ));' -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 --- Comment #3 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 166968 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166968&action=edit Bug 36908: Sort branches based on branchcode This adds a sort based on branchcode, it's a fallback for an edge case that should be rare so I think is acceptable, as long as documented. System preference descriptions are updated as well -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |36665 Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | CC| |jonathan.druart@gmail.com, | |katrin.fischer@bsz-bw.de Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36665 [Bug 36665] Auto location and IP recognition -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |26176 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26176 [Bug 26176] AutoLocation is badly named -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #4 from Jonathan Druart <jonathan.druart@gmail.com> --- See bug 36665 comment 23. "AutoLocation" (login restriction by IP) is not working as the syspref's description is saying. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166966|0 |1 is obsolete| | Attachment #166967|0 |1 is obsolete| | Attachment #166968|0 |1 is obsolete| | --- Comment #5 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 167044 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=167044&action=edit Bug 36908: Expand, clarify, and tidy tests for AutoLocation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 --- Comment #6 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 167045 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=167045&action=edit Bug 36908: Additional unit tests to identify flaw when two branches have same IP This could be considered a configuration flaw, but when: StaffLoginBranchBasedOnIP enabled and not AutoLocation or AutoLocation enabledand no IP set in user's branch AND two branches have the same IP set the user can be logged in randomly to one of the matching branches. These test often pass, but will also randomly fail Easier to verify with a one liner demonstrating current code: perl -e 'use Koha::Libraries; use List::MoreUtils qw(uniq); my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search->as_list }; my $branchcode="CPL"; warn Data::Dumper::Dumper( uniq( $branchcode, keys %$branches ));' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 --- Comment #7 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 167046 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=167046&action=edit Bug 36908: Sort branches based on branchcode This adds a sort based on branchcode, it's a fallback for an edge case that should be rare so I think is acceptable, as long as documented. I added test coverage, but it may no longer be possible to encounter this scenario. System preference descriptions are updated as well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #167044|0 |1 is obsolete| | Attachment #167045|0 |1 is obsolete| | Attachment #167046|0 |1 is obsolete| | --- Comment #8 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 167073 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=167073&action=edit Bug 36908: Expand, clarify, and tidy tests for AutoLocation Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 --- Comment #9 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 167074 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=167074&action=edit Bug 36908: Additional unit tests to identify flaw when two branches have same IP This could be considered a configuration flaw, but when: StaffLoginBranchBasedOnIP enabled and not AutoLocation or AutoLocation enabledand no IP set in user's branch AND two branches have the same IP set the user can be logged in randomly to one of the matching branches. These test often pass, but will also randomly fail Easier to verify with a one liner demonstrating current code: perl -e 'use Koha::Libraries; use List::MoreUtils qw(uniq); my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search->as_list }; my $branchcode="CPL"; warn Data::Dumper::Dumper( uniq( $branchcode, keys %$branches ));' Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 --- Comment #10 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 167075 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=167075&action=edit Bug 36908: Sort branches based on branchcode This adds a sort based on branchcode, it's a fallback for an edge case that should be rare so I think is acceptable, as long as documented. I added test coverage, but it may no longer be possible to encounter this scenario. System preference descriptions are updated as well. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_24_05_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #167073|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 167084 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=167084&action=edit Bug 36908: Expand, clarify, and tidy tests for AutoLocation Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #167074|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 167085 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=167085&action=edit Bug 36908: Additional unit tests to identify flaw when two branches have same IP This could be considered a configuration flaw, but when: StaffLoginBranchBasedOnIP enabled and not AutoLocation or AutoLocation enabledand no IP set in user's branch AND two branches have the same IP set the user can be logged in randomly to one of the matching branches. These test often pass, but will also randomly fail Easier to verify with a one liner demonstrating current code: perl -e 'use Koha::Libraries; use List::MoreUtils qw(uniq); my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search->as_list }; my $branchcode="CPL"; warn Data::Dumper::Dumper( uniq( $branchcode, keys %$branches ));' Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #167075|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 167086 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=167086&action=edit Bug 36908: Sort branches based on branchcode This adds a sort based on branchcode, it's a fallback for an edge case that should be rare so I think is acceptable, as long as documented. I added test coverage, but it may no longer be possible to encounter this scenario. System preference descriptions are updated as well. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 167087 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=167087&action=edit Bug 36908: (QA follow-up) Proposed improvement to prefernce description I found the initial addition of 'or the branch chosen at login' to the system preference description more misleading than the original. After discussion on mattermost and with training staff here at PTFS the best alternative we could come up with is proposed here. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm happy to PQA this.. but I think we would like Nicks final +1 on the proposed follow-up. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #167087|0 |1 is obsolete| | --- Comment #16 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 167090 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=167090&action=edit Bug 36908: (QA follow-up) Proposed improvement to prefernce description I found the initial addition of 'or the branch chosen at login' to the system preference description more misleading than the original. After discussion on mattermost and with training staff here at PTFS the best alternative we could come up with is proposed here. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #17 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- (In reply to Martin Renvoize from comment #15)
I'm happy to PQA this.. but I think we would like Nicks final +1 on the proposed follow-up.
Looks good to me - I corrected a typo in my SO patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |24.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 --- Comment #18 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Status|Pushed to main |Pushed to stable Version(s)|24.05.00 |24.05.00,23.11.06 released in| | --- Comment #19 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Pushed to stable |Pushed to oldstable Version(s)|24.05.00,23.11.06 |24.05.00,23.11.06,23.05.12 released in| | --- Comment #20 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 23.05.x for upcoming 23.05.12 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_24_05_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|martin.renvoize@ptfs-europe | |.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Documentation| |Caroline Cyr La Rose contact| | CC| |caroline.cyr-la-rose@inlibr | |o.com Documentation| |https://gitlab.com/koha-com submission| |munity/koha-manual/-/merge_ | |requests/975 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Bug 36908 depends on bug 36665, which changed state. Bug 36665 Summary: Auto location and IP recognition https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36665 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36908 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to oldstable |RESOLVED CC| |wainuiwitikapark@catalyst.n | |et.nz --- Comment #21 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11 unless requested -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org