Attachment 71518 Details for Bug 18403 – Bug 18403: Patron modification requests

[patch] Bug 18403: Patron modification requests

Bug-18403-Patron-modification-requests.patch (text/plain), 8.33 KB, created by Jonathan Druart on 2018-02-12 19:34:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2018-02-12 19:34:48 UTC

Size: 8.33 KB

patch

obsolete

>From fb2550de6e2c47ad83b63a196f50cbe1e0e16b43 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>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 <jon.mcgowan@ptfs-europe.com>
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>---
> 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

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