Bug 15839 - Move the reviews related code to Koha::Reviews
Summary: Move the reviews related code to Koha::Reviews
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 15449 19808 19843
  Show dependency treegraph
 
Reported: 2016-02-17 11:39 UTC by Jonathan Druart
Modified: 2018-06-04 14:58 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15839: Koha::Reviews - Add Koha::Review[s] classes (6.28 KB, patch)
2016-02-17 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getreviews (12.74 KB, patch)
2016-02-17 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getallreviews (5.23 KB, patch)
2016-02-17 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove approvereview & unapprovereview (4.64 KB, patch)
2016-02-17 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Change default value for approved to 0 (2.03 KB, patch)
2016-02-17 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove deletereview (1.80 KB, patch)
2016-02-17 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getreview (3.45 KB, patch)
2016-02-17 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getnumberofreviews (4.53 KB, patch)
2016-02-17 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove updatereview (2.65 KB, patch)
2016-02-17 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove savereview (2.09 KB, patch)
2016-02-17 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove C4::Review residue (20.10 KB, patch)
2016-02-17 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Add Koha::Review[s] classes (6.46 KB, patch)
2016-02-22 12:40 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getreviews (12.88 KB, patch)
2016-02-22 12:41 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getallreviews (5.36 KB, patch)
2016-02-22 12:42 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove approvereview & unapprovereview (4.79 KB, patch)
2016-02-22 12:43 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Change default value for approved to 0 (2.17 KB, patch)
2016-02-22 12:44 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove deletereview (1.94 KB, patch)
2016-02-22 12:45 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getreview (3.59 KB, patch)
2016-02-22 12:46 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getnumberofreviews (4.67 KB, patch)
2016-02-22 12:47 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove updatereview (2.79 KB, patch)
2016-02-22 12:49 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove savereview (2.23 KB, patch)
2016-02-22 12:50 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove C4::Review residue (20.24 KB, patch)
2016-02-22 12:51 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Add Koha::Review[s] classes (6.46 KB, patch)
2016-05-04 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getreviews (12.84 KB, patch)
2016-05-04 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getallreviews (5.36 KB, patch)
2016-05-04 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove approvereview & unapprovereview (4.78 KB, patch)
2016-05-04 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Change default value for approved to 0 (2.16 KB, patch)
2016-05-04 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove deletereview (1.93 KB, patch)
2016-05-04 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getreview (3.59 KB, patch)
2016-05-04 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getnumberofreviews (4.65 KB, patch)
2016-05-04 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove updatereview (2.78 KB, patch)
2016-05-04 16:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove savereview (2.22 KB, patch)
2016-05-04 16:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove C4::Review residue (20.15 KB, patch)
2016-05-04 16:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Add Koha::Review[s] classes (6.47 KB, patch)
2016-07-29 09:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getreviews (12.86 KB, patch)
2016-07-29 09:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getallreviews (5.37 KB, patch)
2016-07-29 09:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove approvereview & unapprovereview (4.79 KB, patch)
2016-07-29 09:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Change default value for approved to 0 (2.18 KB, patch)
2016-07-29 09:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove deletereview (1.95 KB, patch)
2016-07-29 09:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getreview (3.60 KB, patch)
2016-07-29 09:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove getnumberofreviews (4.67 KB, patch)
2016-07-29 09:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove updatereview (2.79 KB, patch)
2016-07-29 09:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove savereview (2.24 KB, patch)
2016-07-29 09:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15839: Koha::Reviews - Remove C4::Review residue (20.16 KB, patch)
2016-07-29 09:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15839: [QA Follow-up] Update existing rows in dbrev (1.01 KB, patch)
2016-07-29 09:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15839: [QA Follow-up] Paging on opac-showreviews (4.31 KB, patch)
2016-07-29 09:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15839: [QA Follow-up] Error checking in opac-review.pl (4.66 KB, patch)
2016-07-29 09:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2016-02-17 11:39:03 UTC
The subroutines in C4::Reviews are only doing CRUD operations.
This module can be moved easily to Koha::Reviews.
Comment 1 Jonathan Druart 2016-02-17 13:45:23 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-02-17 13:45:26 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-02-17 13:45:29 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-02-17 13:45:31 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2016-02-17 13:45:34 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2016-02-17 13:45:37 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-02-17 13:45:40 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2016-02-17 13:45:43 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2016-02-17 13:45:46 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2016-02-17 13:45:49 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2016-02-17 13:45:52 UTC Comment hidden (obsolete)
Comment 12 Marc Véron 2016-02-22 12:40:17 UTC Comment hidden (obsolete)
Comment 13 Marc Véron 2016-02-22 12:41:41 UTC Comment hidden (obsolete)
Comment 14 Marc Véron 2016-02-22 12:42:34 UTC Comment hidden (obsolete)
Comment 15 Marc Véron 2016-02-22 12:43:43 UTC Comment hidden (obsolete)
Comment 16 Marc Véron 2016-02-22 12:44:33 UTC Comment hidden (obsolete)
Comment 17 Marc Véron 2016-02-22 12:45:27 UTC Comment hidden (obsolete)
Comment 18 Marc Véron 2016-02-22 12:46:22 UTC Comment hidden (obsolete)
Comment 19 Marc Véron 2016-02-22 12:47:33 UTC Comment hidden (obsolete)
Comment 20 Marc Véron 2016-02-22 12:49:03 UTC Comment hidden (obsolete)
Comment 21 Marc Véron 2016-02-22 12:50:02 UTC Comment hidden (obsolete)
Comment 22 Marc Véron 2016-02-22 12:51:04 UTC Comment hidden (obsolete)
Comment 23 Katrin Fischer 2016-04-03 13:56:10 UTC
Please rebase!

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 15839: Koha::Reviews - Add Koha::Review[s] classes
Applying: Bug 15839: Koha::Reviews - Remove getreviews
Applying: Bug 15839: Koha::Reviews - Remove getallreviews
Applying: Bug 15839: Koha::Reviews - Remove approvereview & unapprovereview
Applying: Bug 15839: Koha::Reviews - Change default value for approved to 0
Applying: Bug 15839: Koha::Reviews - Remove deletereview
fatal: sha1 information is lacking or useless (C4/Review.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 15839: Koha::Reviews - Remove deletereview
The copy of the patch that failed is found in:
   /home/katrin/kohaclone/.git/rebase-apply/patch
Comment 24 Jonathan Druart 2016-05-04 16:19:26 UTC Comment hidden (obsolete)
Comment 25 Jonathan Druart 2016-05-04 16:19:31 UTC Comment hidden (obsolete)
Comment 26 Jonathan Druart 2016-05-04 16:19:35 UTC Comment hidden (obsolete)
Comment 27 Jonathan Druart 2016-05-04 16:19:40 UTC Comment hidden (obsolete)
Comment 28 Jonathan Druart 2016-05-04 16:19:44 UTC Comment hidden (obsolete)
Comment 29 Jonathan Druart 2016-05-04 16:19:49 UTC Comment hidden (obsolete)
Comment 30 Jonathan Druart 2016-05-04 16:19:53 UTC Comment hidden (obsolete)
Comment 31 Jonathan Druart 2016-05-04 16:19:57 UTC Comment hidden (obsolete)
Comment 32 Jonathan Druart 2016-05-04 16:20:01 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2016-05-04 16:20:06 UTC Comment hidden (obsolete)
Comment 34 Jonathan Druart 2016-05-04 16:20:11 UTC Comment hidden (obsolete)
Comment 35 Marcel de Rooy 2016-07-22 07:55:58 UTC
QA: Looking here now..
Comment 36 Marcel de Rooy 2016-07-22 08:14:59 UTC
qa tools:
Processing files after patches
fatal: ambiguous argument 'C4/Review.pm': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions
Comment 37 Marcel de Rooy 2016-07-22 08:43:05 UTC
QA Comment:
If I submit a new review, the approved status will be NULL.
But reviewswaiting.pl will not show it:

my $status   = $query->param('status') || 0;
...
my $reviews = Koha::Reviews->search(
    { approved => $status },
Comment 38 Jonathan Druart 2016-07-22 09:43:13 UTC
(In reply to Marcel de Rooy from comment #37)
> QA Comment:
> If I submit a new review, the approved status will be NULL.
> But reviewswaiting.pl will not show it:
> 
> my $status   = $query->param('status') || 0;
> ...
> my $reviews = Koha::Reviews->search(
>     { approved => $status },

Marcel,
This is fixed by "Change default value for approved to 0", please execute the updatedatabase.pl and update the schema.
Comment 39 Marcel de Rooy 2016-07-29 07:45:07 UTC
(In reply to Jonathan Druart from comment #38)
> (In reply to Marcel de Rooy from comment #37)
> > QA Comment:
> > If I submit a new review, the approved status will be NULL.
> > But reviewswaiting.pl will not show it:
> > 
> > my $status   = $query->param('status') || 0;
> > ...
> > my $reviews = Koha::Reviews->search(
> >     { approved => $status },
> 
> Marcel,
> This is fixed by "Change default value for approved to 0", please execute
> the updatedatabase.pl and update the schema.

QA: Looking again
Comment 40 Marcel de Rooy 2016-07-29 09:43:40 UTC
Marc,
The é in Véron still makes me crazy. Especially in 11 patches :)
Comment 41 Marcel de Rooy 2016-07-29 09:44:12 UTC
Created attachment 53794 [details] [review]
Bug 15839: Koha::Reviews - Add Koha::Review[s] classes

The subroutines in C4::Reviews are only doing CRUD operations.
This patch set moves them to 2 new Koha::Review[s] classes.

Test plan for the whole patch set:
0/ Set the reviewson syspref
1/ At the OPAC, create a couple of reviews for a record
Try and create a review without html tags different that br b i em big
small strong: You should get a warning.
2/ Edit a review
3/ On the Staff interface, you should see a "Comments pending approval"
link on the mainpage
4/ Approve 1 review and delete the other
5/ At the OPAC you should only see 1 review (the approved one) in the
"Comments" tab
6/ Modify the review
7/ The review should appear again in the "comments awaiting moderation"
tab.
Note that even the comment has not been changed, it will have to be
reapproved (FIXME later). This behavior already existed prior to this
patch.

Tested all patches together (with database updated)
Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 42 Marcel de Rooy 2016-07-29 09:44:16 UTC
Created attachment 53795 [details] [review]
Bug 15839: Koha::Reviews - Remove getreviews

The C4::Reviews::getreviews subroutine retrieved the reviews for a given
record, depending on their status.
This can be achieve with a call to Koha::Reviews->search.
There were 2 calls to this subroutine. The one from opac-ISBDdetail.pl
does not look in used: the reviews are not display on this page. It
certainly comes from an old copy/paste from opac-detail.pl.
The one from opac-detail only asked for the approved reviews. So the
logged in user does not see its own review if it is pending approval.
Actually this pending approval review is only displayed when the user
submits it (because of a unecessary complex text replacement done in
JS).
With this patch, the approved reviews AND the unaproved review from the
logged in user will be displayed.
It will allow a future enhancement to add a way to delete our own
reviews.

Moreover, the reviews were retrieved even if they were not displayed (if
reviewson is off), it's now fixed.

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 43 Marcel de Rooy 2016-07-29 09:44:20 UTC
Created attachment 53796 [details] [review]
Bug 15839: Koha::Reviews - Remove getallreviews

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 44 Marcel de Rooy 2016-07-29 09:44:24 UTC
Created attachment 53797 [details] [review]
Bug 15839: Koha::Reviews - Remove approvereview & unapprovereview

This patch adds 2 new methods to Koha::Review: approve and unapprove.

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 45 Marcel de Rooy 2016-07-29 09:44:28 UTC
Created attachment 53798 [details] [review]
Bug 15839: Koha::Reviews - Change default value for approved to 0

The reviews.approved column had a default value set to NULL.
It does not make sense, the default value should be 0, this will avoid
to have to specify the approved value when creating a new review.

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 46 Marcel de Rooy 2016-07-29 09:44:32 UTC
Created attachment 53799 [details] [review]
Bug 15839: Koha::Reviews - Remove deletereview

Note that a future enhancement should take into account the possible
error returned by ->approve, ->unapprove and ->delete

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 47 Marcel de Rooy 2016-07-29 09:44:36 UTC
Created attachment 53800 [details] [review]
Bug 15839: Koha::Reviews - Remove getreview

Note that this (biblionumber, borrowernumber) pair should be defined as
a unique key at the DB level (FIXME added)

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 48 Marcel de Rooy 2016-07-29 09:44:40 UTC
Created attachment 53801 [details] [review]
Bug 15839: Koha::Reviews - Remove getnumberofreviews

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 49 Marcel de Rooy 2016-07-29 09:44:44 UTC
Created attachment 53802 [details] [review]
Bug 15839: Koha::Reviews - Remove updatereview

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 50 Marcel de Rooy 2016-07-29 09:44:48 UTC
Created attachment 53803 [details] [review]
Bug 15839: Koha::Reviews - Remove savereview

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 51 Marcel de Rooy 2016-07-29 09:44:52 UTC
Created attachment 53804 [details] [review]
Bug 15839: Koha::Reviews - Remove C4::Review residue

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 52 Marcel de Rooy 2016-07-29 09:44:57 UTC
Created attachment 53805 [details] [review]
Bug 15839: [QA Follow-up] Update existing rows in dbrev

Adding the default is fine for new records; we also should take care
of the pending existing ones.
Trivial addition to atomic update file.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested the dbrev again with existing unapproved reviews.
Comment 53 Marcel de Rooy 2016-07-29 09:45:02 UTC
Created attachment 53806 [details] [review]
Bug 15839: [QA Follow-up] Paging on opac-showreviews

Paging is kind of messy here. This patch at least improves somewhat.
The page number should be rounded.
The results per page should be passed to the template too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested a number of reviews and played with count parameter in URL.
Comment 54 Marcel de Rooy 2016-07-29 09:45:07 UTC
Created attachment 53807 [details] [review]
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).
Comment 55 Marcel de Rooy 2016-07-29 09:46:10 UTC
qa tools: OK
t/db_dependent/Koha/Reviews.t OK
Added a follow-up for db rev
Loop in opac-detail repeatedly calls GetMember for possibly same borrower?
Enabled ShowRecentComments 
opac-showreviews: Improved paging in a small follow-up. (The count parameter is not kept when paging.)
reviewswaiting: When I delete an approved review, the display jumps back to awaiting moderation.
Just to mention: If I have two comments on a book, these two icons are weird imo. (Even one, but that is personal taste. :) 
opac-review: See follow-up

Passed QA
Comment 56 Kyle M Hall 2016-09-09 11:38:34 UTC
Pushed to master for 16.11, thanks Jonathan, Marcel!