[Bug 17089] New: Move the star ratings related code to Koha::Ratings
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Bug ID: 17089 Summary: Move the star ratings related code to Koha::Ratings Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org The subroutines in C4::Reviews are only doing CRUD operations. This module can be moved easily to Koha::Reviews, especially as it already uses DBIx::Class -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 54207 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54207&action=edit Bug 17089: Koha::Ratings - Add Koha::Rating[s] classes The subroutines in C4::Ratings are only CRUD operations. This patch set moves the entire module to Koha::Rating[s] based on Koha::Object[s] Test plan for the whole patch set: 0/ Set OpacStarRatings to "results and details" 1/ Log in at the OPAC 2/ Launch a search, you should see the star ratings 3/ Go on a detail page, you should see the star ratings 4/ Rate the record 5/ Log in with another patron 6/ Go on the same detail page, you should see the previous rating 7/ Rate the racord, you should see "2 votes" and the average rating value. 8/ Cancel your rate 9/ Disable JavaScript and repeat 7 Note: You cannot cancel your rate if JavaScript is disabled (not a regression). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 54208 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54208&action=edit Bug 17089: Koha::Ratings - move tests Rewrite and move the tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 54209 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54209&action=edit Bug 17089: Koha::Ratings - Remove AddRating -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 54210 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54210&action=edit Bug 17089: Koha::Ratings - Remove ModRating -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 54211 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54211&action=edit Bug 17089: Koha::Ratings - Remove DelRating -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 54212 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54212&action=edit Bug 17089: Koha::Ratings - Remove GetRating -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 54213 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54213&action=edit Bug 17089: Koha::Ratings - Remove C4::Ratings -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 54214 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54214&action=edit Bug 17089: Mark out the boundaries of ratings In the doc of C4::Ratings, it was written that the rate values should be
=0 and <=5, but it was not taken into account. This bug should be fixed on stable releases as well.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #9 from Owen Leonard <oleonard@myacpl.org> --- I'm getting an error in the console when I try to cancel a rating: Can't call method "rating_value" on an undefined value at opac/opac-ratings-ajax.pl line 90 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 54225 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54225&action=edit Bug 17089: Improve errors handling -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54207|0 |1 is obsolete| | --- Comment #11 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 54230 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54230&action=edit [SIGNED-OFF] Bug 17089: Koha::Ratings - Add Koha::Rating[s] classes The subroutines in C4::Ratings are only CRUD operations. This patch set moves the entire module to Koha::Rating[s] based on Koha::Object[s] Test plan for the whole patch set: 0/ Set OpacStarRatings to "results and details" 1/ Log in at the OPAC 2/ Launch a search, you should see the star ratings 3/ Go on a detail page, you should see the star ratings 4/ Rate the record 5/ Log in with another patron 6/ Go on the same detail page, you should see the previous rating 7/ Rate the racord, you should see "2 votes" and the average rating value. 8/ Cancel your rate 9/ Disable JavaScript and repeat 7 Note: You cannot cancel your rate if JavaScript is disabled (not a regression). Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54208|0 |1 is obsolete| | --- Comment #12 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 54231 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54231&action=edit [SIGNED-OFF] Bug 17089: Koha::Ratings - move tests Rewrite and move the tests. Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54209|0 |1 is obsolete| | --- Comment #13 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 54232 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54232&action=edit [SIGNED-OFF] Bug 17089: Koha::Ratings - Remove AddRating Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54210|0 |1 is obsolete| | --- Comment #14 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 54233 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54233&action=edit [SIGNED-OFF] Bug 17089: Koha::Ratings - Remove ModRating Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54211|0 |1 is obsolete| | --- Comment #15 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 54234 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54234&action=edit [SIGNED-OFF] Bug 17089: Koha::Ratings - Remove DelRating Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54212|0 |1 is obsolete| | --- Comment #16 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 54235 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54235&action=edit [SIGNED-OFF] Bug 17089: Koha::Ratings - Remove GetRating Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54213|0 |1 is obsolete| | --- Comment #17 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 54236 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54236&action=edit [SIGNED-OFF] Bug 17089: Koha::Ratings - Remove C4::Ratings Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54214|0 |1 is obsolete| | --- Comment #18 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 54237 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54237&action=edit [SIGNED-OFF] Bug 17089: Mark out the boundaries of ratings In the doc of C4::Ratings, it was written that the rate values should be
=0 and <=5, but it was not taken into account. This bug should be fixed on stable releases as well.
Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54225|0 |1 is obsolete| | --- Comment #19 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 54238 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54238&action=edit [SIGNED-OFF] Bug 17089: Improve errors handling Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54230|0 |1 is obsolete| | --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 54332 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54332&action=edit Bug 17089: Koha::Ratings - Add Koha::Rating[s] classes The subroutines in C4::Ratings are only CRUD operations. This patch set moves the entire module to Koha::Rating[s] based on Koha::Object[s] Test plan for the whole patch set: 0/ Set OpacStarRatings to "results and details" 1/ Log in at the OPAC 2/ Launch a search, you should see the star ratings 3/ Go on a detail page, you should see the star ratings 4/ Rate the record 5/ Log in with another patron 6/ Go on the same detail page, you should see the previous rating 7/ Rate the racord, you should see "2 votes" and the average rating value. 8/ Cancel your rate 9/ Disable JavaScript and repeat 7 Note: You cannot cancel your rate if JavaScript is disabled (not a regression). Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54231|0 |1 is obsolete| | Attachment #54232|0 |1 is obsolete| | Attachment #54233|0 |1 is obsolete| | Attachment #54234|0 |1 is obsolete| | Attachment #54235|0 |1 is obsolete| | Attachment #54236|0 |1 is obsolete| | Attachment #54237|0 |1 is obsolete| | Attachment #54238|0 |1 is obsolete| | --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 54333 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54333&action=edit Bug 17089: Koha::Ratings - move tests Rewrite and move the tests. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #23 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 54334 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54334&action=edit Bug 17089: Koha::Ratings - Remove AddRating Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #24 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 54335 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54335&action=edit Bug 17089: Koha::Ratings - Remove ModRating Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #25 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 54336 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54336&action=edit Bug 17089: Koha::Ratings - Remove DelRating Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #26 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 54337 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54337&action=edit Bug 17089: Koha::Ratings - Remove GetRating Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #27 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 54338 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54338&action=edit Bug 17089: Koha::Ratings - Remove C4::Ratings Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #28 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 54339 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54339&action=edit Bug 17089: Mark out the boundaries of ratings In the doc of C4::Ratings, it was written that the rate values should be
=0 and <=5, but it was not taken into account. This bug should be fixed on stable releases as well.
Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #29 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 54340 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54340&action=edit Bug 17089: Improve errors handling Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #30 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 54341 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54341&action=edit Bug 17089: [QA Follow-up] Add some test descriptions to Ratings.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Medium patch QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | --- Comment #31 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA Comment: Code looks good to me. eval { $avg = $sum / $total }; Not your code, but an eval around a non-zero division seems overkill. [% rating_avg_int = BLOCK %][% rating_avg | format("%.0f") %][% END %] This kind of construct does not look very pretty. Simplify? Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 --- Comment #32 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #31)
[% rating_avg_int = BLOCK %][% rating_avg | format("%.0f") %][% END %] This kind of construct does not look very pretty. Simplify?
It's not possible to set a variable to something using a filter: [% SET rating_avg_int = rating_avg | format("%.0f") %] This does not work. The construct is the same I already used in facets.inc: [% url = BLOCK %][% url %][% "&sort_by=" _ sort_by |url %][% END %] If you find something easier to write/read I will be happy to know it :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |kyle@bywatersolutions.com --- Comment #33 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 16.11, thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17089 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |15449 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15449 [Bug 15449] Move stuffs to the Koha namespace -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org