From 4d9d47b2a138d551bee8d70721a04d0214837f19 Mon Sep 17 00:00:00 2001 From: Alex Buckley Date: Fri, 7 May 2021 01:30:59 +0000 Subject: [PATCH] Bug 26346: Fix unit tests Test plan: 1. Run Patron.t and Virtualshelves.t unit tests: sudo koha-shell prove t/db_dependent/Koha/Patron.t prove t/db_dependent/Koha/Virtualshelves.t Sponsored-by: Horowhenua Library Trust, New Zealand --- Koha/Patron.pm | 2 +- Koha/Virtualshelf.pm | 2 + t/db_dependent/Koha/Patron.t | 32 ++++++++++++++- t/db_dependent/Virtualshelves.t | 91 +++++++++++++++++++++++++++++++++-------- 4 files changed, 107 insertions(+), 20 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 44241af65e..825c353338 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -1925,7 +1925,7 @@ Otherwise, return 0. sub can_patron_change_staff_only_lists { my ( $self, $params ) = @_; - return 1 if C4::Auth->haspermission( $self->userid, { 'catalogue' => 1 }); + return 1 if C4::Auth::haspermission( $self->userid, { 'catalogue' => 1 }); return 0; } diff --git a/Koha/Virtualshelf.pm b/Koha/Virtualshelf.pm index 3a937bc9d3..69efcd92a2 100644 --- a/Koha/Virtualshelf.pm +++ b/Koha/Virtualshelf.pm @@ -64,6 +64,8 @@ sub store { unless defined $self->allow_change_from_owner; $self->allow_change_from_others( 0 ) unless defined $self->allow_change_from_others; + $self->allow_change_from_staff( 0 ) + unless defined $self->allow_change_from_staff; $self->created_on( dt_from_string ) unless defined $self->created_on; diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index f5a329960d..19dc041a49 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 8; +use Test::More tests => 9; use Test::Exception; use Test::Warn; @@ -716,3 +716,33 @@ subtest 'can_log_into() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'can_patron_change_staff_only_lists() tests' => sub { + + plan tests => 3; + + $schema->storage->txn_begin; + + # make a user with no special permissions + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + flags => undef + } + } + ); + is( $patron->can_patron_change_staff_only_lists(), 0, 'Patron without permissions cannot change staff only lists'); + + # make it a 'Catalogue' permission + $patron->set({ flags => 4 })->store->discard_changes; + is( $patron->can_patron_change_staff_only_lists(), 1, 'Catalogue patron can change staff only lists'); + + + # make it a superlibrarian + $patron->set({ flags => 1 })->store->discard_changes; + is( $patron->can_patron_change_staff_only_lists(), 1, 'Superlibrarian patron can change staff only lists'); + + $schema->storage->txn_rollback; +}; + diff --git a/t/db_dependent/Virtualshelves.t b/t/db_dependent/Virtualshelves.t index d349b7b39b..0fdc7438b1 100755 --- a/t/db_dependent/Virtualshelves.t +++ b/t/db_dependent/Virtualshelves.t @@ -22,7 +22,7 @@ my $dbh = C4::Context->dbh; teardown(); subtest 'CRUD' => sub { - plan tests => 13; + plan tests => 14; my $patron = $builder->build({ source => 'Borrower', }); @@ -44,6 +44,7 @@ subtest 'CRUD' => sub { is( $number_of_shelves, 1, '1 shelf should have been inserted' ); is( $shelf->allow_change_from_owner, 1, 'The default value for allow_change_from_owner should be 1' ); is( $shelf->allow_change_from_others, 0, 'The default value for allow_change_from_others should be 0' ); + is ( $shelf->allow_change_from_staff, 0, 'The default value for allow_change_from_staff should be 0'); is( t::lib::Dates::compare( $shelf->created_on, dt_from_string), 0, 'The creation time should have been set to today' ); # Test if creation date will not be overwritten by store @@ -171,9 +172,10 @@ subtest 'Sharing' => sub { subtest 'Shelf content' => sub { - plan tests => 18; + plan tests => 21; my $patron1 = $builder->build( { source => 'Borrower', } ); my $patron2 = $builder->build( { source => 'Borrower', } ); + my $patron3 = $builder->build( { source => 'Borrower', value => {flags => 1} }); my $biblio1 = $builder->build_sample_biblio; my $biblio2 = $builder->build_sample_biblio; my $biblio3 = $builder->build_sample_biblio; @@ -247,18 +249,31 @@ subtest 'Shelf content' => sub { $number_of_contents = Koha::Virtualshelfcontents->search->count; is( $number_of_contents, 3, 'Back to three entries' ); + # allow_change_from_staff == 1 and allow_change_from_others == 0 + $shelf->allow_change_from_staff( 1 ); + $shelf->allow_change_from_others( 0 ); + $content4 = $shelf->add_biblio( $biblio3->biblionumber, $patron3->{borrowernumber} ); + $number_of_contents = Koha::Virtualshelfcontents->search->count; + is( $number_of_contents, 4, 'The biblio should have been added to the shelf by patron 2'); + $number_of_deleted_biblios = $shelf->remove_biblios( { biblionumbers => [ $biblio3->biblionumber ], borrowernumber => $patron3->{borrowernumber} } ); + is( $number_of_deleted_biblios, 1, 'Biblio 3 deleted by patron 2' ); + $number_of_contents = Koha::Virtualshelfcontents->search->count; + is( $number_of_contents, 3, 'Back to three entries' ); + teardown(); }; subtest 'Shelf permissions' => sub { - plan tests => 40; + plan tests => 70; my $patron1 = $builder->build( { source => 'Borrower', value => { flags => '2096766' } } ); # 2096766 is everything checked but not superlibrarian my $patron2 = $builder->build( { source => 'Borrower', value => { flags => '1048190' } } ); # 1048190 is everything checked but not superlibrarian and delete_public_lists + my $patron3 = $builder->build( { source => 'Borrower', value => { flags => '0' } } ); # this is a patron with no special permissions my $biblio1 = $builder->build_sample_biblio; my $biblio2 = $builder->build_sample_biblio; my $biblio3 = $builder->build_sample_biblio; my $biblio4 = $builder->build_sample_biblio; + my $biblio5 = $builder->build_sample_biblio; my $public_shelf = Koha::Virtualshelf->new( { shelfname => "my first shelf", @@ -266,42 +281,53 @@ subtest 'Shelf permissions' => sub { category => 2, allow_change_from_owner => 0, allow_change_from_others => 0, + allow_change_from_staff => 0, } )->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( $patron2->{borrowernumber} ), 1, 'Public list should be viewed by someone else' ); + 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_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete his list' ); - is( $public_shelf->can_be_deleted( $patron2->{borrowernumber} ), 0, 'Public list should not be deleted by someone else' ); + 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_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage his list' ); - is( $public_shelf->can_be_managed( $patron2->{borrowernumber} ), 0, 'Public list should not be managed by someone else' ); + 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_biblios_be_added( $patron1->{borrowernumber} ), 0, 'The owner should not 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 someone else' ); + 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_removed( $patron1->{borrowernumber} ), 0, 'The owner should not 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 someone else' ); + 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 (removed) by someone with no special permissions' ); $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( $patron2->{borrowernumber} ), 1, 'Public list should be viewed by someone else' ); + 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_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete his list' ); - is( $public_shelf->can_be_deleted( $patron2->{borrowernumber} ), 0, 'Public list should not be deleted by someone else' ); + 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_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage his list' ); - is( $public_shelf->can_be_managed( $patron2->{borrowernumber} ), 0, 'Public list should not be managed by someone else' ); + 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_biblios_be_added( $patron1->{borrowernumber} ), 1, 'The owner should be able to add biblios to his list' ); - is( $public_shelf->can_biblios_be_added( $patron2->{borrowernumber} ), 0, 'Public list should not be modified (add) by someone else' ); + 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->{borrowerbumber} ), 0, 'Public list should not be modified (add) by someone with no special permissions' ); 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( $patron2->{borrowernumber} ), 0, 'Public list should not be modified (remove) by someone else' ); + 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->{borroernumber} ), 0, 'Public list should not be modified (remove) by someone with no special permissions' ); my $private_shelf = Koha::Virtualshelf->new( @@ -310,24 +336,53 @@ subtest 'Shelf permissions' => sub { category => 1, allow_change_from_owner => 0, allow_change_from_others => 0, + allow_change_from_staff => 0, } )->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( $patron2->{borrowernumber} ), 0, 'Private list should not be viewed by someone else' ); + 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_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete his list' ); - is( $private_shelf->can_be_deleted( $patron2->{borrowernumber} ), 0, 'Private list should not be deleted by someone else' ); + 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_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage his list' ); - is( $private_shelf->can_be_managed( $patron2->{borrowernumber} ), 0, 'Private list should not be managed by someone else' ); + 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_biblios_be_added( $patron1->{borrowernumber} ), 0, 'The owner should not be able to add biblios to their list' ); - is( $private_shelf->can_biblios_be_added( $patron2->{borrowernumber} ), 0, 'Private list should not be modified (add) by someone else' ); + is( $private_shelf->can_biblios_be_added( $patron2->{borrowernumber} ), 0, 'Private list should not be modified (add) by another staff member' ); + 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_removed( $patron1->{borrowernumber} ), 0, 'The owner should not be able to remove biblios to their list' ); - is( $private_shelf->can_biblios_be_removed( $patron2->{borrowernumber} ), 0, 'Private list should not be modified (remove) by someone else' ); + is( $private_shelf->can_biblios_be_removed( $patron2->{borrowernumber} ), 0, 'Private list should not be modified (remove) by another staff member' ); + is( $private_shelf->can_biblios_be_removed( $patron3->{borrowernumber} ), 0, 'Private list should not be modified (remove) by someone with no special permissions' ); + + $private_shelf->allow_change_from_owner(1); + $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( $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_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete his 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_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage his 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_biblios_be_added( $patron1->{borrowernumber} ), 1, 'The owner should be able to add biblios to his 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} ), 0, 'Private list could be modified (add) by someone with no special permissions' ); + + 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( $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} ), 0, 'Private list could be modified (remove) by someone with no special permissions' ); $private_shelf->allow_change_from_owner(1); $private_shelf->allow_change_from_others(1); -- 2.11.0