From f05a8c839f7bd2c3f1da070b81701f0c4dc9d997 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 1 Nov 2024 11:35:04 +0000 Subject: [PATCH] Bug 37392: (follow-up) Limit a borrower not in a group and fix tests The previous patches took into account all the groups for a patron, but missed the case where a patron didn't have permission to see outside their library, and their library is not in a group. Code updated and a test added. Other tests adjusted to ensure the feature to limit patrons was set in those groups. Signed-off-by: Tomas Cohen Arazi --- Koha/Patron.pm | 3 ++- t/db_dependent/ArticleRequests.t | 4 ++-- t/db_dependent/Koha/Patrons.t | 21 +++++++++++++++++++-- t/db_dependent/Koha/Reviews.t | 4 ++-- t/db_dependent/Patron/Borrower_Discharge.t | 4 ++-- 5 files changed, 27 insertions(+), 9 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index f97e104e21..bc031f8bcf 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -2041,8 +2041,9 @@ sub libraries_where_can_see_things { } } + } else { + push @restricted_branchcodes, $self->branchcode; } - } } diff --git a/t/db_dependent/ArticleRequests.t b/t/db_dependent/ArticleRequests.t index f3d5b317c9..289015b431 100755 --- a/t/db_dependent/ArticleRequests.t +++ b/t/db_dependent/ArticleRequests.t @@ -174,8 +174,8 @@ subtest 'search_limited' => sub { plan tests => 2; my $nb_article_requests = Koha::ArticleRequests->count; - my $group_1 = Koha::Library::Group->new( { title => 'TEST Group 1' } )->store; - my $group_2 = Koha::Library::Group->new( { title => 'TEST Group 2' } )->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; Koha::Library::Group->new({ parent_id => $group_1->id, branchcode => $patron->branchcode })->store(); Koha::Library::Group->new({ parent_id => $group_2->id, branchcode => $patron_2->branchcode })->store(); t::lib::Mocks::mock_userenv( { patron => $patron } ); # Is superlibrarian diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 8f94ab2945..994d3fe409 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -1322,12 +1322,15 @@ subtest my $library_11 = $builder->build( { source => 'Branch' } ); my $library_12 = $builder->build( { source => 'Branch' } ); my $library_21 = $builder->build( { source => 'Branch' } ); + my $library_31 = $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} ); + $library_31 = Koha::Libraries->find( $library_31->{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; + # Library 31, not in any group my $sth = C4::Context->dbh->prepare(q|INSERT INTO user_permissions( borrowernumber, module_bit, code ) VALUES (?, ?, ?)|); @@ -1359,6 +1362,12 @@ subtest $patron_21 = Koha::Patrons->find( $patron_21->{borrowernumber} ); $sth->execute( $patron_21->borrowernumber, 4, 'edit_borrowers' ); + # 1 patron from library_31 (no group) can only see patron's info from its library + my $patron_31 = $builder->build( + { source => 'Borrower', value => { branchcode => $library_31->branchcode, flags => undef, } } ); + $patron_31 = Koha::Patrons->find( $patron_31->{borrowernumber} ); + $sth->execute( $patron_31->borrowernumber, 4, 'edit_borrowers' ); + # Pfiou, we can start now! subtest 'libraries_where_can_see_things' => sub { plan tests => 4; @@ -1430,7 +1439,7 @@ subtest }; subtest 'libraries_where_can_see_patrons' => sub { - plan tests => 3; + plan tests => 4; my @branchcodes; @@ -1451,6 +1460,13 @@ subtest \@branchcodes, [ $library_21->branchcode ], q|patron_21 has not view_borrower_infos_from_any_libraries => Can only see patron's from its group| ); + + t::lib::Mocks::mock_userenv( { patron => $patron_31 } ); + @branchcodes = $patron_31->libraries_where_can_see_patrons; + is_deeply( + \@branchcodes, [ $library_31->branchcode ], + q|patron_31 has not view_borrower_infos_from_any_libraries => Can only see patron's from its library that is not in a group| + ); }; subtest 'can_see_patron_infos' => sub { plan tests => 6; @@ -1475,7 +1491,7 @@ subtest 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 + 5; #we added five 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, @@ -1502,6 +1518,7 @@ subtest $patron_11_2->delete; $patron_12->delete; $patron_21->delete; + $patron_31->delete; }; subtest 'account_locked' => sub { diff --git a/t/db_dependent/Koha/Reviews.t b/t/db_dependent/Koha/Reviews.t index 57c651f43c..0c8dcdc9c4 100755 --- a/t/db_dependent/Koha/Reviews.t +++ b/t/db_dependent/Koha/Reviews.t @@ -70,8 +70,8 @@ is( $retrieved_review_1_1->review, $new_review_1_1->review, 'Find a review by id subtest 'search_limited' => sub { plan tests => 2; - my $group_1 = Koha::Library::Group->new( { title => 'TEST Group 1' } )->store; - my $group_2 = Koha::Library::Group->new( { title => 'TEST Group 2' } )->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; Koha::Library::Group->new({ parent_id => $group_1->id, branchcode => $patron_1->branchcode })->store(); Koha::Library::Group->new({ parent_id => $group_2->id, branchcode => $patron_2->branchcode })->store(); t::lib::Mocks::mock_userenv( { patron => $patron_1 } ); diff --git a/t/db_dependent/Patron/Borrower_Discharge.t b/t/db_dependent/Patron/Borrower_Discharge.t index 6c06ecff90..4b04530b9f 100755 --- a/t/db_dependent/Patron/Borrower_Discharge.t +++ b/t/db_dependent/Patron/Borrower_Discharge.t @@ -154,8 +154,8 @@ is( ref(Koha::Patron::Discharge::request({ borrowernumber => $patron->borrowernu subtest 'search_limited' => sub { plan tests => 4; $dbh->do(q|DELETE FROM discharges|); - my $group_1 = Koha::Library::Group->new( { title => 'TEST Group 1' } )->store; - my $group_2 = Koha::Library::Group->new( { title => 'TEST Group 2' } )->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; # $patron and $patron2 are from the same library, $patron3 from another one # Logged in user is $patron, superlibrarian t::lib::Mocks::mock_userenv({ patron => $patron }); -- 2.39.5