Bug 11889 - Option to keep public or shared lists when deleting patron
Summary: Option to keep public or shared lists when deleting patron
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Lists (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Aleisha Amohia
QA Contact: Marcel de Rooy
URL:
Keywords:
: 13844 28889 (view as bug list)
Depends on:
Blocks: 25498
  Show dependency treegraph
 
Reported: 2014-03-04 15:08 UTC by Marcel de Rooy
Modified: 2023-12-28 20:43 UTC (History)
16 users (show)

See Also:
Change sponsored?: 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 report adds the preference ListOwnershipUponPatronDeletion. It allows you to choose between the existing behavior of just deleting all lists when deleting a patron, or transfer ownership of public and shared lists to the staff member that deleted the patron. Follow-up reports will allow for even more flexibility.
Version(s) released in:
22.11.00


Attachments
Example code (831 bytes, text/plain)
2014-03-10 08:25 UTC, Marcel de Rooy
Details
Bug 11889: Disown a list upon deletion of owner (6.87 KB, patch)
2017-02-14 23:08 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 11889: Disown a list upon deletion of owner (6.96 KB, patch)
2017-02-16 09:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: [Follow-up] Remove share if you are the new owner (1.02 KB, patch)
2017-02-16 09:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: [QA Follow-up] Add entry to sysprefs.sql (2.88 KB, patch)
2017-02-16 09:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: [FOLLOW-UP] Tests and new get_shared_shelves subroutine (10.82 KB, patch)
2017-02-20 21:53 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 11889: Disown a list upon deletion of owner (6.92 KB, patch)
2017-03-20 14:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11889: [Follow-up] Remove share if you are the new owner (1000 bytes, patch)
2017-03-20 14:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11889: [QA Follow-up] Add entry to sysprefs.sql (2.84 KB, patch)
2017-03-20 14:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11889: [FOLLOW-UP] Tests and new get_shared_shelves subroutine (10.83 KB, patch)
2017-03-20 14:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11889: Disown a list upon deletion of owner (7.06 KB, patch)
2017-03-22 14:05 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11889: [Follow-up] Remove share if you are the new owner (1.12 KB, patch)
2017-03-22 14:05 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11889: [QA Follow-up] Add entry to sysprefs.sql (2.98 KB, patch)
2017-03-22 14:05 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11889: [FOLLOW-UP] Tests and new get_shared_shelves subroutine (11.02 KB, patch)
2017-03-22 14:05 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11889: Disown a list upon deletion of owner (7.14 KB, patch)
2020-08-14 08:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11889: [Follow-up] Remove share if you are the new owner (1.13 KB, patch)
2020-08-14 08:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11889: [QA Follow-up] Add entry to sysprefs.sql (2.96 KB, patch)
2020-08-14 08:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11889: [FOLLOW-UP] Tests and new get_shared_shelves subroutine (10.52 KB, patch)
2020-08-14 08:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11889: Disown a list upon deletion of owner (7.18 KB, patch)
2022-06-08 13:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (follow-up) Remove share if you are the new owner (1.17 KB, patch)
2022-06-08 13:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Add entry to sysprefs.sql (3.35 KB, patch)
2022-06-08 13:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (follow-up) Tests and new get_shared_shelves (10.37 KB, patch)
2022-06-08 13:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Add entry to sysprefs.sql (3.04 KB, patch)
2022-06-08 13:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (follow-up) Tests and new get_shared_shelves (10.37 KB, patch)
2022-06-08 13:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: Disown a list upon deletion of owner (6.41 KB, patch)
2022-06-08 14:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (follow-up) Remove share if you are the new owner (1.17 KB, patch)
2022-06-08 14:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Add entry to sysprefs.sql (1.89 KB, patch)
2022-06-08 14:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (follow-up) Tests and new get_shared_shelves (10.37 KB, patch)
2022-06-08 14:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Add POD ground work (1.66 KB, patch)
2022-06-08 14:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Convert to new atomicupdate style (1.29 KB, patch)
2022-06-08 14:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Changes for wantarray move in ->search (1.92 KB, patch)
2022-06-08 14:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Fix tests (3.29 KB, patch)
2022-06-08 14:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: Disown a list upon deletion of owner (6.41 KB, patch)
2022-06-09 09:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (follow-up) Remove share if you are the new owner (1.17 KB, patch)
2022-06-09 09:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Add entry to sysprefs.sql (1.89 KB, patch)
2022-06-09 09:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (follow-up) Tests and new get_shared_shelves (10.43 KB, patch)
2022-06-09 09:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Add POD ground work (1.66 KB, patch)
2022-06-09 09:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Convert to new atomicupdate style (1.29 KB, patch)
2022-06-09 09:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Changes for wantarray move in ->search (1.92 KB, patch)
2022-06-09 09:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Fix tests (3.55 KB, patch)
2022-06-09 09:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (follow-up) Get rid of FIXME in Koha::Patron (12.28 KB, patch)
2022-06-09 09:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (follow-up) We did not need get_shared_shelves after all (1.92 KB, patch)
2022-06-09 09:40 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: Disown a list upon deletion of owner (6.41 KB, patch)
2022-06-09 09:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (follow-up) Remove share if you are the new owner (1.17 KB, patch)
2022-06-09 09:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Add entry to sysprefs.sql (1.89 KB, patch)
2022-06-09 09:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (follow-up) Tests and new get_shared_shelves (10.43 KB, patch)
2022-06-09 09:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Add POD ground work (1.75 KB, patch)
2022-06-09 09:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Convert to new atomicupdate style (1.35 KB, patch)
2022-06-09 09:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Changes for wantarray move in ->search (1.97 KB, patch)
2022-06-09 09:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Fix tests (3.55 KB, patch)
2022-06-09 09:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (follow-up) Get rid of FIXME in Koha::Patron (12.34 KB, patch)
2022-06-09 09:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: (follow-up) We did not need get_shared_shelves after all (1.98 KB, patch)
2022-06-09 09:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11889: Disown a list upon deletion of owner (6.43 KB, patch)
2022-06-09 14:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11889: (follow-up) Remove share if you are the new owner (1.19 KB, patch)
2022-06-09 14:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Add entry to sysprefs.sql (1.91 KB, patch)
2022-06-09 14:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11889: (follow-up) Tests and new get_shared_shelves (10.46 KB, patch)
2022-06-09 14:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Add POD ground work (1.77 KB, patch)
2022-06-09 14:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Convert to new atomicupdate style (1.37 KB, patch)
2022-06-09 14:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Changes for wantarray move in ->search (2.00 KB, patch)
2022-06-09 14:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Fix tests (3.58 KB, patch)
2022-06-09 14:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11889: (follow-up) Get rid of FIXME in Koha::Patron (12.37 KB, patch)
2022-06-09 14:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11889: (follow-up) We did not need get_shared_shelves after all (2.00 KB, patch)
2022-06-09 14:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11889: (QA follow-up) Spelling (857 bytes, patch)
2022-06-09 14:58 UTC, Martin Renvoize
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 2014-03-04 15:08:12 UTC

    
Comment 1 Marcel de Rooy 2014-03-10 08:24:37 UTC
Please see bug 9032 also. One of the last patches on that report made Koha behavior consistent when deleting a list or deleting a patron.
But what if we would like to keep public lists or shared lists when deleting a patron that owned these lists?

We could add a syspref that controlled this. If enabled, we could allow owner NULL for public and shared lists if the patron is deleted, or if the patron deletes one of those lists. (Instead of the NULL value, we could also move them to a designated patron for this purpose.)

In the attachment some code that shows what should go into HandleDelBorrower. Similar changes should be made to DelShelf.

What do you think? Is this a welcome feature or is it confusing and should we just delete these lists?
Comment 2 Marcel de Rooy 2014-03-10 08:25:42 UTC
Created attachment 25972 [details]
Example code
Comment 3 Christopher Brannon 2015-03-18 14:23:24 UTC
(In reply to M. de Rooy from comment #1)
> Please see bug 9032 also. One of the last patches on that report made Koha
> behavior consistent when deleting a list or deleting a patron.
> But what if we would like to keep public lists or shared lists when deleting
> a patron that owned these lists?
> 
> We could add a syspref that controlled this. If enabled, we could allow
> owner NULL for public and shared lists if the patron is deleted, or if the
> patron deletes one of those lists. (Instead of the NULL value, we could also
> move them to a designated patron for this purpose.)
> 
> In the attachment some code that shows what should go into
> HandleDelBorrower. Similar changes should be made to DelShelf.
> 
> What do you think? Is this a welcome feature or is it confusing and should
> we just delete these lists?

Lists should not just automatically be deleted.  It should be a preference.  Some public lists created by a former staff account, for example.

Christopher
Comment 4 kholt 2015-03-19 16:38:36 UTC
It would be great to be able to keep public lists.  We had this problem when the staff person who maintained our Newbery Awards list retired and her staff account was deleted.  A system administrator should be able to transfer ownership of a list so that it can continue to be added to.
Comment 5 Aleisha Amohia 2017-02-14 23:08:08 UTC
Created attachment 60224 [details] [review]
Bug 11889: Disown a list upon deletion of owner

This patch adds a syspref 'ListOwnershipUponPatronDeletion' which
decides if all the owner's lists (public and private) are deleted
when the owner is deleted, or if the ownership of their public and
shared lists are transfered to the user who deleted the owner (private
lists that are not shared are always deleted).

To test:
1) Apply patch and update database
2) Set the ListOwnershipUponPatronDeletion syspref to 'Transfer...'
3) Set up a superlibrarian user
4) Log in as this superlibrarian user and set up three lists, a public
list (maybe named 'public'), and two private lists (named 'private' and
'shared')
5) Log in as this user to the opac (if you haven't already) and share
the 'shared' list with someone
6) View the virtualshelves table in mysql to see the three lists you
just created under your new user
7) Also view the virtualshelfshares table in mysql to see the 'shared'
list
8) Log into the staff client with your regular user (a different user to
the one you just created)
9) Search for the user you just created and delete them
10) Go to your lists
11) You should see the 'shared' list under your private lists, and their
'public' list under your public lists, both with you as the owner
12) if you view both the virtualshelves table and the virtualshelfshares
table in mysql again, youll see you are now the owner and the 'private'
list will no longer exist.
13) Set the ListOwnershipUponPatronDeletion syspref back to 'Delete...'
14) Repeat steps 3 - 10 again (it may help to delete the 'shared' and
'public' lists before you re-create them)
15) You should not see any of the lists you just made under private or
public lists this time. To confirm, check the virtualshelves table in
mysql and make sure that all three lists have been deleted.

Sponsored-by: Catalyst IT
Comment 6 Christopher Brannon 2017-02-15 15:27:23 UTC
*** Bug 13844 has been marked as a duplicate of this bug. ***
Comment 7 Marcel de Rooy 2017-02-16 09:41:15 UTC
Created attachment 60306 [details] [review]
Bug 11889: Disown a list upon deletion of owner

This patch adds a syspref 'ListOwnershipUponPatronDeletion' which
decides if all the owner's lists (public and private) are deleted
when the owner is deleted, or if the ownership of their public and
shared lists are transfered to the user who deleted the owner (private
lists that are not shared are always deleted).

To test:
1) Apply patch and update database
2) Set the ListOwnershipUponPatronDeletion syspref to 'Transfer...'
3) Set up a superlibrarian user
4) Log in as this superlibrarian user and set up three lists, a public
list (maybe named 'public'), and two private lists (named 'private' and
'shared')
5) Log in as this user to the opac (if you haven't already) and share
the 'shared' list with someone
6) View the virtualshelves table in mysql to see the three lists you
just created under your new user
7) Also view the virtualshelfshares table in mysql to see the 'shared'
list
8) Log into the staff client with your regular user (a different user to
the one you just created)
9) Search for the user you just created and delete them
10) Go to your lists
11) You should see the 'shared' list under your private lists, and their
'public' list under your public lists, both with you as the owner
12) if you view both the virtualshelves table and the virtualshelfshares
table in mysql again, youll see you are now the owner and the 'private'
list will no longer exist.
13) Set the ListOwnershipUponPatronDeletion syspref back to 'Delete...'
14) Repeat steps 3 - 10 again (it may help to delete the 'shared' and
'public' lists before you re-create them)
15) You should not see any of the lists you just made under private or
public lists this time. To confirm, check the virtualshelves table in
mysql and make sure that all three lists have been deleted.

Sponsored-by: Catalyst IT

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2017-02-16 09:41:21 UTC
Created attachment 60307 [details] [review]
Bug 11889: [Follow-up] Remove share if you are the new owner

This will probably be exceptional. But if the staff member already had
a share for a private list of the patron he deletes now, he will be
the new owner and we should remove the share record.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2017-02-16 09:41:25 UTC
Created attachment 60308 [details] [review]
Bug 11889: [QA Follow-up] Add entry to sysprefs.sql

Also changes the insert in the atomic update to an insert ignore, as
we generally do for inserting sysprefs (see updatedatabase).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2017-02-16 09:42:18 UTC
Thanks, Aleisha, for picking up this older bug.
Comment 11 Jonathan Druart 2017-02-16 11:08:15 UTC
Aleisha,
Several things here:
1. I think 
my @sharedlists = Koha::Virtualshelves->search({ 'me.owner' => $self->borrowernumber, 'me.shelfnumber' => { -ident => 'virtualshelfshares.shelfnumber' }  }, { prefetch => 'virtualshelfshares' })

Should be a new Koha::Virtualshelves method, like get_public_shelves and get_privates_shelves

2. The following FIXME should be kept
  # FIXME Could be $patron->get_lists

3. You should provide tests for this change.

Marked as Failed QA.
Comment 12 Aleisha Amohia 2017-02-20 21:53:06 UTC
Created attachment 60481 [details] [review]
Bug 11889: [FOLLOW-UP] Tests and new get_shared_shelves subroutine

To test:
1) prove t/db_dependent/Virtualshelves.t
2) prove t/db_dependent/Koha/Patrons.t

Sponsored-by: Catalyst IT
Comment 13 Eric 2017-03-20 14:44:09 UTC
I tried setting up the patch in a sandbox, but twice now I've gotten this email and i can't get to test it.

The sandbox you've requested is not ready.
Some problems occurred applying patches from bug 11889:
<h1>Something went wrong !</h1>Applying: Bug 11889: Disown a list upon deletion of owner
Applying: Bug 11889: [Follow-up] Remove share if you are the new owner
Applying: Bug 11889: [QA Follow-up] Add entry to sysprefs.sql
Applying: Bug 11889: [FOLLOW-UP] Tests and new get_shared_shelves subroutine
fatal: sha1 information is lacking or useless (Koha/Patron.pm).
Patch failed at 0001 Bug 11889: [FOLLOW-UP] Tests and new get_shared_shelves subroutine
Comment 14 Jonathan Druart 2017-03-20 14:53:09 UTC
Created attachment 61319 [details] [review]
Bug 11889: Disown a list upon deletion of owner

This patch adds a syspref 'ListOwnershipUponPatronDeletion' which
decides if all the owner's lists (public and private) are deleted
when the owner is deleted, or if the ownership of their public and
shared lists are transfered to the user who deleted the owner (private
lists that are not shared are always deleted).

To test:
1) Apply patch and update database
2) Set the ListOwnershipUponPatronDeletion syspref to 'Transfer...'
3) Set up a superlibrarian user
4) Log in as this superlibrarian user and set up three lists, a public
list (maybe named 'public'), and two private lists (named 'private' and
'shared')
5) Log in as this user to the opac (if you haven't already) and share
the 'shared' list with someone
6) View the virtualshelves table in mysql to see the three lists you
just created under your new user
7) Also view the virtualshelfshares table in mysql to see the 'shared'
list
8) Log into the staff client with your regular user (a different user to
the one you just created)
9) Search for the user you just created and delete them
10) Go to your lists
11) You should see the 'shared' list under your private lists, and their
'public' list under your public lists, both with you as the owner
12) if you view both the virtualshelves table and the virtualshelfshares
table in mysql again, youll see you are now the owner and the 'private'
list will no longer exist.
13) Set the ListOwnershipUponPatronDeletion syspref back to 'Delete...'
14) Repeat steps 3 - 10 again (it may help to delete the 'shared' and
'public' lists before you re-create them)
15) You should not see any of the lists you just made under private or
public lists this time. To confirm, check the virtualshelves table in
mysql and make sure that all three lists have been deleted.

Sponsored-by: Catalyst IT

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Jonathan Druart 2017-03-20 14:53:14 UTC
Created attachment 61320 [details] [review]
Bug 11889: [Follow-up] Remove share if you are the new owner

This will probably be exceptional. But if the staff member already had
a share for a private list of the patron he deletes now, he will be
the new owner and we should remove the share record.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Jonathan Druart 2017-03-20 14:53:19 UTC
Created attachment 61321 [details] [review]
Bug 11889: [QA Follow-up] Add entry to sysprefs.sql

Also changes the insert in the atomic update to an insert ignore, as
we generally do for inserting sysprefs (see updatedatabase).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 17 Jonathan Druart 2017-03-20 14:53:23 UTC
Created attachment 61322 [details] [review]
Bug 11889: [FOLLOW-UP] Tests and new get_shared_shelves subroutine

To test:
1) prove t/db_dependent/Virtualshelves.t
2) prove t/db_dependent/Koha/Patrons.t

Sponsored-by: Catalyst IT
Comment 18 Jonathan Druart 2017-03-20 14:53:59 UTC
(In reply to Eric from comment #13)
> I tried setting up the patch in a sandbox, but twice now I've gotten this
> email and i can't get to test it.

Hi Éric, I have rebased the patches, please try again.
Comment 19 Christopher Brannon 2017-03-20 15:22:30 UTC
What if I don't want ownership of a list.  Having ownership means I am responsible for and have to maintain the list.  Is there a way I can assign the list to someone else that can maintain it?
Comment 20 Eric 2017-03-20 19:33:07 UTC
Hello,

5) Log in as this user to the opac (if you haven't already) and *share*
the 'shared' list with someone

I will sign it off, but I think it needs to be specified that the list needs to have records in it so you can share. I tried testing it with empty lists but no sharing button would appear. Thanks !
Comment 21 Katrin Fischer 2017-03-20 20:59:54 UTC
I am having a bid of a stomach ache about the transfer of the shared lists. I feel they are still private - only shared with someone of your choice and should not go to library staff. I'd suggest to change the ownership to the first person the list was shared with instead.
Comment 22 Christopher Brannon 2017-03-20 21:41:51 UTC
(In reply to Katrin Fischer from comment #21)
> I am having a bid of a stomach ache about the transfer of the shared lists.
> I feel they are still private - only shared with someone of your choice and
> should not go to library staff. I'd suggest to change the ownership to the
> first person the list was shared with instead.

I agree that this needs to be handled a little better.  As I stated above, ownership shouldn't be thrown onto someone that doesn't want it.  There needs to be a method for changing the owner, other than deleting the owner.

I think the idea of changing ownership to the first person in the shared list is fine, but what if they don't want it?  Is there a way to hand it off?
Comment 23 Katrin Fischer 2017-03-21 06:41:14 UTC
At the moment you can't transfer list ownership - so the staff patron would get stuck with the lists of the deleted patron until they delete them. It would be good to have an option to do it, but I think that would be a new bug.
Comment 24 Marcel de Rooy 2017-03-21 07:47:00 UTC
(In reply to Christopher Brannon from comment #22)
> I agree that this needs to be handled a little better.  As I stated above,
> ownership shouldn't be thrown onto someone that doesn't want it.  There
> needs to be a method for changing the owner, other than deleting the owner.
> 
> I think the idea of changing ownership to the first person in the shared
> list is fine, but what if they don't want it?  Is there a way to hand it off?

Well, maybe we should return to the idea (from comment1 here) to have a designated user, perhaps a virtual staff member, to be the list owner. It could be a pref; if you do not fill it, the list gets deleted or is transferred to the current staff member (yet another option?).

Another bug should be: transfer ownership.
Comment 25 Katrin Fischer 2017-03-21 08:23:09 UTC
I'd prefer if we found a way to treat shared lists as private lists - only those who were invited should see them. If we have a designated user, the library can still easily access with the password they have given. I know it's all in the database anyway, but it adds a bit of more privacy. And database access can be somewhat restricted by the reports permissions.
Comment 26 Biblibre Sandboxes 2017-03-22 14:04:11 UTC
Patch tested with a sandbox, by Cédric Vita <cedric.vita@dracenie.com>
Comment 27 Biblibre Sandboxes 2017-03-22 14:05:06 UTC
Created attachment 61451 [details] [review]
Bug 11889: Disown a list upon deletion of owner

This patch adds a syspref 'ListOwnershipUponPatronDeletion' which
decides if all the owner's lists (public and private) are deleted
when the owner is deleted, or if the ownership of their public and
shared lists are transfered to the user who deleted the owner (private
lists that are not shared are always deleted).

To test:
1) Apply patch and update database
2) Set the ListOwnershipUponPatronDeletion syspref to 'Transfer...'
3) Set up a superlibrarian user
4) Log in as this superlibrarian user and set up three lists, a public
list (maybe named 'public'), and two private lists (named 'private' and
'shared')
5) Log in as this user to the opac (if you haven't already) and share
the 'shared' list with someone
6) View the virtualshelves table in mysql to see the three lists you
just created under your new user
7) Also view the virtualshelfshares table in mysql to see the 'shared'
list
8) Log into the staff client with your regular user (a different user to
the one you just created)
9) Search for the user you just created and delete them
10) Go to your lists
11) You should see the 'shared' list under your private lists, and their
'public' list under your public lists, both with you as the owner
12) if you view both the virtualshelves table and the virtualshelfshares
table in mysql again, youll see you are now the owner and the 'private'
list will no longer exist.
13) Set the ListOwnershipUponPatronDeletion syspref back to 'Delete...'
14) Repeat steps 3 - 10 again (it may help to delete the 'shared' and
'public' lists before you re-create them)
15) You should not see any of the lists you just made under private or
public lists this time. To confirm, check the virtualshelves table in
mysql and make sure that all three lists have been deleted.

Sponsored-by: Catalyst IT

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 28 Biblibre Sandboxes 2017-03-22 14:05:11 UTC
Created attachment 61452 [details] [review]
Bug 11889: [Follow-up] Remove share if you are the new owner

This will probably be exceptional. But if the staff member already had
a share for a private list of the patron he deletes now, he will be
the new owner and we should remove the share record.

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 29 Biblibre Sandboxes 2017-03-22 14:05:15 UTC
Created attachment 61453 [details] [review]
Bug 11889: [QA Follow-up] Add entry to sysprefs.sql

Also changes the insert in the atomic update to an insert ignore, as
we generally do for inserting sysprefs (see updatedatabase).

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 30 Biblibre Sandboxes 2017-03-22 14:05:38 UTC
Created attachment 61454 [details] [review]
Bug 11889: [FOLLOW-UP] Tests and new get_shared_shelves subroutine

To test:
1) prove t/db_dependent/Virtualshelves.t
2) prove t/db_dependent/Koha/Patrons.t

Sponsored-by: Catalyst IT

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 31 Marcel de Rooy 2017-03-22 14:24:32 UTC
Thx for your signoff, Cedric.
The comments earlier from Christopher and Katrin still need attention.
Comment 32 Christopher Brannon 2017-03-22 14:50:10 UTC
(In reply to Katrin Fischer from comment #23)
> At the moment you can't transfer list ownership - so the staff patron would
> get stuck with the lists of the deleted patron until they delete them. It
> would be good to have an option to do it, but I think that would be a new
> bug.

The subject of this bug is to disown a list.  The heart of this bug is to transfer ownership of a list.  I think this patch needs to focus on this topic as a whole and not as a piece.  Transferring ownership because the original owner is deleted is only a piece of this puzzle, and if it only focuses on that one piece, a lot of people are going to be annoyed when ownership is transferred and there isn't much that can be done about it besides deleting it.  I think a mechanism needs to be in place to move the owner before or during deletion.

Perhaps what can be done is to setup a button on the list to transfer ownership.  If it is the patron, they can have the option to transfer it to someone it is already shared with.  They get to pick.  And it is a request to the new patron in which they have to accept or decline, with the ability to add a quick note.  If they accept, ownership is transferred.  If they decline, the owner is notified.

I think a superlibrarian should also have the ability to transfer, with the same options, but they get the ability to chose any patron to send the request to.

Another approach for patrons being deleted with shared lists is to have the disowned list be listed in a group of unowned shared lists.  A transfer does not necessarily have to occur when the patron/staff account is deleted.  Then a superlibrarian or staff with appropriate permissions can use the transfer tool to ask someone if they will take ownership, or they can choose to delete it if they decide the list does not need to be kept.

Another question arises...does a shared list have to be owned?

If a list is shared or public, couldn't it remain unowned?  Maybe it is a part of an unowned list, in which a superlibrarian could either transfer ownership, or discard if needed.  But if the sole purpose of a list is to be public, does it have to have an owner to keep being public?  Of course it does if it is being modified.  Otherwise, if there is just some way to get to and manage unowned lists, do they have to reattach to someone to keep being public/shared?
Comment 33 Marcel de Rooy 2017-03-27 07:08:22 UTC
(In reply to Christopher Brannon from comment #32)
> I think this patch needs to focus on this topic as a whole and not as a
> piece.
I think we can't solve all issues on one report. So we have to agree on the goal we want to reach, but do it step by step.

> Perhaps what can be done is to setup a button on the list to transfer
> ownership.  
Would be a nice feature.

> I think a superlibrarian should also have the ability to transfer, with the
> same options, but they get the ability to chose any patron to send the
> request to.
Why exactly? The privacy discussion comes up here..

> Another approach for patrons being deleted with shared lists is to have the
> disowned list be listed in a group of unowned shared lists.
> Another question arises...does a shared list have to be owned?
> If a list is shared or public, couldn't it remain unowned? 
The lists module has not really been designed that way.
The option that I mentioned already a few times, is a compromize in this direction: create a fake staff user that owns all these lists; could be a pref.
Comment 34 Katrin Fischer 2017-03-27 07:49:52 UTC
I think there is a different understanding in shared and shared with public - shared means shared with selected other users by invitation. For those privacy applies. For the public/shared with public, I think the idea of having a general inheriting user is fine.
Comment 35 Marcel de Rooy 2017-03-27 08:07:37 UTC
(In reply to Katrin Fischer from comment #34)
> For the public/shared with public, I think the idea of
> having a general inheriting user is fine.

If we would have consensus about that, we could perhaps apply the following algorithm to public lists now ? Would this provide sufficient flexibility? 

When deleting a public list,
if pref GeneralPublicListOwner[?] is a specific userid then
  move ownership to that user
elsif pref == current user then
  move to current staff member
else
  delete list
endif

And open another report to handle shared lists. And another one for transferring ownership?
Comment 36 Katrin Fischer 2017-03-27 08:39:58 UTC
+1 for moving shared lists and transfer to another bug report.

We could use the Anonymous patron - not sure if that would make sense. Otherwise for me that sounds fine - my only worries are only about the shared ones.
Comment 37 Marcel de Rooy 2018-07-12 13:21:17 UTC
Anyone having an idea how we can get this running again ?
Comment 38 Katrin Fischer 2018-07-14 11:38:00 UTC
I am ok with your suggestion in comment#35. I was only suggesting to maybe reuse the Anonymous patron.
Comment 39 David Cook 2020-05-14 04:55:13 UTC
(In reply to kholt from comment #4)
> It would be great to be able to keep public lists.  We had this problem when
> the staff person who maintained our Newbery Awards list retired and her
> staff account was deleted.  A system administrator should be able to
> transfer ownership of a list so that it can continue to be added to.

I 100% agree. We've had someone retire and we need to change the owner of the list.

As a vendor, usually I would just do a backend database update to do this, but I don't have remote access to this Koha, so that's a problem.

I've opened an issue specifically about this topic:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25498

I'm not planning on working on this right now, but maybe someone else will.
Comment 40 Jonathan Druart 2020-08-14 08:26:41 UTC
(In reply to Marcel de Rooy from comment #35)
> (In reply to Katrin Fischer from comment #34)
> > For the public/shared with public, I think the idea of
> > having a general inheriting user is fine.
> 
> If we would have consensus about that, we could perhaps apply the following
> algorithm to public lists now ? Would this provide sufficient flexibility? 
> 
> When deleting a public list,
> if pref GeneralPublicListOwner[?] is a specific userid then
>   move ownership to that user
> elsif pref == current user then
>   move to current staff member
> else
>   delete list
> endif
> 
> And open another report to handle shared lists. And another one for
> transferring ownership?

Late in the party...
Yes Marcel I agree with that. Are you still willing to work on it?
Comment 41 Jonathan Druart 2020-08-14 08:31:53 UTC
Created attachment 108244 [details] [review]
Bug 11889: Disown a list upon deletion of owner

This patch adds a syspref 'ListOwnershipUponPatronDeletion' which
decides if all the owner's lists (public and private) are deleted
when the owner is deleted, or if the ownership of their public and
shared lists are transfered to the user who deleted the owner (private
lists that are not shared are always deleted).

To test:
1) Apply patch and update database
2) Set the ListOwnershipUponPatronDeletion syspref to 'Transfer...'
3) Set up a superlibrarian user
4) Log in as this superlibrarian user and set up three lists, a public
list (maybe named 'public'), and two private lists (named 'private' and
'shared')
5) Log in as this user to the opac (if you haven't already) and share
the 'shared' list with someone
6) View the virtualshelves table in mysql to see the three lists you
just created under your new user
7) Also view the virtualshelfshares table in mysql to see the 'shared'
list
8) Log into the staff client with your regular user (a different user to
the one you just created)
9) Search for the user you just created and delete them
10) Go to your lists
11) You should see the 'shared' list under your private lists, and their
'public' list under your public lists, both with you as the owner
12) if you view both the virtualshelves table and the virtualshelfshares
table in mysql again, youll see you are now the owner and the 'private'
list will no longer exist.
13) Set the ListOwnershipUponPatronDeletion syspref back to 'Delete...'
14) Repeat steps 3 - 10 again (it may help to delete the 'shared' and
'public' lists before you re-create them)
15) You should not see any of the lists you just made under private or
public lists this time. To confirm, check the virtualshelves table in
mysql and make sure that all three lists have been deleted.

Sponsored-by: Catalyst IT

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 42 Jonathan Druart 2020-08-14 08:32:01 UTC
Created attachment 108245 [details] [review]
Bug 11889: [Follow-up] Remove share if you are the new owner

This will probably be exceptional. But if the staff member already had
a share for a private list of the patron he deletes now, he will be
the new owner and we should remove the share record.

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 43 Jonathan Druart 2020-08-14 08:32:09 UTC
Created attachment 108246 [details] [review]
Bug 11889: [QA Follow-up] Add entry to sysprefs.sql

Also changes the insert in the atomic update to an insert ignore, as
we generally do for inserting sysprefs (see updatedatabase).

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 44 Jonathan Druart 2020-08-14 08:32:16 UTC
Created attachment 108247 [details] [review]
Bug 11889: [FOLLOW-UP] Tests and new get_shared_shelves subroutine

To test:
1) prove t/db_dependent/Virtualshelves.t
2) prove t/db_dependent/Koha/Patrons.t

Sponsored-by: Catalyst IT

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 45 Marcel de Rooy 2020-08-17 07:24:14 UTC
(In reply to Jonathan Druart from comment #40)
> Late in the party...
> Yes Marcel I agree with that. Are you still willing to work on it?

Well, I am still interested in getting this further ;)
Comment 46 Christopher Brannon 2021-08-20 20:08:52 UTC
*** Bug 28889 has been marked as a duplicate of this bug. ***
Comment 47 Christopher Brannon 2021-08-20 20:15:51 UTC
I'm confused.  If this is signed off, why is it in discussion.  Is there a reason it isn't going through QA?
Comment 48 Marcel de Rooy 2022-06-08 13:47:30 UTC
Created attachment 135808 [details] [review]
Bug 11889: Disown a list upon deletion of owner

This patch adds a syspref 'ListOwnershipUponPatronDeletion' which
decides if all the owner's lists (public and private) are deleted
when the owner is deleted, or if the ownership of their public and
shared lists are transfered to the user who deleted the owner (private
lists that are not shared are always deleted).

To test:
1) Apply patch and update database
2) Set the ListOwnershipUponPatronDeletion syspref to 'Transfer...'
3) Set up a superlibrarian user
4) Log in as this superlibrarian user and set up three lists, a public
list (maybe named 'public'), and two private lists (named 'private' and
'shared')
5) Log in as this user to the opac (if you haven't already) and share
the 'shared' list with someone
6) View the virtualshelves table in mysql to see the three lists you
just created under your new user
7) Also view the virtualshelfshares table in mysql to see the 'shared'
list
8) Log into the staff client with your regular user (a different user to
the one you just created)
9) Search for the user you just created and delete them
10) Go to your lists
11) You should see the 'shared' list under your private lists, and their
'public' list under your public lists, both with you as the owner
12) if you view both the virtualshelves table and the virtualshelfshares
table in mysql again, youll see you are now the owner and the 'private'
list will no longer exist.
13) Set the ListOwnershipUponPatronDeletion syspref back to 'Delete...'
14) Repeat steps 3 - 10 again (it may help to delete the 'shared' and
'public' lists before you re-create them)
15) You should not see any of the lists you just made under private or
public lists this time. To confirm, check the virtualshelves table in
mysql and make sure that all three lists have been deleted.

Sponsored-by: Catalyst IT

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 49 Marcel de Rooy 2022-06-08 13:47:34 UTC
Created attachment 135809 [details] [review]
Bug 11889: (follow-up) Remove share if you are the new owner

This will probably be exceptional. But if the staff member already had
a share for a private list of the patron he deletes now, he will be
the new owner and we should remove the share record.

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 50 Marcel de Rooy 2022-06-08 13:47:38 UTC
Created attachment 135810 [details] [review]
Bug 11889: (QA follow-up) Add entry to sysprefs.sql

Also changes the insert in the atomic update to an insert ignore, as
we generally do for inserting sysprefs (see updatedatabase).

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 51 Marcel de Rooy 2022-06-08 13:47:43 UTC
Created attachment 135811 [details] [review]
Bug 11889: (follow-up) Tests and new get_shared_shelves

This is a manual reconstruction of an older patch, that git
did not want to apply anymore (sha trouble etc.)
Comment 52 Marcel de Rooy 2022-06-08 13:49:37 UTC
Created attachment 135812 [details] [review]
Bug 11889: (QA follow-up) Add entry to sysprefs.sql

Also changes the insert in the atomic update to an insert ignore, as
we generally do for inserting sysprefs (see updatedatabase).

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 53 Marcel de Rooy 2022-06-08 13:49:42 UTC
Created attachment 135813 [details] [review]
Bug 11889: (follow-up) Tests and new get_shared_shelves

This is a manual reconstruction of an older patch, that git
did not want to apply anymore (sha trouble etc.)
Comment 54 Marcel de Rooy 2022-06-08 13:50:46 UTC
Trying to get this a bit further again ;)
Comment 55 Marcel de Rooy 2022-06-08 14:22:38 UTC
As expected, bitten by 28959 which on itself needs a fix !
Comment 56 Marcel de Rooy 2022-06-08 14:50:27 UTC
The method Koha::Virtualshelves->owner is not covered by tests!

=> Bug 29844
Comment 57 Marcel de Rooy 2022-06-08 14:56:57 UTC
Created attachment 135814 [details] [review]
Bug 11889: Disown a list upon deletion of owner

This patch adds a syspref 'ListOwnershipUponPatronDeletion' which
decides if all the owner's lists (public and private) are deleted
when the owner is deleted, or if the ownership of their public and
shared lists are transfered to the user who deleted the owner (private
lists that are not shared are always deleted).

To test:
1) Apply patch and update database
2) Set the ListOwnershipUponPatronDeletion syspref to 'Transfer...'
3) Set up a superlibrarian user
4) Log in as this superlibrarian user and set up three lists, a public
list (maybe named 'public'), and two private lists (named 'private' and
'shared')
5) Log in as this user to the opac (if you haven't already) and share
the 'shared' list with someone
6) View the virtualshelves table in mysql to see the three lists you
just created under your new user
7) Also view the virtualshelfshares table in mysql to see the 'shared'
list
8) Log into the staff client with your regular user (a different user to
the one you just created)
9) Search for the user you just created and delete them
10) Go to your lists
11) You should see the 'shared' list under your private lists, and their
'public' list under your public lists, both with you as the owner
12) if you view both the virtualshelves table and the virtualshelfshares
table in mysql again, youll see you are now the owner and the 'private'
list will no longer exist.
13) Set the ListOwnershipUponPatronDeletion syspref back to 'Delete...'
14) Repeat steps 3 - 10 again (it may help to delete the 'shared' and
'public' lists before you re-create them)
15) You should not see any of the lists you just made under private or
public lists this time. To confirm, check the virtualshelves table in
mysql and make sure that all three lists have been deleted.

Sponsored-by: Catalyst IT

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2022-06-08 Removed old atomic update. See new follow-up.
Comment 58 Marcel de Rooy 2022-06-08 14:57:02 UTC
Created attachment 135815 [details] [review]
Bug 11889: (follow-up) Remove share if you are the new owner

This will probably be exceptional. But if the staff member already had
a share for a private list of the patron he deletes now, he will be
the new owner and we should remove the share record.

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 59 Marcel de Rooy 2022-06-08 14:57:06 UTC
Created attachment 135816 [details] [review]
Bug 11889: (QA follow-up) Add entry to sysprefs.sql

Also changes the insert in the atomic update to an insert ignore, as
we generally do for inserting sysprefs (see updatedatabase).

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 60 Marcel de Rooy 2022-06-08 14:57:11 UTC
Created attachment 135817 [details] [review]
Bug 11889: (follow-up) Tests and new get_shared_shelves

This is a manual reconstruction of an older patch, that git
did not want to apply anymore (sha trouble etc.)
Comment 61 Marcel de Rooy 2022-06-08 14:57:16 UTC
Created attachment 135818 [details] [review]
Bug 11889: (QA follow-up) Add POD ground work
Comment 62 Marcel de Rooy 2022-06-08 14:57:20 UTC
Created attachment 135819 [details] [review]
Bug 11889: (QA follow-up) Convert to new atomicupdate style

The first patch contained an old .sql atomic update.
Comment 63 Marcel de Rooy 2022-06-08 14:57:25 UTC
Created attachment 135820 [details] [review]
Bug 11889: (QA follow-up) Changes for wantarray move in ->search

See bug 29844.
Comment 64 Marcel de Rooy 2022-06-08 14:57:30 UTC
Created attachment 135821 [details] [review]
Bug 11889: (QA follow-up) Fix tests

Noise from 28959 changing category to public.
Comment 65 Marcel de Rooy 2022-06-08 14:58:09 UTC
Please wait. Not ready yet.
Comment 66 Marcel de Rooy 2022-06-09 09:39:09 UTC
Created attachment 135857 [details] [review]
Bug 11889: Disown a list upon deletion of owner

This patch adds a syspref 'ListOwnershipUponPatronDeletion' which
decides if all the owner's lists (public and private) are deleted
when the owner is deleted, or if the ownership of their public and
shared lists are transfered to the user who deleted the owner (private
lists that are not shared are always deleted).

To test:
1) Apply patch and update database
2) Set the ListOwnershipUponPatronDeletion syspref to 'Transfer...'
3) Set up a superlibrarian user
4) Log in as this superlibrarian user and set up three lists, a public
list (maybe named 'public'), and two private lists (named 'private' and
'shared')
5) Log in as this user to the opac (if you haven't already) and share
the 'shared' list with someone
6) View the virtualshelves table in mysql to see the three lists you
just created under your new user
7) Also view the virtualshelfshares table in mysql to see the 'shared'
list
8) Log into the staff client with your regular user (a different user to
the one you just created)
9) Search for the user you just created and delete them
10) Go to your lists
11) You should see the 'shared' list under your private lists, and their
'public' list under your public lists, both with you as the owner
12) if you view both the virtualshelves table and the virtualshelfshares
table in mysql again, youll see you are now the owner and the 'private'
list will no longer exist.
13) Set the ListOwnershipUponPatronDeletion syspref back to 'Delete...'
14) Repeat steps 3 - 10 again (it may help to delete the 'shared' and
'public' lists before you re-create them)
15) You should not see any of the lists you just made under private or
public lists this time. To confirm, check the virtualshelves table in
mysql and make sure that all three lists have been deleted.

Sponsored-by: Catalyst IT

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2022-06-08 Removed old atomic update. See new follow-up.
Comment 67 Marcel de Rooy 2022-06-09 09:39:14 UTC
Created attachment 135858 [details] [review]
Bug 11889: (follow-up) Remove share if you are the new owner

This will probably be exceptional. But if the staff member already had
a share for a private list of the patron he deletes now, he will be
the new owner and we should remove the share record.

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 68 Marcel de Rooy 2022-06-09 09:39:19 UTC
Created attachment 135859 [details] [review]
Bug 11889: (QA follow-up) Add entry to sysprefs.sql

Also changes the insert in the atomic update to an insert ignore, as
we generally do for inserting sysprefs (see updatedatabase).

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 69 Marcel de Rooy 2022-06-09 09:39:24 UTC
Created attachment 135860 [details] [review]
Bug 11889: (follow-up) Tests and new get_shared_shelves

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
This is a manual reconstruction of an older patch, that git
did not want to apply anymore (sha trouble etc.)
Comment 70 Marcel de Rooy 2022-06-09 09:39:33 UTC
Created attachment 135861 [details] [review]
Bug 11889: (QA follow-up) Add POD ground work
Comment 71 Marcel de Rooy 2022-06-09 09:39:40 UTC
Created attachment 135862 [details] [review]
Bug 11889: (QA follow-up) Convert to new atomicupdate style

The first patch contained an old .sql atomic update.
Comment 72 Marcel de Rooy 2022-06-09 09:39:45 UTC
Created attachment 135863 [details] [review]
Bug 11889: (QA follow-up) Changes for wantarray move in ->search

See bug 29844.
Comment 73 Marcel de Rooy 2022-06-09 09:39:52 UTC
Created attachment 135864 [details] [review]
Bug 11889: (QA follow-up) Fix tests

We need some changes for 28959 changing category to public.
Added the userenv mocking.

Note: A follow-up does a bit more refactoring here. But they
should pass at least now.

Test plan:
Run t/db_dependent/Virtualshelves.t
Run t/db_dependent/Koha/Patrons.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 74 Marcel de Rooy 2022-06-09 09:39:56 UTC
Created attachment 135865 [details] [review]
Bug 11889: (follow-up) Get rid of FIXME in Koha::Patron

Instead of get_lists, I added ->virtualshelves.

I removed the addition of the suffix to the shelf name here; if we
really want that (doubts), we should do it pref based. Removing a
staff member with public lists would result in weird names with
shelf numbers on the OPAC. (Should be very easy to restore.)

Used this opportunity to refactor the patron->delete subtest a bit:
switching to build_object, removing the 'retrieved patrons',
differentiating between staff patron and patron for sharing,
adding a few tests.

Test plan:
Enable transfer ownership. Delete a patron with lists in interface.
Check what happened.
Run t/db_dependent/Koha/Patrons.t.
Comment 75 Marcel de Rooy 2022-06-09 09:40:01 UTC
Created attachment 135866 [details] [review]
Bug 11889: (follow-up) We did not need get_shared_shelves after all

Test plan:
Run t/db_dependent/Virtualshelves.t
Comment 76 Marcel de Rooy 2022-06-09 09:48:19 UTC
Created attachment 135867 [details] [review]
Bug 11889: Disown a list upon deletion of owner

This patch adds a syspref 'ListOwnershipUponPatronDeletion' which
decides if all the owner's lists (public and private) are deleted
when the owner is deleted, or if the ownership of their public and
shared lists are transfered to the user who deleted the owner (private
lists that are not shared are always deleted).

To test:
1) Apply patch and update database
2) Set the ListOwnershipUponPatronDeletion syspref to 'Transfer...'
3) Set up a superlibrarian user
4) Log in as this superlibrarian user and set up three lists, a public
list (maybe named 'public'), and two private lists (named 'private' and
'shared')
5) Log in as this user to the opac (if you haven't already) and share
the 'shared' list with someone
6) View the virtualshelves table in mysql to see the three lists you
just created under your new user
7) Also view the virtualshelfshares table in mysql to see the 'shared'
list
8) Log into the staff client with your regular user (a different user to
the one you just created)
9) Search for the user you just created and delete them
10) Go to your lists
11) You should see the 'shared' list under your private lists, and their
'public' list under your public lists, both with you as the owner
12) if you view both the virtualshelves table and the virtualshelfshares
table in mysql again, youll see you are now the owner and the 'private'
list will no longer exist.
13) Set the ListOwnershipUponPatronDeletion syspref back to 'Delete...'
14) Repeat steps 3 - 10 again (it may help to delete the 'shared' and
'public' lists before you re-create them)
15) You should not see any of the lists you just made under private or
public lists this time. To confirm, check the virtualshelves table in
mysql and make sure that all three lists have been deleted.

Sponsored-by: Catalyst IT

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2022-06-08 Removed old atomic update. See new follow-up.
Comment 77 Marcel de Rooy 2022-06-09 09:48:23 UTC
Created attachment 135868 [details] [review]
Bug 11889: (follow-up) Remove share if you are the new owner

This will probably be exceptional. But if the staff member already had
a share for a private list of the patron he deletes now, he will be
the new owner and we should remove the share record.

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 78 Marcel de Rooy 2022-06-09 09:48:28 UTC
Created attachment 135869 [details] [review]
Bug 11889: (QA follow-up) Add entry to sysprefs.sql

Also changes the insert in the atomic update to an insert ignore, as
we generally do for inserting sysprefs (see updatedatabase).

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

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Comment 79 Marcel de Rooy 2022-06-09 09:48:33 UTC
Created attachment 135870 [details] [review]
Bug 11889: (follow-up) Tests and new get_shared_shelves

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
This is a manual reconstruction of an older patch, that git
did not want to apply anymore (sha trouble etc.)
Comment 80 Marcel de Rooy 2022-06-09 09:48:38 UTC
Created attachment 135871 [details] [review]
Bug 11889: (QA follow-up) Add POD ground work

Make QA tools a fraction happier ;)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 81 Marcel de Rooy 2022-06-09 09:48:43 UTC
Created attachment 135872 [details] [review]
Bug 11889: (QA follow-up) Convert to new atomicupdate style

The first patch contained an old .sql atomic update.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 82 Marcel de Rooy 2022-06-09 09:48:48 UTC
Created attachment 135873 [details] [review]
Bug 11889: (QA follow-up) Changes for wantarray move in ->search

See bug 29844.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 83 Marcel de Rooy 2022-06-09 09:48:54 UTC
Created attachment 135874 [details] [review]
Bug 11889: (QA follow-up) Fix tests

We need some changes for 28959 changing category to public.
Added the userenv mocking.

Note: A follow-up does a bit more refactoring here. But they
should pass at least now.

Test plan:
Run t/db_dependent/Virtualshelves.t
Run t/db_dependent/Koha/Patrons.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 84 Marcel de Rooy 2022-06-09 09:48:59 UTC
Created attachment 135875 [details] [review]
Bug 11889: (follow-up) Get rid of FIXME in Koha::Patron

Instead of get_lists, I added ->virtualshelves.

I removed the addition of the suffix to the shelf name here; if we
really want that (doubts), we should do it pref based. Removing a
staff member with public lists would result in weird names with
shelf numbers on the OPAC. (Should be very easy to restore.)

Used this opportunity to refactor the patron->delete subtest a bit:
switching to build_object, removing the 'retrieved patrons',
differentiating between staff patron and patron for sharing,
adding a few tests.

Test plan:
Enable transfer ownership. Delete a patron with lists in interface.
Check what happened.
Run t/db_dependent/Koha/Patrons.t.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 85 Marcel de Rooy 2022-06-09 09:49:04 UTC
Created attachment 135876 [details] [review]
Bug 11889: (follow-up) We did not need get_shared_shelves after all

Test plan:
Run t/db_dependent/Virtualshelves.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 86 Marcel de Rooy 2022-06-09 10:06:24 UTC
This was reported by myself in 2014 !
And Aleisha did a brave attempt to get this in in 2017. And then we started a discussion that kept this development in permafrost until now. And it will probably will stay there if we want to address all issues reported here.

I would really love to get this further now. So I would propose the following:

[1] Aleisha has 2 patches here, rest I added. There was already a signoff but since I added and refreshed it now, I take the signoff and ask for another QAer to have a look. Martin or Jonathan perhaps?

[2] This development allows you to choose between current behavior or move ownership of public and shared lists to the staff member deleting a user. If you dont like transferring ownership like this, do not yet enable the pref. The fact that we have this pref, allows us to push this on its own.

Important: note that deleting users with cron in an 'anonymized workflow' will still lead to deleting lists. We should still address that on a new report!
Bug 30933. Allowing to designate a special user as owner.

[3] We still need a separate transfer ownership feature. Will try to add some code on bug 25498. 
With [2] and [3] I think we covered all concerns in the discussion here, providing a foundation for several workflows with or without specially designated users for general lists.
Comment 87 Marcel de Rooy 2022-06-09 11:33:30 UTC
Changed title. This better describes what we are actually doing here.
Comment 88 Martin Renvoize 2022-06-09 14:57:40 UTC
Created attachment 135891 [details] [review]
Bug 11889: Disown a list upon deletion of owner

This patch adds a syspref 'ListOwnershipUponPatronDeletion' which
decides if all the owner's lists (public and private) are deleted
when the owner is deleted, or if the ownership of their public and
shared lists are transfered to the user who deleted the owner (private
lists that are not shared are always deleted).

To test:
1) Apply patch and update database
2) Set the ListOwnershipUponPatronDeletion syspref to 'Transfer...'
3) Set up a superlibrarian user
4) Log in as this superlibrarian user and set up three lists, a public
list (maybe named 'public'), and two private lists (named 'private' and
'shared')
5) Log in as this user to the opac (if you haven't already) and share
the 'shared' list with someone
6) View the virtualshelves table in mysql to see the three lists you
just created under your new user
7) Also view the virtualshelfshares table in mysql to see the 'shared'
list
8) Log into the staff client with your regular user (a different user to
the one you just created)
9) Search for the user you just created and delete them
10) Go to your lists
11) You should see the 'shared' list under your private lists, and their
'public' list under your public lists, both with you as the owner
12) if you view both the virtualshelves table and the virtualshelfshares
table in mysql again, youll see you are now the owner and the 'private'
list will no longer exist.
13) Set the ListOwnershipUponPatronDeletion syspref back to 'Delete...'
14) Repeat steps 3 - 10 again (it may help to delete the 'shared' and
'public' lists before you re-create them)
15) You should not see any of the lists you just made under private or
public lists this time. To confirm, check the virtualshelves table in
mysql and make sure that all three lists have been deleted.

Sponsored-by: Catalyst IT
Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2022-06-08 Removed old atomic update. See new follow-up.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 89 Martin Renvoize 2022-06-09 14:57:46 UTC
Created attachment 135892 [details] [review]
Bug 11889: (follow-up) Remove share if you are the new owner

This will probably be exceptional. But if the staff member already had
a share for a private list of the patron he deletes now, he will be
the new owner and we should remove the share record.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 90 Martin Renvoize 2022-06-09 14:57:52 UTC
Created attachment 135893 [details] [review]
Bug 11889: (QA follow-up) Add entry to sysprefs.sql

Also changes the insert in the atomic update to an insert ignore, as
we generally do for inserting sysprefs (see updatedatabase).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 91 Martin Renvoize 2022-06-09 14:57:57 UTC
Created attachment 135894 [details] [review]
Bug 11889: (follow-up) Tests and new get_shared_shelves

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
This is a manual reconstruction of an older patch, that git
did not want to apply anymore (sha trouble etc.)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 92 Martin Renvoize 2022-06-09 14:58:03 UTC
Created attachment 135895 [details] [review]
Bug 11889: (QA follow-up) Add POD ground work

Make QA tools a fraction happier ;)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 93 Martin Renvoize 2022-06-09 14:58:11 UTC
Created attachment 135896 [details] [review]
Bug 11889: (QA follow-up) Convert to new atomicupdate style

The first patch contained an old .sql atomic update.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 94 Martin Renvoize 2022-06-09 14:58:17 UTC
Created attachment 135897 [details] [review]
Bug 11889: (QA follow-up) Changes for wantarray move in ->search

See bug 29844.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 95 Martin Renvoize 2022-06-09 14:58:24 UTC
Created attachment 135898 [details] [review]
Bug 11889: (QA follow-up) Fix tests

We need some changes for 28959 changing category to public.
Added the userenv mocking.

Note: A follow-up does a bit more refactoring here. But they
should pass at least now.

Test plan:
Run t/db_dependent/Virtualshelves.t
Run t/db_dependent/Koha/Patrons.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 96 Martin Renvoize 2022-06-09 14:58:30 UTC
Created attachment 135899 [details] [review]
Bug 11889: (follow-up) Get rid of FIXME in Koha::Patron

Instead of get_lists, I added ->virtualshelves.

I removed the addition of the suffix to the shelf name here; if we
really want that (doubts), we should do it pref based. Removing a
staff member with public lists would result in weird names with
shelf numbers on the OPAC. (Should be very easy to restore.)

Used this opportunity to refactor the patron->delete subtest a bit:
switching to build_object, removing the 'retrieved patrons',
differentiating between staff patron and patron for sharing,
adding a few tests.

Test plan:
Enable transfer ownership. Delete a patron with lists in interface.
Check what happened.
Run t/db_dependent/Koha/Patrons.t.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 97 Martin Renvoize 2022-06-09 14:58:37 UTC
Created attachment 135900 [details] [review]
Bug 11889: (follow-up) We did not need get_shared_shelves after all

Test plan:
Run t/db_dependent/Virtualshelves.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 98 Martin Renvoize 2022-06-09 14:58:46 UTC
Created attachment 135901 [details] [review]
Bug 11889: (QA follow-up) Spelling

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 99 Martin Renvoize 2022-06-09 15:01:50 UTC
I'm in agreement with Marcel here.. lets be pragmatic and get this moving.

Whilst there's lots of patches here, the final diff is actually small and well formed.. lots of good golfing.

I think 'virtualshelves' is the wrong naming for the Koha:: namespace really.. but that's not new here and I think should be handled in it's own bug.. 'Koha::Biblio::Lists' or 'Koha::Lists::Biblios' perhaps.

After all that, the QA scripts is passing, tests are happy and I think this is a solid improvement.

Passing QA
Comment 100 Marcel de Rooy 2022-06-10 06:04:37 UTC
(In reply to Martin Renvoize from comment #99)
> 
> Passing QA

Great! Thx
Comment 101 Tomás Cohen Arazi 2022-07-20 12:09:34 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 102 Lucas Gass 2022-08-23 19:46:20 UTC
Enhancement will not be backported to the 22.05.x series