Bug 9043 - Adding system preferences to control the visibility of the content on the Advanced Search page
Summary: Adding system preferences to control the visibility of the content on the Adv...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Katrin Fischer
URL:
Keywords:
: 6567 (view as bug list)
Depends on:
Blocks: 12802
  Show dependency treegraph
 
Reported: 2012-11-09 03:20 UTC by Srikanth Dhondi
Modified: 2015-06-04 23:32 UTC (History)
19 users (show)

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


Attachments
Bug 9043 - Changes to the advanced search page (10.85 KB, patch)
2012-11-12 05:01 UTC, Srikanth Dhondi
Details | Diff | Splinter Review
Typos in Advanced Search Options preferences (52.34 KB, image/jpeg)
2012-12-03 20:54 UTC, Marc Véron
Details
Bug 9043 - Changes to the advanced search page (10.68 KB, patch)
2013-02-14 04:22 UTC, Srikanth Dhondi
Details | Diff | Splinter Review
screenshot (37.80 KB, image/tiff)
2013-02-14 16:39 UTC, Melia Meggs
Details
[Signed off] Bug 9043 - Changes to the advanced search page (10.73 KB, patch)
2013-02-14 16:41 UTC, Melia Meggs
Details | Diff | Splinter Review
[ALT 1/2] Bug 9043: Syspref improvement: add new type "multiple" (4.86 KB, patch)
2013-02-19 13:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALT 2/2] Bug 9043: Changes to the advanced search page (6.53 KB, patch)
2013-02-19 13:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9043: Changes to the advanced search page (6.55 KB, patch)
2013-03-01 09:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALT 1/2] Bug 9043: Syspref improvement: add new type "multiple" (4.86 KB, patch)
2013-06-19 10:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALT 2/2] Bug 9043: Changes to the advanced search page (6.55 KB, patch)
2013-06-19 10:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALT 1/3] Bug 9043: Syspref improvement: add new type "multiple" (4.86 KB, patch)
2013-08-13 14:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALT 2/3] Bug 9043: Changes to the advanced search page (6.67 KB, patch)
2013-08-13 14:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALT 3/3] Bug 9043: Add the dropdown check list jquery plugin (48.00 KB, patch)
2013-08-13 14:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALT 2/3] Bug 9043: Changes to the advanced search page (6.80 KB, patch)
2013-08-29 07:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALT 3/3] Bug 9043: Add the dropdown check list jquery plugin (47.08 KB, patch)
2013-08-29 07:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALT 1/3] Bug 9043: Syspref improvement: add new type "multiple" (4.81 KB, patch)
2013-09-30 08:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALT 2/3] Bug 9043: Changes to the advanced search page (6.80 KB, patch)
2013-09-30 08:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALT 3/3] Bug 9043: Add the dropdown check list jquery plugin (47.08 KB, patch)
2013-09-30 08:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALT 1/3] Bug 9043: Syspref improvement: add new type "multiple" (5.32 KB, patch)
2014-04-24 11:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALT 2/3] Bug 9043: Add the dropdown check list jquery plugin (47.08 KB, patch)
2014-04-24 11:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALT 3/3] Bug 9043: Changes to the advanced search page (10.96 KB, patch)
2014-04-24 11:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9043: Syspref improvement: add new type "multiple" (5.37 KB, patch)
2014-06-19 11:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9043: Add the dropdown check list jquery plugin (47.10 KB, patch)
2014-06-19 11:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9043: Changes to the advanced search page (10.87 KB, patch)
2014-06-19 11:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9043: [SIGNED OFF] Syspref improvement: add new type "multiple" (5.43 KB, patch)
2014-07-21 09:01 UTC, Jacek Ablewicz
Details | Diff | Splinter Review
Bug 9043: [SIGNED-OFF] Add the dropdown check list jquery plugin (45.42 KB, patch)
2014-07-21 09:01 UTC, Jacek Ablewicz
Details | Diff | Splinter Review
Bug 9043: [SIGNED-OFF] Changes to the advanced search page (10.93 KB, patch)
2014-07-21 09:01 UTC, Jacek Ablewicz
Details | Diff | Splinter Review
Bug 9043: [SIGNED-OFF] Changes to the advanced search page (10.98 KB, patch)
2014-07-22 07:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9043: Add the dropdown check list jquery plugin in the license list. (1.84 KB, patch)
2014-07-22 07:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 9043: [SIGNED OFF] Syspref improvement: add new type "multiple" (5.50 KB, patch)
2014-07-28 06:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9043: [SIGNED-OFF] Add the dropdown check list jquery plugin (45.48 KB, patch)
2014-07-28 06:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9043: [SIGNED-OFF] Changes to the advanced search page (11.05 KB, patch)
2014-07-28 06:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9043: Add the dropdown check list jquery plugin in the license list. (2.23 KB, patch)
2014-07-28 06:32 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 9043: Syspref improvement: add new type "multiple" (5.42 KB, patch)
2014-08-19 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9043: Changes to the advanced search page (10.97 KB, patch)
2014-08-19 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9043: Add the multiple select jquery plugin (34.44 KB, patch)
2014-08-19 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9043: Make strings translatable (1.45 KB, patch)
2014-08-27 07:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 9043: Syspref improvement: add new type "multiple" (5.48 KB, patch)
2014-09-12 14:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 9043: Changes to the advanced search page (11.15 KB, patch)
2014-09-12 14:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 9043: Add the multiple select jquery plugin (34.49 KB, patch)
2014-09-12 14:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 9043: Make strings translatable (1.51 KB, patch)
2014-09-12 14:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 9043 [QA Followup] - Don't show "More options" are there aren't any (1.74 KB, patch)
2014-09-12 14:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9043: Syspref improvement: add new type "multiple" (5.47 KB, patch)
2014-11-10 15:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9043: Changes to the advanced search page (11.18 KB, patch)
2014-11-10 15:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9043: Add the multiple select jquery plugin (34.71 KB, patch)
2014-11-10 15:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9043: Make strings translatable (1.50 KB, patch)
2014-11-10 15:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9043 [QA Followup] - Don't show "More options" are there aren't any (1.73 KB, patch)
2014-11-10 15:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9043: The comma (,) should be kept to separate multi-valuated prefs (5.57 KB, patch)
2014-11-17 16:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9043: The comma (,) should be kept to separate multi-valuated prefs (5.83 KB, patch)
2014-11-17 17:40 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 9043: Add a wonderful image (5.25 KB, patch)
2014-11-26 11:41 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Srikanth Dhondi 2012-11-09 03:20:35 UTC
There are several options on the advanced search page using which a user can refine/limit their search. Some of that content may be too technical or may not be relavant. By adding additional system preferences the librarians can control what to display on the OPAC advanced search page.
Comment 1 Katrin Fischer 2012-11-10 21:21:07 UTC
Maybe an easier way is to make sure all elements have proper id's and can be hidden using CSS?
Comment 2 Srikanth Dhondi 2012-11-11 22:23:00 UTC
I see following issues with CSS implementation as a solution for this. 

1) The advanced search page has two options: Fewer Options and More Options. Both pages more or less contain the same content, thus hiding elements by giving them unique id's, on Fewer and More Options pages independently, is going to a bit tricky and clunky.

2) Library staff probably don't need to deal with CSS. It would be nice to have an integrated solution for all librarians implemented in Koha. 

I think adding controls to system preferences is the best solution as it gives librarians a chance to customize the advanced search page(s) to suit their patrons needs.
Comment 3 Srikanth Dhondi 2012-11-12 05:01:54 UTC Comment hidden (obsolete)
Comment 4 Marc Véron 2012-12-03 20:54:49 UTC
Created attachment 13858 [details]
Typos in Advanced Search Options preferences

I had to resolve conflicts in sysprefs.sql to appy the patch.

It behaved as expected, I could turn off/on the display of the 6 sections.

However, 2 syspreferenceshave a typo: OpacAdvSeachSorting and OpacAdvSeachSubtype

See screenshot attached.
Comment 5 Srikanth Dhondi 2013-02-14 04:22:35 UTC Comment hidden (obsolete)
Comment 6 Srikanth Dhondi 2013-02-14 04:23:33 UTC
Fixed the typos and done some testing.
Comment 7 Melia Meggs 2013-02-14 16:39:07 UTC
Created attachment 15328 [details]
screenshot

Hello,

I applied this patch and tested.

I see the sys prefs for all six options (with the typos corrected!).  I tested each one by setting it to always, never, and only on More Options page and found everything to be working correctly.  So I am going to add my signoff.  

However, I would like to point out one small cosmetic detail.  Usually when looking at the advanced search page, you see this line at both the top and bottom of the screen: 
[Search] [More/Fewer Options] [New Search]
Now if you set all six of these new sys prefs to only show on More Options page, when you're looking at the Fewer Options page, you see this line repeated twice with nothing in between, which looks a bit funny.  I have added a screenshot so you can see what I mean.  I wonder if it is possible to remove one of these in this case.

Thanks,

Melia
Comment 8 Melia Meggs 2013-02-14 16:41:51 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2013-02-19 13:53:36 UTC
This patch works as expected but it adds a lot of sysprefs for just one thing.

I will suggest 2 alternative patchs, for discussion. The idea is to improve the syspref's engine in order to create a new type which allow to selected several values for one syspref.
Comment 10 Jonathan Druart 2013-02-19 13:56:15 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2013-02-19 13:56:22 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2013-02-28 10:33:29 UTC
Srikanth and Melia,
Do you think my proposal solves the problem or not?
Comment 13 Srikanth Dhondi 2013-03-01 01:49:22 UTC
Hi Jonathan, I like the idea. But there are couple of issues I would like to raise

1) In the current state, the OpacAdvSearch syspref applies the same behavior to Advanced search page with both Fewer and More options. This needs to be rectified. 

2) I tried unselecting/disabling a previously selected/enabled option by clicking on it, this doesn't seem to work. How does one unselect/disable one of the options from the scroll-down menu?
Comment 14 Jonathan Druart 2013-03-01 09:06:20 UTC
(In reply to comment #13)
> Hi Jonathan, I like the idea. But there are couple of issues I would like to
> raise
> 
> 1) In the current state, the OpacAdvSearch syspref applies the same behavior
> to Advanced search page with both Fewer and More options. This needs to be
> rectified. 

Ok, I didn't understand what you want. New patch coming.

> 2) I tried unselecting/disabling a previously selected/enabled option by
> clicking on it, this doesn't seem to work. How does one unselect/disable one
> of the options from the scroll-down menu?

You have to press the ctrl key and click on the entry.
Comment 15 Jonathan Druart 2013-03-01 09:06:45 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2013-06-19 10:51:00 UTC
Need feedback on my last patch, switch to needs signoff.
Comment 17 Jonathan Druart 2013-06-19 10:55:17 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2013-06-19 10:55:21 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2013-06-19 10:58:34 UTC
Note: The 3 patches are not compatible.

Either you apply:
  [Signed off] Bug 9043 - Changes to the advanced search page

Either both:
  [ALT 1/2] Bug 9043: Syspref improvement: add new type "multiple"
  [ALT 2/2] Bug 9043: Changes to the advanced search page
Comment 20 Kyle M Hall 2013-08-02 17:13:30 UTC
Jonathan, your patches work quite well, but there is no way to disable all the options at once. When I deslect them all, and hit the save button, I get the "Nothing to save" message.

Not required, but it might be nice to implement something like https://code.google.com/p/dropdown-check-list/ as a fair number of people may not know about ctrl-click to select/deselect individual items.
Comment 21 Jonathan Druart 2013-08-13 14:59:11 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2013-08-13 14:59:18 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2013-08-13 14:59:26 UTC Comment hidden (obsolete)
Comment 24 Jonathan Druart 2013-08-13 15:01:58 UTC
(In reply to Kyle M Hall from comment #20)
> Jonathan, your patches work quite well, but there is no way to disable all
> the options at once. When I deslect them all, and hit the save button, I get
> the "Nothing to save" message.
> 
> Not required, but it might be nice to implement something like
> https://code.google.com/p/dropdown-check-list/ as a fair number of people
> may not know about ctrl-click to select/deselect individual items.

Thank for the idea, done!
I cannot reproduce your issue, but should be fixed by the last patch.
Comment 25 Campbell Reid-Tait 2013-08-29 02:15:50 UTC
 [ALT 2/3] Bug 9043: Changes to the advanced search page (6.67 KB, patch)
2013-08-13 14:59 UTC, Jonathan Druart 

Failed to apply.

Applying: Bug 9043: Changes to the advanced search page
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-advsearch.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
Auto-merging installer/data/mysql/sysprefs.sql
CONFLICT (content): Merge conflict in installer/data/mysql/sysprefs.sql
Failed to merge in the changes.
Comment 26 Jonathan Druart 2013-08-29 07:49:54 UTC Comment hidden (obsolete)
Comment 27 Jonathan Druart 2013-08-29 07:50:07 UTC Comment hidden (obsolete)
Comment 28 Julian Maurice 2013-09-24 14:04:56 UTC
Found a bug:

When you deselect all options from a 'multiple' syspref. The '(modified)' text is displayed but clicking on "Save" button doesn't save the syspref value and message "Nothing to save" appears.
Comment 29 Julian Maurice 2013-09-24 14:15:22 UTC
Forget that, I cleared my browser cache and it works!
Comment 30 I'm just a bot 2013-09-29 06:13:29 UTC
Applying: Bug 9043 - Changes to the advanced search page
Using index info to reconstruct a base tree...
M	installer/data/mysql/sysprefs.sql
M	installer/data/mysql/updatedatabase.pl
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
M	koha-tmpl/opac-tmpl/prog/en/modules/opac-advsearch.tt
M	opac/opac-search.pl
Falling back to patching base and 3-way merge...
Auto-merging opac/opac-search.pl
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-advsearch.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
Auto-merging installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/sysprefs.sql
CONFLICT (content): Merge conflict in installer/data/mysql/sysprefs.sql
Patch failed at 0001 Bug 9043 - Changes to the advanced search page
The copy of the patch that failed is found in:
   /home/christopher/git/koha/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Comment 31 Jonathan Druart 2013-09-30 08:13:21 UTC Comment hidden (obsolete)
Comment 32 Jonathan Druart 2013-09-30 08:13:29 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2013-09-30 08:13:35 UTC Comment hidden (obsolete)
Comment 34 I'm just a bot 2013-10-28 06:13:33 UTC
Applying: Bug 9043 - Changes to the advanced search page
Using index info to reconstruct a base tree...
M	installer/data/mysql/sysprefs.sql
M	installer/data/mysql/updatedatabase.pl
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
M	koha-tmpl/opac-tmpl/prog/en/modules/opac-advsearch.tt
M	opac/opac-search.pl
Falling back to patching base and 3-way merge...
Auto-merging opac/opac-search.pl
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-advsearch.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
Auto-merging installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/sysprefs.sql
CONFLICT (content): Merge conflict in installer/data/mysql/sysprefs.sql
Patch failed at 0001 Bug 9043 - Changes to the advanced search page
The copy of the patch that failed is found in:
   /home/christopher/git/koha/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Comment 35 Jonathan Druart 2013-10-28 09:04:50 UTC
Could it be possible to get some feedback on my proposed patches? The idea was not to block the enh but to improve it!
Comment 36 jmbroust 2014-03-13 08:28:18 UTC
<h1>Something went wrong !</h1>Applying: Bug 9043 - Changes to the advanced
search page
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging installer/data/mysql/sysprefs.sql
CONFLICT (content): Merge conflict in installer/data/mysql/sysprefs.sql
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
CONFLICT (content): Merge conflict in
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-advsearch.tt
Auto-merging opac/opac-search.pl
Failed to merge in the changes.
Patch failed at 0001 Bug 9043 - Changes to the advanced search page
When you have resolved this problem run git bz apply --continue.
If you would prefer to skip this patch, instead run git bz apply --skip.
To restore the original branch and stop patching run git bz apply --abort.
Bug 9043 - Adding system preferences to control the visibility of the content
on the Advanced Search page

15329 - [Signed off] Bug 9043 - Changes to the advanced search page
21643 - [ALT 1/3] Bug 9043: Syspref improvement: add new type multiple
21644 - [ALT 2/3] Bug 9043: Changes to the advanced search page
21645 - [ALT 3/3] Bug 9043: Add the dropdown check list jquery plugin
Comment 37 Jonathan Druart 2014-03-13 09:55:27 UTC
You cannot test this in a sandbox, the first patch should not be applied.
Comment 38 David Cook 2014-04-24 03:01:58 UTC
(In reply to Jonathan Druart from comment #35)
> Could it be possible to get some feedback on my proposed patches? The idea
> was not to block the enh but to improve it!

_One_

It appears that one of your patches includes the smart match operator, which is something we're avoiding due to its deprecated/experimental status in Perl 5.18, I believe.

_Two_

Do you have a test plan to accompany your patches, Jonathan? It seems that the concept behind your patch and the original are at odds in that the original allows  finer control of when the options appear. In yours, it appears that the options always show when viewing "more options" and only show when viewing "fewer options" if that option is included in the system preference.

I think the idea was to choose between 3 scenarios:

1) Never show the option in advanced search
2) Show it when viewing "more" and "fewer"
3) Show it only when viewing "more"

I think your patch only covers scenarios #2 and #3. Is that correct?

I'm not a huge fan of governing this behaviour using system preferences, but I think I prefer the 3 choices rather than 2.

_Three_

I think Owen has been making an effort to move the Javascript libraries out of the /en/ directory and put it higher up so that it isn't needlessly duplicated in the translations, so it would probably make sense to move the JS higher up the directory tree.

_Four_

I think there might be another bug out there that seeks to add multiple options for system preferences...but I might be mistaken.

_Five_

It looks like this patch will need to rebase due to the updatedatabase.pl being too old now.

---

I hope that helps!
Comment 39 Jonathan Druart 2014-04-24 11:05:12 UTC
(In reply to David Cook from comment #38)

Thanks David to revive this bug!

> It appears that one of your patches includes the smart match operator, which
> is something we're avoiding due to its deprecated/experimental status in
> Perl 5.18, I believe.

Will be fixed.

> Do you have a test plan to accompany your patches, Jonathan?

Will be added.

> In yours, it
> appears that the options always show when viewing "more options" and only
> show when viewing "fewer options" if that option is included in the system
> preference.

> I think your patch only covers scenarios #2 and #3. Is that correct?

Yes, good catch. I will add a new pref for that.

> I'm not a huge fan of governing this behaviour using system preferences, but
> I think I prefer the 3 choices rather than 2.

I don't see another way to do that.

> I think Owen has been making an effort to move the Javascript libraries out
> of the /en/ directory and put it higher up so that it isn't needlessly
> duplicated in the translations, so it would probably make sense to move the
> JS higher up the directory tree.

At the OPAC, not at the staff interface.

> I think there might be another bug out there that seeks to add multiple
> options for system preferences...but I might be mistaken.

Not sure. If someone agree with my patches, I think the first patch should be abandoned. Otherwise, We are going to create 6 prefs and remove them just after.

> It looks like this patch will need to rebase due to the updatedatabase.pl
> being too old now.

Will be rebased.

> I hope that helps!

Of course!
Comment 40 Jonathan Druart 2014-04-24 11:07:16 UTC Comment hidden (obsolete)
Comment 41 Jonathan Druart 2014-04-24 11:07:27 UTC Comment hidden (obsolete)
Comment 42 Jonathan Druart 2014-04-24 11:07:35 UTC Comment hidden (obsolete)
Comment 43 Koha Team University Lyon 3 2014-06-19 09:58:52 UTC
This patch works well :  adds 2 new sysprefs: OpacAdvSearchOptions and
OpacAdvSearchMoreOptions.

There is just one thing to remember when configuring the advance search options : 
When you fill OpacAdvSearchOptions with Item types and Languages (for example)you should fill also OpacAdvSearchMoreOptions with the same options : Item types, Languages, otherwise, when you click on more options, you get fiewer options (the options filled in OpacAdvSearchOptions are not showed)which is quite surprising, it's a logical mistake in configuring the advance search options : more options must be filled with at least the options filled in the default options.
Comment 44 Jonathan Druart 2014-06-19 11:01:34 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2014-06-19 11:01:43 UTC Comment hidden (obsolete)
Comment 46 Jonathan Druart 2014-06-19 11:01:49 UTC Comment hidden (obsolete)
Comment 47 Jonathan Druart 2014-07-18 09:12:11 UTC
*** Bug 6567 has been marked as a duplicate of this bug. ***
Comment 48 Jacek Ablewicz 2014-07-21 09:01:33 UTC Comment hidden (obsolete)
Comment 49 Jacek Ablewicz 2014-07-21 09:01:43 UTC Comment hidden (obsolete)
Comment 50 Jacek Ablewicz 2014-07-21 09:01:53 UTC Comment hidden (obsolete)
Comment 51 Katrin Fischer 2014-07-21 22:07:27 UTC
Please add a follow-up to include the new jQuery plugin on the About page. 
Continuing testing...
Comment 52 Katrin Fischer 2014-07-21 22:13:00 UTC
I think maybe rename location to 'location and availability' like on the advanced search form and to avoid confusion with the locations tab that you can add with AdvancedSearchTypes.
Comment 53 Jonathan Druart 2014-07-22 07:38:58 UTC Comment hidden (obsolete)
Comment 54 Jonathan Druart 2014-07-22 07:39:04 UTC Comment hidden (obsolete)
Comment 55 Jonathan Druart 2014-07-22 07:39:56 UTC
(In reply to Katrin Fischer from comment #51)
> Please add a follow-up to include the new jQuery plugin on the About page. 

Done.

(In reply to Katrin Fischer from comment #52)
> I think maybe rename location to 'location and availability' like on the
> advanced search form and to avoid confusion with the locations tab that you
> can add with AdvancedSearchTypes.

Done too, I amended the last patch and just changed the string in opac.pref.
Comment 56 Katrin Fischer 2014-07-28 06:26:39 UTC
There are 2 small things:

- we might want to rename "Itemtypes" as it could also be locations or collections for a library. But I am not sure what would be a better name :(
- the font size and script of the new plugin created pull down seems a bit off and some styling to make it fit better into the page would be nice.

Overall this works as it should and I think those 2 small things could be resolved in a follow-up.
Comment 57 Katrin Fischer 2014-07-28 06:31:05 UTC Comment hidden (obsolete)
Comment 58 Katrin Fischer 2014-07-28 06:31:20 UTC Comment hidden (obsolete)
Comment 59 Katrin Fischer 2014-07-28 06:31:34 UTC Comment hidden (obsolete)
Comment 60 Katrin Fischer 2014-07-28 06:32:09 UTC Comment hidden (obsolete)
Comment 61 Owen Leonard 2014-07-28 15:59:01 UTC
I'm concerned about adding a jQuery plugin dependency that comes with this disclaimer:

"DDCL IS NO LONGER ACTIVELY SUPPORTED

It has not been rebuilt in over 2 years and is stuck on jQuery release 1.7. However, if anybody feels they can devote time to maintaining and developing DDCL, then feel free to get in contact."

We are still in the process of updating or replacing existing plugins which block an upgrade of jQuery in Koha. I don't think it's a good idea to add another one.

Are there any alternatives we could consider?
Comment 62 Owen Leonard 2014-07-28 18:10:05 UTC
I haven't tested it but this looks interesting:

http://wenzhixin.net.cn/p/multiple-select/docs/
Comment 63 Katrin Fischer 2014-07-28 19:30:32 UTC
Hi Owen, thx for taking a look and finding this. I hope we find a solution that won't block the update.
Comment 64 Jonathan Druart 2014-08-19 09:05:36 UTC Comment hidden (obsolete)
Comment 65 Jonathan Druart 2014-08-19 09:05:45 UTC Comment hidden (obsolete)
Comment 66 Jonathan Druart 2014-08-19 09:05:55 UTC Comment hidden (obsolete)
Comment 67 Jonathan Druart 2014-08-19 09:06:36 UTC
(In reply to Owen Leonard from comment #62)
> I haven't tested it but this looks interesting:
> 
> http://wenzhixin.net.cn/p/multiple-select/docs/

The last patch set uses this multiple select plugin.
Comment 68 Katrin Fischer 2014-08-21 09:11:11 UTC
This works as described, but introduces a couple of untranslatable strings - Select all, All selected, x of y selected etc. 

Marking failed for now, hoping there is a solution.
Comment 69 Jonathan Druart 2014-08-21 09:54:33 UTC
(In reply to Katrin Fischer from comment #68)
> This works as described, but introduces a couple of untranslatable strings -
> Select all, All selected, x of y selected etc. 
> 
> Marking failed for now, hoping there is a solution.

The solution would be to modify the multiple-select plugin.
Since the strings are only displayed when editing 2 sysprefs, I am not sure it is worth forking it.
Comment 70 Owen Leonard 2014-08-26 16:21:19 UTC
(In reply to Jonathan Druart from comment #69)
> The solution would be to modify the multiple-select plugin.

This worked in my test:

        $("select[multiple='multiple']").multipleSelect({
            placeholder: _("Please select..."),
            selectAllText: _("Select all"),
            allSelected: _("All selected"),
            countSelected: _("# of % selected"),
            noMatchesFound: _("No matches found")
        });

...and the translation script seemed to pick it up.
Comment 71 Jonathan Druart 2014-08-27 07:36:45 UTC Comment hidden (obsolete)
Comment 72 Kyle M Hall 2014-09-12 14:39:16 UTC Comment hidden (obsolete)
Comment 73 Kyle M Hall 2014-09-12 14:39:39 UTC Comment hidden (obsolete)
Comment 74 Kyle M Hall 2014-09-12 14:39:44 UTC Comment hidden (obsolete)
Comment 75 Kyle M Hall 2014-09-12 14:39:48 UTC Comment hidden (obsolete)
Comment 76 Kyle M Hall 2014-09-12 14:39:52 UTC Comment hidden (obsolete)
Comment 77 Kyle M Hall 2014-09-12 14:43:57 UTC
Everything looks good. Passes koha-qa, tested and functions well. Had to rebase due to updates to the advanced search template. Added small qa followup to keep this train rolling.

Great work Jonathan!
Comment 78 Jonathan Druart 2014-11-10 15:23:27 UTC
Created attachment 33428 [details] [review]
Bug 9043: Syspref improvement: add new type "multiple"

This patch adds a new type "multiple" for syspref.
This new type allows to select several values for one syspref.

Signed-off-by: Koha Team Lyon 3 <koha@univ-lyon3.fr>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 79 Jonathan Druart 2014-11-10 15:23:32 UTC
Created attachment 33429 [details] [review]
Bug 9043: Changes to the advanced search page

This patch adds 2 new sysprefs: OpacAdvSearchOptions and
OpacAdvSearchMoreOptions.

These sysprefs are multivalued and you can add or remove some elements on
the adv search page.

This patch allows to display/hide some entries in the advanced search
page at the OPAC.
On this way, it is possible to configure the search options to display.

Test plan:
1/
Fill OpacAdvSearchOptions with Item types and Languages.
Fill OpacAdvSearchMoreOptions with Item types, Languages and Location.
On the advanced search page, verify the Location is only displayed for
the "More options" view.
2/ Try different settings for these prefs
3/ Remove all entries for the OpacAdvSearchMoreOptions and verify that
the "More options" does not displayed additional options.
4/ Remove all entries for the OpacAdvSearchOptions and verify that the
"normal view" does not displayed any options.

Note that this patch only affects the bootstrap theme.

Signed-off-by: Koha Team Lyon 3 <koha@univ-lyon3.fr>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 80 Jonathan Druart 2014-11-10 15:23:38 UTC
Created attachment 33430 [details] [review]
Bug 9043: Add the multiple select jquery plugin

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 81 Jonathan Druart 2014-11-10 15:23:43 UTC
Created attachment 33431 [details] [review]
Bug 9043: Make strings translatable

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 82 Jonathan Druart 2014-11-10 15:23:48 UTC
Created attachment 33432 [details] [review]
Bug 9043 [QA Followup] - Don't show "More options" are there aren't any

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 83 Tomás Cohen Arazi 2014-11-10 15:56:44 UTC
Patches pushed to master.

Thanks Jonathan! Nice addition!
Comment 84 Jonathan Druart 2014-11-17 16:00:39 UTC Comment hidden (obsolete)
Comment 85 Owen Leonard 2014-11-17 17:40:58 UTC
Created attachment 33620 [details] [review]
[SIGNED-OFF] Bug 9043: The comma (,) should be kept to separate multi-valuated prefs

The prefs language and opaclanguages used the comma to separate the
different values.

The new prefs OpacAdvSearchMoreOptions and OpacAdvSearchOptions should
do the same.

To reproduce the issue: update the language pref (or opaclanguages) and
refresh the page.
=> The pref values are not checked anymore and the language selection
(bottom of the page) does not appear.

Test plan:
1/ Verify that the behavior described above is fixed.
2/ Verify that the original test plan of bug 9043 still passes.

Note: The 2 OpacAdvSearchMoreOptions and OpacAdvSearchOptions pref
values are overwritten but the feature have just been pushed recently.
It should not affect a production environment.

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

I was able to reproduce the issue and verify that this patch corrected
the problem. Langage selection and OpacAdvSearchOptions worked
correctly.
Comment 86 Tomás Cohen Arazi 2014-11-17 17:55:43 UTC
Followup pushed to master.

Thanks Jonathan and Owen for the fast response!
Comment 87 Jonathan Druart 2014-11-26 11:41:27 UTC
Created attachment 33947 [details] [review]
Bug 9043: Add a wonderful image

This patch adds the missing multiple-select.png image.

Test plan:
- search the pref 'advsearch'
- without this patch, you get a 404 error for the image
- with this patch, you don't get the error but a wonderful image on the
  right of the "multiple" pref
Comment 88 Tomás Cohen Arazi 2014-11-26 14:38:15 UTC
Minor followup pushed to master.

Thanks Jonathan!