Bug 1985 - Email notification of new OPAC comments
Summary: Email notification of new OPAC comments
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P3 enhancement
Assignee: Mark Tompsett
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-26 08:47 UTC by Owen Leonard
Modified: 2019-08-08 17:22 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 1985 - Email notification of new OPAC comments (7.74 KB, patch)
2014-11-25 16:24 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 1985 - Email notification of new OPAC comments (7.61 KB, patch)
2014-11-25 16:42 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 1985 - Email notification of new OPAC comments (7.79 KB, patch)
2015-02-20 11:26 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 1985 - Email notification of new OPAC comments (7.79 KB, patch)
2015-03-05 02:49 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985 - Add missing table variables, tweak License, and cleanup (9.59 KB, patch)
2015-03-05 02:49 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985 - Email notification of new OPAC comments (7.79 KB, patch)
2015-03-05 04:43 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985 - tweak License and lots of cleanup (16.81 KB, patch)
2015-03-05 04:43 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED OFF] Bug 1985 - Email notification of new OPAC comments (8.36 KB, patch)
2015-03-05 15:06 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985 - tweak License and lots of cleanup (17.08 KB, patch)
2015-03-05 15:06 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985 - Email notification of new OPAC comments (8.36 KB, patch)
2015-03-05 16:46 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985 - tweak License and lots of cleanup (17.12 KB, patch)
2015-03-05 16:47 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 13635: Unimarc - On editing a notice, the title should be displayed (3.65 KB, patch)
2015-03-06 09:12 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 1985 - Email notification of new OPAC comments (8.42 KB, patch)
2015-03-06 09:12 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 1985 - tweak License and lots of cleanup (17.18 KB, patch)
2015-03-06 09:12 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
[PASSED-QA] Bug 1985 - Email notification of new OPAC comments (8.49 KB, patch)
2015-03-06 11:01 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
[PASSED-QA] Bug 1985 - tweak License and lots of cleanup (17.25 KB, patch)
2015-03-06 11:03 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 1985 - Follow up: Add sample letter to other languages (15.25 KB, patch)
2015-03-06 16:58 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985 - Email notification of new OPAC comments (8.17 KB, patch)
2015-05-06 20:47 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985 - tweak License and lots of cleanup (16.16 KB, patch)
2015-05-06 20:47 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985 - Follow up: Add sample letter to other languages (15.25 KB, patch)
2015-05-06 20:47 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 1985: Follow up - Add sample letter to other languages (15.37 KB, patch)
2015-05-12 11:23 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 1985: Fix whitespace in opac/opac-review.pl (2.01 KB, patch)
2017-05-26 17:50 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Introduction of review alerts (16.79 KB, patch)
2017-05-26 17:51 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Follow up - Add sample letter to other languages (15.24 KB, patch)
2017-05-26 17:51 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: perlcritic (3.68 KB, patch)
2017-05-26 17:51 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: perltidy opac/opac-review.pl (5.60 KB, patch)
2017-05-26 17:51 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: perlcritic (5.28 KB, patch)
2017-05-26 17:51 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: perltidy Koha/Review.pm (2.32 KB, patch)
2017-05-26 17:51 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Fix whitespace in opac/opac-review.pl (2.14 KB, patch)
2017-05-27 06:25 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 1985: Introduction of review alerts (16.93 KB, patch)
2017-05-27 06:27 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 1985: Follow up - Add sample letter to other languages (15.37 KB, patch)
2017-05-27 06:38 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 1985: Fix whitespace in opac/opac-review.pl (2.14 KB, patch)
2018-04-23 18:14 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Send email alert when comment posted. (17.77 KB, patch)
2018-04-23 18:14 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Follow up - Add sample letter to other languages (15.40 KB, patch)
2018-04-23 18:14 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Send email alert when comment posted. (16.97 KB, patch)
2018-04-23 18:23 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Follow up - Add sample letter to other languages (15.40 KB, patch)
2018-04-23 18:23 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Fix whitespace in opac/opac-review.pl (2.14 KB, patch)
2018-04-23 22:23 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Send email alert when comment posted. (17.48 KB, patch)
2018-04-23 22:23 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Follow up - Add sample letter to other languages (15.45 KB, patch)
2018-04-23 22:23 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Fix whitespace in opac/opac-review.pl (1.81 KB, patch)
2019-07-02 17:54 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Send email alert when comment posted. (16.95 KB, patch)
2019-07-02 17:54 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Follow up - Add sample letter to other languages (15.50 KB, patch)
2019-07-02 17:54 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Fix whitespace in opac/opac-review.pl (1.81 KB, patch)
2019-07-03 00:18 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Send email alert when comment posted. (16.95 KB, patch)
2019-07-03 00:18 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: (follow-up) correct http:// in staff URL (2.41 KB, patch)
2019-07-03 00:18 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: (follow-up) Add letter to other languages (15.49 KB, patch)
2019-07-03 00:18 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: (follow-up) fix staff URL in other letters (9.06 KB, patch)
2019-07-03 00:19 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: (follow-up) Fix Reviews.t logic (2.87 KB, patch)
2019-07-03 00:19 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 1985: Fix whitespace in opac/opac-review.pl (1.88 KB, patch)
2019-07-26 15:41 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 1985: Send email alert when comment posted. (17.02 KB, patch)
2019-07-26 15:41 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 1985: (follow-up) correct http:// in staff URL (2.47 KB, patch)
2019-07-26 15:41 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 1985: (follow-up) Add letter to other languages (15.56 KB, patch)
2019-07-26 15:41 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 1985: (follow-up) fix staff URL in other letters (9.12 KB, patch)
2019-07-26 15:41 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 1985: (follow-up) Fix Reviews.t logic (2.93 KB, patch)
2019-07-26 15:41 UTC, ByWater Sandboxes
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 00:36:53 UTC


---- Reported by oleonard@myacpl.org 2008-03-26 08:47:48 ----

Many blog systems have a feature where the admin gets an email when someone posts a new comment that needs to be moderated. It would be nice to incorporate this into Koha. Ideally, you could set an email address in system preferences where notifications should be sent (since the Koha administrator is probably not the one approving comments). The body of the email would contain the comments, and offer a direct link to approve or delete the comment.



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 00:36 UTC  ---

This bug was previously known as _bug_ 1985 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=1985

Actual time not defined. Setting to 0.0

Comment 1 Olli-Antti Kivilahti 2014-11-25 16:24:32 UTC Comment hidden (obsolete)
Comment 2 Olli-Antti Kivilahti 2014-11-25 16:25:40 UTC
Pending unit tests. I might not be able to do them in a while because I am quite busy with other issues.
Comment 3 Olli-Antti Kivilahti 2014-11-25 16:42:59 UTC Comment hidden (obsolete)
Comment 4 Mark Tompsett 2014-12-16 06:54:24 UTC Comment hidden (obsolete)
Comment 5 Mark Tompsett 2014-12-16 06:55:05 UTC
See my comments in comment 4 as to why I changed this to Failed QA.
Comment 6 Katrin Fischer 2014-12-16 22:24:29 UTC
I have run the QA script on it and there is indeed:
"return" statement with explicit "undef" at line 207, column 9.  See page 199 of PBP.  (Severity: 5)

There are also some other things that need to be fixed - Olli, can you provide a follow-up please fixing those?
Comment 7 Jonathan Druart 2015-02-20 11:26:53 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2015-02-20 11:30:18 UTC
(In reply to M. Tompsett from comment #4)
> I think this will fail the koha qa tests. No need to be explicit with the
> undef;

Does this mean the patch work as expected?

The failed QA status should not be used to raise QA issues when the bug is on the NSO step.
It removes the patch from the NSO queue and won't be tested.
However you can use it if the patch does not work as expected.

This kind of issues will be caught on the QA step and can be fixed by the QAer.
Comment 9 Jonathan Druart 2015-02-20 11:30:51 UTC
The last patch now passes the qa script.
Comment 10 Jonathan Druart 2015-02-20 11:31:37 UTC
Comment on attachment 36065 [details] [review]
Bug 1985 - Email notification of new OPAC comments

Review of attachment 36065 [details] [review]:
-----------------------------------------------------------------

::: C4/Review.pm
@@ +200,5 @@
> +
> +=cut
> +
> +sub SendReviewAlert {
> +    my $review = shift;

Olli, this $review is not used in the subroutine.
Comment 11 Koha Team University Lyon 3 2015-03-04 16:11:03 UTC
I tested this patch on a sandbox :
1. set syspref CommentModeratorsEmail with my email adress
2. go to public interface and add a comment on a book
3. go to saved reports to check to table message_queue : I have a new record with the letter_code ''COMMENT_CREATED' : Ok, it works !
4. Go to notices and slips to change the message of ''COMMENT_CREATED' model : in email tab, I try to add the title of the book in the message (<<biblio.title>>) and the biblionumber (<<biblio.biblionumber>>)
5. Go to the opac and add a new comment on an other book
6. go to saved reports to check to table message_queue :  I don't retrieve the title of the book and the biblionumber in the email : it's a pity because it can be usefull informations for the librarian.
Comment 12 Mark Tompsett 2015-03-04 16:58:13 UTC
I think that may be a cronjob configuration problem. I'll recheck this.
Comment 13 Mark Tompsett 2015-03-05 01:19:01 UTC
Another reason this is currently failed qa. This modifies C4/Review.pm without adding any tests. Don't worry, the patch I'm working on for this will handle comment #11 and add tests.
Comment 14 Mark Tompsett 2015-03-05 02:49:11 UTC Comment hidden (obsolete)
Comment 15 Mark Tompsett 2015-03-05 02:49:24 UTC Comment hidden (obsolete)
Comment 16 Mark Tompsett 2015-03-05 02:50:39 UTC
Obviously item numbers and the like don't make sense in the context of this bug, but fixing what is listed for what reports would require a separate enhancement.
Comment 17 Mark Tompsett 2015-03-05 02:52:21 UTC
And by the way, comment 10 is still a valid issue. I figure it might be useful to include it in the letter in the future, but no code was added to make that possible.
Comment 18 Mark Tompsett 2015-03-05 04:43:33 UTC Comment hidden (obsolete)
Comment 19 Mark Tompsett 2015-03-05 04:43:50 UTC Comment hidden (obsolete)
Comment 20 Mark Tompsett 2015-03-05 04:45:24 UTC
Ignore comments #10 and #17. My second commit covers all that. :)
Comment 21 Koha Team University Lyon 3 2015-03-05 09:12:50 UTC
I have just tested it. Everything is fine except that <<staffClientBaseURL>> isn't filled in the email : 
...
BEGIN COMMENT --- A lire absolument --- END COMMENT --- To approve: http:///cgi-bin/koha/review/reviewswaiting.pl?op=approve&reviewid=10 To delete: http:///cgi-bin/koha/review/reviewswaiting.pl?op=delete&reviewid=10 Or to review all comments: http:///cgi-bin/koha/review/reviewswaiting.pl Check it out! Your library.
Comment 22 Mark Tompsett 2015-03-05 14:33:18 UTC
(In reply to Koha Team Lyon 3 from comment #21)
> I have just tested it. Everything is fine except that <<staffClientBaseURL>>
> isn't filled in the email : 
> ...
> BEGIN COMMENT --- A lire absolument --- END COMMENT --- To approve:
> http:///cgi-bin/koha/review/reviewswaiting.pl?op=approve&reviewid=10 To
> delete: http:///cgi-bin/koha/review/reviewswaiting.pl?op=delete&reviewid=10
> Or to review all comments: http:///cgi-bin/koha/review/reviewswaiting.pl
> Check it out! Your library.

Not to ask the obvious question, but did you set staffClientBaseURL?
Comment 23 Mark Tompsett 2015-03-05 15:06:13 UTC Comment hidden (obsolete)
Comment 24 Mark Tompsett 2015-03-05 15:06:27 UTC Comment hidden (obsolete)
Comment 25 Koha Team University Lyon 3 2015-03-05 15:13:58 UTC
Yes, you're right :-) I forgot to set the staffClientBaseURL syspref ...
I'll test one more time
Comment 26 Koha Team University Lyon 3 2015-03-05 15:52:49 UTC
Tested on a sandbox :
- go to OPAC to add a comment without filling the syspref CommentModeratorsEmail : nothing is added in message_queue table : OK

- set syspref CommentModeratorsEmail with my email adress and set the syspref staffClientBaseURL

- add a comment on OPAC

- try SELECT * FROM message_queue WHERE letter_code='COMMENT_CREATED';
I have a new record : OK

- reedit the comment on the OPAC

- try the SQL request : 2 records : Ok

- go to notices and slips : try to change the model

- go to the OPAC : add a new comment

- retry the SQL reequest : I have a new record with the change I made in the model : OK

It works as it is described in the test plan but I have a remarks in the url to manage the comments : in the model, it's : http://<<staffClientBaseURL>>/cgi-bin/koha/review/reviewswaiting.pl?

but the right URL is

http://<<staffClientBaseURL>>/cgi-bin/koha/reviewS/reviewswaiting.pl?
(there's a S and the end of review)
Comment 27 Mark Tompsett 2015-03-05 16:46:50 UTC Comment hidden (obsolete)
Comment 28 Mark Tompsett 2015-03-05 16:47:02 UTC Comment hidden (obsolete)
Comment 29 Biblibre Sandboxes 2015-03-06 09:12:12 UTC
Patch tested with a sandbox, by sonia bouis <sonia.bouis@univ-lyon3.fr>
Comment 30 Biblibre Sandboxes 2015-03-06 09:12:50 UTC Comment hidden (obsolete)
Comment 31 Biblibre Sandboxes 2015-03-06 09:12:54 UTC Comment hidden (obsolete)
Comment 32 Biblibre Sandboxes 2015-03-06 09:12:58 UTC Comment hidden (obsolete)
Comment 33 Koha Team University Lyon 3 2015-03-06 09:17:33 UTC
I've done again all the tests (until point 6 but I think point 6 and 7 can be done at the QA step ?)
Everything works correctly
Comment 34 Kyle M Hall (khall) 2015-03-06 11:01:28 UTC Comment hidden (obsolete)
Comment 35 Kyle M Hall (khall) 2015-03-06 11:03:21 UTC Comment hidden (obsolete)
Comment 36 Kyle M Hall (khall) 2015-03-06 11:06:23 UTC
One more addition is required for this to pass qa. Can you please add the new notice to all the translations?
Comment 37 Mark Tompsett 2015-03-06 15:48:25 UTC
ARG! Kyle, did you have to make me notice another bug? :P I'm cleaning it up (bug 13808). In the mean time, I'll just add this notice into every language as a separate patch.
Comment 38 Mark Tompsett 2015-03-06 16:58:08 UTC Comment hidden (obsolete)
Comment 39 Mark Tompsett 2015-05-06 20:47:17 UTC Comment hidden (obsolete)
Comment 40 Mark Tompsett 2015-05-06 20:47:33 UTC Comment hidden (obsolete)
Comment 41 Mark Tompsett 2015-05-06 20:47:43 UTC Comment hidden (obsolete)
Comment 42 Mark Tompsett 2015-05-06 21:00:04 UTC
This is a rebase. The last patch (Follow up: Add sample letter to other languages) is what addresses Kyle's comment #36. The first two patches have passed QA, the last one needs sign off.

prove xt/sample_notices.t might be in order too. :)

If there is a failure, check if bug 14119 has been applied, since I discovered a problem elsewhere. :)
Comment 43 Bernardo Gonzalez Kriegel 2015-05-12 11:23:17 UTC Comment hidden (obsolete)
Comment 44 Jonathan Druart 2015-05-26 11:53:07 UTC
Comment on attachment 38909 [details] [review]
Bug 1985 - tweak License and lots of cleanup

Review of attachment 38909 [details] [review]:
-----------------------------------------------------------------

::: t/db_dependent/Review.t
@@ +21,5 @@
> +$biblionumber = $row->{'biblionumber'} if $row;
> +
> +SKIP: {
> +    skip "Missing borrowers or biblios",13
> +      unless $biblionumber and $borrowernumber;

You have to generate the data you want!
Try TestBuilder, it will do that for you.
Comment 45 Jonathan Druart 2015-05-26 11:54:34 UTC
Comment on attachment 38908 [details] [review]
Bug 1985 - Email notification of new OPAC comments

Review of attachment 38908 [details] [review]:
-----------------------------------------------------------------

::: C4/Review.pm
@@ +83,5 @@
>      $sth->execute( $borrowernumber, $biblionumber, $review);
> +
> +    unless ($sth->err()) {
> +        SendReviewAlert( $review, $borrowernumber );
> +    }

else ? What happen if an error occurs?
Comment 46 Mark Tompsett 2015-06-22 18:11:28 UTC
I'm setting this back to Olli-Antti Kivilahti, since I have no more time to follow up on this, and it was his initially.
Comment 47 Olli-Antti Kivilahti 2017-03-22 08:50:26 UTC
We are migrating to a national OPAC (finna.fi) as we speak and OPAC stuff is no longer relevant for us!

Thanks for signing off.
Comment 48 Mark Tompsett 2017-05-20 03:20:46 UTC
I'm close to a resurrection of this.
Comment 49 Mark Tompsett 2017-05-26 17:50:49 UTC Comment hidden (obsolete)
Comment 50 Mark Tompsett 2017-05-26 17:51:07 UTC Comment hidden (obsolete)
Comment 51 Mark Tompsett 2017-05-26 17:51:21 UTC Comment hidden (obsolete)
Comment 52 Mark Tompsett 2017-05-26 17:51:31 UTC Comment hidden (obsolete)
Comment 53 Mark Tompsett 2017-05-26 17:51:40 UTC Comment hidden (obsolete)
Comment 54 Mark Tompsett 2017-05-26 17:51:51 UTC Comment hidden (obsolete)
Comment 55 Mark Tompsett 2017-05-26 17:51:59 UTC Comment hidden (obsolete)
Comment 56 Mark Tompsett 2017-05-26 17:53:21 UTC
If you apply everything and run the test plans in comment #50 and comment #51, then it should suffice.
Comment 57 Mark Tompsett 2017-05-26 17:56:08 UTC
(In reply to M. Tompsett from comment #56)
> If you apply everything and run the test plans in comment #50 and comment
> #51, then it should suffice.

Generally running perlcritic -1 {file}, and ignoring some things, with the default perlcritic. This puts the code at a higher perlcritic level than just -5 as used by the koha qa test tools.
Comment 58 Jonathan Druart 2017-05-26 20:25:51 UTC
(In reply to M. Tompsett from comment #57)
> (In reply to M. Tompsett from comment #56)
> > If you apply everything and run the test plans in comment #50 and comment
> > #51, then it should suffice.
> 
> Generally running perlcritic -1 {file}, and ignoring some things, with the
> default perlcritic. This puts the code at a higher perlcritic level than
> just -5 as used by the koha qa test tools.

We are not following perlcritic -1, this patch is out of the scope of this bug report.
Comment 59 Mark Tompsett 2017-05-26 21:44:27 UTC
(In reply to Jonathan Druart from comment #58)
> We are not following perlcritic -1, this patch is out of the scope of this
> bug report.

While you are correct about scope creep for the last 4 patches, I do believe the first 3 are fine and should not creep it as far as I recall.
Comment 60 Mark Tompsett 2017-05-26 22:06:42 UTC Comment hidden (obsolete)
Comment 61 Mark Tompsett 2017-05-26 22:06:58 UTC Comment hidden (obsolete)
Comment 62 Mark Tompsett 2017-05-26 22:07:13 UTC Comment hidden (obsolete)
Comment 63 Mark Tompsett 2017-05-26 22:07:24 UTC Comment hidden (obsolete)
Comment 64 Marc Véron 2017-05-27 06:23:16 UTC
Tested all patches together, followed test plan from comment #50, works as expected. Will sign off.
Comment 65 Marc Véron 2017-05-27 06:25:54 UTC Comment hidden (obsolete)
Comment 66 Marc Véron 2017-05-27 06:27:16 UTC Comment hidden (obsolete)
Comment 67 Marc Véron 2017-05-27 06:38:41 UTC Comment hidden (obsolete)
Comment 68 Mark Tompsett 2018-04-23 18:14:37 UTC Comment hidden (obsolete)
Comment 69 Mark Tompsett 2018-04-23 18:14:41 UTC Comment hidden (obsolete)
Comment 70 Mark Tompsett 2018-04-23 18:14:45 UTC Comment hidden (obsolete)
Comment 71 Mark Tompsett 2018-04-23 18:23:46 UTC Comment hidden (obsolete)
Comment 72 Mark Tompsett 2018-04-23 18:23:50 UTC Comment hidden (obsolete)
Comment 73 Mark Tompsett 2018-04-23 22:23:22 UTC
Created attachment 74769 [details] [review]
Bug 1985: Fix whitespace in opac/opac-review.pl

$ git diff -w origin/master
-- should have no differences

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 74 Mark Tompsett 2018-04-23 22:23:26 UTC
Created attachment 74770 [details] [review]
Bug 1985: Send email alert when comment posted.

Many blog systems have a feature where the admin gets
an email when someone posts a new comment that needs
to be moderated. It would be nice to incorporate this
into Koha. Ideally, you could set an email address in
system preferences where notifications should be sent
(since the Koha administrator is probably not the one
approving comments). The body of the email would contain
the comments, and offer a direct link to approve or delete
the comment.

The authorship was corrected to Olli-Antti Kivilahti.

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/Koha/Reviews.t
   -- six new tests should run.
7) Run the koha qa test tool.
Comment 75 Mark Tompsett 2018-04-23 22:23:31 UTC
Created attachment 74771 [details] [review]
Bug 1985: Follow up - Add sample letter to other languages

As per comment #36, the letter COMMENT_CREATED has been added
to the other languages.

TEST PLAN
---------
0) Back up your letters table
1) In your mysql client after selected your Koha database,
   Run this:

SET foreign_key_checks = 0;
truncate letter;
source installer/data/mysql/de-DE/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/es-ES/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/it-IT/necessari/notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/nb-NO/1-Obligatorisk/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/pl-PL/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/ru-RU/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/uk-UA/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/en/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';

   -- there should be 9 counts of 1.
2) Restore your letters table.

Left signed off, even though uk-UA was rebased.

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 76 Mark Tompsett 2018-04-23 22:26:04 UTC
The middle patch was rebased by manual entry, hence the "Needs Signoff". The first patch was untouched, and the last only needed a minor tweak to rebased properly.
Comment 77 Liz Rea 2019-04-16 17:23:44 UTC
Patch no longer applies, sorry.
Comment 78 Mark Tompsett 2019-07-02 17:54:28 UTC Comment hidden (obsolete)
Comment 79 Mark Tompsett 2019-07-02 17:54:32 UTC Comment hidden (obsolete)
Comment 80 Mark Tompsett 2019-07-02 17:54:37 UTC Comment hidden (obsolete)
Comment 81 Mark Tompsett 2019-07-02 18:05:55 UTC Comment hidden (obsolete)
Comment 82 Mark Tompsett 2019-07-03 00:18:43 UTC
Created attachment 91193 [details] [review]
Bug 1985: Fix whitespace in opac/opac-review.pl

$ git diff -w origin/master
-- should have no differences

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 83 Mark Tompsett 2019-07-03 00:18:47 UTC
Created attachment 91194 [details] [review]
Bug 1985: Send email alert when comment posted.

Many blog systems have a feature where the admin gets
an email when someone posts a new comment that needs
to be moderated. It would be nice to incorporate this
into Koha. Ideally, you could set an email address in
system preferences where notifications should be sent
(since the Koha administrator is probably not the one
approving comments). The body of the email would contain
the comments, and offer a direct link to approve or delete
the comment.

The authorship was corrected to Olli-Antti Kivilahti.

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/Koha/Reviews.t
   -- six new tests should run.
7) Run the koha qa test tool.
Comment 84 Mark Tompsett 2019-07-03 00:18:51 UTC
Created attachment 91195 [details] [review]
Bug 1985: (follow-up) correct http:// in staff URL
Comment 85 Mark Tompsett 2019-07-03 00:18:55 UTC
Created attachment 91196 [details] [review]
Bug 1985: (follow-up) Add letter to other languages

As per comment #36, the letter COMMENT_CREATED has been added
to the other languages.

TEST PLAN
---------
0) Back up your letters table
1) In your mysql client after selected your Koha database,
   Run this:

SET foreign_key_checks = 0;
truncate letter;
source installer/data/mysql/de-DE/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/es-ES/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/it-IT/necessari/notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/nb-NO/1-Obligatorisk/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/pl-PL/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/ru-RU/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/uk-UA/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/en/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';

   -- there should be 9 counts of 1.
2) Restore your letters table.

Left signed off, even though uk-UA was rebased.

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 86 Mark Tompsett 2019-07-03 00:19:00 UTC
Created attachment 91197 [details] [review]
Bug 1985: (follow-up) fix staff URL in other letters
Comment 87 Mark Tompsett 2019-07-03 00:19:05 UTC
Created attachment 91198 [details] [review]
Bug 1985: (follow-up) Fix Reviews.t logic

Read the tests to make sure they make sense.
Before the patch is applied, they don't.

$nb_of_approved_reviews + 2 did not make sense, because the three
reviews added were not approved.
search->count counts everything, so the use of approved+3 didn't
make sense.
Rather than duplicate the search->count test, changed the
first one to reflect that the added reviews were all unapproved.

Before applying patch, prove fails.

After applying patch, prove passes.
Comment 88 ByWater Sandboxes 2019-07-26 15:41:12 UTC
Created attachment 91818 [details] [review]
Bug 1985: Fix whitespace in opac/opac-review.pl

$ git diff -w origin/master
-- should have no differences

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 89 ByWater Sandboxes 2019-07-26 15:41:16 UTC
Created attachment 91819 [details] [review]
Bug 1985: Send email alert when comment posted.

Many blog systems have a feature where the admin gets
an email when someone posts a new comment that needs
to be moderated. It would be nice to incorporate this
into Koha. Ideally, you could set an email address in
system preferences where notifications should be sent
(since the Koha administrator is probably not the one
approving comments). The body of the email would contain
the comments, and offer a direct link to approve or delete
the comment.

The authorship was corrected to Olli-Antti Kivilahti.

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/Koha/Reviews.t
   -- six new tests should run.
7) Run the koha qa test tool.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 90 ByWater Sandboxes 2019-07-26 15:41:19 UTC
Created attachment 91820 [details] [review]
Bug 1985: (follow-up) correct http:// in staff URL

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 91 ByWater Sandboxes 2019-07-26 15:41:23 UTC
Created attachment 91821 [details] [review]
Bug 1985: (follow-up) Add letter to other languages

As per comment #36, the letter COMMENT_CREATED has been added
to the other languages.

TEST PLAN
---------
0) Back up your letters table
1) In your mysql client after selected your Koha database,
   Run this:

SET foreign_key_checks = 0;
truncate letter;
source installer/data/mysql/de-DE/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/es-ES/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/it-IT/necessari/notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/nb-NO/1-Obligatorisk/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/pl-PL/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/ru-RU/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/uk-UA/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';
truncate letter;
source installer/data/mysql/en/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='COMMENT_CREATED';

   -- there should be 9 counts of 1.
2) Restore your letters table.

Left signed off, even though uk-UA was rebased.

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 92 ByWater Sandboxes 2019-07-26 15:41:26 UTC
Created attachment 91822 [details] [review]
Bug 1985: (follow-up) fix staff URL in other letters

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 93 ByWater Sandboxes 2019-07-26 15:41:31 UTC
Created attachment 91823 [details] [review]
Bug 1985: (follow-up) Fix Reviews.t logic

Read the tests to make sure they make sense.
Before the patch is applied, they don't.

$nb_of_approved_reviews + 2 did not make sense, because the three
reviews added were not approved.
search->count counts everything, so the use of approved+3 didn't
make sense.
Rather than duplicate the search->count test, changed the
first one to reflect that the added reviews were all unapproved.

Before applying patch, prove fails.

After applying patch, prove passes.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 94 Katrin Fischer 2019-08-08 17:18:02 UTC
QA review:

1) Letter text

+We want to inform you that borrower <<borrowers.cardnumber>> has just created a new comment on <<biblio.title>>, which is biblionumber <<biblio.biblionumber>>.

Instead of telling the biblionumber in the text, why not add a link to the record instead? To make this title more distinguishable we could also make use of the new fields (subtitle, parts, number).

These seem a bit unusual and very developery for formatting:

+--- BEGIN COMMENT ---
+<<reviews.review>>
+--- END COMMENT ---

I think using standard formatting or just ----- would work better here.

Signing a letter to the libray with 'the library' seems a little odd to me :)

2) Convert database update to .perl

The RM will be happier.

3) Capitalization

+('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'),

Biblio and Borrower... Same for the description in the SQL statements.
Comment 95 Katrin Fischer 2019-08-08 17:18:40 UTC
Thx for taking this up - also impressed with letters added for all translations :)
Comment 96 Katrin Fischer 2019-08-08 17:22:46 UTC
4) Method naming

I believe we agreed on lower case using _ for dividing words for all methods in the new Koha namespace.

SendReviewAlert