Bug 18214 - Cannot edit list permissions of a private list
Summary: Cannot edit list permissions of a private list
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Lists (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 18228
  Show dependency treegraph
 
Reported: 2017-03-06 08:44 UTC by Marcel de Rooy
Modified: 2017-06-22 07:13 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 18214: Cannot edit list permissions of a private list (2.24 KB, patch)
2017-03-06 08:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18214: Cannot edit list permissions of a private list (2.27 KB, patch)
2017-03-06 09:25 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 18214: Add check for shared or public list (2.05 KB, patch)
2017-06-22 07:08 UTC, Marcel de Rooy
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-03-06 08:44:25 UTC
If you have disabled the pref OpacAllowPublicListCreation, your users are not able to edit the list permissions for private/shared lists.
For a private list they may only be theoretically relevant, but for a shared list they are relevant.
Since we do not always know the history of a list (has it been public or shared, does it contains entries from other users) and therefore permissions are even relevant for a currently private list, we should just allow editing these permissions.
Trivial change in opac-shelves template. No need to change staff side.
Comment 1 Marcel de Rooy 2017-03-06 08:52:16 UTC Comment hidden (obsolete)
Comment 2 Magnus Enger 2017-03-06 09:25:30 UTC
Created attachment 60822 [details] [review]
Bug 18214: Cannot edit list permissions of a private list

If you have disabled the pref OpacAllowPublicListCreation, your users are
not able to edit the list permissions for private/shared lists.
For a private list they may only be theoretically relevant, but for a shared
list they are relevant.
Since we do not always know the history of a list (has it been public or
shared, does it contains entries from other users) and therefore permissions
are even relevant for a currently private list, we should just allow editing
these permissions.

Test plan:
[1] Do not yet apply this patch.
[2] Disable OpacAllowPublicListCreation.
[3] Create a private list in OPAC. Edit the list. Verify that you do not
    see the permission combo boxes.
[4] Apply this patch. Edit the list again. Do they appear now?

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised.
Comment 3 Jonathan Druart 2017-03-07 13:58:40 UTC
There is a Koha::VirtualShelf->is_shared method, should not we use it to know if the permissions must be displayed or not?
Comment 4 Marcel de Rooy 2017-03-07 14:12:35 UTC
(In reply to Jonathan Druart from comment #3)
> There is a Koha::VirtualShelf->is_shared method, should not we use it to
> know if the permissions must be displayed or not?

You dont need the check, opac-shelves will already say: You do not have permission to update this list.
Comment 5 Marcel de Rooy 2017-03-07 14:14:07 UTC
(In reply to Jonathan Druart from comment #3)
> There is a Koha::VirtualShelf->is_shared method, should not we use it to
> know if the permissions must be displayed or not?

And it may have been shared. Or it was a public list. So the list may contain items from other people..
Comment 6 Marcel de Rooy 2017-03-09 11:32:42 UTC
Temporarily blocking this one in favor of bug 18228.
That bug includes this change too, but simplifies things.
Comment 7 Katrin Fischer 2017-06-19 14:45:16 UTC
We just ran into this:

If OpacAllowPublicListCreation is turned off, you can't allow users to add to your shared list. This renders the sharing a bit useless :(

It's especially odd since the system preference OpacAllowSharingPrivateLists talks about sharing private lists and I would expect that to enable the permission options.
Comment 8 Marcel de Rooy 2017-06-21 12:43:23 UTC
(In reply to Katrin Fischer from comment #7)
> We just ran into this:
> 
> If OpacAllowPublicListCreation is turned off, you can't allow users to add
> to your shared list. This renders the sharing a bit useless :(
> 
> It's especially odd since the system preference OpacAllowSharingPrivateLists
> talks about sharing private lists and I would expect that to enable the
> permission options.

This problem is definitely solved with bug 18228. The trivial change on this report is incorporated in the larger solution on that report.

*** This bug has been marked as a duplicate of bug 18228 ***
Comment 9 Katrin Fischer 2017-06-21 15:39:16 UTC
I understand, but not sure if the bigger solution can be backported as it also changes behaviour.
Comment 10 Marcel de Rooy 2017-06-22 06:51:40 UTC
(In reply to Katrin Fischer from comment #9)
> I understand, but not sure if the bigger solution can be backported as it
> also changes behaviour.

Reopening this bug
Comment 11 Marcel de Rooy 2017-06-22 06:55:04 UTC
(In reply to Jonathan Druart from comment #3)
> There is a Koha::VirtualShelf->is_shared method, should not we use it to
> know if the permissions must be displayed or not?

On second thought, I will add a check for shared or public list here.
Comment 12 Marcel de Rooy 2017-06-22 07:08:37 UTC
Created attachment 64504 [details] [review]
Bug 18214: Add check for shared or public list

Following the idea behind bug 10865, we are only showing the permissions
when the list is shared or public.
Adding a simple test in opac-shelves here.

Note 1: Since the owner can always add or delete entries, the permissions
will not be relevant anymore for a strictly private list.

Note 2: Staff view always shows the permissions. This could have been
changed here too, but that change is far less urgent (bug 10865 did not
touch staff view and bug 18228 will rearrange permissions anyway).

Test plan:
[1] Verify on OPAC that you see the permissions for a private list with
    shares or a public list. And you do not see them for a private list
    without shares.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Marcel de Rooy 2017-06-22 07:10:42 UTC
(In reply to Katrin Fischer from comment #9)
> I understand, but not sure if the bigger solution can be backported as it
> also changes behaviour.

The two current patches should be easy to backport. I will rebase 18228/834.
Comment 14 Marcel de Rooy 2017-06-22 07:12:48 UTC
Moving these trivial patches to the RM queue now. They had enough attention now imo.