[Koha-bugs] [Bug 5668] Star ratings in the opac

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668

--- Comment #46 from Srdjan Jankovic <srdjan at catalyst.net.nz> ---
This is a patch that I'm trying to upstream, so I cannot give exact answers for
all:

1/ Why did you comment some unit tests ? They are ok.

Fixed

2/ You declare OpacStarRatings for template in C4::Auth, but it is only used in
2 tmpl (opac-results.tt and opac-detail.tt). Isn't it better to declare them in
their respective pl file ?

I'm not sure. I will move it if requested.

3/ Why have you set the POD in the end of the module and not before each
function like (nearly) everywhere in Koha ? (C4::Ratings)

I cannot see it being in the end. I've added missing doc headers.

4/ There is a TODO in opac/opac-ratings-ajax.pl :) Moreover, in this script,
why the "use warnings" line is commented ?
+ I think the use of C4::Dates can be deleted

That's fixed

5/ Why don't you create a foreign key for biblionumber and borrowernumber with
"on delete cascade" in the ratings table ?

Done

6/ It appears that a more recent version of jquery.ratings.js exists (v3.13 vs
v3.10) it is certainly not important, but why don't have choose the lastest ? I
don't have checked the difference between both.

Updated

7/ Some useless comment lines are still present (#use Smart::Comments '####';)

Renoved

8/ As said Katrin, javascript code is generated into a .pl file, isn't it
possible to open an alert into the javascript code ? After returns of ajax
response for example ?

I believe this has been changed

Comment 45: 'checked="1"' should be 'checked="checked"', and embedding [% IF %]
inside a tag goes against a coding guideline we have to prevent errors with the
translation script:

Changed checked. 
Embeding IF tags: that is a great pain. I have a workaround, but I don't really
understand the issue here. "checked" needs no translation, but regardless. I
think that if this is a problem for the translation script, then the script
needs fixing.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are watching all bug changes.