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> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This commit is contained in:
Nick Clemens
Katrin Fischer
parent
7e88035372
commit
46cecfdd72
|
|
@ -1214,8 +1214,8 @@ sub checkauth {
|
|||
$register_id = $register->id if ($register);
|
||||
$register_name = $register->name if ($register);
|
||||
}
|
||||
my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search->as_list };
|
||||
if ( $type ne 'opac' ) {
|
||||
my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search->as_list };
|
||||
if ( C4::Context->preference('AutoLocation') ) {
|
||||
# we have to check they are coming from the right ip range
|
||||
my $domain = $branches->{$branchcode}->{'branchip'};
|
||||
|
|
@ -1248,7 +1248,8 @@ sub checkauth {
|
|||
&& $branches->{$branchcode}->{'branchip'} )
|
||||
)
|
||||
{
|
||||
foreach my $br ( uniq( $branchcode, keys %$branches ) ) {
|
||||
my @branchcodes = sort { lc $a cmp lc $b } keys %$branches;
|
||||
foreach my $br ( uniq( $branchcode, @branchcodes ) ) {
|
||||
|
||||
# now we work with the treatment of ip
|
||||
my $domain = $branches->{$br}->{'branchip'};
|
||||
|
|
|
|||
|
|
@ -91,14 +91,14 @@ Administration:
|
|||
class: integer
|
||||
- Adding d will specify it in days, e.g. 1d is timeout of one day.
|
||||
-
|
||||
- "Require staff to log in from a computer in the IP address range specified by their library (if any): "
|
||||
- "Require staff to log in from a computer in the IP address range specified by their library or the branch chosen at login (if any): "
|
||||
- pref: AutoLocation
|
||||
default: 0
|
||||
choices:
|
||||
1: "Yes"
|
||||
0: "No"
|
||||
- <a href="/cgi-bin/koha/admin/branches.pl">Link to library administration</a>
|
||||
- 'This setting will override the <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=StaffLoginBranchBasedOnIP">StaffLoginBranchBasedOnIP</a> system preference.'
|
||||
- 'Staff can only choose thier branch on login if they have "loggedinlibrary" permission. This setting will override the <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=StaffLoginBranchBasedOnIP">StaffLoginBranchBasedOnIP</a> system preference. In the event of multiple branches with matching IPs, branchcode (alphabetically) will be the tie breaker.'
|
||||
-
|
||||
- "Enable check for change in remote IP address for session security: "
|
||||
- pref: SessionRestrictionByIP
|
||||
|
|
@ -115,7 +115,7 @@ Administration:
|
|||
1: "Yes"
|
||||
0: "No"
|
||||
- "Note: If IPs overlap, the first found match will be used."
|
||||
- 'This setting will be overridden by <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=AutoLocation">AutoLocation</a> system preference.'
|
||||
- 'This setting will be overridden by <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=AutoLocation">AutoLocation</a> system preference. In the event of multiple branches with matching IPs, branchcode (alphabetically) will be the tie breaker.'
|
||||
# PostgreSQL is supported by CGI::Session but not by Koha.
|
||||
-
|
||||
- "Storage of login session information: "
|
||||
|
|
|
|||
|
|
@ -1341,17 +1341,25 @@ subtest 'StaffLoginBranchBasedOnIP' => sub {
|
|||
);
|
||||
|
||||
t::lib::Mocks::mock_preference( 'AutoLocation', 0 );
|
||||
my $other_branch = $builder->build_object( { class => 'Koha::Libraries', value => { branchip => "127.0.0.1" } } );
|
||||
my $other_branch = $builder->build_object(
|
||||
{
|
||||
class => 'Koha::Libraries',
|
||||
value => { branchip => "127.0.0.1", branchcode => substr( "z" . $branch->branchcode, 0, 10 ) }
|
||||
}
|
||||
);
|
||||
( $userid, $cookie, $sessionID, $flags ) = C4::Auth::checkauth( $cgi, 0, { catalogue => 1 }, 'intranet' );
|
||||
is( $userid, $patron->userid, "User successfully logged in" );
|
||||
$session = C4::Auth::get_session($sessionID);
|
||||
is( $session->param('branch'), $branch->branchcode, "Logged in branch is set based which branch when two libraries have same IP?" );
|
||||
is(
|
||||
$session->param('branch'), $branch->branchcode,
|
||||
"Logged in branch is set based which branch when two libraries have same IP?"
|
||||
);
|
||||
|
||||
};
|
||||
|
||||
subtest 'AutoLocation' => sub {
|
||||
|
||||
plan tests => 11;
|
||||
plan tests => 12;
|
||||
|
||||
$schema->storage->txn_begin;
|
||||
|
||||
|
|
@ -1428,19 +1436,37 @@ subtest 'AutoLocation' => sub {
|
|||
( $userid, $cookie, $sessionID, $flags, $template ) =
|
||||
C4::Auth::checkauth( $cgi, 0, { catalogue => 1 }, 'intranet' );
|
||||
$session = C4::Auth::get_session($sessionID);
|
||||
is( $session->param('branch'), $noip_library->branchcode, "When a branch with no IP set is chosen, we respect the choice regardless of current IP" );
|
||||
is(
|
||||
$session->param('branch'), $noip_library->branchcode,
|
||||
"When a branch with no IP set is chosen, we respect the choice regardless of current IP"
|
||||
);
|
||||
|
||||
$ENV{REMOTE_ADDR} = '129.0.0.1'; # Set current IP to match other_branch
|
||||
$cgi->param( 'branch', undef ); # Do not pass a branch
|
||||
$cgi->param( 'branch', '' ); # Do not pass a branch
|
||||
$patron->library->branchip('')->store; # Unset user branch IP, to allow IP matching on any branch
|
||||
# Add a second branch with same IP
|
||||
my $another_library = $builder->build_object( { class => 'Koha::Libraries', value => { branchip => '129.0.0.1' } } );
|
||||
# Add a second branch with same IP
|
||||
my $another_library = $builder->build_object(
|
||||
{
|
||||
class => 'Koha::Libraries',
|
||||
value => { branchip => "129.0.0.1", branchcode => substr( "z" . $other_library->branchcode, 0, 10 ) }
|
||||
}
|
||||
);
|
||||
( $userid, $cookie, $sessionID, $flags, $template ) =
|
||||
C4::Auth::checkauth( $cgi, 0, { catalogue => 1 }, 'intranet', undef, undef, { do_not_print => 1 } );
|
||||
$session = C4::Auth::get_session($sessionID);
|
||||
is(
|
||||
$session->param('branch'), $other_library->branchcode,
|
||||
"Which branch is chosen when home branch has no IP and more than 1 library matches?"
|
||||
$session->param('branch'), $patron->library->branchcode,
|
||||
"When user branch has no IP, and no branch chosen, user is logged in to their homebranch"
|
||||
);
|
||||
|
||||
$cgi->param( 'branch', $another_library->branchcode )
|
||||
; # Choose branch with duplicate IP and alphabetically later branchcode
|
||||
( $userid, $cookie, $sessionID, $flags, $template ) =
|
||||
C4::Auth::checkauth( $cgi, 0, { catalogue => 1 }, 'intranet', undef, undef, { do_not_print => 1 } );
|
||||
$session = C4::Auth::get_session($sessionID);
|
||||
is(
|
||||
$session->param('branch'), $another_library->branchcode,
|
||||
"When there is an IP conflict, we use the chosen branch if it matches"
|
||||
);
|
||||
|
||||
$schema->storage->txn_rollback;
|
||||
|
|
|
|||
Loading…
Reference in New Issue