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: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Bug 18403: (follow-up) Patron discharges

Fix QA issue:
forbidden pattern: Do not assume male gender, use they/them instead (bug 18432) (line 150)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/Koha/Patron/Discharge.pm b/Koha/Patron/Discharge.pm
index a9a6b8d..44bcc5d 100644 (file)
--- a/Koha/Patron/Discharge.pm
+++ b/Koha/Patron/Discharge.pm
@@ -27,8 +27,7 @@ sub count {
         $values->{validated} = { '!=', undef };
     }
 
-    my $rs = Koha::Database->new->schema->resultset('Discharge');
-    return $rs->search( $values )->count;
+    return search_limited( $values )->count;
 }
 
 sub can_be_discharged {
@@ -50,9 +49,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;
@@ -163,9 +162,7 @@ sub get_pendings {
         ( defined $branchcode ? ( 'borrower.branchcode' => $branchcode ) : () ),
     };
 
-    my $rs = Koha::Database->new->schema->resultset('Discharge');
-    my @rs = $rs->search( $cond, { join => 'borrower' } );
-    return \@rs;
+    return search_limited( $cond );
 }
 
 sub get_validated {
@@ -179,10 +176,23 @@ sub get_validated {
         ( defined $branchcode ? ( 'borrower.branchcode' => $branchcode ) : () ),
     };
 
-    my $rs = Koha::Database->new->schema->resultset('Discharge');
-    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';
+
+    my $rs = Koha::Database->new->schema->resultset('Discharge');
+    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 (file)
--- 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 @@
             </tr>
           </thead>
           <tbody>
-            [% FOR d IN pending_discharges %]
+            [% FOREACH d IN pending_discharges %]
               <tr>
                 <td><a href="/cgi-bin/koha/members/discharge.pl?borrowernumber=[% d.borrower.borrowernumber %]">[% d.borrower.surname %], [% d.borrower.firstname %]</a></td>
                 <td><a href="/cgi-bin/koha/members/discharges.pl?op=allow&borrowernumber=[% d.borrower.borrowernumber %]">Allow</a></td>

diff --git a/members/discharge.pl b/members/discharge.pl
index ed1c7ac..ca7c44f 100755 (executable)
--- 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 (executable)
--- 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 ddc6835..cf08a88 100644 (file)
--- 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 <http://www.gnu.org/licenses>.
 
 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,5 +131,37 @@ 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 their 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,
+        '',                      ''
+    );
+}