Bug 26296 - Use new table column selection modal for OPAC suggestion fields
Summary: Use new table column selection modal for OPAC suggestion fields
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
Depends on: 23420 27263
Blocks: 29695
  Show dependency treegraph
 
Reported: 2020-08-25 12:19 UTC by Owen Leonard
Modified: 2022-12-12 21:23 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This changes the selection of values for OPACSuggestionUnwantedFields and OPACSuggestionMandatoryFields from drop down lists to a modal.
Version(s) released in:
22.05.00
Circulation function:


Attachments
Bug 26296: Use new table column selection modal for OPAC suggestion fields (7.54 KB, patch)
2021-01-08 21:03 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 26296: (follow-up) Make sure title remains mandatory (18.00 KB, patch)
2021-11-08 17:43 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 26296: Use new table column selection modal for OPAC suggestion fields (7.56 KB, patch)
2021-12-01 15:22 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 26296: (follow-up) Make sure title remains mandatory (18.00 KB, patch)
2021-12-01 15:22 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 26296: (follow-up) Make sure title remains mandatory (18.05 KB, patch)
2021-12-01 20:44 UTC, David Nind
Details | Diff | Splinter Review
Bug 26296: Use new table column selection modal for OPAC suggestion fields (7.61 KB, patch)
2021-12-01 20:52 UTC, David Nind
Details | Diff | Splinter Review
Bug 26296: (follow-up) Make sure title remains mandatory (18.05 KB, patch)
2021-12-01 20:52 UTC, David Nind
Details | Diff | Splinter Review
Bug 26296: Use new table column selection modal for OPAC suggestion fields (7.69 KB, patch)
2021-12-15 06:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26296: (follow-up) Make sure title remains mandatory (18.13 KB, patch)
2021-12-15 06:10 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 Owen Leonard 2020-08-25 12:19:15 UTC
We can use the same modal-style selection process for choosing columns to be included in OPACSuggestionUnwantedFields and OPACSuggestionMandatoryFields.
Comment 1 Owen Leonard 2021-01-08 21:03:58 UTC Comment hidden (obsolete)
Comment 2 Lucas Gass (lukeg) 2021-01-08 22:55:45 UTC
Owen,

In testing this I found something wrong with OPACSuggestionMandatoryFields. 

I have:

-applied the patch
-updatedatabase (a few times)
-by default title is set as required on /cgi-bin/koha/opac-suggestions.pl
-in OPACSuggestionMandatoryFields I set something othen than title to mandatory.
-title then becomes NOT required
-Since title isn't in the modal for OPACSuggestionMandatoryFields I now have no way of making it required.
Comment 3 Séverine Queune 2021-04-02 10:16:53 UTC
(In reply to Lucas Gass from comment #2)
> Owen,
> 
> In testing this I found something wrong with OPACSuggestionMandatoryFields. 
> 
> I have:
> 
> -applied the patch
> -updatedatabase (a few times)
> -by default title is set as required on /cgi-bin/koha/opac-suggestions.pl
> -in OPACSuggestionMandatoryFields I set something othen than title to
> mandatory.
> -title then becomes NOT required
> -Since title isn't in the modal for OPACSuggestionMandatoryFields I now have
> no way of making it required.

I noticed the same problems Lucas described.
I add that required field are not anymore displayed in red.
Comment 4 Séverine Queune 2021-04-02 10:17:38 UTC
Oups, wrong status...
Comment 5 Owen Leonard 2021-11-08 17:43:04 UTC Comment hidden (obsolete)
Comment 6 David Nind 2021-11-28 17:30:08 UTC
Patch no longer applies 8-(...

Applying: Bug 26296: Use new table column selection modal for OPAC suggestion fields
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref).
error: could not build fake ancestor
Patch failed at 0001 Bug 26296: Use new table column selection modal for OPAC suggestion fields
Comment 7 Owen Leonard 2021-12-01 15:22:08 UTC Comment hidden (obsolete)
Comment 8 Owen Leonard 2021-12-01 15:22:12 UTC Comment hidden (obsolete)
Comment 9 David Nind 2021-12-01 20:44:03 UTC Comment hidden (obsolete)
Comment 10 David Nind 2021-12-01 20:52:06 UTC Comment hidden (obsolete)
Comment 11 David Nind 2021-12-01 20:52:12 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2021-12-13 14:09:57 UTC
We are loosing the ability to translate syspref's values, is that expected?
Comment 13 Katrin Fischer 2021-12-13 23:03:01 UTC
I am not sure if .json is parsed by the translation tools, but maybe introducing HTML tags would be worth trying? 

For the borrowers prefs we use the database column names, so translations weren't needed, but it would be nice if we could make it possible in this case.
Comment 14 Jonathan Druart 2021-12-14 13:29:03 UTC
I am going to work on the translatability problem on a separate bug report.
Comment 15 Jonathan Druart 2021-12-14 16:04:29 UTC
(In reply to Jonathan Druart from comment #14)
> I am going to work on the translatability problem on a separate bug report.

I've opened bug 29695.
Comment 16 Jonathan Druart 2021-12-15 06:10:35 UTC
Created attachment 128538 [details] [review]
Bug 26296: Use new table column selection modal for OPAC suggestion fields

This patch builds on the change introduced by Bug 22844, changing the
selection mode for the OPACSuggestionMandatoryFields and
OPACSuggestionUnwantedFields preferences to a modal.

This patch includes a database update for converting the format of the
saved values in these preferences. Previously the suggestions table
fields would be separated by a comma. Now they should be separated by a
pipe.

To test, apply the patch run the database update.

 - Go to Administration -> System preferences.
 - Search for 'OPACSuggestion.'
 - The input fields for OPACSuggestionUnwantedFields and
   OPACSuggestionMandatoryFields should appear as "locked" (read-only)
   inputs.
 - Clicking either input field should trigger a modal window with
   checkboxes for each available column in the suggestions table.
 - Test that the "select all" and "clear all" links work correctly.
 - Test that the "cancel" link closes the modal without saving your
   selections.
 - Test that the "Save" button closes the modal, copies your
   selections to the form field, and triggers the preference-saving
   function.
 - Test that changes to both preferences are correctly reflected in the
   OPAC suggestion form.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 17 Jonathan Druart 2021-12-15 06:10:40 UTC
Created attachment 128539 [details] [review]
Bug 26296: (follow-up) Make sure title remains mandatory

This patch adds a flag to the OPAC preferences file marking suggestion
title as mandatory OPACSuggestionMandatoryFields and excluded from
OPACSuggestionUnwantedFields.

The patch also modifies the markup around required fields in the OPAC
suggestion form to comply with changes made in Bug 27668 to mandatory
field styling.

To test, apply the patch and restart services.

- Test the OPACSuggestionMandatoryFields preference. In the modal,
  "title" should be checked and the label in red. It should not be
  possible to uncheck the checkbox.
  - Test that the "Select all" and "Clear all" links don't affect the
    "title" checkbox.
  - Confirm that your selections are still saved correctly.
- Test the OPACSuggestionUnwantedFields preference. In the modal,
  "title" should be unchecked and disabled.
  - Test that the "Select all" and "Clear all" links don't affect the
    "title" checkbox.
  - Confirm that your selections are saved correctly.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 18 Jonathan Druart 2021-12-15 06:11:05 UTC
I would suggest to push this one along with bug 29695, to note lose the translatability.
Comment 19 Fridolin Somers 2021-12-15 07:57:37 UTC
Ah atomic update script needs to be updated to match new DBRev system.
Comment 20 Fridolin Somers 2022-01-04 07:44:47 UTC
(In reply to Fridolin Somers from comment #19)
> Ah atomic update script needs to be updated to match new DBRev system.
Comment 21 Katrin Fischer 2022-01-04 08:25:20 UTC
(In reply to Fridolin Somers from comment #20)
> (In reply to Fridolin Somers from comment #19)
> > Ah atomic update script needs to be updated to match new DBRev system.

So far it was not mandatory, maybe we should add to coding guidelines/communicate - we have a lot of old database updates out there. :)
Comment 22 Fridolin Somers 2022-01-05 23:16:11 UTC
(In reply to Katrin Fischer from comment #21)
> (In reply to Fridolin Somers from comment #20)
> > (In reply to Fridolin Somers from comment #19)
> > > Ah atomic update script needs to be updated to match new DBRev system.
> 
> So far it was not mandatory, maybe we should add to coding
> guidelines/communicate - we have a lot of old database updates out there. :)

Ah I see its is described in :
https://wiki.koha-community.org/wiki/Database_updates

I understand it is mandatory.
Comment 23 Katrin Fischer 2022-01-06 13:47:50 UTC
Yes, but we didn't point it out in QA for old patches, only for new ones. I'll add a note to the next QA email.
Comment 24 Jonathan Druart 2022-01-06 14:24:42 UTC
I volunteered at the time to adjust before push, I was RM.
I can help if you want, but it's trivial to switch from the old to the new version.
Comment 25 Fridolin Somers 2022-01-08 01:06:03 UTC
(In reply to Jonathan Druart from comment #24)
> I volunteered at the time to adjust before push, I was RM.
> I can help if you want, but it's trivial to switch from the old to the new
> version.

OK I'll do it also.
Just wanted a confirmation :)
Comment 26 Fridolin Somers 2022-01-10 07:04:49 UTC
Pushed to master for 22.05, thanks to everybody involved ﷐[U+1F984]﷑