Attachment 36678 Details for Bug 1985 – [PASSED-QA] Bug 1985 - tweak License and lots of cleanup

[patch] [PASSED-QA] Bug 1985 - tweak License and lots of cleanup

PASSED-QA-Bug-1985---tweak-License-and-lots-of-cle.patch (text/plain), 17.25 KB, created by Kyle M Hall (khall) on 2015-03-06 11:03:21 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2015-03-06 11:03:21 UTC

Size: 17.25 KB

patch

obsolete

>From b78aa24a9bd11256d818fc2cd7c46dfd8e9222f2 Mon Sep 17 00:00:00 2001
>From: Mark Tompsett <mtompset@hotmail.com>
>Date: Wed, 4 Mar 2015 19:41:38 -0500
>Subject: [PATCH] [PASSED-QA] Bug 1985 - tweak License and lots of cleanup
>
>In comment #11, it was mentioned that title and biblionumber from
>the biblio table were not available as variables in the letter.
>
>This corrects this by converting the parameter list to a hashref,
>and adding the missing biblio number.
>
>It also cleans up the license on C4/Review.pm to be in line with
>the current coding guidelines license. Also strict and warning
>have been changed to the 'Use Modern::Perl;' now.
>
>Additionally, <<reviews.reviewid>> is now usable in the letter,
>as is <<staffClientBaseURL>>. These were required additions,
>because comment #1 expressly says there should be a direct link
>to approve or delete. I also included a review everything link,
>since there could be multiple comments outstanding.
>
>The default letter was tweaked as well to reflect these changes.
>
>Fixed bad URLs as noted in comment #26.
>
>TEST PLAN
>---------
>1) Create test branch and apply patch
>2) run installer/data/mysql/updatedatabase.pl
>   -- this will add the required default letter.
>3) In the staff client:
>   a) set the CommentModeratorsEmail system preference.
>      -- not setting it will mean you won't get any queued
>         letters.
>   a) set the staffClientBaseURL system preference.
>      -- not setting it will mean your email links won't be
>         valid (i.e http:///...).
>   b) modify the COMMENT_CREATED default letter as desired.
>      -- feel free to add other things.
>4) In the OPAC:
>   a) Log in
>   b) Find something
>   c) Click on the 'Comments' tab
>      -- Holdings/Title notes/Comments (#) is at the bottom
>   d) Click 'Post or edit your comments on this item'
>   e) In the window that pops up, type your comment.
>   f) Click 'Submit and close this window'.
>   g) Click 'Edit' and repeat steps e and f again.
>5) In a mysql client:
>   a) Open the koha database
>   b) SELECT * FROM message_queue WHERE letter_code='COMMENT_CREATED';
>      -- There should be at least two.
>      -- They should be able to contain the borrower number, biblio
>         title, and other borrower and biblio related things.
>      -- Comments are at a biblio level, so items, and issues don't
>         make sense as variables for the letter.
>      -- Confirm the URLs are valid by pasting into a browser.
>6) prove -v t/db_dependent/Review.t
>7) Run the koha qa test tool.
>
>Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
>
>Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>---
> C4/Letters.pm                                      |    6 +-
> C4/Review.pm                                       |   67 ++++++++++----
> .../data/mysql/en/mandatory/sample_notices.sql     |   18 +++-
> installer/data/mysql/updatedatabase.pl             |   62 ++++++++-----
> t/db_dependent/Review.t                            |   97 ++++++++++++++++++++
> 5 files changed, 203 insertions(+), 47 deletions(-)
> create mode 100755 t/db_dependent/Review.t
>
>diff --git a/C4/Letters.pm b/C4/Letters.pm
>index a11c862..dd372d8 100644
>--- a/C4/Letters.pm
>+++ b/C4/Letters.pm
>@@ -625,9 +625,12 @@ sub GetPreparedLetter {
>        }
>     }
> 
>-    my $OPACBaseURL = C4::Context->preference('OPACBaseURL');
>+    my $OPACBaseURL = C4::Context->preference('OPACBaseURL') // '';
>     $letter->{content} =~ s/<<OPACBaseURL>>/$OPACBaseURL/go;
> 
>+    my $staffClientBaseURL = C4::Context->preference('staffClientBaseURL') // '';
>+    $letter->{content} =~ s/<<staffClientBaseURL>>/$staffClientBaseURL/go;
>+
>     if ($want_librarian) {
>         # parsing librarian name
>         my $userenv = C4::Context->userenv;
>@@ -740,6 +743,7 @@ sub _parseletter_sth {
>     ($table eq 'aqbooksellers') ? "SELECT * FROM $table WHERE             id = ?"                                  :
>     ($table eq 'aqorders'     ) ? "SELECT * FROM $table WHERE    ordernumber = ?"                                  :
>     ($table eq 'opac_news'    ) ? "SELECT * FROM $table WHERE          idnew = ?"                                  :
>+    ($table eq 'reviews'      ) ? "SELECT * FROM $table WHERE       reviewid = ?"                                  :
>     ($table eq 'borrower_modifications') ? "SELECT * FROM $table WHERE verification_token = ?" :
>     undef ;
>     unless ($query) {
>diff --git a/C4/Review.pm b/C4/Review.pm
>index dddac55..6ccbf8a 100644
>--- a/C4/Review.pm
>+++ b/C4/Review.pm
>@@ -4,23 +4,23 @@ package C4::Review;
> #
> # This file is part of Koha.
> #
>-# Koha is free software; you can redistribute it and/or modify it under the
>-# terms of the GNU General Public License as published by the Free Software
>-# Foundation; either version 2 of the License, or (at your option) any later
>-# version.
>+# Koha is free software; you can redistribute it and/or modify it
>+# under the terms of the GNU General Public License as published by
>+# the Free Software Foundation; either version 3 of the License, or
>+# (at your option) any later version.
> #
>-# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
>-# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
>-# A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
>+# Koha is distributed in the hope that it will be useful, but
>+# WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+# GNU General Public License for more details.
> #
>-# You should have received a copy of the GNU General Public License along
>-# with Koha; if not, write to the Free Software Foundation, Inc.,
>-# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>+# You should have received a copy of the GNU General Public License
>+# along with Koha; if not, see <http://www.gnu.org/licenses>.
> 
>-use strict;
>-use warnings;
>+use Modern::Perl;
> 
> use C4::Context;
>+require C4::Letters;
> 
> use vars qw($VERSION @ISA @EXPORT);
> 
>@@ -29,8 +29,10 @@ BEGIN {
>     $VERSION = 3.07.00.049;
> 	require Exporter;
> 	@ISA    = qw(Exporter);
>-	@EXPORT = qw(getreview savereview updatereview numberofreviews numberofreviewsbybiblionumber
>-		getreviews getallreviews approvereview unapprovereview deletereview);
>+    @EXPORT = qw(getreview savereview updatereview numberofreviews
>+                 numberofreviewsbybiblionumber getreviews
>+                 getallreviews approvereview unapprovereview
>+                 deletereview SendReviewAlert);
> }
> 
> =head1 NAME
>@@ -79,11 +81,20 @@ sub savereview {
>     my $query = "INSERT INTO reviews (borrowernumber,biblionumber,
> 	review,approved,datereviewed) VALUES 
>   (?,?,?,0,now())";
>+    my $GetReviewID = q{
>+SELECT reviewid FROM reviews WHERE borrowernumber=? AND biblionumber=?;
>+                      };
>     my $sth = $dbh->prepare($query);
>     $sth->execute( $borrowernumber, $biblionumber, $review);
>+    $sth = $dbh->prepare($GetReviewID);
>+    $sth->execute( $borrowernumber, $biblionumber );
>+    my $reviewid = $sth->fetchrow();
> 
>     unless ($sth->err()) {
>-        SendReviewAlert( $review, $borrowernumber );
>+        SendReviewAlert( {
>+                           'reviewid'       => $reviewid,
>+                           'borrowernumber' => $borrowernumber,
>+                           'biblionumber'   => $biblionumber } );
>     }
> }
> 
>@@ -91,11 +102,20 @@ sub updatereview {
>     my ( $biblionumber, $borrowernumber, $review ) = @_;
>     my $dbh   = C4::Context->dbh;
>     my $query = "UPDATE reviews SET review=?,datereviewed=now(),approved=0  WHERE borrowernumber=? and biblionumber=?";
>+    my $GetReviewID = q{
>+SELECT reviewid FROM reviews WHERE borrowernumber=? AND biblionumber=?;
>+                      };
>     my $sth = $dbh->prepare($query);
>     $sth->execute( $review, $borrowernumber, $biblionumber );
>+    $sth = $dbh->prepare($GetReviewID);
>+    $sth->execute( $borrowernumber, $biblionumber );
>+    my $reviewid = $sth->fetchrow();
> 
>     unless ($sth->err()) {
>-        SendReviewAlert( $review, $borrowernumber );
>+        SendReviewAlert( {
>+                           'reviewid'       => $reviewid,
>+                           'borrowernumber' => $borrowernumber,
>+                           'biblionumber'   => $biblionumber } );
>     }
> }
> 
>@@ -196,23 +216,30 @@ sub deletereview {
> 
> =head2 SendReviewAlert
> 
>-  SendReviewAlert( $review, $borrowernumber );
>+  SendReviewAlert( { 'reviewid'       => $reviewid,
>+                     'borrowernumber' => $borrowernumber,
>+                     'biblionumber'   => $biblionumber } );
> 
> =cut
> 
> sub SendReviewAlert {
>-    my $review = shift;
>-    my $borrowernumber = shift;
>+    my $params = shift;
> 
>     my $moderatorEmail = C4::Context->preference('CommentModeratorsEmail');
>-
>     return unless $moderatorEmail;
> 
>+    my $reviewid       = $params->{'reviewid'};
>+    my $borrowernumber = $params->{'borrowernumber'};
>+    my $biblionumber   = $params->{'biblionumber'};
>+
>     my $letter =  C4::Letters::GetPreparedLetter (
>         module => 'members',
>         letter_code => 'COMMENT_CREATED',
>         tables => {
>+            'reviews'     => $reviewid,
>             'borrowers'   => $borrowernumber,
>+            'biblio'      => $biblionumber,
>+            'biblioitems' => $biblionumber,
>         }
>     ) or return;
> 
>diff --git a/installer/data/mysql/en/mandatory/sample_notices.sql b/installer/data/mysql/en/mandatory/sample_notices.sql
>index 506e43b..8967c09 100644
>--- a/installer/data/mysql/en/mandatory/sample_notices.sql
>+++ b/installer/data/mysql/en/mandatory/sample_notices.sql
>@@ -119,9 +119,23 @@ If you did not initiate this request, you may safely ignore this one-time messag
> );
> 
> INSERT INTO  letter (module, code, branchcode, name, is_html, title, content, message_transport_type)
>-VALUES ( 'members', 'COMMENT_CREATED', '', 'Comment created notification', '0', 'Comment from <<borrowers.cardnumber>> is waiting for moderation', 'Dear moderator,
>+VALUES ( 'members', 'COMMENT_CREATED', '', 'Comment created notification', '0', 'Comment from <<borrowers.cardnumber>> is waiting for moderation',
>+'Dear moderator,
> 
>-We want to inform you that borrower <<borrowers.cardnumber>> has just created a new comment.
>+We want to inform you that borrower <<borrowers.cardnumber>> has just created a new comment on <<biblio.title>>, which is biblionumber <<biblio.biblionumber>>.
>+
>+--- BEGIN COMMENT ---
>+<<reviews.review>>
>+--- END COMMENT ---
>+
>+To approve:
>+http://<<staffClientBaseURL>>/cgi-bin/koha/reviews/reviewswaiting.pl?op=approve&reviewid=<<reviews.reviewid>>
>+
>+To delete:
>+http://<<staffClientBaseURL>>/cgi-bin/koha/reviews/reviewswaiting.pl?op=delete&reviewid=<<reviews.reviewid>>
>+
>+Or to review all comments:
>+http://<<staffClientBaseURL>>/cgi-bin/koha/reviews/reviewswaiting.pl
> 
> Check it out!
> 
>diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl
>index a68d67f..c20862c 100755
>--- a/installer/data/mysql/updatedatabase.pl
>+++ b/installer/data/mysql/updatedatabase.pl
>@@ -8314,30 +8314,6 @@ if ( CheckVersion($DBversion) ) {
>     SetVersion($DBversion);
> }
> 
>-$DBversion = "3.17.00.XXX";
>-if ( CheckVersion($DBversion) ) {
>-    $dbh->do(q|
>-INSERT INTO  letter (module, code, branchcode, name, is_html, title, content, message_transport_type)
>-VALUES ( 'members', 'COMMENT_CREATED', '', 'Comment created notification', '0', 'Comment from <<borrowers.cardnumber>> is waiting for moderation', 'Dear moderator,
>-
>-We want to inform you that borrower <<borrowers.cardnumber>> has just created a new comment.
>-
>-Check it out!
>-
>-Your library.',
>-'email'
>-);
>-    |);
>-    $dbh->do(q|
>-        INSERT INTO systempreferences
>-            (variable,value,explanation,options,type)
>-        VALUES
>-        ('CommentModeratorsEmail','','','The email address where to send a notification (template code COMMENT_CREATED) when a Borrower adds/modifies a review/comment for a Biblio. Set to empty to disable sending email notifications.','Textarea')
>-    |);
>-    print "Upgrade to $DBversion done (Bug 1985 - Email notification of new OPAC comments)\n";
>-    SetVersion($DBversion);
>-}
>-
> $DBversion = "3.15.00.041";
> if ( CheckVersion($DBversion) ) {
>     my $name = $dbh->selectcol_arrayref(q|
>@@ -9828,6 +9804,44 @@ if ( CheckVersion($DBversion) ) {
>     SetVersion($DBversion);
> }
> 
>+$DBversion = "3.19.00.XXX";
>+if ( CheckVersion($DBversion) ) {
>+    $dbh->do(q|
>+INSERT INTO  letter (module, code, branchcode, name, is_html, title, content, message_transport_type)
>+VALUES ( 'members', 'COMMENT_CREATED', '', 'Comment created notification', '0', 'Comment from <<borrowers.cardnumber>> is waiting for moderation',
>+'Dear moderator,
>+
>+We want to inform you that borrower <<borrowers.cardnumber>> has just created a new comment on <<biblio.title>>, which is biblionumber <<biblio.biblionumber>>.
>+
>+--- BEGIN COMMENT ---
>+<<reviews.review>>
>+--- END COMMENT ---
>+
>+To approve:
>+http://<<staffClientBaseURL>>/cgi-bin/koha/reviews/reviewswaiting.pl?op=approve&reviewid=<<reviews.reviewid>>
>+
>+To delete:
>+http://<<staffClientBaseURL>>/cgi-bin/koha/reviews/reviewswaiting.pl?op=delete&reviewid=<<reviews.reviewid>>
>+
>+Or to review all comments:
>+http://<<staffClientBaseURL>>/cgi-bin/koha/reviews/reviewswaiting.pl
>+
>+Check it out!
>+
>+Your library.',
>+'email'
>+);
>+    |);
>+    $dbh->do(q|
>+        INSERT INTO systempreferences
>+            (variable,value,explanation,options,type)
>+        VALUES
>+        ('CommentModeratorsEmail','','','The email address where to send a notification (template code COMMENT_CREATED) when a Borrower adds/modifies a review/comment for a Biblio. Set to empty to disable sending email notifications.','Textarea')
>+    |);
>+    print "Upgrade to $DBversion done (Bug 1985 - Email notification of new OPAC comments)\n";
>+    SetVersion($DBversion);
>+}
>+
> =head1 FUNCTIONS
> 
> =head2 TableExists($table)
>diff --git a/t/db_dependent/Review.t b/t/db_dependent/Review.t
>new file mode 100755
>index 0000000..9c990a0
>--- /dev/null
>+++ b/t/db_dependent/Review.t
>@@ -0,0 +1,97 @@
>+#!/usr/bin/perl
>+
>+use Modern::Perl;
>+use Test::More tests => 14;
>+
>+use_ok('C4::Review');
>+
>+my $dbh = C4::Context->dbh;
>+$dbh->{RaiseError} = 1;
>+$dbh->{AutoCommit} = 0;
>+
>+my $sth = $dbh->prepare("SELECT * FROM borrowers;");
>+$sth->execute();
>+my $row = $sth->fetchrow_hashref();
>+my $borrowernumber;
>+$borrowernumber = $row->{'borrowernumber'} if $row;
>+$sth = $dbh->prepare("SELECT * FROM biblio;");
>+$sth->execute();
>+$row = $sth->fetchrow_hashref();
>+my $biblionumber;
>+$biblionumber = $row->{'biblionumber'} if $row;
>+
>+SKIP: {
>+    skip "Missing borrowers or biblios",13
>+      unless $biblionumber and $borrowernumber;
>+
>+    # Make sure there are no reviews
>+    $sth = $dbh->prepare("DELETE FROM reviews;");
>+    $sth->execute();
>+
>+    # Clear out the message_queue to make counting easier.
>+    $sth = $dbh->prepare("DELETE FROM message_queue;");
>+    $sth->execute();
>+
>+    # Add a review
>+    savereview($biblionumber,$borrowernumber,'A sample review.');
>+
>+    # Retrieve review
>+    my $reviews = getallreviews(0);
>+    my $reviewid = $reviews->[0]->{'reviewid'};
>+    my $ReviewText = $reviews->[0]->{'review'};
>+    ok($ReviewText eq 'A sample review.','Saved and retrieved a sample review.');
>+
>+    # Update the review
>+    updatereview($biblionumber,$borrowernumber,'A revised review.');
>+
>+    # Retrieve it differently.
>+    my $review = getreview($biblionumber,$borrowernumber);
>+    $ReviewText = $review->{'review'};
>+    ok($ReviewText eq 'A revised review.','Updated and retrieved a revised review.');
>+
>+    # Confirm 1 unapproved comment.
>+    my $UnapprovedReviewsCount = numberofreviews(0);
>+    ok($UnapprovedReviewsCount==1,'There is one unapproved comment.');
>+
>+    # Approve the comment.
>+    approvereview($reviewid);
>+    $UnapprovedReviewsCount = numberofreviews(0);
>+    ok($UnapprovedReviewsCount==0,'There are no unapproved comments.');
>+    my $ApprovedReviewsCount = numberofreviews(1);
>+    ok($ApprovedReviewsCount==1,'There is one approved comment.');
>+
>+    # Check how many reviews for this biblionumber.
>+    my $ReviewsCount = numberofreviewsbybiblionumber($biblionumber);
>+    ok($ReviewsCount==1,'There is one approved comment for this biblio.');
>+
>+    # Unapprove the comment.
>+    unapprovereview($reviewid);
>+    $UnapprovedReviewsCount = numberofreviews(0);
>+    ok($UnapprovedReviewsCount==1,'There is one unapproved comment.');
>+    $ApprovedReviewsCount = numberofreviews(1);
>+    ok($ApprovedReviewsCount==0,'There are no approved comments.');
>+
>+    # Delete the review.
>+    deletereview($reviewid);
>+    $UnapprovedReviewsCount = numberofreviews(0);
>+    ok($UnapprovedReviewsCount==0,'There are no unapproved comments.');
>+    $ApprovedReviewsCount = numberofreviews(1);
>+    ok($ApprovedReviewsCount==0,'There are no approved comments.');
>+
>+    my $ApprovedReviews   = getreviews($biblionumber,1);
>+    my $UnapprovedReviews = getreviews($biblionumber,0);
>+    ok(scalar @$ApprovedReviews==0,'No approved comment records.');
>+    ok(scalar @$UnapprovedReviews==0,'No unapproved comment records.');
>+
>+    # Only save, update, and SendReviewAlert should trigger letters.
>+    SendReviewAlert( { 'review' => 'A sample review unused.',
>+                       'biblionumber' => $biblionumber,
>+                       'borrowernumber' => $borrowernumber });
>+
>+    $sth = $dbh->prepare("SELECT COUNT(*) FROM message_queue;");
>+    $sth->execute();
>+    my $LetterCount = $sth->fetchrow();
>+    ok($LetterCount==3,'There are three triggered reminders as expected.');
>+}
>+
>+$dbh->rollback();
>-- 
>1.7.10.4

Actions: View | Diff | Splinter Review

Attachments on bug 1985: 33916 | 33918 | 36065 | 36516 | 36517 | 36521 | 36522 | 36613 | 36614 | 36628 | 36629 | 36643 | 36644 | 36645 | 36677 | 36678 | 36720 | 38908 | 38909 | 38910 | 39062 | 63746 | 63747 | 63748 | 63749 | 63750 | 63751 | 63752 | 63761 | 63762 | 63763 | 74760 | 74761 | 74762 | 74763 | 74764 | 74769 | 74770 | 74771 | 91190 | 91191 | 91192 | 91193 | 91194 | 91195 | 91196 | 91197 | 91198 | 91818 | 91819 | 91820 | 91821 | 91822 | 91823