Bug 25498 - Allow to change owner of public or shared list
Summary: Allow to change owner of public or shared list
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Lists (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 40 votes (vote)
Assignee: Marcel de Rooy
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 11889 30578
Blocks: 30933
  Show dependency treegraph
 
Reported: 2020-05-14 04:53 UTC by David Cook
Modified: 2023-12-28 20:42 UTC (History)
14 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/658
Text to go in the release notes:
This enhancement allows staff members with sufficient permissions to change the owner of a public list. It also add the possibility for the owner of a shared list to transfer ownership to one of the other users of the list via the opac shelves form. Implicitly, we hereby add the option of showing which users are currently sharing the list to the owner.
Version(s) released in:
22.11.00


Attachments
Bug 25498: Add transfer button on intranet (6.20 KB, patch)
2022-06-09 14:27 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 25498: Add a transfer form for public lists (7.14 KB, patch)
2022-06-10 09:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 25498: Add transfer button on intranet (6.49 KB, patch)
2022-06-13 12:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 25498: Add a transfer form for public lists (7.14 KB, patch)
2022-06-13 12:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 25498: Show transfer button for shared lists on OPAC (4.06 KB, patch)
2022-06-13 12:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 25498: Transfer ownership shared list on OPAC (7.37 KB, patch)
2022-06-13 12:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 25498: Add transfer button on intranet (6.50 KB, patch)
2022-06-13 15:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 25498: Add a transfer form for public lists (7.15 KB, patch)
2022-06-13 15:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 25498: Show transfer button for shared lists on OPAC (4.07 KB, patch)
2022-06-13 15:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 25498: Transfer ownership shared list on OPAC (7.38 KB, patch)
2022-06-13 15:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 25498: Add transfer button on intranet (6.56 KB, patch)
2022-06-16 13:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25498: Add a transfer form for public lists (7.21 KB, patch)
2022-06-16 13:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25498: Show transfer button for shared lists on OPAC (4.13 KB, patch)
2022-06-16 13:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25498: Transfer ownership shared list on OPAC (7.44 KB, patch)
2022-06-16 13:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25498: (QA follow-up) Rename virtualshelf->patron (1.75 KB, patch)
2022-06-17 07:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 25498: (QA follow-up) Rename virtualshelfshare->patron (1.76 KB, patch)
2022-06-17 07:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 25498: Add transfer button on intranet (6.64 KB, patch)
2022-07-19 12:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 25498: Add a transfer form for public lists (7.25 KB, patch)
2022-07-19 12:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 25498: Show transfer button for shared lists on OPAC (4.17 KB, patch)
2022-07-19 12:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 25498: Transfer ownership shared list on OPAC (7.48 KB, patch)
2022-07-19 12:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 25498: (QA follow-up) Rename virtualshelfshare->patron (1.76 KB, patch)
2022-07-19 12:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 25498: (follow-up) Change for autocomplete change of 30578 (2.28 KB, patch)
2022-07-19 12:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 25498: (follow-up) Change for autocomplete change of 30578 (2.31 KB, patch)
2022-07-19 12:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25498: (follow-up) Show error message when no sharee has email (2.70 KB, patch)
2022-07-20 11:47 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 David Cook 2020-05-14 04:53:48 UTC
Once someone owns a list, there is no way to change the owner. This problem often arises when a library staff member uses an individual account to create a public list, and then they leave their job.

A superlibrarian (or otherwise privileged user) should be able to change the owner of a list on demand.
Comment 1 Marcel de Rooy 2020-05-14 06:07:29 UTC
Duplicate of the see also ?
Comment 2 David Cook 2020-05-14 06:12:20 UTC
(In reply to Marcel de Rooy from comment #1)
> Duplicate of the see also ?

I don't think so. The see also only seems to be concerned with what to do when deleting a patron. 

In this case, I'm just targeting the inability/ability to manually change the owner of a list.

Personally, I would think this issue would make Bug 11889 redundant, but that bug seems to be lost in a swamp of conflicting ideas and expectations. So I opened this one with a very simple goal.
Comment 3 Marcel de Rooy 2020-05-14 06:17:47 UTC
What we will encounter here too, is the "privacy" issue. You should not be able to take ownership of arbitrary private or shared lists.
I am still thinking in terms of moving owner to NULL for a public list when you remove its owner. And allowing superlibrarians to take over such lists.
Comment 4 Katrin Fischer 2020-05-14 06:27:39 UTC
(In reply to Marcel de Rooy from comment #3)
> What we will encounter here too, is the "privacy" issue. You should not be
> able to take ownership of arbitrary private or shared lists.
> I am still thinking in terms of moving owner to NULL for a public list when
> you remove its owner. And allowing superlibrarians to take over such lists.

+1
Comment 5 David Cook 2020-05-14 06:30:02 UTC
(In reply to Marcel de Rooy from comment #3)
> What we will encounter here too, is the "privacy" issue. You should not be
> able to take ownership of arbitrary private or shared lists.
> I am still thinking in terms of moving owner to NULL for a public list when
> you remove its owner. And allowing superlibrarians to take over such lists.

I think that the solution is to allow only superlibrarians to change ownership of lists. Since they have full access to the SQL reporting, they already have full access to that data, so the privacy issue is a bit moot. 

In the use cases described by kholt@roseville.ca.us and myself, we're most concerned about public lists. It really should be possible for a superlibrarian to transfer ownership (ie responsibility) for a list to a different staff person.

I'd be willing to work on a feature where a superlibrarian can only change the ownership of a public list. That would reduce the chance of a superlibrarian accidentally changing the owner of a private list, and thus leaking personal information between private individuals.
Comment 6 Marcel de Rooy 2020-05-14 07:18:34 UTC
(In reply to David Cook from comment #5)
> Since they have full access to the SQL reporting, they
> already have full access to that data, so the privacy issue is a bit moot. 

Agreed. But it may be a matter of Does the system facilitate crossing such privacy borders? The reporting backdoor is a subject on itself ;)
Comment 7 David Cook 2020-05-14 23:05:59 UTC
(In reply to Marcel de Rooy from comment #6)
> (In reply to David Cook from comment #5)
> > Since they have full access to the SQL reporting, they
> > already have full access to that data, so the privacy issue is a bit moot. 
> 
> Agreed. But it may be a matter of Does the system facilitate crossing such
> privacy borders? The reporting backdoor is a subject on itself ;)

Those are good points too heh.

I think in that case maybe private lists should be absolutely private, but public lists should be able to have owners changed by superlibrarians. 

Shared lists... well I don't quite understand the concept of shared lists yet (neither from the user or technical perspective), although I think I get what you'd mean by disowning a list in that context.
Comment 8 Andrew Fuerste-Henry 2020-10-22 18:31:03 UTC
I've been talking to a library that wants to create custom lists for patrons -- the patron fills out a readers advisory form and the library prepares a list of recommendations. It'd be great if this would allow the library to create a private list and make the patron the owner.
Comment 9 Martin Renvoize 2020-11-24 08:49:36 UTC
Are you still intending on working on this David?  We'd love to see such functionality too.
Comment 10 David Cook 2020-11-24 22:42:13 UTC
(In reply to Martin Renvoize from comment #9)
> Are you still intending on working on this David?  We'd love to see such
> functionality too.

No, I have no plans to work on this, unfortunately.
Comment 11 Abbey Holt 2021-03-05 20:41:42 UTC
We would love this feature, too! Mainly so a librarian can continue building on existing public lists when the original list owner is no longer involved.
Comment 12 Amy Muchmore 2021-03-05 21:03:31 UTC
I would love for there to be a way to change ownership of a list or to transfer ownership to another staff member when the original list owner is no longer involved.
Comment 13 ssmith 2021-03-11 15:47:04 UTC
(In reply to Andrew Fuerste-Henry from comment #8)
> I've been talking to a library that wants to create custom lists for patrons
> -- the patron fills out a readers advisory form and the library prepares a
> list of recommendations. It'd be great if this would allow the library to
> create a private list and make the patron the owner.

Andrew's suggestion could be a useful tool too!

But more than an anything, an ability to transfer ownership of public lists would be very helpful.

A staff member doesn't even need to leave an institution for this to be helpful. A change in position within the same institution might mean that someone new needs to maintain an old list, and it'd be very nice not to have to scrap that old list and start from scratch.
Comment 14 Ben Eagle 2021-03-11 16:02:26 UTC
This feature would be very helpful! As someone who makes lists frequently and collaborates with other coworkers in making lists, it'd be helpful to transfer ownership to update a list that was started by a coworker (or to give ownership to someone else).
Comment 15 Barbara Johnson 2022-02-17 16:47:33 UTC
This functionality would be very useful at our library.
Comment 16 Marcel de Rooy 2022-06-09 11:37:23 UTC
Will be proposing something here shortly in terms of patches
Comment 17 Marcel de Rooy 2022-06-09 14:27:09 UTC
Created attachment 135889 [details] [review]
Bug 25498: Add transfer button on intranet

Note: Adjusting shelves_results template too for not allowing delete
when you do not have delete_public_lists (or higher). If you are the
owner, you are allowed to edit or delete. But transfer specifically
needs edit_public_lists (or higher).

Test plan:
Check if you see the transfer button on public lists only when having
permission edit_public_lists (or higher). And never on private lists.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 18 Marcel de Rooy 2022-06-09 14:27:32 UTC
By far not ready :)
Comment 19 Martin Renvoize 2022-06-09 15:10:16 UTC
I've never really investigated this area of Koha much.. I was surprised to see the 'share' options only work from the OPAC.. I thought you'd be able to share a private list with other staff via the staff client... that belief made me think perhaps the transfer of ownership stuff would be tied to who you shared a list with (and what capacity the share was.. read only, edit, owner.. somewhat like google docs and similar friends).

Just some meandering thoughts.. I'll be interested to see where you take this Marcel.
Comment 20 Marcel de Rooy 2022-06-10 06:21:19 UTC
(In reply to Martin Renvoize from comment #19)
> I've never really investigated this area of Koha much.. I was surprised to
> see the 'share' options only work from the OPAC.. I thought you'd be able to
> share a private list with other staff via the staff client... that belief
> made me think perhaps the transfer of ownership stuff would be tied to who
> you shared a list with (and what capacity the share was.. read only, edit,
> owner.. somewhat like google docs and similar friends).
> 
> Just some meandering thoughts.. I'll be interested to see where you take
> this Marcel.

Thinking along the following lines:
Allow transfer ownership of public list on staff interface. Staff member with permission can transfer to any user.
Allow the owner of a shared list to transfer ownership to another member of the list like you wrote above.
Still not completely sure about handling deletion of a shared list. Since the owner will have the choice between transfer [and remove share] or delete, a deliberate delete should probably be performed as now. On the other hand, bug 11889 may transfer the shared list to a staff member when deleting the owner as patron. Easy to defend: you shared that list..
Private lists should not be transferred.
Comment 21 Marcel de Rooy 2022-06-10 09:22:33 UTC
Created attachment 135921 [details] [review]
Bug 25498: Add a transfer form for public lists

Test plan:
[1] Valid transfer:
    Log in with staff patron C having edit_public_lists.
    Pick a public list. Choose Transfer. Select a patron.
    Submit and check if transfer was successful.
[2] Permission problem:
    Pick a staff user D. Temporary enable edit_public_lists.
    Add another private tab. Login with D.
    Pick a public list. Transfer. Choose another patron. Wait.
    Switch tab: remove added permission from D.
    Switch tab again: Submit transfer form. Error? Close tab.
[3] Bonus test - Shelf disappeared:
    Pick a public list. Transfer. Choose another patron. Wait.
    Open another tab. Delete the selected public list.
    Close tab. Submit the transfer. Not exist error?
[4] Bonus test - Patron not found:
    Pick a public list. Transfer. Choose another patron. Wait.
    Open another tab. Delete selected patron.
    Close tab. Submit transfer. Patron not found error?

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 22 Marcel de Rooy 2022-06-10 10:10:12 UTC
(In reply to Marcel de Rooy from comment #20)
> Allow the owner of a shared list to transfer ownership to another member of
> the list like you wrote above.

Does anyone already have some thoughts about the sensitivity of this OPAC operation? I'm thinking of a simple select containing the names only of the members that accepted a share of this list. But actually we only mailed an invitation via the Share button; very strictly speaking this would reveal more information. On the other hand, the shared list owner should be entitled somehow to see which people are using the list.

Note: Instead of a name, we could list an email address with a borrowernumber behind it in the select option?
Comment 23 Marcel de Rooy 2022-06-13 12:15:33 UTC
Created attachment 136008 [details] [review]
Bug 25498: Add transfer button on intranet

Note: Adjusting shelves_results template too for not allowing delete
when you do not have delete_public_lists (or higher). If you are the
owner, you are allowed to edit or delete. But transfer specifically
needs edit_public_lists (or higher).

Note: Removed a few useless POD lines to make qa tools happy.

Test plan:
Check if you see the transfer button on public lists only when having
permission edit_public_lists (or higher). And never on private lists.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 24 Marcel de Rooy 2022-06-13 12:15:38 UTC
Created attachment 136009 [details] [review]
Bug 25498: Add a transfer form for public lists

Test plan:
[1] Valid transfer:
    Log in with staff patron C having edit_public_lists.
    Pick a public list. Choose Transfer. Select a patron.
    Submit and check if transfer was successful.
[2] Permission problem:
    Pick a staff user D. Temporary enable edit_public_lists.
    Add another private tab. Login with D.
    Pick a public list. Transfer. Choose another patron. Wait.
    Switch tab: remove added permission from D.
    Switch tab again: Submit transfer form. Error? Close tab.
[3] Bonus test - Shelf disappeared:
    Pick a public list. Transfer. Choose another patron. Wait.
    Open another tab. Delete the selected public list.
    Close tab. Submit the transfer. Not exist error?
[4] Bonus test - Patron not found:
    Pick a public list. Transfer. Choose another patron. Wait.
    Open another tab. Delete selected patron.
    Close tab. Submit transfer. Patron not found error?

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 25 Marcel de Rooy 2022-06-13 12:15:43 UTC
Created attachment 136010 [details] [review]
Bug 25498: Show transfer button for shared lists on OPAC

Test plan:
Pick or create a shared list.
Verify that OPAC only shows the Transfer button when you are the
owner of a shared list.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 26 Marcel de Rooy 2022-06-13 12:15:48 UTC
Created attachment 136011 [details] [review]
Bug 25498: Transfer ownership shared list on OPAC

Test plan:
Pick or create a shared list. Login as owner.
Transfer to one of the other members.
Verify that you do no longer have Transfer, but see Remove share.
Login as the other member. Transfer it back.

Bonus: Working with two tabs, you can open the Transfer form and
in the meantime delete the list, the patron or remove the share.
Or change permissions. Submit after that and check the error message.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 27 Marcel de Rooy 2022-06-13 12:23:35 UTC Comment hidden (obsolete)
Comment 28 Marcel de Rooy 2022-06-13 12:27:13 UTC
Current stand:
[1] Allow transfer ownership of public list on staff interface. Staff member with permission can transfer to any user.
[2] Allow the owner of a shared list to transfer ownership to another member of the list (currently only via OPAC form).
[3] Private lists should not be transferred.

(In reply to Marcel de Rooy from comment #22)
> Does anyone already have some thoughts about the sensitivity of this OPAC
> operation? 
Not yet. But this implementation only copies a few borrowernumbers (not directly visible) and email addresses to the new user combo. The owner is imo entitled to that information; we can assume that he already knows them in general too. Other patron data is not revealed.
Comment 29 Marcel de Rooy 2022-06-13 12:46:47 UTC
QA team:

 FAIL   Koha/Virtualshelfshare.pm
   FAIL   valid
                Constant subroutine Koha::Virtualshelfshare::SHARE_INVITATION_EXPIRY_DAYS redefined

This is a false negative. Nothing wrong with that constant. It wasnt changed either.
Comment 30 Marcel de Rooy 2022-06-13 12:48:22 UTC
Why does this report not make it possible to change the owner of a shared list on staff?
In short, share a list is an OPAC feature.
Note that I opened bug 30953 to add an intranet counterpart. It could be addressed there.
Comment 31 Marcel de Rooy 2022-06-13 12:52:41 UTC
Also opened bug 30955 for an additional notice for a new owner of a shared list.
Comment 32 Owen Leonard 2022-06-13 15:08:28 UTC
Created attachment 136033 [details] [review]
Bug 25498: Add transfer button on intranet

Note: Adjusting shelves_results template too for not allowing delete
when you do not have delete_public_lists (or higher). If you are the
owner, you are allowed to edit or delete. But transfer specifically
needs edit_public_lists (or higher).

Note: Removed a few useless POD lines to make qa tools happy.

Test plan:
Check if you see the transfer button on public lists only when having
permission edit_public_lists (or higher). And never on private lists.

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

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 33 Owen Leonard 2022-06-13 15:08:33 UTC
Created attachment 136034 [details] [review]
Bug 25498: Add a transfer form for public lists

Test plan:
[1] Valid transfer:
    Log in with staff patron C having edit_public_lists.
    Pick a public list. Choose Transfer. Select a patron.
    Submit and check if transfer was successful.
[2] Permission problem:
    Pick a staff user D. Temporary enable edit_public_lists.
    Add another private tab. Login with D.
    Pick a public list. Transfer. Choose another patron. Wait.
    Switch tab: remove added permission from D.
    Switch tab again: Submit transfer form. Error? Close tab.
[3] Bonus test - Shelf disappeared:
    Pick a public list. Transfer. Choose another patron. Wait.
    Open another tab. Delete the selected public list.
    Close tab. Submit the transfer. Not exist error?
[4] Bonus test - Patron not found:
    Pick a public list. Transfer. Choose another patron. Wait.
    Open another tab. Delete selected patron.
    Close tab. Submit transfer. Patron not found error?

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

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 34 Owen Leonard 2022-06-13 15:08:38 UTC
Created attachment 136035 [details] [review]
Bug 25498: Show transfer button for shared lists on OPAC

Test plan:
Pick or create a shared list.
Verify that OPAC only shows the Transfer button when you are the
owner of a shared list.

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

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 35 Owen Leonard 2022-06-13 15:08:43 UTC
Created attachment 136036 [details] [review]
Bug 25498: Transfer ownership shared list on OPAC

Test plan:
Pick or create a shared list. Login as owner.
Transfer to one of the other members.
Verify that you do no longer have Transfer, but see Remove share.
Login as the other member. Transfer it back.

Bonus: Working with two tabs, you can open the Transfer form and
in the meantime delete the list, the patron or remove the share.
Or change permissions. Submit after that and check the error message.

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

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 36 Owen Leonard 2022-06-13 15:10:02 UTC
This is working well, but I think we ought to wait and rework the autocomplete parts of it after Bug 30578 makes progress.
Comment 37 Marcel de Rooy 2022-06-14 11:33:07 UTC
(In reply to Owen Leonard from comment #36)
> This is working well, but I think we ought to wait and rework the
> autocomplete parts of it after Bug 30578 makes progress.

Thanks Owen.
It should not be that hard to add a tiny follow-up changing the ysearch call to REST API at either side. Without already committing ourselves to dependency order now?
Comment 38 Martin Renvoize 2022-06-16 13:52:44 UTC
Created attachment 136166 [details] [review]
Bug 25498: Add transfer button on intranet

Note: Adjusting shelves_results template too for not allowing delete
when you do not have delete_public_lists (or higher). If you are the
owner, you are allowed to edit or delete. But transfer specifically
needs edit_public_lists (or higher).

Note: Removed a few useless POD lines to make qa tools happy.

Test plan:
Check if you see the transfer button on public lists only when having
permission edit_public_lists (or higher). And never on private lists.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 39 Martin Renvoize 2022-06-16 13:52:49 UTC
Created attachment 136167 [details] [review]
Bug 25498: Add a transfer form for public lists

Test plan:
[1] Valid transfer:
    Log in with staff patron C having edit_public_lists.
    Pick a public list. Choose Transfer. Select a patron.
    Submit and check if transfer was successful.
[2] Permission problem:
    Pick a staff user D. Temporary enable edit_public_lists.
    Add another private tab. Login with D.
    Pick a public list. Transfer. Choose another patron. Wait.
    Switch tab: remove added permission from D.
    Switch tab again: Submit transfer form. Error? Close tab.
[3] Bonus test - Shelf disappeared:
    Pick a public list. Transfer. Choose another patron. Wait.
    Open another tab. Delete the selected public list.
    Close tab. Submit the transfer. Not exist error?
[4] Bonus test - Patron not found:
    Pick a public list. Transfer. Choose another patron. Wait.
    Open another tab. Delete selected patron.
    Close tab. Submit transfer. Patron not found error?

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 40 Martin Renvoize 2022-06-16 13:52:54 UTC
Created attachment 136168 [details] [review]
Bug 25498: Show transfer button for shared lists on OPAC

Test plan:
Pick or create a shared list.
Verify that OPAC only shows the Transfer button when you are the
owner of a shared list.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 41 Martin Renvoize 2022-06-16 13:52:59 UTC
Created attachment 136169 [details] [review]
Bug 25498: Transfer ownership shared list on OPAC

Test plan:
Pick or create a shared list. Login as owner.
Transfer to one of the other members.
Verify that you do no longer have Transfer, but see Remove share.
Login as the other member. Transfer it back.

Bonus: Working with two tabs, you can open the Transfer form and
in the meantime delete the list, the patron or remove the share.
Or change permissions. Submit after that and check the error message.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 42 Martin Renvoize 2022-06-16 13:54:39 UTC
This works well, tests pass and QA script is happy.

Minor wonderment, should the permissions check be moved up in the the module level rather than being in the controllers.. so we can have it unit tested consistently?

Otherwise, this is all good and gets my PQA stamp, great work Marcel
Comment 43 Jonathan Druart 2022-06-16 14:00:36 UTC
Isn't Koha::Virtualshelfshare->patron confusing?
Comment 44 Marcel de Rooy 2022-06-16 14:04:28 UTC
(In reply to Martin Renvoize from comment #42)
> This works well, tests pass and QA script is happy.
> 
> Minor wonderment, should the permissions check be moved up in the the module
> level rather than being in the controllers.. so we can have it unit tested
> consistently?
> 
> Otherwise, this is all good and gets my PQA stamp, great work Marcel

Thanks Martin
Comment 45 Marcel de Rooy 2022-06-16 14:04:50 UTC
(In reply to Jonathan Druart from comment #43)
> Isn't Koha::Virtualshelfshare->patron confusing?

Hmm Good question. Do you have a suggestion already?
Comment 46 Marcel de Rooy 2022-06-16 14:08:03 UTC
(In reply to Martin Renvoize from comment #42)
> Minor wonderment, should the permissions check be moved up in the the module
> level rather than being in the controllers.. so we can have it unit tested
> consistently?

Could do that. Will move it to the next report. 30933
Comment 47 Jonathan Druart 2022-06-16 14:21:12 UTC
(In reply to Marcel de Rooy from comment #45)
> (In reply to Jonathan Druart from comment #43)
> > Isn't Koha::Virtualshelfshare->patron confusing?
> 
> Hmm Good question. Do you have a suggestion already?

Not really, "contributor" maybe?
Comment 48 Martin Renvoize 2022-06-16 15:17:42 UTC
Do they contribute or just get access to view?

'Recipient' or 'Beneficiary'


or 'Sharer' and 'Sharee'.. those feel funky but might actually be the clearest two quasi-terms.?
Comment 49 Katrin Fischer 2022-06-16 16:43:27 UTC
> 
> or 'Sharer' and 'Sharee'.. those feel funky but might actually be the
> clearest two quasi-terms.?

I like those best from the suggested. user I guess is too general in context here?
Comment 50 Marcel de Rooy 2022-06-17 06:51:43 UTC
(In reply to Katrin Fischer from comment #49)
> > 
> > or 'Sharer' and 'Sharee'.. those feel funky but might actually be the
> > clearest two quasi-terms.?
> 
> I like those best from the suggested. user I guess is too general in context
> here?

Thx.
Lets go with Sharee here, since this patron received the share and is not the owner who shared (which would be the sharer).
Comment 51 Marcel de Rooy 2022-06-17 06:54:52 UTC
Altough I must admit that $share->sharee is a bit funky indeed :)
Comment 52 Marcel de Rooy 2022-06-17 07:02:17 UTC Comment hidden (obsolete)
Comment 53 Marcel de Rooy 2022-06-17 07:05:23 UTC
Created attachment 136221 [details] [review]
Bug 25498: (QA follow-up) Rename virtualshelfshare->patron

This patron is a 'sharee' (not the sharer).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 54 Marcel de Rooy 2022-07-19 12:21:30 UTC
Created attachment 137862 [details] [review]
Bug 25498: Add transfer button on intranet

Note: Adjusting shelves_results template too for not allowing delete
when you do not have delete_public_lists (or higher). If you are the
owner, you are allowed to edit or delete. But transfer specifically
needs edit_public_lists (or higher).

Note: Removed a few useless POD lines to make qa tools happy.

Test plan:
Check if you see the transfer button on public lists only when having
permission edit_public_lists (or higher). And never on private lists.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 55 Marcel de Rooy 2022-07-19 12:21:35 UTC
Created attachment 137863 [details] [review]
Bug 25498: Add a transfer form for public lists

Test plan:
[1] Valid transfer:
    Log in with staff patron C having edit_public_lists.
    Pick a public list. Choose Transfer. Select a patron.
    Submit and check if transfer was successful.
[2] Permission problem:
    Pick a staff user D. Temporary enable edit_public_lists.
    Add another private tab. Login with D.
    Pick a public list. Transfer. Choose another patron. Wait.
    Switch tab: remove added permission from D.
    Switch tab again: Submit transfer form. Error? Close tab.
[3] Bonus test - Shelf disappeared:
    Pick a public list. Transfer. Choose another patron. Wait.
    Open another tab. Delete the selected public list.
    Close tab. Submit the transfer. Not exist error?
[4] Bonus test - Patron not found:
    Pick a public list. Transfer. Choose another patron. Wait.
    Open another tab. Delete selected patron.
    Close tab. Submit transfer. Patron not found error?

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 56 Marcel de Rooy 2022-07-19 12:21:40 UTC
Created attachment 137864 [details] [review]
Bug 25498: Show transfer button for shared lists on OPAC

Test plan:
Pick or create a shared list.
Verify that OPAC only shows the Transfer button when you are the
owner of a shared list.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 57 Marcel de Rooy 2022-07-19 12:21:45 UTC
Created attachment 137865 [details] [review]
Bug 25498: Transfer ownership shared list on OPAC

Test plan:
Pick or create a shared list. Login as owner.
Transfer to one of the other members.
Verify that you do no longer have Transfer, but see Remove share.
Login as the other member. Transfer it back.

Bonus: Working with two tabs, you can open the Transfer form and
in the meantime delete the list, the patron or remove the share.
Or change permissions. Submit after that and check the error message.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 58 Marcel de Rooy 2022-07-19 12:21:50 UTC
Created attachment 137866 [details] [review]
Bug 25498: (QA follow-up) Rename virtualshelfshare->patron

This patron is a 'sharee' (not the sharer).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 59 Marcel de Rooy 2022-07-19 12:21:55 UTC
Created attachment 137867 [details] [review]
Bug 25498: (follow-up) Change for autocomplete change of 30578

Test plan:
Check if the patron search still works on the intranet shelves form
when transferring ownership.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 60 Jonathan Druart 2022-07-19 12:59:19 UTC
Created attachment 137871 [details] [review]
Bug 25498: (follow-up) Change for autocomplete change of 30578

Test plan:
Check if the patron search still works on the intranet shelves form
when transferring ownership.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 61 Jonathan Druart 2022-07-19 13:00:38 UTC
Just wanted to test the last patch. All good.

A quick remark when testing: I think we need the "transfer" action available from the show list view (/virtualshelves/shelves.pl?op=view&shelfnumber=1). Not considering blocker however.
Comment 62 Marcel de Rooy 2022-07-20 11:47:42 UTC
Created attachment 137908 [details] [review]
Bug 25498: (follow-up) Show error message when no sharee has email

Test plan:
Share a list with a patron that has no email address.
Try to transfer ownership. Verify that you get the error message.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 63 Tomás Cohen Arazi 2022-07-20 12:09:42 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 64 Lucas Gass 2022-08-23 19:47:09 UTC
Enhancement will not be backported to 22.05.x series
Comment 65 Caroline Cyr La Rose 2023-04-14 18:05:43 UTC
Added to manual https://gitlab.com/koha-community/koha-manual/-/merge_requests/658