Bug 11375

Summary: Improve patrons permissions display
Product: Koha Reporter: paxed <pasi.kallinen>
Component: I18N/L10NAssignee: Owen Leonard <oleonard>
Status: RESOLVED FIXED QA Contact: Josef Moravec <josef.moravec>
Severity: enhancement    
Priority: P5 - low CC: bibliwho, frederic, josef.moravec, katrin.fischer, martin.renvoize, maryse.simard, nick, sandboxes, severine.queune, veron
Version: master   
Target Milestone: ---   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20813
Whiteboard:
Change sponsored?: --- Patch complexity: Small patch
Bot Control: --- When did the bot last check this:
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
Bug Depends on: 21751, 21964    
Bug Blocks: 22553    
Attachments: Idea for a patron permissions interface redesign
Bug 11375: Improve patrons permissions display
Bug 11375: Improve patrons permissions display
Bug 11375: Improve patrons permissions display
Bug 11375: Improve patrons permissions display
Bug 11375: Improve patrons permissions display
Bug 11375: Improve patrons permissions display
Bug 11375: (follow-up) Add links to show all/hide all
Bug 11375: (follow-up) Add links to show all/hide all
Bug 11375: Improve patrons permissions display
Bug 11375: (follow-up) Add links to show all/hide all
Bug 11375: Improve patrons permissions display
Bug 11375: (follow-up) Add links to show all/hide all
Bug 11375: Improve patrons permissions display
Bug 11375: (follow-up) Add links to show all/hide all
Bug 11375: Improve patrons permissions display
Bug 11375: (follow-up) Add links to show all/hide all
Bug 11375: (QA follow-up) Fix typo
Bug 11375: (QA follow-up) Remove some more treeview occurences
Bug 11375: Improve patrons permissions display
Bug 11375: (follow-up) Add links to show all/hide all
Bug 11375: (QA follow-up) Fix typo
Bug 11375: (QA follow-up) Remove some more treeview occurences

Description paxed 2013-12-11 12:33:39 UTC
Patron permimssions (eg. "superlibrarian", "reserveforothers", etc) and their explanations are not translatable. These strings come from installer/data/mysql/en/mandatory/subtag_registry.sql and installer/data/mysql/en/mandatory/userpermissions.sql
Comment 1 Katrin Fischer 2013-12-11 12:36:15 UTC
The way to translate those is creating a Finnish installer currently. I have translated those and other files into German as have others (French, Spanish, etc.)
Comment 2 paxed 2013-12-11 12:47:57 UTC
(In reply to Katrin Fischer from comment #1)
> The way to translate those is creating a Finnish installer currently. I have
> translated those and other files into German as have others (French,
> Spanish, etc.)

Ah, OK.

I don't think it's possible to translate the permission names themselves though? (That is, "superlibrarian")
Comment 3 Katrin Fischer 2013-12-11 13:45:29 UTC
Those are codes and it's not possible to translate them as they are used in the code. But you can have translated descriptions.
Comment 4 Katrin Fischer 2014-05-17 11:06:08 UTC
I think we could discuss not showing the codes, although I find them useful sometimes. I look at them like at the codes for notices. paxed, would you be ok with WONTFIX?
Comment 5 Owen Leonard 2016-05-26 18:29:42 UTC
Created attachment 51847 [details]
Idea for a patron permissions interface redesign

I wonder if we should deemphasize the permission code? As a developer I look for the code because I usually know what I'm looking for. Perhaps it would be friendlier to put the focus on the translatable text?

(also I want to get rid of jstree)
Comment 6 Katrin Fischer 2017-05-01 12:23:52 UTC
I like it! Only thing maybe is that I like the top level permission clickable, as that seems a bit faster than the 'show details' link when you have to enter a lot of accounts as we usually do.
Comment 7 Katrin Fischer 2018-05-16 23:55:24 UTC
Hi Owen, are you still working on this?
Comment 8 Owen Leonard 2018-05-24 15:05:47 UTC
(In reply to Katrin Fischer from comment #7)
> Hi Owen, are you still working on this?

Whoops, I missed this question when you first asked. I still have an up-to-date branch with this. I'm thinking about trying to get it in with Bug 20813 (unless that bug proves to be controversial?)
Comment 9 Owen Leonard 2018-06-11 17:25:25 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2018-07-31 14:50:35 UTC
Hi Owen, 

I've rebased the patch to add the new permissions, hope I got everything right.

Some things I notice:

1) Currently, when you click on a sub-permission, the top-level permission checkbox gets unchecked. With the new patches this no longer happens, which I think could lead to errors. Could we restore this behaviour?

2) Ok... searched up and down, otherwise this appears to work great!
Comment 11 Katrin Fischer 2018-07-31 14:51:19 UTC Comment hidden (obsolete)
Comment 12 sandboxes@biblibre.com 2018-08-05 13:44:14 UTC
Patch tested with a sandbox, by Cab Vinton <bibliwho@gmail.com>
Comment 13 sandboxes@biblibre.com 2018-08-05 13:44:44 UTC Comment hidden (obsolete)
Comment 14 Cab Vinton 2018-08-05 13:50:07 UTC
All works as expected. A couple minor suggestions to consider:

1./ Add Save/ Cancel buttons to the top of the screen (to avoid having to scroll to the bottom of the screen).

2./ Add Show/ Hide all details buttons (for easier navigation)

Very nice work!
Comment 15 Owen Leonard 2018-12-06 18:42:15 UTC Comment hidden (obsolete)
Comment 16 Claire Gravely 2018-12-17 15:02:14 UTC
Created attachment 83296 [details] [review]
Bug 11375: Improve patrons permissions display

This patch updates the patron permissions interface in order to
emphasize permission descriptions (which can be translated) over
permission code (which cannot).

The tree structure (and jQuery plugin) is removed, and permissions are
displayed in a table-like way. Sub-permissions are shown or hidden with
a link.

A free-text filter is added to the top for narrowing the list by
keyword.

To test, apply the patch and clear your browser cache if necessary.

 - Open the "Set permissions" page for any patron.
 - Confirm that the filter works correctly to show only lines which
   match your entry
   - Note that the superlibrarian line stays visible all the time. This
     is to make it clearer that sub-permissions cannot be selected if
     superlibrarian is checked.
 - Check the superlibrarain "hint" text for clarity.
 - Test the show/hide controls.
 - Test that checking a "parent" permission displays the
   sub-permissions.
 - Test that changing and saving permissions works correctly.

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 17 Owen Leonard 2019-01-17 14:56:42 UTC
Created attachment 84140 [details] [review]
Bug 11375: Improve patrons permissions display

This patch updates the patron permissions interface in order to
emphasize permission descriptions (which can be translated) over
permission code (which cannot).

The tree structure (and jQuery plugin) is removed, and permissions are
displayed in a table-like way. Sub-permissions are shown or hidden with
a link.

A free-text filter is added to the top for narrowing the list by
keyword.

Save and cancel buttons are now in a floating toolbar.

To test, apply the patch, regenerate the staff client CSS, and clear
your browser cache if necessary.

 - Open the "Set permissions" page for any patron.
 - Confirm that the filter works correctly to show only lines which
   match your entry
   - Note that the superlibrarian line stays visible all the time. This
     is to make it clearer that sub-permissions cannot be selected if
     superlibrarian is checked.
 - Check the superlibrarain "hint" text for clarity.
 - Test the show/hide controls.
 - Test that checking a "parent" permission displays the
   sub-permissions.
 - Test that the toolbar with Save and Cancel floats when scrolling.
 - Test that changing and saving permissions works correctly.
Comment 18 Owen Leonard 2019-01-17 15:41:24 UTC
Created attachment 84143 [details] [review]
Bug 11375: (follow-up) Add links to show all/hide all

This patch adds links in the toolbar to show or hide all
sub-permissions.

To test, apply the patch and view the permissions page for a patron.
Clicking the "Show all" or "Hide all" links should work to show or hide
all sub-permissions. The corresponding "Show details" and "Hide details"
links for each permission set should be correctly toggled.
Comment 19 Katrin Fischer 2019-02-02 13:55:20 UTC
Passed QA on depenency, unlocking this one.
Comment 20 Katrin Fischer 2019-02-02 13:56:31 UTC
Tried to apply on top of patches for 21751, but got a conflict:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 11375: Improve patrons permissions display
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss).
error: could not build fake ancestor
Patch failed at 0001 Bug 11375: Improve patrons permissions display
The copy of the patch that failed is found in: .git/rebase-apply/patch
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".
Patch left in /tmp/Bug-11375-Improve-patrons-permissions-display-jTsU_R.patch
Comment 21 Owen Leonard 2019-02-04 13:17:47 UTC
Created attachment 84700 [details] [review]
Bug 11375: (follow-up) Add links to show all/hide all

This patch adds links in the toolbar to show or hide all
sub-permissions.

To test, apply the patch and view the permissions page for a patron.
Clicking the "Show all" or "Hide all" links should work to show or hide
all sub-permissions. The corresponding "Show details" and "Hide details"
links for each permission set should be correctly toggled.
Comment 22 Owen Leonard 2019-02-04 13:19:02 UTC
Created attachment 84701 [details] [review]
Bug 11375: Improve patrons permissions display

This patch updates the patron permissions interface in order to
emphasize permission descriptions (which can be translated) over
permission code (which cannot).

The tree structure (and jQuery plugin) is removed, and permissions are
displayed in a table-like way. Sub-permissions are shown or hidden with
a link.

A free-text filter is added to the top for narrowing the list by
keyword.

Save and cancel buttons are now in a floating toolbar.

To test, apply the patch, regenerate the staff client CSS, and clear
your browser cache if necessary.

 - Open the "Set permissions" page for any patron.
 - Confirm that the filter works correctly to show only lines which
   match your entry
   - Note that the superlibrarian line stays visible all the time. This
     is to make it clearer that sub-permissions cannot be selected if
     superlibrarian is checked.
 - Check the superlibrarain "hint" text for clarity.
 - Test the show/hide controls.
 - Test that checking a "parent" permission displays the
   sub-permissions.
 - Test that the toolbar with Save and Cancel floats when scrolling.
 - Test that changing and saving permissions works correctly.
Comment 23 Owen Leonard 2019-02-04 13:19:06 UTC
Created attachment 84702 [details] [review]
Bug 11375: (follow-up) Add links to show all/hide all

This patch adds links in the toolbar to show or hide all
sub-permissions.

To test, apply the patch and view the permissions page for a patron.
Clicking the "Show all" or "Hide all" links should work to show or hide
all sub-permissions. The corresponding "Show details" and "Hide details"
links for each permission set should be correctly toggled.
Comment 24 Maryse Simard 2019-02-15 21:09:07 UTC
Patch doesn't apply

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 11375: Improve patrons permissions display
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss
M	koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/jquery.treeview.css
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/treeview-red.gif
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/treeview-red-line.gif
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/treeview-gray.gif
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/treeview-gray-line.gif
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/treeview-famfamfam.gif
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/treeview-famfamfam-line.gif
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/treeview-default.gif
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/treeview-default-line.gif
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/treeview-black.gif
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/treeview-black-line.gif
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/plus.gif
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/minus.gif
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/folder.gif
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/folder-closed.gif
Removing koha-tmpl/intranet-tmpl/prog/css/treeview/images/file.gif
Auto-merging koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss
Removing koha-tmpl/intranet-tmpl/lib/jquery/plugins/jquery.treeview.pack.js
error: Failed to merge in the changes.
Patch failed at 0001 Bug 11375: Improve patrons permissions display
The copy of the patch that failed is found in: .git/rebase-apply/patch
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".
Patch left in /tmp/Bug-11375-Improve-patrons-permissions-display-P1z7Qv.patch
Comment 25 Owen Leonard 2019-02-20 13:21:19 UTC
Created attachment 85351 [details] [review]
Bug 11375: Improve patrons permissions display

This patch updates the patron permissions interface in order to
emphasize permission descriptions (which can be translated) over
permission code (which cannot).

The tree structure (and jQuery plugin) is removed, and permissions are
displayed in a table-like way. Sub-permissions are shown or hidden with
a link.

A free-text filter is added to the top for narrowing the list by
keyword.

Save and cancel buttons are now in a floating toolbar.

To test, apply the patch, regenerate the staff client CSS, and clear
your browser cache if necessary.

 - Open the "Set permissions" page for any patron.
 - Confirm that the filter works correctly to show only lines which
   match your entry
   - Note that the superlibrarian line stays visible all the time. This
     is to make it clearer that sub-permissions cannot be selected if
     superlibrarian is checked.
 - Check the superlibrarain "hint" text for clarity.
 - Test the show/hide controls.
 - Test that checking a "parent" permission displays the
   sub-permissions.
 - Test that the toolbar with Save and Cancel floats when scrolling.
 - Test that changing and saving permissions works correctly.
Comment 26 Owen Leonard 2019-02-20 13:21:24 UTC
Created attachment 85352 [details] [review]
Bug 11375: (follow-up) Add links to show all/hide all

This patch adds links in the toolbar to show or hide all
sub-permissions.

To test, apply the patch and view the permissions page for a patron.
Clicking the "Show all" or "Hide all" links should work to show or hide
all sub-permissions. The corresponding "Show details" and "Hide details"
links for each permission set should be correctly toggled.
Comment 27 Maryse Simard 2019-02-22 20:26:05 UTC
Created attachment 85576 [details] [review]
Bug 11375: Improve patrons permissions display

This patch updates the patron permissions interface in order to
emphasize permission descriptions (which can be translated) over
permission code (which cannot).

The tree structure (and jQuery plugin) is removed, and permissions are
displayed in a table-like way. Sub-permissions are shown or hidden with
a link.

A free-text filter is added to the top for narrowing the list by
keyword.

Save and cancel buttons are now in a floating toolbar.

To test, apply the patch, regenerate the staff client CSS, and clear
your browser cache if necessary.

 - Open the "Set permissions" page for any patron.
 - Confirm that the filter works correctly to show only lines which
   match your entry
   - Note that the superlibrarian line stays visible all the time. This
     is to make it clearer that sub-permissions cannot be selected if
     superlibrarian is checked.
 - Check the superlibrarain "hint" text for clarity.
 - Test the show/hide controls.
 - Test that checking a "parent" permission displays the
   sub-permissions.
 - Test that the toolbar with Save and Cancel floats when scrolling.
 - Test that changing and saving permissions works correctly.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Comment 28 Maryse Simard 2019-02-22 20:26:43 UTC
Created attachment 85577 [details] [review]
Bug 11375: (follow-up) Add links to show all/hide all

This patch adds links in the toolbar to show or hide all
sub-permissions.

To test, apply the patch and view the permissions page for a patron.
Clicking the "Show all" or "Hide all" links should work to show or hide
all sub-permissions. The corresponding "Show details" and "Hide details"
links for each permission set should be correctly toggled.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Comment 29 Maryse Simard 2019-02-22 20:28:05 UTC
It works well and looks good!

But there's one thing that I feel is missing. When a permission is checked and you uncheck one or more of it's subpermissions, the parent doesn't uncheck. That is current behavior and can be useful if you only want to have one subpermissions unchecked among a dozen.
Comment 30 Josef Moravec 2019-02-25 14:52:07 UTC
Created attachment 85622 [details] [review]
Bug 11375: Improve patrons permissions display

This patch updates the patron permissions interface in order to
emphasize permission descriptions (which can be translated) over
permission code (which cannot).

The tree structure (and jQuery plugin) is removed, and permissions are
displayed in a table-like way. Sub-permissions are shown or hidden with
a link.

A free-text filter is added to the top for narrowing the list by
keyword.

Save and cancel buttons are now in a floating toolbar.

To test, apply the patch, regenerate the staff client CSS, and clear
your browser cache if necessary.

 - Open the "Set permissions" page for any patron.
 - Confirm that the filter works correctly to show only lines which
   match your entry
   - Note that the superlibrarian line stays visible all the time. This
     is to make it clearer that sub-permissions cannot be selected if
     superlibrarian is checked.
 - Check the superlibrarain "hint" text for clarity.
 - Test the show/hide controls.
 - Test that checking a "parent" permission displays the
   sub-permissions.
 - Test that the toolbar with Save and Cancel floats when scrolling.
 - Test that changing and saving permissions works correctly.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 31 Josef Moravec 2019-02-25 14:52:12 UTC
Created attachment 85623 [details] [review]
Bug 11375: (follow-up) Add links to show all/hide all

This patch adds links in the toolbar to show or hide all
sub-permissions.

To test, apply the patch and view the permissions page for a patron.
Clicking the "Show all" or "Hide all" links should work to show or hide
all sub-permissions. The corresponding "Show details" and "Hide details"
links for each permission set should be correctly toggled.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 32 Josef Moravec 2019-02-25 14:52:15 UTC
Created attachment 85624 [details] [review]
Bug 11375: (QA follow-up) Fix typo

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 33 Josef Moravec 2019-02-25 14:52:19 UTC
Created attachment 85625 [details] [review]
Bug 11375: (QA follow-up) Remove some more treeview occurences

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 34 Josef Moravec 2019-03-11 12:38:33 UTC
Created attachment 86437 [details] [review]
Bug 11375: Improve patrons permissions display

This patch updates the patron permissions interface in order to
emphasize permission descriptions (which can be translated) over
permission code (which cannot).

The tree structure (and jQuery plugin) is removed, and permissions are
displayed in a table-like way. Sub-permissions are shown or hidden with
a link.

A free-text filter is added to the top for narrowing the list by
keyword.

Save and cancel buttons are now in a floating toolbar.

To test, apply the patch, regenerate the staff client CSS, and clear
your browser cache if necessary.

 - Open the "Set permissions" page for any patron.
 - Confirm that the filter works correctly to show only lines which
   match your entry
   - Note that the superlibrarian line stays visible all the time. This
     is to make it clearer that sub-permissions cannot be selected if
     superlibrarian is checked.
 - Check the superlibrarain "hint" text for clarity.
 - Test the show/hide controls.
 - Test that checking a "parent" permission displays the
   sub-permissions.
 - Test that the toolbar with Save and Cancel floats when scrolling.
 - Test that changing and saving permissions works correctly.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 35 Josef Moravec 2019-03-11 12:38:37 UTC
Created attachment 86438 [details] [review]
Bug 11375: (follow-up) Add links to show all/hide all

This patch adds links in the toolbar to show or hide all
sub-permissions.

To test, apply the patch and view the permissions page for a patron.
Clicking the "Show all" or "Hide all" links should work to show or hide
all sub-permissions. The corresponding "Show details" and "Hide details"
links for each permission set should be correctly toggled.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 36 Josef Moravec 2019-03-11 12:38:41 UTC
Created attachment 86439 [details] [review]
Bug 11375: (QA follow-up) Fix typo

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 37 Josef Moravec 2019-03-11 12:38:45 UTC
Created attachment 86440 [details] [review]
Bug 11375: (QA follow-up) Remove some more treeview occurences

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 38 Josef Moravec 2019-03-11 12:40:25 UTC
Rebased, conflict with bug 15774, which added new permission manage_additional_field
Comment 39 Nick Clemens 2019-03-11 13:10:16 UTC
Awesome work all!

Pushed to master for 19.05
Comment 40 Martin Renvoize 2019-03-12 09:42:19 UTC
Enhancement will not be backported to 18.11.x series.