From 2a7787e51f9bf60dafec2134663535699065305c 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 Content-Type: text/plain; charset=utf-8 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 --- opac/opac-shelves.pl | 22 +++++++++------------- virtualshelves/shelves.pl | 25 +++++++++++++------------ 2 files changed, 22 insertions(+), 25 deletions(-) diff --git a/opac/opac-shelves.pl b/opac/opac-shelves.pl index d93ae2c52a..378c2f5633 100755 --- a/opac/opac-shelves.pl +++ b/opac/opac-shelves.pl @@ -262,31 +262,27 @@ 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 ) { push @$patrons, { email => $share->sharee->notice_email_address, borrowernumber => $share->get_column('borrowernumber') }; } $template->param( shared_users => $patrons ); - $op = 'transfer'; - } 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 0386268cb6..fd9229762c 100755 --- a/virtualshelves/shelves.pl +++ b/virtualshelves/shelves.pl @@ -59,6 +59,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 }; @@ -238,23 +239,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); @@ -374,6 +374,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.20.1