http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #2 from Mason JAMES <mason@kohaaloha.com> 2011-01-30 21:16:40 UTC --- Created attachment 3035 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=3035 detail.pl screenshot -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> 2011-04-25 00:30:25 UTC --- What is the status of this bug? -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Robin Sheat <robin@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |robin@catalyst.net.nz AssignedTo|oleonard@myacpl.org |robin@catalyst.net.nz --- Comment #5 from Robin Sheat <robin@catalyst.net.nz> 2011-08-17 03:11:20 UTC --- I've got a star ratings patch (I think the same one from here) that I'm going to polish up and send upstream. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #7 from Robin Sheat <robin@catalyst.net.nz> 2011-08-23 00:39:15 UTC --- Created attachment 5097 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=5097 Bug 5668 - star ratings in the OPAC This takes the work done by Mason James and tidies it up a bit to work with master. This patch allows OPAC users to assign star ratings to biblios, and other users to see the average of what other people have rated. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #8 from Robin Sheat <robin@catalyst.net.nz> 2011-08-23 00:44:37 UTC --- Created attachment 5098 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=5098 Bug 5668 - adds the 'delete.gif' image This was accidentally left off the first commit. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Nicole C. Engard <nengard@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch Status|Needs Signoff |Failed QA --- Comment #9 from Nicole C. Engard <nengard@gmail.com> 2011-08-24 16:18:01 UTC --- Notes: 1. White space errors Applying: Bug 5668 - star ratings in the OPAC /home/nengard/kohaclone/.git/rebase-apply/patch:210: trailing whitespace. is 0, then the rating will be deleted. If the value is out of the range of /home/nengard/kohaclone/.git/rebase-apply/patch:225: space before tab in indent. carp "Attempted to add a rating for borrower number 0"; /home/nengard/kohaclone/.git/rebase-apply/patch:226: space before tab in indent. return; /home/nengard/kohaclone/.git/rebase-apply/patch:354: trailing whitespace. /home/nengard/kohaclone/.git/rebase-apply/patch:358: trailing whitespace. warning: squelched 63 whitespace errors warning: 68 lines add whitespace errors. 2. In the sys pref it says "Dont't" - we need to remove the first 't' 3. When you're not logged in you can click on the stars on the search results and it puts the starts in but says (0 votes) - it shouldn't be clickable if you're not logged in (like when you're on the detail page) 4. On the detail page it might be nice (not necessary) to say - log in to rate this title - just like we do for comments and tags. Marking Failed QA because of #3 which is a misleading problem - but the other things are here in case they too can be fixed. I have only tested patch 1 - not the follow up with the delete image. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Robin Sheat <robin@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch Status|Failed QA |Needs Signoff --- Comment #11 from Robin Sheat <robin@catalyst.net.nz> 2011-08-25 00:17:34 UTC --- This should fix up the issues you mention, although probably not the whitespace ones. It also includes the delete.gif. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Nicole C. Engard <nengard@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #5097|0 |1 is obsolete| | -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Nicole C. Engard <nengard@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #5136|0 |1 is obsolete| | --- Comment #13 from Nicole C. Engard <nengard@gmail.com> 2011-08-25 01:08:52 UTC --- Created attachment 5137 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=5137 [SIGNED-OFF] Bug 5668 - star ratings in the OPAC This takes the work done by Mason James and tidies it up a bit to work with master. This patch allows OPAC users to assign star ratings to biblios, and other users to see the average of what other people have rated. Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #15 from Robin Sheat <robin@catalyst.net.nz> 2011-08-25 01:25:09 UTC --- I think you applied on top of the already applied patch, rather than from master, which is what caused those messages. But, all good that it's working :) -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|robin@catalyst.net.nz |mtj@kohaaloha.com -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #18 from Mason James <mtj@kohaaloha.com> 2011-10-08 11:11:50 UTC --- (In reply to comment #16)

Some issues, in order of descending importance:

hi Ian i have subitted a patch to address your 3 BLOCKER issues...

1) Clearing does not work on search results; no AJAX seems to be posted at all. -- must fix to pass QA

i have made the ratings 'readonly' on the search results page. i think the removal of this feature is wise, and will help stop people (read: teenagers) abusing the ratings system my intention was for this to be an *experimental* feature, as i had reservations about including this feature in my patch to kc.org

2) Star ratings are rounded down (a rating of 2 and rating of 3 show a rating of 2 stars, rather than two and a half -- I believe this will be confusing to users if it's not fixed; please attempt.

i did attempt this enhancment, and discovered it breaks the page in no-JS mode, which is bad... i think it is wise to keep the ratings as whole-numbers only, until a solution is found that does not break paging viewing in no-JS mode (which will probably be another rating system?)

3) My ratings and other peoples ratings are the same color (yellow), making it difficult to tell quickly what I've already rated -- same as 2, though a little less urgent

the difference between my-ratings and average ratings is now clearly stated as text 'your rating: 5, average: 3.2 (12 votes)' is issue is now fixed -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #19 from Mason James <mtj@kohaaloha.com> 2011-10-08 11:16:22 UTC --- Created attachment 5794 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=5794 pix -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch Status|Needs Signoff |Pushed For QA -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch Status|Pushed For QA |Needs Signoff --- Comment #22 from Katrin Fischer <katrin.fischer@bsz-bw.de> 2011-11-02 20:31:16 UTC --- I am confused by the patch status here, but it seems Robin's patch still needs a sign-off? I am changing the status to that, so it does not get lost. Please change if I am wrong. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wizzyrea@gmail.com --- Comment #24 from Liz Rea <wizzyrea@gmail.com> 2011-11-04 20:31:34 UTC --- It doesn't seem to matter what I do, I can't get the ratings to stick? It always says "your rating: none" and "average rating: null" The ratings don't seem to stick when I refresh the page, either. (It is possible that I have some kind of corruption in my database, but please check on these issues all the same) -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #26 from Liz Rea <wizzyrea@gmail.com> 2011-11-04 20:36:06 UTC --- http://screencast.com/t/tt56vcqqQT38 here is a screencast of what I was experiencing. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #28 from Magnus Enger <magnus@enger.priv.no> 2011-12-29 19:27:38 UTC --- When I run updatedatabase.pl I get this error: "DBD::mysql::db do failed: Key column 'rating_id' doesn't exist in table at installer/data/mysql/updatedatabase.pl line 4590." Looks like rating_id is declared as the primary key without actuelly being defined first? + CREATE TABLE `ratings` ( + `borrowernumber` int(11) NOT NULL, + `biblionumber` int(11) NOT NULL, + `value` tinyint(1) NOT NULL, + `timestamp` timestamp NOT NULL default CURRENT_TIMESTAMP, + PRIMARY KEY (`rating_id`), -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #30 from Robin Sheat <robin@catalyst.net.nz> 2011-12-29 19:34:05 UTC --- The dependency on Smart::Comments should be removed, and the SQL error is due to a change from having an ID column to having a composite key (as you don't really need an ID when biblio+borrower are unique :) Not sure when I'll get time to look at this. Hopefully Mason beats me to it ;) -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #32 from Robin Sheat <robin@catalyst.net.nz> 2011-12-29 19:44:13 UTC --- IIRC Smart::Comments is a development/debugging thing, so there's no real need nor want to keep it in the production code. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #34 from Mason James <mtj@kohaaloha.com> 2012-01-09 02:47:59 UTC --- (In reply to comment #30)

The dependency on Smart::Comments should be removed, and the SQL error is due to a change from having an ID column to having a composite key (as you don't really need an ID when biblio+borrower are unique :)

Not sure when I'll get time to look at this. Hopefully Mason beats me to it ;)

thanks for the input everyone, i'll make some corrections and send a patch up this week :) -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|rel_3_6 |master Patch Status|Failed QA |Needs Signoff --- Comment #36 from Mason James <mtj@kohaaloha.com> 2012-01-12 22:56:29 UTC --- for the curious i have the ratings stuff running on a demo (user:pwd = test:testtest) http://demo2.kohaaloha.com/cgi-bin/koha/opac-search.pl?q=the http://demo2.kohaaloha.com/cgi-bin/koha/opac-detail.pl?biblionumber=1 to test, after successfully applying the patch... 1) run the updatedatedase.pl script $ perl installer/data/mysql/updatedatabase.pl Upgrade to 9.99.99.XXX done (Add 'ratings' table and 'OpacStarRatings' syspref) 2) run the test suite $ perl t/db_dependent/Ratings.t 1..9 ok 1 - use C4::Ratings; ok 2 - add a rating ok 3 - add another rating ok 4 - update a rating ok 5 - get a rating, with userid ok 6 - get a rating, without userid ok 7 - verify user's bib rating ok 8 - delete a rating ok 9 - delete another rating 3) set syspref 'OpacStarRatings' to 'Show star-ratings on "results and details" pages' 4) search the OPAC fyi: this patch was created from commit 756a1f6b72c976a342b510f3cfc25105f3719cd0 -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #38 from Katrin Fischer <katrin.fischer@bsz-bw.de> 2012-01-15 22:05:54 UTC --- Created attachment 7166 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=7166 Bug 5668: Follow up - Star ratings - adds new system preference OpacStarRatings to sysprefs.sql file - changes NULL NULL to 'yes|no|details','Choice' - changes updatedatabase.pl - removes mysqlisms from the table definition - changes INSERT to make it more robust and match sysprefs.sql -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7166|0 |1 is obsolete| | -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@biblibre.co | |m --- Comment #41 from Jonathan Druart <jonathan.druart@biblibre.com> 2012-01-19 15:08:07 UTC --- QA comment: 1/ Why did you comment some unit tests ? They are ok. 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 ? 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) 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 5/ Why don't you create a foreign key for biblionumber and borrowernumber with "on delete cascade" in the ratings table ? 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. 7/ Some useless comment lines are still present (#use Smart::Comments '####';) 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 ? Else, all works fine, great job :) Marked as Failed QA. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #43 from Paul Poulain <paul.poulain@biblibre.com> 2012-02-17 14:39:10 UTC --- The script devel/misc/testbugzillapatches.pl has detected that this bug does not apply anymore, so i'm marking this bug as "does not apply". The script is supposed to handle conflicts related to updatedatabase, but no other cases of merge conflict. So the conflict may be trivial to fix. If it happens that the patch attached does apply cleanly, please send me an email, there's something wrong in the script. It's possible, as it's a recent script. If the patch applies does not apply as I think, please rebase and resubmit, thanks -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #46 from Srdjan Jankovic <srdjan@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.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #49 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Yes, it uses ?. I thought I could trick, it but the script is too clever for me :) I would change it if there was no other way, but the whole thing *is* wrong: * ? (or IF, does not matter) is used to set an attribute conditionally; that is not such a rare occasion * in this particular case, we have 2 conditional attributes, which brings us to one line being replaced with four; I hate to think of a poor soul that would have to deal with three conditional attributes I will not make any changes now, because Mason James is working on an improved version, so we'll deal with it when the new version is ready. BTW, how do you set that DO-NOT-USE status? -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|srdjan@catalyst.net.nz |mtj@kohaaloha.com -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8027|0 |1 is obsolete| | --- Comment #51 from Mason James <mtj@kohaaloha.com> --- Created attachment 8199 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8199&action=edit Bug 5668 - Star ratings in the opac applied to commit 49c387cddf6de083343fe6c88ad9f167844521b4 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Nicole C. Engard <nengard@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8199|0 |1 is obsolete| | --- Comment #53 from Nicole C. Engard <nengard@gmail.com> --- Created attachment 8202 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8202&action=edit [SIGNED-OFF] Bug 5668 - Star ratings in the opac applied to commit 49c387cddf6de083343fe6c88ad9f167844521b4 Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Turned on star ratings in the opac on details and results Searched for titles - saw the stars Clicked on a title Clicked on the stars Clicked on the stars to change my rating Logged out Tried to click on stars Logged in as different user Rated items that were rated already and saw average change Changed preference to show only on detail and repeated tests Changed preference to now show stars All above tests passed. Signing off. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #54 from Ian Walls <koha.sekjal@gmail.com> --- Created attachment 8312 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8312&action=edit Bug 5668 - Star ratings in the opac applied to commit 49c387cddf6de083343fe6c88ad9f167844521b4 Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Turned on star ratings in the opac on details and results Searched for titles - saw the stars Clicked on a title Clicked on the stars Clicked on the stars to change my rating Logged out Tried to click on stars Logged in as different user Rated items that were rated already and saw average change Changed preference to show only on detail and repeated tests Changed preference to now show stars All above tests passed. Signing off. Rebased 3-19-12 by Ian Walls -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #55 from Ian Walls <koha.sekjal@gmail.com> --- I've rebased this as part of the QA process, and figured I'd attach it here since I'm at a stopping point for the moment. This seems to work alright, but t/db_dependent/Ratings.t fails on me. There is a note that it will fail on any database that doesn't have a blank 'ratings' table... this seems like a less than useful test to me. And, it failed both with and without lines in the ratings table. I'm not sure if this is a problem with the code or the unit test. Could someone take a look before I resume QA? -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8312|0 |1 is obsolete| | --- Comment #57 from Mason James <mtj@kohaaloha.com> --- Created attachment 8326 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8326&action=edit Bug 5668 - Star ratings in the opac applied to commit b3cae766949599c88095930b8fd37dabfc79a5cc Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Turned on star ratings in the opac on details and results Searched for titles - saw the stars Clicked on a title Clicked on the stars Clicked on the stars to change my rating Logged out Tried to click on stars Logged in as different user Rated items that were rated already and saw average change Changed preference to show only on detail and repeated tests Changed preference to now show stars All above tests passed. Signing off. Rebased 3-19-12 by Ian Walls -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #59 from Ian Walls <koha.sekjal@gmail.com> --- Looking good, almost there. Problems: Unit tests don't work on other database than the testing db, leading to errors in auto testing suites. --- I have a followup patch to fix this, will attach shortly Breaks Baker & Taylor on opac-details.tt (easy fix, change BakerTaylorContenturl to BakerTaylorContentURL) System preference is classed as "Choice", between 'yes', 'no' and 'details', but the default value is 0, and the code tests on 1|details. Need to sort out this to consistent terms, either all textual (recommended) or all numeric. Non-problems: Lots of extra whitespace lines are introduced... no need for them, but no problem, either. I'm going on the notion that it's better to avoid unnecessary changes, even if they're benign. opac-ratings-ajax.pl references opac-detail.tmpl instead of opac-detail.tt... no biggie, that happens a lot, but better to clean it up as new code is created. COinSinOPACResults gets moved in opac-search.pl... again, unnecessary and benign, just observing. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #61 from Ian Walls <koha.sekjal@gmail.com> --- Created attachment 8514 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8514&action=edit Enh 5668 Followup: fix BakerTaylorContentURL typo Restores opac-detail.tt reference to BakerTaylorContentURL, instead of BakerTaylorContenturl -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #63 from Mason James <mtj@kohaaloha.com> --- (In reply to comment #62)

...

System preference is classed as "Choice", between 'yes', 'no' and 'details', but the default value is 0, and the code tests on 1|details. Need to sort out this to consistent terms, either all textual (recommended) or all numeric.

i'll fix this issue now

hmm, this is almost a wont-fix, for me agreed, the numeric values idea is bad bad bad some bug (or designed feature?) in preferences.pl makes it currently impossible to save the literal strings 'yes' and 'no' to the sys-prefs table either preferences.pl (or mysql itself?), seems to transform the values 'yes' and 'no', to 1 or 0 when saving i think the only work around here, is to use some-other values (disable|details|all)? rather than (no|details|yes) thats the best i can do... ps: its also the reason why the goofy '1|details' syntax was used in the first place, i had simply forgotten the initial problem -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8359|0 |1 is obsolete| | Attachment #8459|0 |1 is obsolete| | Attachment #8514|0 |1 is obsolete| | --- Comment #65 from Mason James <mtj@kohaaloha.com> --- Created attachment 8537 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8537&action=edit Bug 5668 - Star ratings in the opac applied to commit b3cae766949599c88095930b8fd37dabfc79a5cc Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Turned on star ratings in the opac on details and results Searched for titles - saw the stars Clicked on a title Clicked on the stars Clicked on the stars to change my rating Logged out Tried to click on stars Logged in as different user Rated items that were rated already and saw average change Changed preference to show only on detail and repeated tests Changed preference to now show stars All above tests passed. Signing off. Rebased 3-19-12 by Ian Walls -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #67 from Ian Walls <koha.sekjal@gmail.com> --- Created attachment 8548 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8548&action=edit [PASSED QA] Bug 5668 - Star ratings in the opac Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Turned on star ratings in the opac on details and results Searched for titles - saw the stars Clicked on a title Clicked on the stars Clicked on the stars to change my rating Logged out Tried to click on stars Logged in as different user Rated items that were rated already and saw average change Changed preference to show only on detail and repeated tests Changed preference to now show stars All above tests passed. Signing off. Rebased 3-22-12 by Ian Walls -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA CC| |paul.poulain@biblibre.com Version|master |rel_3_8 --- Comment #69 from Paul Poulain <paul.poulain@biblibre.com> --- QA comment: I checked perlcritic and there's an error: opac/opac-ratings-ajax.pl: Subroutine prototypes used at line 107, column 1. See page 194 of PBP. (Severity: 5) please fix it & switch to passed QA again. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #71 from Mason James <mtj@kohaaloha.com> --- Created attachment 8613 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8613&action=edit Attachment to Bug 5668 - Star ratings in the opac mason@xen1:~/g/head2$ perlcritic opac/opac-ratings-ajax.pl opac/opac-ratings-ajax.pl source OK modified: opac/opac-ratings-ajax.pl -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #72 from Paul Poulain <paul.poulain@biblibre.com> --- sorry, there's still a problem: * many files now have a conflict (just pushed bug 7470, that is probably the cause) CONFLICT (content): Merge conflict in installer/data/mysql/kohastructure.sql CONFLICT (content): Merge conflict in installer/data/mysql/sysprefs.sql CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt CONFLICT (content): Merge conflict in opac/opac-search.pl I thought they were easy to solve, but seems I can't merge opac-search.pl easily. After trying, I get a : syntax error at opac/opac-search.pl line 746, near "elsif" Missing right curly or square bracket at opac/opac-search.pl line 798, at end of line I prefer not doing something wrong, and ask for a rebased patch, really sorry (Even if I "feature freeze" today, i'll accept this patch if you provide a rebased version this week) -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8548|0 |1 is obsolete| | --- Comment #74 from Mason James <mtj@kohaaloha.com> --- Comment on attachment 8548 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8548 [PASSED QA] Bug 5668 - Star ratings in the opac this patch was a rebase from an older patch, marking this obsolete -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA --- Comment #75 from Mason James <mtj@kohaaloha.com> --- (In reply to comment #72)

sorry, there's still a problem: * many files now have a conflict (just pushed bug 7470, that is probably the cause) I prefer not doing something wrong, and ask for a rebased patch, really sorry

(Even if I "feature freeze" today, i'll accept this patch if you provide a rebased version this week)

heya Paul, no problem :) here's the rebased patch, tested and applied to commit eb3dc448d2378e01a524d082fc6389484b0cc7d8 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8537|0 |1 is obsolete| | Attachment #8613|0 |1 is obsolete| | Attachment #8704|0 |1 is obsolete| | --- Comment #77 from Mason James <mtj@kohaaloha.com> --- Created attachment 8705 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8705&action=edit Bug 5668 - Star ratings in the opac patch applied to commit eb3dc448d2378e01a524d082fc6389484b0cc7d8 Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Turned on star ratings in the opac on details and results Searched for titles - saw the stars Clicked on a title Clicked on the stars Clicked on the stars to change my rating Logged out Tried to click on stars Logged in as different user Rated items that were rated already and saw average change Changed preference to show only on detail and repeated tests Changed preference to now show stars All above tests passed. Signing off. Rebased 3-19-12 by Ian Walls -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #79 from Paul Poulain <paul.poulain@biblibre.com> --- patch pushed, string freeze very very soon ;-) -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 --- Comment #81 from Marc Véron <veron@veron.ch> --- I got the following Error in the Web Installer (Step 3) ERROR 1064 (42000) at line 2821: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '- biblionumber int(11) NOT NULL, - rating_value tinyint(1) NOT NULL, - ' at line 2 The sql code near the Error is: -------------------- CREATE TABLE ratings ( borrowernumber int(11) NOT NULL, --- the borrower this rating is for biblionumber int(11) NOT NULL, --- the biblio it's for rating_value tinyint(1) NOT NULL, --- the rating, from 1-5 timestamp timestamp NOT NULL default CURRENT_TIMESTAMP, PRIMARY KEY (borrowernumber,biblionumber), CONSTRAINT ratings_ibfk_1 FOREIGN KEY (borrowernumber) REFERENCES borrowers (borrowernumber) ON DELETE CASCADE ON UPDATE CASCADE, CONSTRAINT ratings_ibfk_2 FOREIGN KEY (biblionumber) REFERENCES biblio (biblionumber) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8; --------------------- The error is due to the comments (--- instead of --) Additional issue: After fixing the error, the scipt complains about an existing table pending_offline_operations There should be at line: DROP TABLE IF EXISTS `pending_offline_operations`; ...at the begin of the section: -- Table structure for table `pending_offline_operations` -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |ASSIGNED -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #83 from Mason James <mtj@kohaaloha.com> --- (In reply to comment #81)

I got the following Error in the Web Installer (Step 3)

ERROR 1064 (42000) at line 2821: You have an error in your SQL syntax; check

The error is due to the comments (--- instead of --)

bah, Robin had spotted and fixed that typo before thanks Marc! -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|critical |blocker --- Comment #85 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Mason, can you attach your signed off patch to the bug please? Thank you! Also updating severity, because currently it breaks installation. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #9163|0 |1 is obsolete| | --- Comment #87 from Mason James <mtj@kohaaloha.com> --- Created attachment 9164 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=9164&action=edit [SIGNED-OFF] Bug 5668 Error during during insall with web installer, Step 3 Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Signed-off-by: Mason James <mtj at kohaaloha.com> Before patch web installer dies with error message on creating database tables: ERROR 1064 (42000) at line 2821: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '- biblionumber int(11) NOT NULL, - rating_value tinyint(1) NOT NULL, - ' at line 2 After patch installation completes normally. I also checked that tables ratings and pending_offline_operations were created correctly. Note: This is Marc Veron's first patch for Koha - congratulations! -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com --- Comment #89 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 7954 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5668 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master Severity|blocker |enhancement --- Comment #91 from Paul Poulain <paul.poulain@biblibre.com> --- follow-up to fix error during installation pushed -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.