From 177542bf5208e5e053ac9b1500d2536e1eff5ae2 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 29 Jul 2016 10:24:44 +0200 Subject: [PATCH] Bug 15839: [QA Follow-up] Error checking in opac-review.pl [1] Adds a check on biblionumber. (Prevents a DBIx error.) [2] If you have a reviewid, search on that and check results. Add an unauthorized error in template. [3] If you add a new review, check that there is no review yet. If so, edit the existing one. This supports the added FIXME on a unique constraint. Note: This script could receive further attention. Signed-off-by: Marcel de Rooy Tested all crud ops with opac-review.pl (incl URL manipulation). Signed-off-by: Kyle M Hall --- .../bootstrap/en/modules/opac-review.tt | 6 +++++ opac/opac-review.pl | 26 +++++++++++++------ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-review.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-review.tt index b6f968d7b6..7e537b601e 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-review.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-review.tt @@ -18,6 +18,12 @@ [% IF ( ERRORS ) %]
[% FOREACH ERROR IN ERRORS %] + [% IF ( ERROR.nobiblio ) %] +

Error: we cannot find this bibliographic record.

+ [% END %] + [% IF ( ERROR.unauthorized ) %] +

Sorry, only the creator of this comment is allowed to change it.

+ [% END %] [% IF ( ERROR.scrubbed ) %]

Note: your comment contained illegal markup code. It has been saved with the markup removed, as below. You can edit the comment further, or cancel to retain the comment as is.

[% END %] diff --git a/opac/opac-review.pl b/opac/opac-review.pl index 917036b7cc..f8e86bac13 100755 --- a/opac/opac-review.pl +++ b/opac/opac-review.pl @@ -32,6 +32,7 @@ use Koha::Reviews; my $query = new CGI; my $biblionumber = $query->param('biblionumber'); my $review = $query->param('review'); +my $reviewid = $query->param('reviewid'); my ( $template, $borrowernumber, $cookie ) = get_template_and_user( { template_name => "opac-review.tt", @@ -43,11 +44,21 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( # FIXME: need to allow user to delete their own comment(s) +my ( $clean, @errors, $savedreview ); my $biblio = GetBiblioData($biblionumber); -# FIXME biblionumber, borrowernumber should be a unique key of reviews -my $savedreview = Koha::Reviews->search({ biblionumber => $biblionumber, borrowernumber => $borrowernumber })->next; -my ($clean, @errors); -if (defined $review) { + +if( !$biblio ) { + push @errors, { nobiblio => 1 }; +} elsif( $reviewid ) { # edit existing one, check on creator + $savedreview = Koha::Reviews->search({ reviewid => $reviewid, borrowernumber => $borrowernumber })->next; + push @errors, { unauthorized => 1 } if !$savedreview; +} else { # this check prevents adding multiple comments + # FIXME biblionumber, borrowernumber should be a unique key of reviews + $savedreview = Koha::Reviews->search({ biblionumber => $biblionumber, borrowernumber => $borrowernumber })->next; + $review = $savedreview? $savedreview->review: $review; +} + +if( !@errors && defined $review ) { if ($review !~ /\S/) { push @errors, {empty=>1}; } else { @@ -70,12 +81,12 @@ if (defined $review) { } )->store; } else { - Koha::Review->new( + $reviewid = Koha::Review->new( { biblionumber => $biblionumber, borrowernumber => $borrowernumber, review => $clean, } - )->store; + )->store->reviewid; } unless (@errors){ $template->param(WINDOW_CLOSE=>1); } } @@ -89,9 +100,8 @@ $template->param( 'biblionumber' => $biblionumber, 'borrowernumber' => $borrowernumber, 'review' => $review, - 'reviewid' => scalar $query->param('reviewid') || 0, + 'reviewid' => $reviewid || 0, 'title' => $biblio->{'title'}, ); output_html_with_http_headers $query, $cookie, $template->output; - -- 2.39.5