Attachment 62030 Details for Bug 18403 – Bug 18403: Patron discharges

[patch] Bug 18403: Patron discharges

Bug-18403-Patron-discharges.patch (text/plain), 11.70 KB, created by Jonathan Druart on 2017-04-10 21:33:50 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2017-04-10 21:33:50 UTC

Size: 11.70 KB

patch

obsolete

>From c38a408dbb1fe9f3e2d1ae8535468a11f07d225e Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>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.
>---
> 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 @@
>             </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
>--- 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 <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,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.9.3

Actions: View | Diff | Splinter Review

Attachments on bug 18403: 62010 | 62011 | 62012 | 62013 | 62014 | 62015 | 62016 | 62017 | 62018 | 62019 | 62020 | 62021 | 62022 | 62023 | 62024 | 62025 | 62026 | 62027 | 62028 | 62029 | 62030 | 62031 | 62032 | 62033 | 62302 | 62303 | 62830 | 62831 | 62832 | 62833 | 62834 | 62835 | 62836 | 62837 | 62838 | 62839 | 62840 | 62841 | 62842 | 62843 | 62844 | 62845 | 62846 | 62847 | 62848 | 62849 | 62850 | 62851 | 62852 | 62853 | 62854 | 62855 | 71392 | 71394 | 71500 | 71501 | 71502 | 71503 | 71504 | 71505 | 71506 | 71507 | 71508 | 71509 | 71510 | 71511 | 71512 | 71513 | 71514 | 71515 | 71516 | 71517 | 71518 | 71519 | 71520 | 71521 | 71522 | 71523 | 71524 | 71525 | 71526 | 71527 | 71528 | 71529 | 71530 | 71767