From b70ae1d50f07d1449630bf738bd0e9d53d542c82 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Wed, 22 Jun 2022 12:14:37 +0000 Subject: [PATCH] Bug 30933: (follow-up) Use cannot_be_transferred in shelves scripts Test plan: Verify if transfer shared list on OPAC still works as expected. Same for intranet counterpart for public lists. Signed-off-by: Marcel de Rooy Signed-off-by: David Nind Signed-off-by: Martin Renvoize Signed-off-by: Tomas Cohen Arazi --- opac/opac-shelves.pl | 21 +++++++++------------ virtualshelves/shelves.pl | 25 +++++++++++++------------ 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/opac/opac-shelves.pl b/opac/opac-shelves.pl index bc7b70d70a..15ee4da412 100755 --- a/opac/opac-shelves.pl +++ b/opac/opac-shelves.pl @@ -262,13 +262,11 @@ if ( $op eq 'add_form' ) { $shelfnumber = $query->param('shelfnumber'); $shelf = Koha::Virtualshelves->find($shelfnumber) if $shelfnumber; my $new_owner = $query->param('new_owner'); # borrowernumber or undef + my $error_code = $shelf + ? $shelf->cannot_be_transferred({ by => $loggedinuser, to => $new_owner, interface => 'opac' }) + : 'does_not_exist'; - $op = 'list'; - if( !$shelf ) { - push @messages, { type => 'error', code => 'does_not_exist' }; - } elsif( $shelf->public or !$shelf->is_shared or !$shelf->can_be_managed($loggedinuser) ) { - push @messages, { type => 'error', code => 'unauthorized_transfer' }; - } elsif( !$new_owner ) { + if( !$new_owner && $error_code eq 'missing_to_parameter' ) { # show transfer form my $patrons = []; my $shares = $shelf->get_shares->search({ borrowernumber => { '!=' => undef } }); while( my $share = $shares->next ) { @@ -281,17 +279,16 @@ if ( $op eq 'add_form' ) { } else { push @messages, { type => 'error', code => 'no_email_found' }; } - } elsif( !Koha::Patrons->find($new_owner) ) { - push @messages, { type => 'error', code => 'new_owner_not_found' }; - } elsif( !$shelf->get_shares->search({ borrowernumber => $new_owner })->count ) { - push @messages, { type => 'error', code => 'new_owner_has_no_share' }; - } else { - # Remove from virtualshelfshares new_owner, add loggedinuser + } elsif( $error_code ) { + push @messages, { type => 'error', code => $error_code }; + $op = 'list'; + } else { # transfer; remove new_owner from virtualshelfshares, add loggedinuser $shelf->_result->result_source->schema->txn_do( sub { $shelf->get_shares->search({ borrowernumber => $new_owner })->delete; Koha::Virtualshelfshare->new({ shelfnumber => $shelfnumber, borrowernumber => $loggedinuser, sharedate => dt_from_string })->store; $shelf->owner($new_owner)->store; }); + $op = 'list'; } } diff --git a/virtualshelves/shelves.pl b/virtualshelves/shelves.pl index 4e9404c87b..b512fe310b 100755 --- a/virtualshelves/shelves.pl +++ b/virtualshelves/shelves.pl @@ -60,6 +60,7 @@ my $referer = $query->param('referer') || $op; my $public = $query->param('public') ? 1 : 0; my ( $shelf, $shelfnumber, @messages, $allow_transfer ); +# PART1: Perform a few actions if ( $op eq 'add_form' ) { # Only pass default $shelf = { allow_change_from_owner => 1 }; @@ -239,23 +240,22 @@ if ( $op eq 'add_form' ) { $shelfnumber = $query->param('shelfnumber'); $shelf = Koha::Virtualshelves->find($shelfnumber) if $shelfnumber; my $new_owner = $query->param('new_owner'); # is a borrowernumber + my $error_code = $shelf + ? $shelf->cannot_be_transferred({ by => $loggedinuser, to => $new_owner, interface => 'intranet' }) + : 'does_not_exist'; - if( $new_owner ) { + if( !$new_owner && $error_code eq 'missing_to_parameter' ) { + # show form + } elsif( $error_code ) { + push @messages, { type => 'error', code => $error_code }; + $op = 'list'; + } else { + $shelf->owner($new_owner)->store; $op = 'list'; - # First check: shelf found, permission, patron found? - if( !$shelf ) { - push @messages, { type => 'alert', code => 'does_not_exist' }; - } elsif( !haspermission(C4::Context->userenv->{id}, { lists => 'edit_public_lists' }) ) { - push @messages, { type => 'alert', code => 'unauthorized_transfer' }; - } elsif( !Koha::Patrons->find($new_owner) ) { - push @messages, { type => 'alert', code => 'new_owner_not_found' }; - $op = 'transfer'; # find again.. - } else { # success - $shelf->owner($new_owner)->store; - } } } +# PART2: After a possible action, further prepare form if ( $op eq 'view' ) { $shelfnumber ||= $query->param('shelfnumber'); $shelf = Koha::Virtualshelves->find($shelfnumber); @@ -375,6 +375,7 @@ if ( $op eq 'view' ) { } } elsif( $op eq 'list' ) { $allow_transfer = haspermission( C4::Context->userenv->{id}, { lists => 'edit_public_lists' } ) ? 1 : 0; + # this check only serves for button display } $template->param( -- 2.39.5