Attachment 62849 Details for Bug 18403 – Bug 18403: Patron reviews

[patch] Bug 18403: Patron reviews

Bug-18403-Patron-reviews.patch (text/plain), 7.03 KB, created by Martin Renvoize (ashimema) on 2017-04-28 15:30:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2017-04-28 15:30:51 UTC

Size: 7.03 KB

patch

obsolete

>From 57f95a631cc405191ee0e18f7354c9c03bdc90f5 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Fri, 7 Apr 2017 14:37:47 -0300
>Subject: [PATCH] Bug 18403: Patron reviews
>
>This patch adds a new method Koha::Reviews->search_limited to return the
>reviews
>a logged in user is allowed to see depending his permissions.
>
>Test plan:
>Create some reviews at the OPAC and make sure a staff user is limited
>(or not) to approve
>or decline it.
>The number of reviews displayed on the mainpage should be correct as
>well.
>
>Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
>---
> Koha/Reviews.pm               | 23 +++++++++++++++++++++++
> mainpage.pl                   |  2 +-
> reviews/reviewswaiting.pl     |  4 ++--
> t/db_dependent/Koha/Reviews.t | 37 +++++++++++++++++++++++++++++++------
> tools/tools-home.pl           |  2 +-
> 5 files changed, 58 insertions(+), 10 deletions(-)
>
>diff --git a/Koha/Reviews.pm b/Koha/Reviews.pm
>index 13b0314..a5fcae3 100644
>--- a/Koha/Reviews.pm
>+++ b/Koha/Reviews.pm
>@@ -35,6 +35,29 @@ Koha::Reviews - Koha Review Object set class
> 
> =cut
> 
>+=head2 search_limited
>+
>+my $reviews = Koha::Reviews->search_limited( $params, $attributes );
>+
>+Search for reviews according to logged in patron restrictions
>+
>+=cut
>+
>+sub search_limited {
>+    my ( $self, $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;
>+    }
>+    # TODO This 'borrowernumber' relation name is confusing and needs to be renamed
>+    $params->{'borrowernumber.branchcode'} = { -in => \@restricted_branchcodes } if @restricted_branchcodes;
>+    $attributes->{join} = 'borrowernumber';
>+    return $self->search( $params, $attributes );
>+}
>+
> =head3 type
> 
> =cut
>diff --git a/mainpage.pl b/mainpage.pl
>index c1df82b..f4105be 100755
>--- a/mainpage.pl
>+++ b/mainpage.pl
>@@ -63,7 +63,7 @@ my $branch =
>   ? C4::Context->userenv()->{'branch'}
>   : undef;
> 
>-my $pendingcomments    = Koha::Reviews->search({ approved => 0 })->count;
>+my $pendingcomments    = Koha::Reviews->search_limited({ approved => 0 })->count;
> my $pendingtags        = get_count_by_tag_status(0);
> my $pendingsuggestions = CountSuggestion("ASKED");
> my $pending_borrower_modifications = Koha::Patron::Modifications->pending_count( $branch );
>diff --git a/reviews/reviewswaiting.pl b/reviews/reviewswaiting.pl
>index 2669664..ae30c78 100755
>--- a/reviews/reviewswaiting.pl
>+++ b/reviews/reviewswaiting.pl
>@@ -42,7 +42,7 @@ my $status   = $query->param('status') || 0;
> my $reviewid = $query->param('reviewid');
> my $page     = $query->param('page') || 1;
> my $count    = C4::Context->preference('numSearchResults') || 20;
>-my $total    = Koha::Reviews->search({ approved => $status })->count;
>+my $total    = Koha::Reviews->search_limited({ approved => $status })->count;
> 
> if ( $op eq 'approve' ) {
>     my $review = Koha::Reviews->find( $reviewid );
>@@ -57,7 +57,7 @@ elsif ( $op eq 'delete' ) {
>     $review->delete if $review;
> }
> 
>-my $reviews = Koha::Reviews->search(
>+my $reviews = Koha::Reviews->search_limited(
>     { approved => $status },
>     {
>         rows => $count,
>diff --git a/t/db_dependent/Koha/Reviews.t b/t/db_dependent/Koha/Reviews.t
>index da164f7..b6b4b6c 100644
>--- a/t/db_dependent/Koha/Reviews.t
>+++ b/t/db_dependent/Koha/Reviews.t
>@@ -19,9 +19,9 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 7;
>+use Test::More tests => 8;
> 
>-use Koha::Review;
>+use Koha::Patrons;
> use Koha::Reviews;
> use Koha::Database;
> 
>@@ -31,24 +31,26 @@ my $schema = Koha::Database->new->schema;
> $schema->storage->txn_begin;
> 
> my $builder = t::lib::TestBuilder->new;
>-my $patron_1 = $builder->build({ source => 'Borrower' });
>+my $patron_1 = $builder->build({ source => 'Borrower', value => { flags => undef } });
> my $patron_2 = $builder->build({ source => 'Borrower' });
>+$patron_1 = Koha::Patrons->find( $patron_1->{borrowernumber} );
>+$patron_2 = Koha::Patrons->find( $patron_2->{borrowernumber} );
> my $biblio_1 = $builder->build({ source => 'Biblio' });
> my $biblio_2 = $builder->build({ source => 'Biblio' });
> my $nb_of_reviews = Koha::Reviews->search->count;
> my $nb_of_approved_reviews = Koha::Reviews->search({ approved => 1 })->count;
> my $new_review_1_1 = Koha::Review->new({
>-    borrowernumber => $patron_1->{borrowernumber},
>+    borrowernumber => $patron_1->borrowernumber,
>     biblionumber => $biblio_1->{biblionumber},
>     review => 'a kind review',
> })->store;
> my $new_review_1_2 = Koha::Review->new({
>-    borrowernumber => $patron_1->{borrowernumber},
>+    borrowernumber => $patron_1->borrowernumber,
>     biblionumber => $biblio_2->{biblionumber},
>     review => 'anoter kind review',
> })->store;
> my $new_review_2_1 = Koha::Review->new({
>-    borrowernumber => $patron_2->{borrowernumber},
>+    borrowernumber => $patron_2->borrowernumber,
>     biblionumber => $biblio_1->{biblionumber},
>     review => 'just anoter review',
> })->store;
>@@ -65,9 +67,32 @@ is( Koha::Reviews->search({approved => 1})->count, $nb_of_approved_reviews, 'The
> my $retrieved_review_1_1 = Koha::Reviews->find( $new_review_1_1->reviewid );
> is( $retrieved_review_1_1->review, $new_review_1_1->review, 'Find a review by id should return the correct review' );
> 
>+subtest 'search_limited' => sub {
>+    plan tests => 2;
>+    C4::Context->_new_userenv('xxx');
>+    my $group_1 = Koha::Library::Group->new( { title => 'TEST Group 1' } )->store;
>+    my $group_2 = Koha::Library::Group->new( { title => 'TEST Group 2' } )->store;
>+    Koha::Library::Group->new({ parent_id => $group_1->id,  branchcode => $patron_1->branchcode })->store();
>+    Koha::Library::Group->new({ parent_id => $group_2->id,  branchcode => $patron_2->branchcode })->store();
>+    set_logged_in_user( $patron_1 );
>+    is( Koha::Reviews->search->count, $nb_of_approved_reviews + 3, 'Koha::Reviews->search should return all reviews' );
>+    is( Koha::Reviews->search_limited->count, $nb_of_approved_reviews + 2, 'Koha::Reviews->search_limited should return reviews depending on patron permissions' );
>+};
>+
> $retrieved_review_1_1->delete;
> is( Koha::Reviews->search->count, $nb_of_reviews + 2, 'Delete should have deleted the review' );
> 
> $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;
>diff --git a/tools/tools-home.pl b/tools/tools-home.pl
>index 86cfb03..99fde63 100755
>--- a/tools/tools-home.pl
>+++ b/tools/tools-home.pl
>@@ -35,7 +35,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
>     }
> );
> 
>-my $pendingcomments = Koha::Reviews->search({ approved => 0 })->count;
>+my $pendingcomments = Koha::Reviews->search_limited({ approved => 0 })->count;
> my $pendingtags = get_count_by_tag_status(0);
> 
> $template->param(
>-- 
>2.1.4

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