@@ -, +, @@ - 'Owner only' - 'staff only' - 'Anyone seeing this list' --- Koha/Virtualshelf.pm | 12 ++++++++---- Koha/Virtualshelves.pm | 9 +++++++-- opac/opac-shelves.pl | 3 ++- t/db_dependent/Virtualshelves.t | 42 ++++++++++++++++++++--------------------- 4 files changed, 38 insertions(+), 28 deletions(-) --- a/Koha/Virtualshelf.pm +++ a/Koha/Virtualshelf.pm @@ -186,7 +186,8 @@ sub add_biblio { return if $already_exists; # Check permissions - return unless ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) || ( $self->allow_change_from_staff && Koha::Patrons->find( $borrowernumber )->can_patron_change_staff_only_lists ) || $self->allow_change_from_others; + my $patron = Koha::Patrons->find( $borrowernumber ) or return 0; + return 0 unless ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) || ( $self->allow_change_from_staff && $patron->can_patron_change_staff_only_lists ) || $self->allow_change_from_others; my $content = Koha::Virtualshelfcontent->new( { @@ -208,8 +209,9 @@ sub remove_biblios { return unless @$biblionumbers; my $number_removed = 0; + my $patron = Koha::Patrons->find( $borrowernumber ) or return 0; if( ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) - || ( $self->allow_change_from_staff && Koha::Patrons->find( $borrowernumber )->can_patron_change_staff_only_lists ) + || ( $self->allow_change_from_staff && $patron->can_patron_change_staff_only_lists ) || $self->allow_change_from_others ) { $number_removed += $self->get_contents->search({ biblionumber => $biblionumbers, @@ -236,7 +238,7 @@ sub can_be_deleted { return 0 unless $borrowernumber; return 1 if $self->owner == $borrowernumber; - my $patron = Koha::Patrons->find( $borrowernumber ); + my $patron = Koha::Patrons->find( $borrowernumber ) or return 0; return 1 if $self->is_public and haspermission( $patron->userid, { lists => 'delete_public_lists' } ); @@ -257,9 +259,10 @@ sub can_be_managed { sub can_biblios_be_added { my ( $self, $borrowernumber ) = @_; + my $patron = Koha::Patrons->find( $borrowernumber ) or return 0; return 1 if $borrowernumber - and ( ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) or ( $self->allow_change_from_staff && Koha::Patrons->find( $borrowernumber )->can_patron_change_staff_only_lists ) or $self->allow_change_from_others ); + and ( ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) or ( $self->allow_change_from_staff && $patron->can_patron_change_staff_only_lists ) or $self->allow_change_from_others ); return 0; } @@ -273,3 +276,4 @@ sub _type { return 'Virtualshelve'; } +1; --- a/Koha/Virtualshelves.pm +++ a/Koha/Virtualshelves.pm @@ -22,7 +22,6 @@ use Koha::Database; use Koha::Virtualshelf; -use C4::Auth; use base qw(Koha::Objects); @@ -88,8 +87,14 @@ sub get_some_shelves { my $add_allowed = $params->{add_allowed}; my @conditions; + my $patron; + my $staffuser = 0; + if ( $borrowernumber != 0 ) { + $patron = Koha::Patrons->find( $borrowernumber ); + $staffuser = $patron->can_patron_change_staff_only_lists; + } if ( $add_allowed ) { - if ( Koha::Patrons->find( $borrowernumber )->can_patron_change_staff_only_lists ) { + if ( $staffuser ) { push @conditions, { -or => [ --- a/opac/opac-shelves.pl +++ a/opac/opac-shelves.pl @@ -442,7 +442,8 @@ if ( $op eq 'list' ) { ), ); } -my $staffuser = Koha::Patrons->find( $loggedinuser )->can_patron_change_staff_only_lists if $loggedinuser; +my $staffuser; +$staffuser = Koha::Patrons->find( $loggedinuser )->can_patron_change_staff_only_lists if $loggedinuser; $template->param( op => $op, referer => $referer, --- a/t/db_dependent/Virtualshelves.t +++ a/t/db_dependent/Virtualshelves.t @@ -289,17 +289,17 @@ subtest 'Shelf permissions' => sub { } )->store; - is( $public_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view his public list' ); + is( $public_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view their public list' ); is( $public_shelf->can_be_viewed( $patron2->{borrowernumber} ), 1, 'Public list should be viewed by another staff member'); is( $public_shelf->can_be_viewed( $patron3->{borrowernumber} ), 1, 'Public list should be viewed by someone with no special permissions' ); is( $public_shelf->can_be_viewed( $patron4->{borrowernumber} ), 1, 'Public list should be viewed by someone with the edit_public_lists sub-permission checked' ); - is( $public_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete his list' ); + is( $public_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete their list' ); is( $public_shelf->can_be_deleted( $patron2->{borrowernumber} ), 0, 'Public list should not be deleted by another staff member' ); is( $public_shelf->can_be_deleted( $patron3->{borrowernumber} ), 0, 'Public list should not be deleted by someone with no special permissions' ); is( $public_shelf->can_be_deleted( $patron4->{borrowernumber} ), 0, 'Public list should not be deleted by someone with the edit_public_lists sub-permission checked' ); - is( $public_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage his list' ); + is( $public_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage their list' ); is( $public_shelf->can_be_managed( $patron2->{borrowernumber} ), 0, 'Public list should not be managed by another staff member' ); is( $public_shelf->can_be_managed( $patron3->{borrowernumber} ), 0, 'Public list should not be managed by someone with no special permissions' ); is( $public_shelf->can_be_managed( $patron4->{borrowernumber} ), 1, 'Public list should be managed by someone with the edit_public_lists sub-permission checked' ); @@ -317,27 +317,27 @@ subtest 'Shelf permissions' => sub { $public_shelf->allow_change_from_owner(1); $public_shelf->store; - is( $public_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view his public list' ); + is( $public_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view their public list' ); is( $public_shelf->can_be_viewed( $patron2->{borrowernumber} ), 1, 'Public list should be viewed by staff member' ); is( $public_shelf->can_be_viewed( $patron3->{borrowernumber} ), 1, 'Public list should be viewed by someone with no special permissions' ); is( $public_shelf->can_be_viewed( $patron4->{borrowernumber} ), 1, 'Public list should be viewable by someone with the edit_public_lists sub-permission checked' ); - is( $public_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete his list' ); + is( $public_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete their list' ); is( $public_shelf->can_be_deleted( $patron2->{borrowernumber} ), 0, 'Public list should not be deleted by another staff member' ); is( $public_shelf->can_be_deleted( $patron3->{borrowernumber} ), 0, 'Public list should not be deleted by someone with no special permissions' ); is( $public_shelf->can_be_deleted( $patron4->{borrowernumber} ), 0, 'Public list should not be deleted by someone with the edit_public_lists sub-permission checked' ); - is( $public_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage his list' ); + is( $public_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage thier list' ); is( $public_shelf->can_be_managed( $patron2->{borrowernumber} ), 0, 'Public list should not be managed by another staff member' ); is( $public_shelf->can_be_managed( $patron3->{borrowernumber} ), 0, 'Public list should not be managed by someone with no special permissions' ); is( $public_shelf->can_be_managed( $patron4->{borrowernumber} ), 1, 'Public list should be managed by someone with the edit_public_lists sub-permission checked' ); - is( $public_shelf->can_biblios_be_added( $patron1->{borrowernumber} ), 1, 'The owner should be able to add biblios to his list' ); + is( $public_shelf->can_biblios_be_added( $patron1->{borrowernumber} ), 1, 'The owner should be able to add biblios to their list' ); is( $public_shelf->can_biblios_be_added( $patron2->{borrowernumber} ), 0, 'Public list should not be modified (add) by another staff member' ); is( $public_shelf->can_biblios_be_added( $patron3->{borrowernumber} ), 0, 'Public list should not be modified (add) by someone with no special permissions' ); is( $public_shelf->can_biblios_be_added( $patron4->{borrowernumber} ), 0, 'Public list should not be modified (add) by someone with the edit_public_lists sub-permission checked' ); - is( $public_shelf->can_biblios_be_removed( $patron1->{borrowernumber} ), 1, 'The owner should be able to remove biblios to his list' ); + is( $public_shelf->can_biblios_be_removed( $patron1->{borrowernumber} ), 1, 'The owner should be able to remove biblios to their list' ); is( $public_shelf->can_biblios_be_removed( $patron2->{borrowernumber} ), 0, 'Public list should not be modified (remove) by another staff member' ); is( $public_shelf->can_biblios_be_removed( $patron3->{borrowernumber} ), 0, 'Public list should not be modified (remove) by someone with no special permissions' ); is( $public_shelf->can_biblios_be_removed( $patron4->{borrowernumber} ), 0, 'Public list should not be modified (remove) by someone with the edit_public_list sub-permission checked' ); @@ -352,17 +352,17 @@ subtest 'Shelf permissions' => sub { } )->store; - is( $private_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view his list' ); + is( $private_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view their list' ); is( $private_shelf->can_be_viewed( $patron2->{borrowernumber} ), 0, 'Private list should not be viewed by another staff member' ); is( $private_shelf->can_be_viewed( $patron3->{borrowernumber} ), 0, 'Private list should not be viewed by someone with no special permissions' ); is( $private_shelf->can_be_viewed( $patron4->{borrowernumber} ), 0, 'Private list should not be viewed by someone with the edit_public_lists sub-permission checked' ); - is( $private_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete his list' ); + is( $private_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete their list' ); is( $private_shelf->can_be_deleted( $patron2->{borrowernumber} ), 0, 'Private list should not be deleted by another staff member' ); is( $private_shelf->can_be_deleted( $patron3->{borrowernumber} ), 0, 'Private list should not be deleted by someone with no special permissions' ); is( $private_shelf->can_be_deleted( $patron4->{borrowernumber} ), 0, 'Private list should not be deleted by someone with the edit_public_lists sub-permission checked' ); - is( $private_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage his list' ); + is( $private_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage their list' ); is( $private_shelf->can_be_managed( $patron2->{borrowernumber} ), 0, 'Private list should not be managed by another staff member' ); is( $private_shelf->can_be_managed( $patron3->{borrowernumber} ), 0, 'Private list should not be managed by someone with no special permissions' ); is( $private_shelf->can_be_managed( $patron4->{borrowernumber} ), 0, 'Private list should not be managed by someone with the edit_public_lists sub-permission checked' ); @@ -381,27 +381,27 @@ subtest 'Shelf permissions' => sub { $private_shelf->allow_change_from_staff(1); $private_shelf->allow_change_from_others(0); $private_shelf->store; - is( $private_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view his list' ); + is( $private_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view their list' ); is( $private_shelf->can_be_viewed( $patron2->{borrowernumber} ), 0, 'Private list should not be viewed by another staff member' ); is( $private_shelf->can_be_viewed( $patron3->{borrowernumber} ), 0, 'Private list should not be viewed by someone with no special permissions' ); is( $private_shelf->can_be_viewed( $patron4->{borrowernumber} ), 0, 'Private list should not be viewed by someone with the edit_public_lists sub-permission checked' ); - is( $private_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete his list' ); + is( $private_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete their list' ); is( $private_shelf->can_be_deleted( $patron2->{borrowernumber} ), 0, 'Private list should not be deleted by another staff member' ); is( $private_shelf->can_be_deleted( $patron3->{borrowernumber} ), 0, 'Private list should not be deleted by someone with no special permissions' ); is( $private_shelf->can_be_deleted( $patron4->{borrowernumber} ), 0, 'Private list should not be deleted by someone with the edit_public_lists sub-permission checked' ); - is( $private_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage his list' ); + is( $private_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage their list' ); is( $private_shelf->can_be_managed( $patron2->{borrowernumber} ), 0, 'Private list should not be managed by another staff member' ); is( $private_shelf->can_be_managed( $patron3->{borrowernumber} ), 0, 'Private list should not be managed by someone with no special permissions' ); is( $private_shelf->can_be_managed( $patron4->{borrowernumber} ), 0, 'Private list should not be managed by someone with the edit_public_lists sub-permission checked' ); - is( $private_shelf->can_biblios_be_added( $patron1->{borrowernumber} ), 1, 'The owner should be able to add biblios to his list' ); + is( $private_shelf->can_biblios_be_added( $patron1->{borrowernumber} ), 1, 'The owner should be able to add biblios to their list' ); is( $private_shelf->can_biblios_be_added( $patron2->{borrowernumber} ), 1, 'Private list should not modified (add) by another staff member # individual check done later' ); is( $private_shelf->can_biblios_be_added( $patron3->{borrowernumber} ), 0, 'Private list should not be modified (add) by someone with no special permissions' ); is ( $private_shelf->can_biblios_be_added( $patron4->{borrowernumber} ), 0, 'Private list should not be modified (add) by someone with the edit_public_lists sub-permission checked' ); - is( $private_shelf->can_biblios_be_removed( $patron1->{borrowernumber} ), 1, 'The owner should be able to remove biblios to his list' ); + is( $private_shelf->can_biblios_be_removed( $patron1->{borrowernumber} ), 1, 'The owner should be able to remove biblios to their list' ); is( $private_shelf->can_biblios_be_removed( $patron2->{borrowernumber} ), 1, 'Private list should be modified (remove) by another staff member # individual check done later' ); is( $private_shelf->can_biblios_be_removed( $patron3->{borrowernumber} ), 0, 'Private list should not be modified (remove) by someone with no special permissions' ); is( $private_shelf->can_biblios_be_removed( $patron4->{borrowernumber} ), 0, 'Private list should not be modified (remove) by someone with the edit_public_lists sub-permission checked' ); @@ -410,27 +410,27 @@ subtest 'Shelf permissions' => sub { $private_shelf->allow_change_from_others(1); $private_shelf->store; - is( $private_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view his list' ); + is( $private_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view their list' ); is( $private_shelf->can_be_viewed( $patron2->{borrowernumber} ), 0, 'Private list should not be viewed by another staff member' ); is( $private_shelf->can_be_viewed( $patron3->{borrowernumber} ), 0, 'Private list should not be viewed by someone with no special permissions' ); is( $private_shelf->can_be_viewed( $patron4->{borrowernumber} ), 0, 'Private list should not be viewed by someone with the edit_public_lists sub-permission checked' ); - is( $private_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete his list' ); + is( $private_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete their list' ); is( $private_shelf->can_be_deleted( $patron2->{borrowernumber} ), 0, 'Private list should not be deleted by another staff member' ); is( $private_shelf->can_be_deleted( $patron3->{borrowernumber} ), 0, 'Private list should not be deleted by someone with no special permissions' ); is( $private_shelf->can_be_deleted( $patron4->{borrowernumber} ), 0, 'Private list should not be deleted by someone with the edit_public_lists sub-permission checked' ); - is( $private_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage his list' ); + is( $private_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage their list' ); is( $private_shelf->can_be_managed( $patron2->{borrowernumber} ), 0, 'Private list should not be managed by another staff member' ); is( $private_shelf->can_be_managed( $patron3->{borrowernumber} ), 0, 'Private list should not be managed by someone with no special permissions' ); is( $private_shelf->can_be_managed( $patron4->{borrowernumber} ), 0, 'Private list should not be managed by someone with the edit_public_lists sub-permission checked' ); - is( $private_shelf->can_biblios_be_added( $patron1->{borrowernumber} ), 1, 'The owner should be able to add biblios to his list' ); + is( $private_shelf->can_biblios_be_added( $patron1->{borrowernumber} ), 1, 'The owner should be able to add biblios to their list' ); is( $private_shelf->can_biblios_be_added( $patron2->{borrowernumber} ), 1, 'Private list could be modified (add) by another staff member # individual check done later' ); is( $private_shelf->can_biblios_be_added( $patron3->{borrowernumber} ), 1, 'Private list could be modified (add) by someone with no special permissions' ); is( $private_shelf->can_biblios_be_added( $patron4->{borrowernumber} ), 1, 'Private list could be modified (add) by someone with the edit_public_lists sub-permission checked' ); - is( $private_shelf->can_biblios_be_removed( $patron1->{borrowernumber} ), 1, 'The owner should be able to remove biblios to his list' ); + is( $private_shelf->can_biblios_be_removed( $patron1->{borrowernumber} ), 1, 'The owner should be able to remove biblios to their list' ); is( $private_shelf->can_biblios_be_removed( $patron2->{borrowernumber} ), 1, 'Private list could be modified (remove) by another staff member # individual check done later' ); is( $private_shelf->can_biblios_be_removed( $patron3->{borrowernumber} ), 1, 'Private list could be modified (remove) by someone with no special permissions' ); is( $private_shelf->can_biblios_be_removed( $patron4->{borrowernumber} ), 1, 'Private list could be modified (remove) by someone with the edit_public_lists sub-permission checked' ); --