Bug 10129 - Babeltheque does not depend on COinSinOPACResults
Summary: Babeltheque does not depend on COinSinOPACResults
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact:
URL:
Keywords:
Depends on: 7470
Blocks:
  Show dependency treegraph
 
Reported: 2013-04-26 14:14 UTC by Jonathan Druart
Modified: 2014-12-07 20:03 UTC (History)
4 users (show)

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


Attachments
Bug 10129: Babeltheque does not depend on COinSinOPACResults (5.22 KB, patch)
2013-04-26 14:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10129: Babeltheque does not depend on COinSinOPACResults (5.14 KB, patch)
2013-05-02 08:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 10129: Babeltheque does not depend on COinSinOPACResults (5.29 KB, patch)
2013-05-03 14:23 UTC, Owen Leonard
Details | Diff | Splinter Review
[PASSED QA] Bug 10129: Babeltheque does not depend on COinSinOPACResults (5.53 KB, patch)
2013-05-12 17:46 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2013-04-26 14:14:17 UTC
The Babeltheque information are displayed only if the COinSinOPACResults syspref is enabled.
Comment 1 Jonathan Druart 2013-04-26 14:17:07 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2013-04-30 15:38:42 UTC
This patch breaks display of existing approved tags and of local star ratings. When I apply this patch I no longer see approved tags in search results, and all star ratings indicators are grey even for titles which have ratings.
Comment 3 Jonathan Druart 2013-05-02 08:58:01 UTC
(In reply to comment #2)
> This patch breaks display of existing approved tags and of local star
> ratings. When I apply this patch I no longer see approved tags in search
> results, and all star ratings indicators are grey even for titles which have
> ratings.

Owen, thanks for testing.
I cannot reproduce but I found some inconsistent things. There are 3 'next' statements which might cause problems.
I will provide a new patch.
Comment 4 Jonathan Druart 2013-05-02 08:58:19 UTC Comment hidden (obsolete)
Comment 5 David Cook 2013-05-02 09:30:25 UTC
Jonathan: 

I'm interested in testing your patch, but I'm not sure how to get Babeltheque information to display (is it simply enough to turn on the system preference? Are there some known items that I could import via z39.50 to use to test this?).

Also, what types of information should be displaying?
Comment 6 Jonathan Druart 2013-05-02 10:35:20 UTC
(In reply to comment #5)
> Jonathan: 
> 
> I'm interested in testing your patch, but I'm not sure how to get
> Babeltheque information to display (is it simply enough to turn on the
> system preference? Are there some known items that I could import via z39.50
> to use to test this?).
> 
> Also, what types of information should be displaying?

David,
Babeltheque is a service for french libraries. It requires a account for testing it, so it is not possible to test this feature. But it is possible to test features related to others syspref.

I apply the patch on a vm where the feature is configured:
http://catalogue.master33-dev.biblibre.com/cgi-bin/koha/opac-search.pl?q=harry

Syspref values are:
COinSinOPACResults = 0
Babeltheque = 1
TagsEnabled = 1
TagsShowOnList = 6
OpacStarRatings = all

Code is based on master + this patch.

I just found a bug on the detail page when Babeltheque is enabled (js error), I will open a new report.
Comment 7 Owen Leonard 2013-05-03 14:23:26 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2013-05-12 16:56:17 UTC
This influences quite a lot of system preferences - maybe this is better included in 3.14/3.12.1 with a bit more testing?
Comment 9 Katrin Fischer 2013-05-12 17:46:48 UTC
Created attachment 18099 [details] [review]
[PASSED QA] Bug 10129: Babeltheque does not depend on COinSinOPACResults

This patch groups all loops on the new_results array into one.
It is useless to loop on the same results array several times.

Test plan:
Quite hard to test all cases.
This patch deals with 5 sysprefs:
COinSinOPACResults, Babeltheque, TagsEnabled, TagsShowOnList and
OpacStarRatings.

Try to enable/disable all of them and verify there is no difference with
and without this patch.
The only different will be: The Babeltheque information should be displayed
even if the COinSinOPACResults syspref is disabled.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

This revised patch works fine for me, thanks.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Verified following system preferences still work as expected:
- COinSinOPACResults on/off
- TagsEnabled, TagsShowOnList, TagsInputOnList on/off
- OpacStarRatings

Couldn't test Babeltheque functionality as this requires an account.
Comment 10 Marcel de Rooy 2013-05-23 08:27:57 UTC
Version back to master.
Comment 11 Galen Charlton 2013-05-23 16:29:37 UTC
Pushed to master.  Thanks, Jonathan!