From fb9486ce43019b0e372f604347e1d1242537bbd2 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 7 Apr 2017 14:02:09 -0300 Subject: [PATCH] Bug 18403: Patron modification requests Limit patron's modifications based on logged in patron permissions. Test plan: Create some patron's modification requests at the OPAC Make sure the logged in staff user see (or not) the modification depending his permissions. The number of modification displayed on the mainpage should be correct as well. Signed-off-by: Jon McGowan --- Koha/Patron/Modifications.pm | 49 +++++++++++++++++++++--------- t/db_dependent/Koha/Patron/Modifications.t | 40 ++++++++++++++++-------- 2 files changed, 62 insertions(+), 27 deletions(-) diff --git a/Koha/Patron/Modifications.pm b/Koha/Patron/Modifications.pm index 403b955..1a35894 100644 --- a/Koha/Patron/Modifications.pm +++ b/Koha/Patron/Modifications.pm @@ -52,17 +52,26 @@ sub pending_count { AND borrower_modifications.borrowernumber = borrowers.borrowernumber "; - my @params; - if ($branchcode) { - $query .= " AND borrowers.branchcode = ? "; - push( @params, $branchcode ); + my $userenv = C4::Context->userenv; + my @branchcodes; + if ( $userenv ) { + my $logged_in_user = Koha::Patrons->find( $userenv->{number} ); + if ($branchcode) { + return 0 unless $logged_in_user->can_see_patrons_from($branchcode); + @branchcodes = ( $branchcode ); + } + else { + @branchcodes = $logged_in_user->libraries_where_can_see_patrons; + } + } + my @sql_params; + if ( @branchcodes ) { + $query .= ' AND borrowers.branchcode IN ( ' . join( ',', ('?') x @branchcodes ) . ' )'; + push( @sql_params, @branchcodes ); } - my $sth = $dbh->prepare($query); - $sth->execute(@params); - my $result = $sth->fetchrow_hashref(); - - return $result->{count}; + my ( $count ) = $dbh->selectrow_array( $query, undef, @sql_params ); + return $count; } =head2 pending @@ -84,14 +93,26 @@ sub pending { AND borrower_modifications.borrowernumber = borrowers.borrowernumber "; - my @params; - if ($branchcode) { - $query .= " AND borrowers.branchcode = ? "; - push( @params, $branchcode ); + my $userenv = C4::Context->userenv; + my @branchcodes; + if ( $userenv ) { + my $logged_in_user = Koha::Patrons->find( $userenv->{number} ); + if ($branchcode) { + return 0 unless $logged_in_user->can_see_patrons_from($branchcode); + @branchcodes = ( $branchcode ); + } + else { + @branchcodes = $logged_in_user->libraries_where_can_see_patrons; + } + } + my @sql_params; + if ( @branchcodes ) { + $query .= ' AND borrowers.branchcode IN ( ' . join( ',', ('?') x @branchcodes ) . ' )'; + push( @sql_params, @branchcodes ); } $query .= " ORDER BY borrowers.surname, borrowers.firstname"; my $sth = $dbh->prepare($query); - $sth->execute(@params); + $sth->execute(@sql_params); my @m; while ( my $row = $sth->fetchrow_hashref() ) { diff --git a/t/db_dependent/Koha/Patron/Modifications.t b/t/db_dependent/Koha/Patron/Modifications.t index 7c5593a..f05220e 100755 --- a/t/db_dependent/Koha/Patron/Modifications.t +++ b/t/db_dependent/Koha/Patron/Modifications.t @@ -270,25 +270,25 @@ subtest 'pending_count() and pending() tests' => sub { my $patron_1 = $builder->build( - { source => 'Borrower', value => { branchcode => $library_1 } } ) - ->{borrowernumber}; + { source => 'Borrower', value => { branchcode => $library_1 } } ); my $patron_2 = $builder->build( - { source => 'Borrower', value => { branchcode => $library_2 } } ) - ->{borrowernumber}; + { source => 'Borrower', value => { branchcode => $library_2 } } ); my $patron_3 = $builder->build( - { source => 'Borrower', value => { branchcode => $library_2 } } ) - ->{borrowernumber}; + { source => 'Borrower', value => { branchcode => $library_2 } } ); + $patron_1 = Koha::Patrons->find( $patron_1->{borrowernumber} ); + $patron_2 = Koha::Patrons->find( $patron_2->{borrowernumber} ); + $patron_3 = Koha::Patrons->find( $patron_3->{borrowernumber} ); my $verification_token_1 = md5_hex( time().{}.rand().{}.$$ ); my $verification_token_2 = md5_hex( time().{}.rand().{}.$$ ); my $verification_token_3 = md5_hex( time().{}.rand().{}.$$ ); - Koha::Patron::Attribute->new({ borrowernumber => $patron_1, code => 'CODE_1', attribute => 'hello' } )->store(); - Koha::Patron::Attribute->new({ borrowernumber => $patron_2, code => 'CODE_2', attribute => 'bye' } )->store(); + Koha::Patron::Attribute->new({ borrowernumber => $patron_1->borrowernumber, code => 'CODE_1', attribute => 'hello' } )->store(); + Koha::Patron::Attribute->new({ borrowernumber => $patron_2->borrowernumber, code => 'CODE_2', attribute => 'bye' } )->store(); my $modification_1 = Koha::Patron::Modification->new( - { borrowernumber => $patron_1, + { borrowernumber => $patron_1->borrowernumber, surname => 'Hall', firstname => 'Kyle', verification_token => $verification_token_1, @@ -300,7 +300,7 @@ subtest 'pending_count() and pending() tests' => sub { 1, 'pending_count() correctly returns 1' ); my $modification_2 = Koha::Patron::Modification->new( - { borrowernumber => $patron_2, + { borrowernumber => $patron_2->borrowernumber, surname => 'Smith', firstname => 'Sandy', verification_token => $verification_token_2, @@ -309,7 +309,7 @@ subtest 'pending_count() and pending() tests' => sub { )->store(); my $modification_3 = Koha::Patron::Modification->new( - { borrowernumber => $patron_3, + { borrowernumber => $patron_3->borrowernumber, surname => 'Smithy', firstname => 'Sandy', verification_token => $verification_token_3 @@ -322,7 +322,7 @@ subtest 'pending_count() and pending() tests' => sub { my $pending = Koha::Patron::Modifications->pending(); is( scalar @{$pending}, 3, 'pending() returns an array with 3 elements' ); - my @filtered_modifications = grep { $_->{borrowernumber} eq $patron_1 } @{$pending}; + my @filtered_modifications = grep { $_->{borrowernumber} eq $patron_1->borrowernumber } @{$pending}; my $p1_pm = $filtered_modifications[0]; my $p1_pm_attribute_1 = $p1_pm->{extended_attributes}->[0]; @@ -330,7 +330,7 @@ subtest 'pending_count() and pending() tests' => sub { is( ref($p1_pm_attribute_1), 'Koha::Patron::Attribute', 'patron modification has single attribute object' ); is( $p1_pm_attribute_1->attribute, '', 'patron 1 has an empty value for the attribute' ); - @filtered_modifications = grep { $_->{borrowernumber} eq $patron_2 } @{$pending}; + @filtered_modifications = grep { $_->{borrowernumber} eq $patron_2->borrowernumber } @{$pending}; my $p2_pm = $filtered_modifications[0]; is( scalar @{$p2_pm->{extended_attributes}}, 2 , 'patron 2 has 2 attribute modifications' ); @@ -344,6 +344,9 @@ subtest 'pending_count() and pending() tests' => sub { is( $p2_pm_attribute_1->attribute, 'chau', 'patron modification has the right attribute change' ); is( $p2_pm_attribute_2->attribute, 'ciao', 'patron modification has the right attribute change' ); + + C4::Context->_new_userenv('xxx'); + set_logged_in_user( $patron_1 ); is( Koha::Patron::Modifications->pending_count($library_1), 1, 'pending_count() correctly returns 1 if filtered by library' ); @@ -368,4 +371,15 @@ subtest 'pending_count() and pending() tests' => sub { $schema->storage->txn_rollback; }; +sub set_logged_in_user { + my ($patron) = @_; + C4::Context->set_userenv( + $patron->borrowernumber, $patron->userid, + $patron->cardnumber, 'firstname', + 'surname', $patron->library->branchcode, + 'Midway Public Library', $patron->flags, + '', '' + ); +} + 1; -- 2.1.4