Bugzilla – Attachment 169413 Details for
Bug 37378
Patron searches can fail when library groups are set to 'Limit patron data access by group'
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 37378: Tidy
Bug-37378-Tidy.patch (text/plain), 11.29 KB, created by
Nick Clemens (kidclamp)
on 2024-07-23 12:27:52 UTC
(
hide
)
Description:
Bug 37378: Tidy
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2024-07-23 12:27:52 UTC
Size:
11.29 KB
patch
obsolete
>From 07c1e48c66b5052ce10709f3761ad1b5a8e65206 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Tue, 23 Jul 2024 12:24:25 +0000 >Subject: [PATCH] Bug 37378: Tidy > >--- > t/db_dependent/Koha/Patrons.t | 107 +++++++++++++++++++++------------- > 1 file changed, 66 insertions(+), 41 deletions(-) > >diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t >index 754b309d1b4..38a322e4444 100755 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -1284,7 +1284,8 @@ subtest 'search_patrons_to_anonymise' => sub { > t::lib::Mocks::mock_preference('IndependentBranches', 0); > }; > >-subtest 'libraries_where_can_see_patrons + libraries_where_can_see_things + can_see_patron_infos + search_limited' => sub { >+subtest 'libraries_where_can_see_patrons + libraries_where_can_see_things + can_see_patron_infos + search_limited' => >+ sub { > plan tests => 4; > > # group1 >@@ -1293,38 +1294,44 @@ subtest 'libraries_where_can_see_patrons + libraries_where_can_see_things + can_ > # group2 > # + library21 > $nb_of_patrons = Koha::Patrons->search->count; >- my $group_1 = Koha::Library::Group->new( { title => 'TEST Group 1', ft_hide_patron_info => 1 } )->store; >- my $group_2 = Koha::Library::Group->new( { title => 'TEST Group 2', ft_hide_patron_info => 1 } )->store; >+ my $group_1 = Koha::Library::Group->new( { title => 'TEST Group 1', ft_hide_patron_info => 1 } )->store; >+ my $group_2 = Koha::Library::Group->new( { title => 'TEST Group 2', ft_hide_patron_info => 1 } )->store; > my $library_11 = $builder->build( { source => 'Branch' } ); > my $library_12 = $builder->build( { source => 'Branch' } ); > my $library_21 = $builder->build( { source => 'Branch' } ); > $library_11 = Koha::Libraries->find( $library_11->{branchcode} ); > $library_12 = Koha::Libraries->find( $library_12->{branchcode} ); > $library_21 = Koha::Libraries->find( $library_21->{branchcode} ); >- Koha::Library::Group->new( >- { branchcode => $library_11->branchcode, parent_id => $group_1->id } )->store; >- Koha::Library::Group->new( >- { branchcode => $library_12->branchcode, parent_id => $group_1->id } )->store; >- Koha::Library::Group->new( >- { branchcode => $library_21->branchcode, parent_id => $group_2->id } )->store; >- >- my $sth = C4::Context->dbh->prepare(q|INSERT INTO user_permissions( borrowernumber, module_bit, code ) VALUES (?, 4, ?)|); # 4 for borrowers >- # 2 patrons from library_11 (group1) >- # patron_11_1 see patron's infos from outside its group >- # Setting flags => undef to not be considered as superlibrarian >- my $patron_11_1 = $builder->build({ source => 'Borrower', value => { branchcode => $library_11->branchcode, flags => undef, }}); >+ Koha::Library::Group->new( { branchcode => $library_11->branchcode, parent_id => $group_1->id } )->store; >+ Koha::Library::Group->new( { branchcode => $library_12->branchcode, parent_id => $group_1->id } )->store; >+ Koha::Library::Group->new( { branchcode => $library_21->branchcode, parent_id => $group_2->id } )->store; >+ >+ my $sth = >+ C4::Context->dbh->prepare(q|INSERT INTO user_permissions( borrowernumber, module_bit, code ) VALUES (?, 4, ?)|) >+ ; # 4 for borrowers >+ # 2 patrons from library_11 (group1) >+ # patron_11_1 see patron's infos from outside its group >+ # Setting flags => undef to not be considered as superlibrarian >+ my $patron_11_1 = $builder->build( >+ { source => 'Borrower', value => { branchcode => $library_11->branchcode, flags => undef, } } ); > $patron_11_1 = Koha::Patrons->find( $patron_11_1->{borrowernumber} ); > $sth->execute( $patron_11_1->borrowernumber, 'edit_borrowers' ); > $sth->execute( $patron_11_1->borrowernumber, 'view_borrower_infos_from_any_libraries' ); >+ > # patron_11_2 can only see patron's info from its group >- my $patron_11_2 = $builder->build({ source => 'Borrower', value => { branchcode => $library_11->branchcode, flags => undef, }}); >+ my $patron_11_2 = $builder->build( >+ { source => 'Borrower', value => { branchcode => $library_11->branchcode, flags => undef, } } ); > $patron_11_2 = Koha::Patrons->find( $patron_11_2->{borrowernumber} ); > $sth->execute( $patron_11_2->borrowernumber, 'edit_borrowers' ); >+ > # 1 patron from library_12 (group1) >- my $patron_12 = $builder->build({ source => 'Borrower', value => { branchcode => $library_12->branchcode, flags => undef, }}); >+ my $patron_12 = $builder->build( >+ { source => 'Borrower', value => { branchcode => $library_12->branchcode, flags => undef, } } ); > $patron_12 = Koha::Patrons->find( $patron_12->{borrowernumber} ); >+ > # 1 patron from library_21 (group2) can only see patron's info from its group >- my $patron_21 = $builder->build({ source => 'Borrower', value => { branchcode => $library_21->branchcode, flags => undef, }}); >+ my $patron_21 = $builder->build( >+ { source => 'Borrower', value => { branchcode => $library_21->branchcode, flags => undef, } } ); > $patron_21 = Koha::Patrons->find( $patron_21->{borrowernumber} ); > $sth->execute( $patron_21->borrowernumber, 'edit_borrowers' ); > >@@ -1354,57 +1361,75 @@ subtest 'libraries_where_can_see_patrons + libraries_where_can_see_things + can_ > > my @branchcodes; > >- t::lib::Mocks::mock_userenv({ patron => $patron_11_1 }); >+ t::lib::Mocks::mock_userenv( { patron => $patron_11_1 } ); > @branchcodes = $patron_11_1->libraries_where_can_see_patrons; > is_deeply( \@branchcodes, [], q|patron_11_1 has view_borrower_infos_from_any_libraries => No restriction| ); > >- t::lib::Mocks::mock_userenv({ patron => $patron_11_2 }); >+ t::lib::Mocks::mock_userenv( { patron => $patron_11_2 } ); > @branchcodes = $patron_11_2->libraries_where_can_see_patrons; >- is_deeply( \@branchcodes, [ sort ( $library_11->branchcode, $library_12->branchcode ) ], q|patron_11_2 has not view_borrower_infos_from_any_libraries => Can only see patron's from its group| ); >+ is_deeply( >+ \@branchcodes, [ sort ( $library_11->branchcode, $library_12->branchcode ) ], >+ q|patron_11_2 has not view_borrower_infos_from_any_libraries => Can only see patron's from its group| >+ ); > >- t::lib::Mocks::mock_userenv({ patron => $patron_21 }); >+ t::lib::Mocks::mock_userenv( { patron => $patron_21 } ); > @branchcodes = $patron_21->libraries_where_can_see_patrons; >- is_deeply( \@branchcodes, [$library_21->branchcode], q|patron_21 has not view_borrower_infos_from_any_libraries => Can only see patron's from its group| ); >+ is_deeply( >+ \@branchcodes, [ $library_21->branchcode ], >+ q|patron_21 has not view_borrower_infos_from_any_libraries => Can only see patron's from its group| >+ ); > }; > subtest 'can_see_patron_infos' => sub { > plan tests => 6; > >- t::lib::Mocks::mock_userenv({ patron => $patron_11_1 }); >+ t::lib::Mocks::mock_userenv( { patron => $patron_11_1 } ); > $patron_11_1 = Koha::Patrons->find( $patron_11_1->borrowernumber ); >- is( $patron_11_1->can_see_patron_infos( $patron_11_2 ), 1, q|patron_11_1 can see patron_11_2, from its library| ); >- is( $patron_11_1->can_see_patron_infos( $patron_12 ), 1, q|patron_11_1 can see patron_12, from its group| ); >- is( $patron_11_1->can_see_patron_infos( $patron_21 ), 1, q|patron_11_1 can see patron_11_2, from another group| ); >+ is( $patron_11_1->can_see_patron_infos($patron_11_2), 1, q|patron_11_1 can see patron_11_2, from its library| ); >+ is( $patron_11_1->can_see_patron_infos($patron_12), 1, q|patron_11_1 can see patron_12, from its group| ); >+ is( $patron_11_1->can_see_patron_infos($patron_21), 1, q|patron_11_1 can see patron_11_2, from another group| ); > >- t::lib::Mocks::mock_userenv({ patron => $patron_11_2 }); >+ t::lib::Mocks::mock_userenv( { patron => $patron_11_2 } ); > $patron_11_2 = Koha::Patrons->find( $patron_11_2->borrowernumber ); >- is( $patron_11_2->can_see_patron_infos( $patron_11_1 ), 1, q|patron_11_2 can see patron_11_1, from its library| ); >- is( $patron_11_2->can_see_patron_infos( $patron_12 ), 1, q|patron_11_2 can see patron_12, from its group| ); >- is( $patron_11_2->can_see_patron_infos( $patron_21 ), 0, q|patron_11_2 can NOT see patron_21, from another group| ); >+ is( $patron_11_2->can_see_patron_infos($patron_11_1), 1, q|patron_11_2 can see patron_11_1, from its library| ); >+ is( $patron_11_2->can_see_patron_infos($patron_12), 1, q|patron_11_2 can see patron_12, from its group| ); >+ is( >+ $patron_11_2->can_see_patron_infos($patron_21), 0, >+ q|patron_11_2 can NOT see patron_21, from another group| >+ ); > }; > subtest 'search_limited' => sub { > plan tests => 6; > >- t::lib::Mocks::mock_userenv({ patron => $patron_11_1 }); >+ t::lib::Mocks::mock_userenv( { patron => $patron_11_1 } ); > $patron_11_1 = Koha::Patrons->find( $patron_11_1->borrowernumber ); >- my $total_number_of_patrons = $nb_of_patrons + 4; #we added four in these tests >+ my $total_number_of_patrons = $nb_of_patrons + 4; #we added four in these tests > is( Koha::Patrons->search->count, $total_number_of_patrons, 'Non-limited search should return all patrons' ); >- is( Koha::Patrons->search_limited->count, $total_number_of_patrons, 'patron_11_1 is allowed to see all patrons' ); >+ is( >+ Koha::Patrons->search_limited->count, $total_number_of_patrons, >+ 'patron_11_1 is allowed to see all patrons' >+ ); > >- t::lib::Mocks::mock_userenv({ patron => $patron_11_2 }); >+ t::lib::Mocks::mock_userenv( { patron => $patron_11_2 } ); > $patron_11_2 = Koha::Patrons->find( $patron_11_2->borrowernumber ); >- is( Koha::Patrons->search->count, $total_number_of_patrons, 'Non-limited search should return all patrons'); >- is( Koha::Patrons->search_limited->count, 3, 'patron_12_1 is not allowed to see patrons from other groups, only patron_11_1, patron_11_2 and patron_12' ); >+ is( Koha::Patrons->search->count, $total_number_of_patrons, 'Non-limited search should return all patrons' ); >+ is( >+ Koha::Patrons->search_limited->count, 3, >+ 'patron_12_1 is not allowed to see patrons from other groups, only patron_11_1, patron_11_2 and patron_12' >+ ); > >- t::lib::Mocks::mock_userenv({ patron => $patron_21 }); >+ t::lib::Mocks::mock_userenv( { patron => $patron_21 } ); > $patron_21 = Koha::Patrons->find( $patron_21->borrowernumber ); >- is( Koha::Patrons->search->count, $total_number_of_patrons, 'Non-limited search should return all patrons'); >- is( Koha::Patrons->search_limited->count, 1, 'patron_21 is not allowed to see patrons from other groups, only himself' ); >+ is( Koha::Patrons->search->count, $total_number_of_patrons, 'Non-limited search should return all patrons' ); >+ is( >+ Koha::Patrons->search_limited->count, 1, >+ 'patron_21 is not allowed to see patrons from other groups, only himself' >+ ); > }; > $patron_11_1->delete; > $patron_11_2->delete; > $patron_12->delete; > $patron_21->delete; >-}; >+ }; > > subtest 'account_locked' => sub { > plan tests => 13; >-- >2.39.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 37378
:
169411
|
169412
|
169413
|
169430
|
169431
|
169432
|
169711
|
169713
|
169714