Summary: | Babeltheque does not depend on COinSinOPACResults | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | OPAC | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | normal | ||
Priority: | P5 - low | CC: | dcook, gmcharlt, m.de.rooy |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Medium patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Bug Depends on: | 7470 | ||
Bug Blocks: | |||
Attachments: |
Bug 10129: Babeltheque does not depend on COinSinOPACResults
Bug 10129: Babeltheque does not depend on COinSinOPACResults [SIGNED-OFF] Bug 10129: Babeltheque does not depend on COinSinOPACResults [PASSED QA] Bug 10129: Babeltheque does not depend on COinSinOPACResults |
Description
Jonathan Druart
2013-04-26 14:14:17 UTC
Created attachment 17716 [details] [review] 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. 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. (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. Created attachment 17884 [details] [review] 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. 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? (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. Created attachment 17932 [details] [review] [SIGNED-OFF] 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. This influences quite a lot of system preferences - maybe this is better included in 3.14/3.12.1 with a bit more testing? 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. Version back to master. Pushed to master. Thanks, Jonathan! |