From e2008bc0c84a3b6fd0a416d86162139b33dcebde Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 7 Apr 2017 15:42:51 -0300 Subject: [PATCH] Bug 18403: Patron discharges This patch deals with patron's discharges. Test plan: Same as previously you will need to request dischages at the OPAC. On the staff interface the logged in user should not be allowed to see discharge from patrons outside his library group. The number of discharges waiting displayed on the mainpage should be correct as well. Signed-off-by: Jon McGowan --- Koha/Patron/Discharge.pm | 26 +++++++--- .../prog/en/modules/members/discharges.tt | 2 +- members/discharge.pl | 4 +- members/discharges.pl | 5 +- t/db_dependent/Patron/Borrower_Discharge.t | 56 ++++++++++++++++++---- 5 files changed, 71 insertions(+), 22 deletions(-) diff --git a/Koha/Patron/Discharge.pm b/Koha/Patron/Discharge.pm index 7452811..52d3abf 100644 --- a/Koha/Patron/Discharge.pm +++ b/Koha/Patron/Discharge.pm @@ -30,7 +30,7 @@ sub count { $values->{validated} = { '!=', undef }; } - return $rs->search( $values )->count; + return search_limited( $values )->count; } sub can_be_discharged { @@ -52,9 +52,9 @@ sub is_discharged { my $borrowernumber = $params->{borrowernumber}; my $restricted = Koha::Patrons->find( $borrowernumber )->is_debarred; - my $validated = get_validated({borrowernumber => $borrowernumber}); + my @validated = get_validated({borrowernumber => $borrowernumber}); - if ($restricted && $validated) { + if ($restricted && @validated) { return 1; } else { return 0; @@ -161,8 +161,7 @@ sub get_pendings { ( defined $branchcode ? ( 'borrower.branchcode' => $branchcode ) : () ), }; - my @rs = $rs->search( $cond, { join => 'borrower' } ); - return \@rs; + return search_limited( $cond ); } sub get_validated { @@ -176,9 +175,22 @@ sub get_validated { ( defined $branchcode ? ( 'borrower.branchcode' => $branchcode ) : () ), }; - my @rs = $rs->search( $cond, { join => 'borrower' } ); - return \@rs; + return search_limited( $cond ); } +# TODO This module should be based on Koha::Object[s] +sub search_limited { + my ( $params, $attributes ) = @_; + my $userenv = C4::Context->userenv; + my @restricted_branchcodes; + if ( $userenv ) { + my $logged_in_user = Koha::Patrons->find( $userenv->{number} ); + @restricted_branchcodes = $logged_in_user->libraries_where_can_see_patrons; + } + $params->{'borrower.branchcode'} = { -in => \@restricted_branchcodes } if @restricted_branchcodes; + $attributes->{join} = 'borrower'; + + return $rs->search( $params, { join => 'borrower' } ); +} 1; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/discharges.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/discharges.tt index 0998acd..91e9091 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/discharges.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/discharges.tt @@ -23,7 +23,7 @@ - [% FOR d IN pending_discharges %] + [% FOREACH d IN pending_discharges %] [% d.borrower.surname %], [% d.borrower.firstname %] Allow diff --git a/members/discharge.pl b/members/discharge.pl index ed1c7ac..ca7c44f 100755 --- a/members/discharge.pl +++ b/members/discharge.pl @@ -102,7 +102,7 @@ if ( $input->param('discharge') and $can_be_discharged ) { } # Already generated discharges -my $validated_discharges = Koha::Patron::Discharge::get_validated({ +my @validated_discharges = Koha::Patron::Discharge::get_validated({ borrowernumber => $borrowernumber, }); @@ -132,7 +132,7 @@ $template->param( branchcode => $patron->branchcode, has_reserves => $has_reserves, can_be_discharged => $can_be_discharged, - validated_discharges => $validated_discharges, + validated_discharges => \@validated_discharges, ); $template->param( dischargeview => 1, ); diff --git a/members/discharges.pl b/members/discharges.pl index 165a1cb..665d273 100755 --- a/members/discharges.pl +++ b/members/discharges.pl @@ -52,10 +52,9 @@ if( $op eq 'allow' ) { }) if $borrowernumber; } -my $pending_discharges = Koha::Patron::Discharge::get_pendings({ +my @pending_discharges = Koha::Patron::Discharge::get_pendings({ branchcode => $branchcode }); - -$template->param( pending_discharges => $pending_discharges ); +$template->param( pending_discharges => \@pending_discharges ); output_html_with_http_headers $input, $cookie, $template->output; diff --git a/t/db_dependent/Patron/Borrower_Discharge.t b/t/db_dependent/Patron/Borrower_Discharge.t index d5e3e2d..e90c4fd 100644 --- a/t/db_dependent/Patron/Borrower_Discharge.t +++ b/t/db_dependent/Patron/Borrower_Discharge.t @@ -15,7 +15,7 @@ # with Koha; if not, see . use Modern::Perl; -use Test::More tests => 18; +use Test::More tests => 19; use Test::Warn; use MARC::Record; @@ -43,13 +43,16 @@ my $another_library = $builder->build({ source => 'Branch' }); my $itemtype = $builder->build({ source => 'Itemtype' })->{itemtype}; C4::Context->_new_userenv('xxx'); -C4::Context->set_userenv(0, 0, 0, 'firstname', 'surname', $library->{branchcode}, $library->{branchcode}, '', '', '', ''); my $patron = $builder->build({ source => 'Borrower', value => { branchcode => $library->{branchcode}, + flags => 1, # superlibrarian } }); +my $p = Koha::Patrons->find( $patron->{borrowernumber} ); +set_logged_in_user( $p ); + my $patron2 = $builder->build({ source => 'Borrower', value => { @@ -60,8 +63,10 @@ my $patron3 = $builder->build({ source => 'Borrower', value => { branchcode => $another_library->{branchcode}, + flags => undef, } }); +my $p3 = Koha::Patrons->find( $patron3->{borrowernumber} ); # Discharge not possible with issues my ( $biblionumber ) = AddBiblio( MARC::Record->new, ''); @@ -80,8 +85,8 @@ is( Koha::Patron::Discharge::can_be_discharged({ borrowernumber => $patron->{bor is( Koha::Patron::Discharge::request({ borrowernumber => $patron->{borrowernumber} }), undef, 'No request done if patron has issues' ); is( Koha::Patron::Discharge::discharge({ borrowernumber => $patron->{borrowernumber} }), undef, 'No discharge done if patron has issues' ); -is_deeply( Koha::Patron::Discharge::get_pendings(), [], 'There is no pending discharge request' ); -is_deeply( Koha::Patron::Discharge::get_validated(), [], 'There is no validated discharge' ); +is_deeply( [ Koha::Patron::Discharge::get_pendings ], [], 'There is no pending discharge request' ); +is_deeply( [ Koha::Patron::Discharge::get_validated ], [], 'There is no validated discharge' ); AddReturn( $barcode ); @@ -96,18 +101,18 @@ Koha::Patron::Discharge::discharge( { borrowernumber => $patron2->{borrowernumbe Koha::Patron::Discharge::discharge( { borrowernumber => $patron3->{borrowernumber} } ); is( Koha::Patron::Discharge::is_discharged( { borrowernumber => $patron->{borrowernumber} } ), 1, 'The patron has been discharged' ); is( Koha::Patrons->find( $patron->{borrowernumber} )->is_debarred, '9999-12-31', 'The patron has been debarred after discharge' ); -is( scalar( @{ Koha::Patron::Discharge::get_validated() } ), 3, 'There are 3 validated discharges' ); -is( scalar( @{ Koha::Patron::Discharge::get_validated( { borrowernumber => $patron->{borrowernumber} } ) } ), 1, 'There is 1 validated discharge for a given patron' ); -is( scalar( @{ Koha::Patron::Discharge::get_validated( { branchcode => $library->{branchcode} } ) } ), 2, 'There is 2 validated discharges for a given branchcode' ); # This is not used in the code yet +is( scalar( Koha::Patron::Discharge::get_validated ), 3, 'There are 3 validated discharges' ); +is( scalar( Koha::Patron::Discharge::get_validated( { borrowernumber => $patron->{borrowernumber} } ) ), 1, 'There is 1 validated discharge for a given patron' ); +is( scalar( Koha::Patron::Discharge::get_validated( { branchcode => $library->{branchcode} } ) ), 2, 'There is 2 validated discharges for a given branchcode' ); # This is not used in the code yet Koha::Patron::Debarments::DelUniqueDebarment( { 'borrowernumber' => $patron->{borrowernumber}, 'type' => 'DISCHARGE' } ); ok( !Koha::Patrons->find( $patron->{borrowernumber} )->is_debarred, 'The debarment has been lifted' ); ok( !Koha::Patron::Discharge::is_discharged( { borrowernumber => $patron->{borrowernumber} } ), 'The patron is not discharged after the restriction has been lifted' ); # Verify that the discharge works multiple times Koha::Patron::Discharge::request({ borrowernumber => $patron->{borrowernumber} }); -is(scalar( @{ Koha::Patron::Discharge::get_pendings() }), 1, 'There is a pending discharge request (second time)'); +is(scalar( Koha::Patron::Discharge::get_pendings ), 1, 'There is a pending discharge request (second time)'); Koha::Patron::Discharge::discharge( { borrowernumber => $patron->{borrowernumber} } ); -is_deeply( Koha::Patron::Discharge::get_pendings(), [], 'There is no pending discharge request (second time)'); +is_deeply( [ Koha::Patron::Discharge::get_pendings ], [], 'There is no pending discharge request (second time)'); # Check if PDF::FromHTML is installed. my $check = eval { require PDF::FromHTML; }; @@ -126,6 +131,39 @@ else { # FIXME Should be a Koha::Object object is( ref(Koha::Patron::Discharge::request({ borrowernumber => $patron->{borrowernumber} })), 'Koha::Schema::Result::Discharge', 'Discharge request sent' ); +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; + # $patron and $patron2 are from the same library, $patron3 from another one + # Logged in user is $patron, superlibrarian + set_logged_in_user( $p ); + Koha::Library::Group->new({ parent_id => $group_1->id, branchcode => $patron->{branchcode} })->store(); + Koha::Library::Group->new({ parent_id => $group_2->id, branchcode => $patron3->{branchcode} })->store(); + Koha::Patron::Discharge::request({ borrowernumber => $patron->{borrowernumber} }); + Koha::Patron::Discharge::request({ borrowernumber => $patron2->{borrowernumber} }); + Koha::Patron::Discharge::request({ borrowernumber => $patron3->{borrowernumber} }); + is( scalar( Koha::Patron::Discharge::get_pendings), 3, 'With permission, all discharges are visible' ); + is( Koha::Patron::Discharge::count({pending => 1}), 3, 'With permission, all discharges are visible' ); + + # With patron 3 logged in, only discharges from his group are visible + set_logged_in_user( $p3 ); + is( scalar( Koha::Patron::Discharge::get_pendings), 1, 'Without permission, only discharge from our group are visible' ); + is( Koha::Patron::Discharge::count({pending => 1}), 1, 'Without permission, only discharge from our group are visible' ); +}; + $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