From 177542bf5208e5e053ac9b1500d2536e1eff5ae2 Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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 <m.de.rooy@rijksmuseum.nl>
Tested all crud ops with opac-review.pl (incl URL manipulation).

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
 .../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 ) %]
                             <div class="alert">
                                 [% FOREACH ERROR IN ERRORS %]
+                                    [% IF ( ERROR.nobiblio ) %]
+                                        <p>Error: we cannot find this bibliographic record.</p>
+                                    [% END %]
+                                    [% IF ( ERROR.unauthorized ) %]
+                                        <p>Sorry, only the creator of this comment is allowed to change it.</p>
+                                    [% END %]
                                     [% IF ( ERROR.scrubbed ) %]
                                         <p>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.</p>
                                     [% 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.2