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?
Created attachment 25972 [details] Example code
(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
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.
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
*** Bug 13844 has been marked as a duplicate of this bug. ***
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>
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>
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>
Thanks, Aleisha, for picking up this older bug.
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.
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
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
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>
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>
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>
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
(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.
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?
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 !
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.
(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?
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.
(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.
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.
Patch tested with a sandbox, by Cédric Vita <cedric.vita@dracenie.com>
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>
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>
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>
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>
Thx for your signoff, Cedric. The comments earlier from Christopher and Katrin still need attention.
(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?
(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.
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.
(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?
+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.
Anyone having an idea how we can get this running again ?
I am ok with your suggestion in comment#35. I was only suggesting to maybe reuse the Anonymous patron.
(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.
(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?
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>
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>
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>
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>
(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 ;)
*** Bug 28889 has been marked as a duplicate of this bug. ***
I'm confused. If this is signed off, why is it in discussion. Is there a reason it isn't going through QA?
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>
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>
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>
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.)
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>
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.)
Trying to get this a bit further again ;)
As expected, bitten by 28959 which on itself needs a fix !
The method Koha::Virtualshelves->owner is not covered by tests! => Bug 29844
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.
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>
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>
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.)
Created attachment 135818 [details] [review] Bug 11889: (QA follow-up) Add POD ground work
Created attachment 135819 [details] [review] Bug 11889: (QA follow-up) Convert to new atomicupdate style The first patch contained an old .sql atomic update.
Created attachment 135820 [details] [review] Bug 11889: (QA follow-up) Changes for wantarray move in ->search See bug 29844.
Created attachment 135821 [details] [review] Bug 11889: (QA follow-up) Fix tests Noise from 28959 changing category to public.
Please wait. Not ready yet.
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.
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>
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>
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.)
Created attachment 135861 [details] [review] Bug 11889: (QA follow-up) Add POD ground work
Created attachment 135862 [details] [review] Bug 11889: (QA follow-up) Convert to new atomicupdate style The first patch contained an old .sql atomic update.
Created attachment 135863 [details] [review] Bug 11889: (QA follow-up) Changes for wantarray move in ->search See bug 29844.
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>
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.
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
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.
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>
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>
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.)
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>
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>
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>
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>
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>
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>
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.
Changed title. This better describes what we are actually doing here.
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Created attachment 135901 [details] [review] Bug 11889: (QA follow-up) Spelling Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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
(In reply to Martin Renvoize from comment #99) > > Passing QA Great! Thx
Pushed to master for 22.11. Nice work everyone, thanks!
Enhancement will not be backported to the 22.05.x series