Bug 18980 - Add an explanation when Anyone permission has no actual effect
Summary: Add an explanation when Anyone permission has no actual effect
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Lists (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-25 13:02 UTC by Marcel de Rooy
Modified: 2017-08-10 16:53 UTC (History)
4 users (show)

See Also:
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:


Attachments
Bug 18980: Add remark next to "Anyone seeing this list" permission (5.75 KB, patch)
2017-07-25 14:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18980: Add js to dynamically show/hide the Anyone remark (6.95 KB, patch)
2017-07-25 14:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18980: Remove js function Check from opac-shelves (3.29 KB, patch)
2017-07-25 14:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18980: Show distinction between shared and private lists in staff (3.98 KB, patch)
2017-07-25 14:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18980: Add remark next to "Anyone seeing this list" permission (5.85 KB, patch)
2017-07-26 12:47 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18980: Add js to dynamically show/hide the Anyone remark (7.04 KB, patch)
2017-07-26 12:48 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18980: Remove js function Check from opac-shelves (3.38 KB, patch)
2017-07-26 12:48 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18980: Show distinction between shared and private lists in staff (4.07 KB, patch)
2017-07-26 12:49 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18980: Add remark next to "Anyone seeing this list" permission (5.90 KB, patch)
2017-07-31 14:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18980: Add js to dynamically show/hide the Anyone remark (7.10 KB, patch)
2017-07-31 14:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18980: Remove js function Check from opac-shelves (3.44 KB, patch)
2017-07-31 14:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18980: Show distinction between shared and private lists in staff (4.13 KB, patch)
2017-07-31 14:05 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2017-07-25 13:02:39 UTC
This implements an alternative solution for bug 18834.
Comment 1 Marcel de Rooy 2017-07-25 14:48:53 UTC
Created attachment 65229 [details] [review]
Bug 18980: Add remark next to "Anyone seeing this list" permission

This is a follow-up of report 18228 that simplifies the permissions for
lists. It reapplies the idea behind bug 10865 by adding a remark next to
the Anyone permission if it is not actually effective. A next patch will
make it behave dynamically.

Note: The Anyone permission is not relevant if it is a private list that
has no shares. Note that looking at the prefs OpacAllowPublicListCreation
and/or OpacAllowSharingPrivateLists is not decisive. You should look at
list permissions and shares in the database; turning off the Sharing pref
does not automatically remove all shares in the system. It only blocks
creating new shares.

We only need changes in opac-shelves.tt and virtualshelves/shelves.tt.

Test plan:
[1] Verify that you see the remark now in OPAC and staff.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2017-07-25 14:48:57 UTC
Created attachment 65230 [details] [review]
Bug 18980: Add js to dynamically show/hide the Anyone remark

If we we move from Private to Public or vice versa, this should reflect
on the Anyone remark from the first patch.
If we go back to a private list without shares, we should show it.

Handled in a js sub AdjustRemark, triggered by onchange of the category
combo or the permission combo.

Test plan:
[1] Edit a private list without shares in OPAC. Toggle category and/or
    permissions. Is the remark shown or hidden accordingly?
    Note: Include a test with OpacAllowPublicListCreation enabled.
[2] Edit a shared list or public list in staff. Toggle category/permissions.
    You should never see the remark.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Marcel de Rooy 2017-07-25 14:49:00 UTC
Created attachment 65231 [details] [review]
Bug 18980: Remove js function Check from opac-shelves

This function only serves to check if the shelf name is not empty. It
does not even work since it incorrectly refers to f.addshelf (while it
should test #shelfname).
We can solve this and improve consistency by doing the same as in the
staff template with html5 required.

Test plan:
[1] Without this patch it was possible to add a list without a name in
    OPAC (with js error: TypeError: f.addshelf is undefined).
[2] With this patch, verify that you cannot do this anymore.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Comment 4 Marcel de Rooy 2017-07-25 14:49:04 UTC
Created attachment 65232 [details] [review]
Bug 18980: Show distinction between shared and private lists in staff

Just as we show this distinction in OPAC, this patch adds a type column
in the Your lists tab that displays Private or Shared. It always contains
Public in the other tab.

Test plan:
[1] Check if you see Shared for a private lists with shares in staff.
[2] Run t/db_dependent/Utils/Datatables_Virtualshelves.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Comment 5 Marcel de Rooy 2017-07-25 14:51:31 UTC
I adjusted the patches from 18834 to arrive here. Kept the same division. Note that I removed the signoff from LeeJ on the first two patches. The other two did not really change, but were merely rebased.
Comment 6 Marcel de Rooy 2017-07-25 14:55:09 UTC
LeeJ: Any chance to retest and sign off on the first two ? Would be great!
Comment 7 Marc Véron 2017-07-26 12:47:03 UTC
Created attachment 65273 [details] [review]
Bug 18980: Add remark next to "Anyone seeing this list" permission

This is a follow-up of report 18228 that simplifies the permissions for
lists. It reapplies the idea behind bug 10865 by adding a remark next to
the Anyone permission if it is not actually effective. A next patch will
make it behave dynamically.

Note: The Anyone permission is not relevant if it is a private list that
has no shares. Note that looking at the prefs OpacAllowPublicListCreation
and/or OpacAllowSharingPrivateLists is not decisive. You should look at
list permissions and shares in the database; turning off the Sharing pref
does not automatically remove all shares in the system. It only blocks
creating new shares.

We only need changes in opac-shelves.tt and virtualshelves/shelves.tt.

Test plan:
[1] Verify that you see the remark now in OPAC and staff.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 8 Marc Véron 2017-07-26 12:48:03 UTC
Created attachment 65274 [details] [review]
Bug 18980: Add js to dynamically show/hide the Anyone remark

If we we move from Private to Public or vice versa, this should reflect
on the Anyone remark from the first patch.
If we go back to a private list without shares, we should show it.

Handled in a js sub AdjustRemark, triggered by onchange of the category
combo or the permission combo.

Test plan:
[1] Edit a private list without shares in OPAC. Toggle category and/or
    permissions. Is the remark shown or hidden accordingly?
    Note: Include a test with OpacAllowPublicListCreation enabled.
[2] Edit a shared list or public list in staff. Toggle category/permissions.
    You should never see the remark.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 9 Marc Véron 2017-07-26 12:48:45 UTC
Created attachment 65275 [details] [review]
Bug 18980: Remove js function Check from opac-shelves

This function only serves to check if the shelf name is not empty. It
does not even work since it incorrectly refers to f.addshelf (while it
should test #shelfname).
We can solve this and improve consistency by doing the same as in the
staff template with html5 required.

Test plan:
[1] Without this patch it was possible to add a list without a name in
    OPAC (with js error: TypeError: f.addshelf is undefined).
[2] With this patch, verify that you cannot do this anymore.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 10 Marc Véron 2017-07-26 12:49:36 UTC
Created attachment 65276 [details] [review]
Bug 18980: Show distinction between shared and private lists in staff

Just as we show this distinction in OPAC, this patch adds a type column
in the Your lists tab that displays Private or Shared. It always contains
Public in the other tab.

Test plan:
[1] Check if you see Shared for a private lists with shares in staff.
[2] Run t/db_dependent/Utils/Datatables_Virtualshelves.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 11 Marcel de Rooy 2017-07-26 12:53:13 UTC
(In reply to Marc Véron from comment #10)
> Signed-off-by: Marc Véron <veron@veron.ch>

Thanks, Marc !
Comment 12 Nick Clemens 2017-07-31 14:05:28 UTC
Created attachment 65347 [details] [review]
Bug 18980: Add remark next to "Anyone seeing this list" permission

This is a follow-up of report 18228 that simplifies the permissions for
lists. It reapplies the idea behind bug 10865 by adding a remark next to
the Anyone permission if it is not actually effective. A next patch will
make it behave dynamically.

Note: The Anyone permission is not relevant if it is a private list that
has no shares. Note that looking at the prefs OpacAllowPublicListCreation
and/or OpacAllowSharingPrivateLists is not decisive. You should look at
list permissions and shares in the database; turning off the Sharing pref
does not automatically remove all shares in the system. It only blocks
creating new shares.

We only need changes in opac-shelves.tt and virtualshelves/shelves.tt.

Test plan:
[1] Verify that you see the remark now in OPAC and staff.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Nick Clemens 2017-07-31 14:05:32 UTC
Created attachment 65348 [details] [review]
Bug 18980: Add js to dynamically show/hide the Anyone remark

If we we move from Private to Public or vice versa, this should reflect
on the Anyone remark from the first patch.
If we go back to a private list without shares, we should show it.

Handled in a js sub AdjustRemark, triggered by onchange of the category
combo or the permission combo.

Test plan:
[1] Edit a private list without shares in OPAC. Toggle category and/or
    permissions. Is the remark shown or hidden accordingly?
    Note: Include a test with OpacAllowPublicListCreation enabled.
[2] Edit a shared list or public list in staff. Toggle category/permissions.
    You should never see the remark.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 14 Nick Clemens 2017-07-31 14:05:35 UTC
Created attachment 65349 [details] [review]
Bug 18980: Remove js function Check from opac-shelves

This function only serves to check if the shelf name is not empty. It
does not even work since it incorrectly refers to f.addshelf (while it
should test #shelfname).
We can solve this and improve consistency by doing the same as in the
staff template with html5 required.

Test plan:
[1] Without this patch it was possible to add a list without a name in
    OPAC (with js error: TypeError: f.addshelf is undefined).
[2] With this patch, verify that you cannot do this anymore.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 15 Nick Clemens 2017-07-31 14:05:38 UTC
Created attachment 65350 [details] [review]
Bug 18980: Show distinction between shared and private lists in staff

Just as we show this distinction in OPAC, this patch adds a type column
in the Your lists tab that displays Private or Shared. It always contains
Public in the other tab.

Test plan:
[1] Check if you see Shared for a private lists with shares in staff.
[2] Run t/db_dependent/Utils/Datatables_Virtualshelves.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 16 Jonathan Druart 2017-08-10 16:45:24 UTC
On the staff interface:
"""
    Private list: Is managed by you and can be seen only by you.
    Public list: Can be seen by everybody, but managed only by you.
"""

Should not we update it? If a list is private but shared, the description is wrong.
Comment 17 Jonathan Druart 2017-08-10 16:49:53 UTC
AdjustRemark is a bit awkward, we could show the note only if the shelf is shared, private and changes are allowed by anyone seeing this list.
Not blocker.
Comment 18 Jonathan Druart 2017-08-10 16:53:19 UTC
Pushed to master for 17.11, thanks to everybody involved!