From 1f1627cf97d911e666b55ba97fd7c666dc7daa47 Mon Sep 17 00:00:00 2001 From: Alex Buckley Date: Fri, 2 Jul 2021 06:59:02 +0000 Subject: [PATCH] Bug 26346: (follow-up) Fixes in response to QA Fixes for the following comments on bug report 26346: 1. Comment #31 2. Comment #32 3. Comment #35 4. Comment #36 5. Comment #37 6. Comment #38 7. Comment #49 Test plan for comment #49 fix: 1. Log into the staff client as User A. Create one of each of the following types of public lists: - 'Owner only' - 'staff only' - 'Anyone seeing this list' 2. Visit the OPAC. Do not login. Confirm you are able to see all three public lists under the 'Lists' dropdown in the OPAC header 3. Perform a search. Select a search result checkbox and view the dropdown options in the 'Select titles to:' dropdown 4. Confirm only the 'Anyone seeing this list' is displayed as an option 5. Log into the OPAC as a user B (a user with no permissions). Confirm you can see all three public lists under the header 'Lists' dropdown 6. Repeat steps 3, and 4 as User B with the same results as un-authenticated user 6. Log into the OPAC as user C (a user with access to the staff client). Confirm you can see all three public lists under the header 'Lists' dropdown 7. Repeat steps 3, and 4 but this time you should see the 'staff only' and 'Anyone seeing this list' public lists in the dropdown 8. Log into the OPAC as user A. Confirm you can see all three poublic lists under the header 'Lists' dropdown 9. Repeat steps 3, and 4 but this time you should see the 'owner only', 'staff only' and 'Anyone seeing this list' public lists in the dropdown Sponsored-By: Catalyst IT --- Koha/Virtualshelf.pm | 10 +++++++--- Koha/Virtualshelves.pm | 10 +++++++--- opac/opac-shelves.pl | 3 ++- t/db_dependent/Virtualshelves.t | 10 +++++----- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/Koha/Virtualshelf.pm b/Koha/Virtualshelf.pm index 676791ff02..e6cab20d73 100644 --- a/Koha/Virtualshelf.pm +++ b/Koha/Virtualshelf.pm @@ -187,7 +187,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 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( { @@ -209,8 +210,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, @@ -258,9 +260,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; } @@ -274,3 +277,4 @@ sub _type { return 'Virtualshelve'; } +1; diff --git a/Koha/Virtualshelves.pm b/Koha/Virtualshelves.pm index 8162dfd815..381db24740 100644 --- a/Koha/Virtualshelves.pm +++ b/Koha/Virtualshelves.pm @@ -23,8 +23,6 @@ use Koha::Database; use Koha::Virtualshelf; -use C4::Auth; - use base qw(Koha::Objects); =head1 NAME @@ -89,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 => [ diff --git a/opac/opac-shelves.pl b/opac/opac-shelves.pl index 2914385d22..6d2def2a4f 100755 --- a/opac/opac-shelves.pl +++ b/opac/opac-shelves.pl @@ -436,7 +436,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, diff --git a/t/db_dependent/Virtualshelves.t b/t/db_dependent/Virtualshelves.t index 0977362d9c..5de065652f 100755 --- a/t/db_dependent/Virtualshelves.t +++ b/t/db_dependent/Virtualshelves.t @@ -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' ); -- 2.11.0