From 42b21a33c1f698ef43a8e348472fc3563b53a69d Mon Sep 17 00:00:00 2001 From: Aleisha Amohia Date: Tue, 14 Feb 2017 22:56:31 +0000 Subject: [PATCH] Bug 11889: Disown a list upon deletion of owner MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=utf-8 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 Signed-off-by: Cédric Vita --- Koha/Patron.pm | 37 ++++++++++--------- ...istOwnershipUponPatronDeletion_syspref.sql | 1 + .../en/modules/admin/preferences/patrons.pref | 6 +++ 3 files changed, 27 insertions(+), 17 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_11889_-_add_ListOwnershipUponPatronDeletion_syspref.sql diff --git a/Koha/Patron.pm b/Koha/Patron.pm index e205e23fd3..2a7280aa01 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -369,8 +369,8 @@ $patron->delete Delete patron's holds, lists and finally the patron. -Lists owned by the borrower are deleted, but entries from the borrower to -other lists are kept. +Lists owned by the borrower are deleted or ownership is transfered depending on the +ListOwnershipUponPatronDeletion pref, but entries from the borrower to other lists are kept. =cut @@ -388,21 +388,24 @@ sub delete { $hold->cancel; } - # Delete all lists and all shares of this borrower - # Consistent with the approach Koha uses on deleting individual lists - # Note that entries in virtualshelfcontents added by this borrower to - # lists of others will be handled by a table constraint: the borrower - # is set to NULL in those entries. - # NOTE: - # We could handle the above deletes via a constraint too. - # But a new BZ report 11889 has been opened to discuss another approach. - # Instead of deleting we could also disown lists (based on a pref). - # In that way we could save shared and public lists. - # The current table constraints support that idea now. - # This pref should then govern the results of other routines/methods such as - # Koha::Virtualshelf->new->delete too. - # FIXME Could be $patron->get_lists - $_->delete for Koha::Virtualshelves->search( { owner => $self->borrowernumber } )->as_list; + # If ListOwnershipUponPatronDeletion = transfer, change ownership of all + # public and shared lists to the user who deleted them. + if ( C4::Context->preference('ListOwnershipUponPatronDeletion') eq 'transfer' ) { + my $userenv = C4::Context->userenv(); + my $usernumber = (ref($userenv) eq 'HASH') ? $userenv->{'number'} : 0; + my @publiclists = Koha::Virtualshelves->get_public_shelves; + my @sharedlists = Koha::Virtualshelves->search({ 'me.owner' => $self->borrowernumber, 'me.shelfnumber' => { -ident => 'virtualshelfshares.shelfnumber' } }, { prefetch => 'virtualshelfshares' }); + foreach my $plist ( @publiclists ) { + $plist->set({ owner => $usernumber })->store; + } + foreach my $slist ( @sharedlists ) { + $slist->set({ owner => $usernumber })->store; + } + } + + # Delete any remaining lists that this user is an owner of (always private lists, + # only public and shared lists if ListOwnershipUponPatronDeletion = delete) + $_->delete for Koha::Virtualshelves->search({ owner => $self->borrowernumber }); # We cannot have a FK on borrower_modifications.borrowernumber, the table is also used # for patron selfreg diff --git a/installer/data/mysql/atomicupdate/bug_11889_-_add_ListOwnershipUponPatronDeletion_syspref.sql b/installer/data/mysql/atomicupdate/bug_11889_-_add_ListOwnershipUponPatronDeletion_syspref.sql new file mode 100644 index 0000000000..4bdcb152a1 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_11889_-_add_ListOwnershipUponPatronDeletion_syspref.sql @@ -0,0 +1 @@ +INSERT INTO systempreferences (`variable`,`value`,`options`,`explanation`,`type`) VALUES ('ListOwnershipUponPatronDeletion', 'delete', 'delete|transfer', 'When deleting a patron who owns public lists, either delete the public lists or transfer ownership to the patron who deleted the owner', 'Choice'); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref index 871c63722f..f763740bd1 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref @@ -27,6 +27,12 @@ Patrons: 1: "Allow" 0: "Don't allow" - "staff to access a patron's checkout and hold history (checkout history is still stored, regardless of staff being allowed access or not)." + - + - "When deleting a patron who owns public or shared lists," + - pref: ListOwnershipUponPatronDeletion + choices: + delete: "delete all of their public and shared lists." + transfer: "transfer ownership of their public and shared lists to the patron who deleted the owner." - - The late fine for all checkouts will only go up to - pref: MaxFine -- 2.20.1