From 4f31ab6208a07c416ac5a7828f88072e291082ca Mon Sep 17 00:00:00 2001 From: Alex Buckley Date: Wed, 14 Apr 2021 14:06:48 +1200 Subject: [PATCH] Bug 26346: Add option to make public lists editable by all staff If a staff member has access to the staff client (either because 'catalogue' permission is enabled or they're a superlibrarian then that user can add items (from OPAC or staff client) to a list marked 'Staff only' Test plan: 1. In the staff client go to: Lists > 'New list'. Notice under 'Allow changes to contents from' there are three options: Nobody, Owner only, Anyone seeing this list 2. Apply patch and run updatedatabase.pl cd installer/data/mysql sudo koha-shell ./updatedatabase.pl 3. Restart memcached and plack 4. Create a public list and select the new 'Staff only' option under 'Allow changes to contents from' 5. Log out of the staff client and log back in as a different user with the 'Staff access, allows viewing of catalogue in staff interface (catalogue)' permission enabled. 6. Navigate to the Lists module, and click on the name of the list you created in #5 7. Select 'Add items' and enter an item barcode and submit 8. Notice the item has been added to the list 9. Search the catalogue in the staff client. Confirm you can add to the list you created in #5 from both the search result page and biblio record detail page 10. Confirm you can remove items from the list in the OPAC and staff client 11. Login to the OPAC as the same user as #5 12. Perform an OPAC search, Select 'Save to lists' under a biblio record, choose the list from #4 in 'Select a list' and save 13. Confirm in the staff client that the record from #11 has been added to the list 14. Logout of OPAC and login again as a user which does not have 'Staff access, allows viewing of catalogue in staff interface (catalogue)' or superlibrarian enabled 15. Do a OPAC search, hit 'Save to lists' and notice you cannot add the record to the list from #4 16. Run Patron.t and Virtualshelves.t unit tests: sudo koha-shell prove t/db_dependent/Koha/Patron.t prove t/db_dependent/Virtualshelves.t 17. Login to the staff client as the same user as #5 18. Create a new list in the staff client, select 'Category'='Private', 'Allow changes to contents from'='Staff only' 19. Notice 'The Staff only permission has no actual effect while this list is strictly private' message hint is displayed 20. Change 'Category'='Public' and notice the hint is removed 21. Switch back to 'Category'='Private' and 'Allow changes to contents from'='Anyone' 22. Notice the 'The Anyone permission has no actual effect while this list is strictly private.' is displayed 23. Repeat steps 18-22 in the OPAC 24. Create a 'staff only' list in the OPAC and then login as a different user with 'Staff access, allows viewing of catalogue in staff interface (catalogue)' permission enabled. Confirm you are able to add items to this new 'staff only' list. 25. Confirm that the 'Staff only' option is not available to patrons with no special permissions when creating lists in the OPAC Sponsored-by: Horowhenua District Council, New Zealand --- Koha/Patron.pm | 15 ++++ Koha/Schema/Result/Letter.pm | 6 +- Koha/Schema/Result/Virtualshelve.pm | 12 ++- Koha/Virtualshelf.pm | 10 ++- Koha/Virtualshelves.pm | 36 ++++++--- installer/data/mysql/atomicupdate/bug_26346.perl | 7 ++ .../prog/en/modules/virtualshelves/shelves.tt | 18 ++++- .../opac-tmpl/bootstrap/en/modules/opac-shelves.tt | 18 ++++- opac/opac-addbybiblionumber.pl | 55 ++++++++++--- opac/opac-shelves.pl | 6 +- t/db_dependent/Koha/Patron.t | 31 +++++++- t/db_dependent/Virtualshelves.t | 91 +++++++++++++++++----- virtualshelves/shelves.pl | 3 + 13 files changed, 253 insertions(+), 55 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_26346.perl diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 19a9215c86..825c353338 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -1914,6 +1914,21 @@ sub queue_notice { return \%return; } +=head3 can_patron_change_staff_only_lists + +$patron->can_patron_change_staff_only_lists; + +Return 1 if a patron has 'Superlibrarian' or 'Catalogue' permission. +Otherwise, return 0. + +=cut + +sub can_patron_change_staff_only_lists { + my ( $self, $params ) = @_; + return 1 if C4::Auth::haspermission( $self->userid, { 'catalogue' => 1 }); + return 0; +} + =head2 Internal methods =head3 _type diff --git a/Koha/Schema/Result/Letter.pm b/Koha/Schema/Result/Letter.pm index 613c4d9b27..b5627b9674 100644 --- a/Koha/Schema/Result/Letter.pm +++ b/Koha/Schema/Result/Letter.pm @@ -29,8 +29,6 @@ __PACKAGE__->table("letter"); is_auto_increment: 1 is_nullable: 0 -primary key identifier - =head2 module data_type: 'varchar' @@ -217,8 +215,8 @@ __PACKAGE__->belongs_to( ); -# Created by DBIx::Class::Schema::Loader v0.07046 @ 2021-02-11 12:33:50 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:qqdTVEicMu5rHppY5qsEuA +# Created by DBIx::Class::Schema::Loader v0.07046 @ 2021-04-09 22:58:34 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:2QkUwMWgZ9WfOk9Ngf7yFA sub koha_object_class { 'Koha::Notice::Template'; diff --git a/Koha/Schema/Result/Virtualshelve.pm b/Koha/Schema/Result/Virtualshelve.pm index 4fa1265959..709b3f9f1e 100644 --- a/Koha/Schema/Result/Virtualshelve.pm +++ b/Koha/Schema/Result/Virtualshelve.pm @@ -97,6 +97,12 @@ can owner change contents? can others change contents? +=head2 allow_change_from_staff + + data_type: 'tinyint' + default_value: 0 + is_nullable: 1 + =cut __PACKAGE__->add_columns( @@ -132,6 +138,8 @@ __PACKAGE__->add_columns( { data_type => "tinyint", default_value => 1, is_nullable => 1 }, "allow_change_from_others", { data_type => "tinyint", default_value => 0, is_nullable => 1 }, + "allow_change_from_staff", + { data_type => "tinyint", default_value => 0, is_nullable => 1 }, ); =head1 PRIMARY KEY @@ -199,8 +207,8 @@ __PACKAGE__->has_many( ); -# Created by DBIx::Class::Schema::Loader v0.07049 @ 2021-01-21 13:39:29 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:6ZQ6kL/0DzyOMymz+4+LhA +# Created by DBIx::Class::Schema::Loader v0.07046 @ 2021-04-09 23:02:21 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:4FyGa/O1QtVe1t5ih9PK5A sub koha_object_class { 'Koha::Virtualshelf'; diff --git a/Koha/Virtualshelf.pm b/Koha/Virtualshelf.pm index c27c695ada..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; @@ -185,7 +187,7 @@ sub add_biblio { return if $already_exists; # Check permissions - return unless ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) || $self->allow_change_from_others; + 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 $content = Koha::Virtualshelfcontent->new( { @@ -208,6 +210,7 @@ sub remove_biblios { my $number_removed = 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_others ) { $number_removed += $self->get_contents->search({ biblionumber => $biblionumbers, @@ -236,7 +239,7 @@ sub can_be_deleted { my $patron = Koha::Patrons->find( $borrowernumber ); - return 1 if $self->is_public and C4::Auth::haspermission( $patron->userid, { lists => 'delete_public_lists' } ); + return 1 if $self->is_public and haspermission( $patron->userid, { lists => 'delete_public_lists' } ); return 0; } @@ -253,7 +256,7 @@ sub can_biblios_be_added { return 1 if $borrowernumber - and ( ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) or $self->allow_change_from_others ); + 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 ); return 0; } @@ -267,4 +270,3 @@ sub _type { return 'Virtualshelve'; } -1; diff --git a/Koha/Virtualshelves.pm b/Koha/Virtualshelves.pm index 3dde3c35da..8162dfd815 100644 --- a/Koha/Virtualshelves.pm +++ b/Koha/Virtualshelves.pm @@ -23,6 +23,8 @@ use Koha::Database; use Koha::Virtualshelf; +use C4::Auth; + use base qw(Koha::Objects); =head1 NAME @@ -88,16 +90,30 @@ sub get_some_shelves { my @conditions; if ( $add_allowed ) { - push @conditions, { - -or => - [ - { - "me.owner" => $borrowernumber, - "me.allow_change_from_owner" => 1, - }, - "me.allow_change_from_others" => 1, - ] - }; + if ( Koha::Patrons->find( $borrowernumber )->can_patron_change_staff_only_lists ) { + push @conditions, { + -or => + [ + { + "me.owner" => $borrowernumber, + "me.allow_change_from_owner" => 1, + }, + "me.allow_change_from_others" => 1, + "me.allow_change_from_staff" => 1 + ] + }; + } else { + push @conditions, { + -or => + [ + { + "me.owner" => $borrowernumber, + "me.allow_change_from_owner" => 1, + }, + "me.allow_change_from_others" => 1, + ] + }; + } } if ( $category == 1 ) { push @conditions, { diff --git a/installer/data/mysql/atomicupdate/bug_26346.perl b/installer/data/mysql/atomicupdate/bug_26346.perl new file mode 100644 index 0000000000..a3470125ad --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_26346.perl @@ -0,0 +1,7 @@ +$DBversion = 'XXX'; +if ( CheckVersion($DBversion) ) { + if ( !column_exists( 'virtualshelves', 'allow_change_from_staff' ) ) { + $dbh->do(q{ALTER TABLE virtualshelves ADD COLUMN `allow_change_from_staff` tinyint(1) DEFAULT '0'}); + } + NewVersion( $DBversion, 26346, "Add allow_change_from_staff to virtualshelves table" ); +} diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt index b2f12a4b4e..1e393292a3 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt @@ -35,8 +35,11 @@ [% IF shelf.allow_change_from_others %][% ELSE %][% END %] + [% IF shelf.allow_change_from_staff %][% ELSE %][% END %] +   +   [% END %] @@ -809,16 +812,27 @@ if( perms < 2 ) { $("#anyone_remark").hide(); + $("#staff_remark").hide(); } else if( category==1 ) { - // If we move to Private (without shares), show Anyone remark + // If we move to Private (without shares), show either: + // Anyone remark if Anyone can change the list + // Or Staff remark is Staff only can change the list // Note: the number of shares is not tested real-time [% IF !shelf.is_shared %] - $("#anyone_remark").show(); + if( perms== 2) { + $("#anyone_remark").show(); + $("#staff_remark").hide(); + } else if ( perms==3 ) { + $("#anyone_remark").hide(); + $("#staff_remark").show(); + } [% ELSE %] $("#anyone_remark").hide(); + $("#staff_remark").hide(); [% END %] } else { // category==2 $("#anyone_remark").hide(); + $("#staff_remark").hide(); } } [% IF op == 'view' %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt index 3bdb18b641..4572f1c54c 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt @@ -51,8 +51,11 @@ [% IF shelf.allow_change_from_others %][% ELSE %][% END %] + [% IF staffuser == 1 %][% IF shelf.allow_change_from_staff %][% ELSE %][% END %][% END %] +   +   [% END %] @@ -1102,16 +1105,27 @@ function AdjustRemark() { if( perms < 2 ) { $("#anyone_remark").hide(); + $("#staff_remark").hide(); } else if( category==1 ) { - // If we move to Private (without shares), show Anyone remark + // If we move to Private (without shares), show one of the following: + // Anyone remark, if anyone can change the list + // Staff remark, if only Staff can change the list // Note: the number of shares is not tested real-time [% IF !shelf.is_shared %] - $("#anyone_remark").show(); + if ( perms==2 ) { + $("#anyone_remark").show(); + $("#staff_remark").hide(); + } else if ( perms==3 ) { + $("#anyone_remark").hide(); + $("#staff_remark").show(); + } [% ELSE %] $("#anyone_remark").hide(); + $("#staff_remark").hide(); [% END %] } else { // category==2 $("#anyone_remark").hide(); + $("#staff_remark").hide(); } } diff --git a/opac/opac-addbybiblionumber.pl b/opac/opac-addbybiblionumber.pl index 3563dfc300..06c5c33d54 100755 --- a/opac/opac-addbybiblionumber.pl +++ b/opac/opac-addbybiblionumber.pl @@ -118,18 +118,51 @@ if ($newvirtualshelf) { }, { join => 'virtualshelfshares', } ); - my $public_shelves = Koha::Virtualshelves->search( - { category => 2, - -or => [ - -and => { - allow_change_from_owner => 1, - owner => $loggedinuser, + my $public_shelves; + if ( $loggedinuser ) { + if ( Koha::Patrons->find( $loggedinuser )->can_patron_change_staff_only_lists ) { + $public_shelves = Koha::Virtualshelves->search( + { category => 2, + -or => [ + -and => { + allow_change_from_owner => 1, + owner => $loggedinuser, + }, + allow_change_from_others => 1, + allow_change_from_staff => 1 + ], }, - allow_change_from_others => 1, - ], - }, - { order_by => 'shelfname' } - ); + { order_by => 'shelfname' } + ); + } else { + $public_shelves = Koha::Virtualshelves->search( + { category => 2, + -or => [ + -and => { + allow_change_from_owner => 1, + owner => $loggedinuser, + }, + allow_change_from_others => 1, + ], + }, + {order_by => 'shelfname' } + ); + } + } else { + $public_shelves = Koha::Virtualshelves->search( + { category => 2, + -or => [ + -and => { + allow_change_from_owner => 1, + owner => $loggedinuser, + }, + allow_change_from_others => 1, + ], + }, + {order_by => 'shelfname' } + ); + } + $template->param( private_shelves => $private_shelves, private_shelves_shared_with_me => $shelves_shared_with_me, diff --git a/opac/opac-shelves.pl b/opac/opac-shelves.pl index b94b4126ea..b034c049cf 100755 --- a/opac/opac-shelves.pl +++ b/opac/opac-shelves.pl @@ -41,6 +41,7 @@ use Koha::Virtualshelves; use Koha::RecordProcessor; use constant ANYONE => 2; +use constant STAFF => 3; my $query = CGI->new; @@ -110,6 +111,7 @@ if ( $op eq 'add_form' ) { category => scalar $query->param('category') || 1, allow_change_from_owner => $allow_changes_from > 0, allow_change_from_others => $allow_changes_from == ANYONE, + allow_change_from_staff => $allow_changes_from == STAFF, owner => scalar $loggedinuser, } ); @@ -141,6 +143,7 @@ if ( $op eq 'add_form' ) { my $allow_changes_from = $query->param('allow_changes_from'); $shelf->allow_change_from_owner( $allow_changes_from > 0 ); $shelf->allow_change_from_others( $allow_changes_from == ANYONE ); + $shelf->allow_change_from_staff( $allow_changes_from == STAFF ); $shelf->category( scalar $query->param('category') ); eval { $shelf->store }; @@ -428,7 +431,7 @@ if ( $op eq 'list' ) { ), ); } - +my $staffuser = Koha::Patrons->find( $loggedinuser )->can_patron_change_staff_only_lists if $loggedinuser; $template->param( op => $op, referer => $referer, @@ -437,6 +440,7 @@ $template->param( category => $category, print => scalar $query->param('print') || 0, listsview => 1, + staffuser => $staffuser, ); my $content_type = $query->param('rss')? 'rss' : 'html'; diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index f5a329960d..cae09ec1c0 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,32 @@ 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); diff --git a/virtualshelves/shelves.pl b/virtualshelves/shelves.pl index d233eb1355..8b8e1bc4e0 100755 --- a/virtualshelves/shelves.pl +++ b/virtualshelves/shelves.pl @@ -36,6 +36,7 @@ use Koha::Patrons; use Koha::Virtualshelves; use constant ANYONE => 2; +use constant STAFF => 3; my $query = CGI->new; @@ -79,6 +80,7 @@ if ( $op eq 'add_form' ) { category => scalar $query->param('category'), allow_change_from_owner => $allow_changes_from > 0, allow_change_from_others => $allow_changes_from == ANYONE, + allow_change_from_staff => $allow_changes_from == STAFF, owner => scalar $query->param('owner'), } ); @@ -108,6 +110,7 @@ if ( $op eq 'add_form' ) { my $allow_changes_from = $query->param('allow_changes_from'); $shelf->allow_change_from_owner( $allow_changes_from > 0 ); $shelf->allow_change_from_others( $allow_changes_from == ANYONE ); + $shelf->allow_change_from_staff( $allow_changes_from == STAFF ); $shelf->category( scalar $query->param('category') ); eval { $shelf->store }; -- 2.11.0