From 7e991d0702dac22e1a578b9544c2029f73a34f17 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 16 Aug 2022 12:14:06 +0200 Subject: [PATCH] Bug 29939: Use the REST API for ratings This patch replaces opac-ratings-ajax.pl with a new REST API route POST /public/biblios/42/ratings Note that we could go further and refactor the 'start_rating' select code. Test plan: Test the "star ratings" feature at the OPAC, on the different page where it's displayed. Signed-off-by: Owen Leonard Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi --- Koha/REST/V1/Biblios.pm | 71 +++++++++++ api/v1/swagger/paths/biblios.yaml | 66 ++++++++++ api/v1/swagger/swagger.yaml | 2 + .../bootstrap/en/includes/doc-head-close.inc | 1 + .../bootstrap/en/modules/opac-detail.tt | 70 +++-------- .../en/modules/opac-readingrecord.tt | 3 - .../bootstrap/en/modules/opac-user.tt | 2 - koha-tmpl/opac-tmpl/bootstrap/js/global.js | 12 ++ koha-tmpl/opac-tmpl/bootstrap/js/ratings.js | 33 +++-- opac/opac-ratings-ajax.pl | 116 ------------------ t/db_dependent/api/v1/biblios.t | 39 +++++- 11 files changed, 224 insertions(+), 191 deletions(-) delete mode 100755 opac/opac-ratings-ajax.pl diff --git a/Koha/REST/V1/Biblios.pm b/Koha/REST/V1/Biblios.pm index c74f5180c6..f923255cfa 100644 --- a/Koha/REST/V1/Biblios.pm +++ b/Koha/REST/V1/Biblios.pm @@ -20,6 +20,7 @@ use Modern::Perl; use Mojo::Base 'Mojolicious::Controller'; use Koha::Biblios; +use Koha::Ratings; use Koha::RecordProcessor; use C4::Biblio qw( DelBiblio ); @@ -409,4 +410,74 @@ sub get_items_public { }; } +=head3 set_rating + +Set rating for the logged in user + +=cut + + +sub set_rating { + my $c = shift->openapi->valid_input or return; + + my $biblio = Koha::Biblios->find( $c->validation->param('biblio_id') ); + + unless ($biblio) { + return $c->render( + status => 404, + openapi => { + error => "Object not found." + } + ); + } + + my $patron = $c->stash('koha.user'); + unless ($patron) { + return $c->render( + status => 403, + openapi => + { error => "Cannot rate. Reason: must be logged-in" } + ); + } + + my $body = $c->validation->param('body'); + my $rating_value = $body->{rating}; + + return try { + + my $rating = Koha::Ratings->find( + { + biblionumber => $biblio->biblionumber, + borrowernumber => $patron->borrowernumber, + } + ); + $rating->delete if $rating; + + if ( $rating_value ) { # Cannot set to 0 from the UI + $rating = Koha::Rating->new( + { + biblionumber => $biblio->biblionumber, + borrowernumber => $patron->borrowernumber, + rating_value => $rating_value, + } + )->store; + }; + my $ratings = + Koha::Ratings->search( { biblionumber => $biblio->biblionumber } ); + my $average = $ratings->get_avg_rating; + + return $c->render( + status => 200, + openapi => { + rating => $rating && $rating->in_storage ? $rating->rating_value : undef, + average => $average, + count => $ratings->count + }, + ); + } + catch { + $c->unhandled_exception($_); + }; +} + 1; diff --git a/api/v1/swagger/paths/biblios.yaml b/api/v1/swagger/paths/biblios.yaml index 60e4e951a4..782de211e7 100644 --- a/api/v1/swagger/paths/biblios.yaml +++ b/api/v1/swagger/paths/biblios.yaml @@ -383,3 +383,69 @@ description: Under maintenance schema: $ref: "../swagger.yaml#/definitions/error" +"/public/biblios/{biblio_id}/ratings": + post: + x-mojo-to: Biblios#set_rating + operationId: setBiblioRating + tags: + - biblios + summary: set biblio rating (public) + parameters: + - $ref: "../swagger.yaml#/parameters/biblio_id_pp" + - name: body + in: body + description: A JSON object containing rating information + schema: + type: object + properties: + rating: + description: the rating + type: + - integer + - "null" + required: + - rating + additionalProperties: false + produces: + - application/json + responses: + "200": + description: Rating set + schema: + type: object + properties: + rating: + description: user's rating + type: + - number + - "null" + average: + description: average rating + type: number + count: + description: number of ratings + type: integer + additionalProperties: false + "401": + description: Authentication required + schema: + $ref: "../swagger.yaml#/definitions/error" + "403": + description: Access forbidden + schema: + $ref: "../swagger.yaml#/definitions/error" + "404": + description: Biblio not found + schema: + $ref: "../swagger.yaml#/definitions/error" + "500": + description: | + Internal server error. Possible `error_code` attribute values: + + * `internal_server_error` + schema: + $ref: "../swagger.yaml#/definitions/error" + "503": + description: Under maintenance + schema: + $ref: "../swagger.yaml#/definitions/error" diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml index 597af62166..a10f2408b6 100644 --- a/api/v1/swagger/swagger.yaml +++ b/api/v1/swagger/swagger.yaml @@ -207,6 +207,8 @@ paths: $ref: "./paths/biblios.yaml#/~1public~1biblios~1{biblio_id}" "/public/biblios/{biblio_id}/items": $ref: "./paths/biblios.yaml#/~1public~1biblios~1{biblio_id}~1items" + "/public/biblios/{biblio_id}/ratings": + $ref: "./paths/biblios.yaml#/~1public~1biblios~1{biblio_id}~1ratings" /public/libraries: $ref: ./paths/libraries.yaml#/~1public~1libraries "/public/libraries/{library_id}": diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/doc-head-close.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/doc-head-close.inc index e29568d76f..816f4a8d4b 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/doc-head-close.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/doc-head-close.inc @@ -63,6 +63,7 @@ [% IF lang && lang != 'en' %] [% Asset.js(lang _ '/js/locale_data.js') | $raw %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt index 802a642d7c..e69c3fac73 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt @@ -299,15 +299,15 @@ [% IF ( OpacStarRatings != 'disable' ) %]
Star ratings -
+
[% SET rating_avg = ratings.get_avg_rating() %] [% rating_avg_int = BLOCK %][% rating_avg | format("%.0f") %][% END %] [% IF ( borrowernumber ) %] - [% ELSE %] - [% END %] [% IF ( rating_avg_int == 0 ) %] @@ -320,24 +320,24 @@ [% END %] [% END %] - + - + [% UNLESS ( rating_readonly ) %]  [% END %]  [% IF my_rating %] - Your rating: [% my_rating.rating_value | html %]. - Cancel rating. + Your rating: [% my_rating.rating_value | html %]. + Cancel rating. [% ELSE %] - - + + [% END %] - Average rating: [% rating_avg | html %] ([% ratings.count | html %] votes) + Average rating: [% rating_avg | html %] ([% ratings.count | html %] votes)
[% END # / IF OpacStarRatings != 'disable' %] @@ -1432,7 +1432,10 @@ [% INCLUDE 'datatables.inc' %] [% INCLUDE 'columns_settings.inc' %] [% INCLUDE greybox.inc %] - [% IF ( OpacStarRatings != 'disable' ) %][% Asset.js("lib/jquery/plugins/jquery.barrating.min.js") | $raw %][% END %] + [% IF ( OpacStarRatings != 'disable' ) %] + [% Asset.js("lib/jquery/plugins/jquery.barrating.min.js") | $raw %] + [% Asset.js("js/ratings.js") | $raw %] + [% END %] [% IF ( OpacHighlightedWords ) %][% Asset.js("lib/jquery/plugins/jquery.highlight-3.js") | $raw %][% END %] [% IF ( Koha.Preference('OPACDetailQRCode') ) %] @@ -1883,51 +1886,6 @@ }); }()); [% END # /IF ( OPACShelfBrowser ) %] - - [% IF ( OpacStarRatings != 'disable' ) %] - // ----------------------------------------------------- - // star-ratings code - // ----------------------------------------------------- - // hide 'rate' button if javascript enabled - - $('input[name="rate_button"]').remove(); - - var rating_enabled = ( $("#star_rating").data("rating-enabled") == "1" ) ? false : true; - $('#star_rating').barrating({ - theme: 'fontawesome-stars', - showSelectedRating: false, - allowEmpty: true, - deselectable: false, - readonly: rating_enabled, - onSelect: function(value, text) { - $("#rating-loading").show(); - $.post("/cgi-bin/koha/opac-ratings-ajax.pl", { - rating_old_value: $("#rating_value").attr("value"), - borrowernumber: "[% borrowernumber | html %]", - biblionumber: "[% biblio.biblionumber | html %]", - rating_value: value, - auth_error: value - }, function (data) { - $("#rating_value").val(data.rating_value); - if (data.rating_value) { - $("#rating_value_text").text(_("Your rating: %s, ").format(data.rating_value)); - $("#cancel_rating_text").show(); - } else { - $("#rating_value_text").text(''); - $("#cancel_rating_text").hide(); - } - $("#rating_text").text(_("Average rating: %s (%s votes)").format(data.rating_avg, data.rating_total)); - $("#rating-loading").hide(); - }, "json"); - } - }); - - $("#cancel_rating_text a").on("click", function(e){ - e.preventDefault(); - $("#star_rating").barrating("set", ""); - }); - - [% END # / IF ( OpacStarRatings != 'disable' )%] }); $(document).ready(function() { diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt index dc5e2aac67..1e4f7d6f3e 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt @@ -228,9 +228,6 @@ } }); }); - var borrowernumber = "[% logged_in_user.borrowernumber | html %]"; - var MSG_YOUR_RATING = _("Your rating: %s, "); - var MSG_AVERAGE_RATING = _("Average rating: %s (%s votes)"); [% IF ( Koha.Preference('OpacStarRatings') == 'all' ) %] [% Asset.js("lib/jquery/plugins/jquery.barrating.min.js") | $raw %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt index 11275fab5f..d131abc175 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt @@ -1347,8 +1347,6 @@ } var borrowernumber = "[% borrowernumber | html %]"; - var MSG_YOUR_RATING = _("Your rating: %s, "); - var MSG_AVERAGE_RATING = _("Average rating: %s (%s votes)"); [% IF ( Koha.Preference('OpacStarRatings') == 'all' ) %] [% Asset.js("lib/jquery/plugins/jquery.barrating.min.js") | $raw %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/js/global.js b/koha-tmpl/opac-tmpl/bootstrap/js/global.js index 1a40118106..724d9d4831 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/js/global.js +++ b/koha-tmpl/opac-tmpl/bootstrap/js/global.js @@ -117,6 +117,18 @@ function confirmModal(message, title, yes_label, no_label, callback) { $("#bootstrap-confirm-box-modal").modal('show'); } + +// Function to check errors from AJAX requests +const checkError = function(response) { + if (response.status >= 200 && response.status <= 299) { + return response.json(); + } else { + console.log("Server returned an error:"); + console.log(response); + alert("%s (%s)".format(response.statusText, response.status)); + } +}; + //Add jQuery :focusable selector (function($) { function visible(element) { diff --git a/koha-tmpl/opac-tmpl/bootstrap/js/ratings.js b/koha-tmpl/opac-tmpl/bootstrap/js/ratings.js index 971e604f2f..f867d6d4c4 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/js/ratings.js +++ b/koha-tmpl/opac-tmpl/bootstrap/js/ratings.js @@ -1,4 +1,3 @@ -/* global borrowernumber MSG_YOUR_RATING MSG_AVERAGE_RATING */ // ----------------------------------------------------- // star-ratings code // ----------------------------------------------------- @@ -14,27 +13,35 @@ $(document).ready(function(){ showSelectedRating: false, allowEmpty: true, deselectable: false, + readonly: !is_logged_in, onSelect: function( value ) { var context = $("#" + this.$elem.data("context") ); $(".rating-loading", context ).show(); - $.post("/cgi-bin/koha/opac-ratings-ajax.pl", { - rating_old_value: $(".rating_value", context ).attr("value"), - borrowernumber: borrowernumber, - biblionumber: this.$elem.data('biblionumber'), - rating_value: value, - auth_error: value - }, function (data) { - $(".rating_value", context ).val(data.rating_value); - if (data.rating_value) { - $(".rating_value_text", context ).text( MSG_YOUR_RATING.format(data.rating_value) ); + let biblionumber = this.$elem.data('biblionumber'); + if ( value == "" ) value = null; + fetch("/api/v1/public/biblios/"+biblionumber+"/ratings", { + method: 'POST', + body: JSON.stringify({ rating: value }), + headers: { + "Content-Type": "application/json;charset=utf-8", + } + }).then(checkError) + .then((data) => { + $(".rating_value", context ).val(data.rating); + console.log(data); + console.log($(".cancel_rating_text", context )); + if (data.rating) { + console.log(data.rating); + $(".rating_value_text", context ).text( __("Your rating: %s.").format(data.rating) ); $(".cancel_rating_text", context ).show(); } else { $(".rating_value_text", context ).text(""); $(".cancel_rating_text", context ).hide(); } - $(".rating_text", context ).text( MSG_AVERAGE_RATING.format(data.rating_avg, data.rating_total) ); + console.log($(".rating_text", context )); + $(".rating_text", context ).text( __("Average rating: %s (%s votes)").format(data.average, data.count) ); $(".rating-loading", context ).hide(); - }, "json"); + }); } }); diff --git a/opac/opac-ratings-ajax.pl b/opac/opac-ratings-ajax.pl deleted file mode 100755 index dcbeaebabb..0000000000 --- a/opac/opac-ratings-ajax.pl +++ /dev/null @@ -1,116 +0,0 @@ -#!/usr/bin/perl - -# Copyright 2011 KohaAloha, NZ -# -# 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 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. -# -# You should have received a copy of the GNU General Public License -# along with Koha; if not, see . - -=head1 DESCRIPTION - -A script that takes an ajax json query, and then inserts or modifies a star-rating. - -=cut - -use Modern::Perl; - -use CGI qw ( -utf8 ); -use CGI::Cookie; # need to check cookies before having CGI parse the POST request - -use C4::Auth qw( get_template_and_user check_cookie_auth ); -use C4::Context; -use C4::Output qw( is_ajax output_ajax_with_http_headers ); - -use Koha::Ratings; - -use JSON; - -my $is_ajax = is_ajax(); - -my ( $query, $auth_status ); -if ($is_ajax) { - ( $query, $auth_status ) = &ajax_auth_cgi( {} ); -} -else { - $query = CGI->new(); -} - -my $biblionumber = $query->param('biblionumber'); -my $rating_value = $query->param('rating_value'); -my $rating_old_value = $query->param('rating_old_value'); - -my ( $template, $loggedinuser, $cookie ); -if ($is_ajax) { - $loggedinuser = C4::Context->userenv->{'number'}; -} -else { - ( $template, $loggedinuser, $cookie ) = get_template_and_user( - { - template_name => "opac-detail.tt", - query => $query, - type => "opac", - authnotrequired => 0, # auth required to add tags - } - ); -} - -my $rating; -$rating_value //= ''; - -if ( $rating_value eq '' ) { - my $rating = Koha::Ratings->find( { biblionumber => $biblionumber, borrowernumber => $loggedinuser } ); - $rating->delete if $rating; -} - -elsif ( $rating_value and !$rating_old_value ) { - Koha::Rating->new( { biblionumber => $biblionumber, borrowernumber => $loggedinuser, rating_value => $rating_value, })->store; -} - -elsif ( $rating_value ne $rating_old_value ) { - my $rating = Koha::Ratings->find( { biblionumber => $biblionumber, borrowernumber => $loggedinuser }); - $rating->rating_value($rating_value)->store if $rating -} - -my $ratings = Koha::Ratings->search({ biblionumber => $biblionumber }); -my $my_rating = $ratings->search({ borrowernumber => $loggedinuser })->next; -my $avg = $ratings->get_avg_rating; - -my %js_reply = ( - rating_total => $ratings->count, - rating_avg => $avg, - rating_avg_int => sprintf("%.0f", $avg), - rating_value => $my_rating ? $my_rating->rating_value : undef, - auth_status => $auth_status, - -); - -my $json_reply = JSON->new->encode( \%js_reply ); - -#### $rating -#### %js_reply -#### $json_reply - -output_ajax_with_http_headers( $query, $json_reply ); -exit; - -# a ratings specific ajax return sub, returns CGI object, and an 'auth_success' value -sub ajax_auth_cgi { - my $needed_flags = shift; - my %cookies = CGI::Cookie->fetch; - my $input = CGI->new; - my $sessid = $cookies{'CGISESSID'}->value || $input->param('CGISESSID'); - my ( $auth_status ) = - check_cookie_auth( $sessid, $needed_flags ); - return $input, $auth_status; -} diff --git a/t/db_dependent/api/v1/biblios.t b/t/db_dependent/api/v1/biblios.t index ca7a23e1e7..b4e98b17f4 100755 --- a/t/db_dependent/api/v1/biblios.t +++ b/t/db_dependent/api/v1/biblios.t @@ -20,7 +20,7 @@ use Modern::Perl; use utf8; use Encode; -use Test::More tests => 7; +use Test::More tests => 8; use Test::MockModule; use Test::Mojo; use Test::Warn; @@ -676,3 +676,40 @@ subtest 'get_checkouts() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'set_rating() tests' => sub { + + plan tests => 12; + + $schema->storage->txn_begin; + + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 0 } + } + ); + my $password = 'thePassword123'; + $patron->set_password( { password => $password, skip_validation => 1 } ); + $patron->discard_changes; + my $userid = $patron->userid; + + my $biblio = $builder->build_sample_biblio(); + $t->post_ok("/api/v1/public/biblios/" . $biblio->biblionumber . "/ratings" => json => { rating => 3 }) + ->status_is(403); + + $t->post_ok("//$userid:$password@/api/v1/public/biblios/" . $biblio->biblionumber . "/ratings" => json => { rating => 3 }) + ->status_is(200) + ->json_is( '/rating', '3' ) + ->json_is( '/average', '3' ) + ->json_is( '/count', '1' ); + + $t->post_ok("//$userid:$password@/api/v1/public/biblios/" . $biblio->biblionumber . "/ratings" => json => { rating => undef }) + ->status_is(200) + ->json_is( '/rating', undef ) + ->json_is( '/average', '0' ) + ->json_is( '/count', '0' ); + + $schema->storage->txn_rollback; + +}; -- 2.39.5