From fb2550de6e2c47ad83b63a196f50cbe1e0e16b43 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: Signed-off-by: Jon McGowan Signed-off-by: Jonathan Druart --- Koha/Patron/Modifications.pm | 49 +++++++++++++++++++++--------- t/db_dependent/Koha/Patron/Modifications.t | 39 ++++++++++++++++-------- 2 files changed, 61 insertions(+), 27 deletions(-) diff --git a/Koha/Patron/Modifications.pm b/Koha/Patron/Modifications.pm index 5e7ed9efa5..730056862a 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 53664f5542..f1e6934c73 100755 --- a/t/db_dependent/Koha/Patron/Modifications.t +++ b/t/db_dependent/Koha/Patron/Modifications.t @@ -272,25 +272,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, @@ -302,7 +302,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, @@ -311,7 +311,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 @@ -324,7 +324,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]; @@ -332,7 +332,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' ); @@ -346,6 +346,9 @@ subtest 'pending_count() and pending() tests' => sub { is( $p2_pm_attribute_1->attribute, 'año', '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' ); @@ -370,3 +373,13 @@ 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, + '', '' + ); +} -- 2.11.0