@@ -, +, @@ staff - User A : Superlibrarian permissions - User B : 'Staff access, allows viewing of catalogue in staff interface - User C : No permissions - User D : 'Staff access, allows viewing of catalogue in staff - Individual list page using the 'Add items' button - Staff client search result page - Staff client biblio detail page - OPAC search result page - OPAC biblio detail page - 'Category'='Private', - 'Allow changes to contents from'='Staff only' - Public = 'Public' - Allow changes to contents from = 'owner only' --- Koha/Patron.pm | 15 ++ Koha/Virtualshelf.pm | 18 +- Koha/Virtualshelves.pm | 41 +++- .../prog/en/includes/permissions.inc | 5 + .../prog/en/modules/virtualshelves/shelves.tt | 14 +- .../bootstrap/en/modules/opac-shelves.tt | 14 +- opac/opac-addbybiblionumber.pl | 55 ++++-- opac/opac-shelves.pl | 7 +- t/db_dependent/Koha/Patron.t | 32 +++- t/db_dependent/Virtualshelves.t | 177 +++++++++++++----- virtualshelves/shelves.pl | 3 + 11 files changed, 307 insertions(+), 74 deletions(-) --- a/Koha/Patron.pm +++ a/Koha/Patron.pm @@ -2069,6 +2069,21 @@ sub recalls { return Koha::Recalls->search({ borrowernumber => $self->borrowernumber }); } +=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 --- a/Koha/Virtualshelf.pm +++ a/Koha/Virtualshelf.pm @@ -56,6 +56,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; @@ -177,7 +179,8 @@ sub add_biblio { return if $already_exists; # Check permissions - return unless ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) || $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( { @@ -199,7 +202,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 && $patron->can_patron_change_staff_only_lists ) || $self->allow_change_from_others ) { $number_removed += $self->get_contents->search({ biblionumber => $biblionumbers, @@ -226,9 +231,9 @@ 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 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; } @@ -237,15 +242,20 @@ sub can_be_managed { my ( $self, $borrowernumber ) = @_; return 1 if $borrowernumber and $self->owner == $borrowernumber; + + my $patron = Koha::Patrons->find( $borrowernumber ) or return 0; + return 1 + if $self->is_public and haspermission( $patron->userid, { lists => 'edit_public_lists' } ); return 0; } 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_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; } --- a/Koha/Virtualshelves.pm +++ a/Koha/Virtualshelves.pm @@ -22,6 +22,7 @@ use Koha::Database; use Koha::Virtualshelf; + use base qw(Koha::Objects); =head1 NAME @@ -86,17 +87,37 @@ 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 ) { - push @conditions, { - -or => - [ - { - "me.owner" => $borrowernumber, - "me.allow_change_from_owner" => 1, - }, - "me.allow_change_from_others" => 1, - ] - }; + if ( $staffuser ) { + 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 ( !$public ) { push @conditions, { --- a/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc +++ a/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc @@ -678,6 +678,11 @@ Delete public lists ([% name | html %]) + [%- CASE 'edit_public_lists' -%] + + Edit public lists + + ([% name | html %]) [%- CASE 'upload_general_files' -%] Upload any file --- a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt @@ -33,8 +33,11 @@ [% IF shelf.allow_change_from_others %][% ELSE %][% END %] + [% IF shelf.allow_change_from_staff %][% ELSE %][% END %] +   +   [% END %] @@ -801,16 +804,25 @@ if( perms < 2 ) { $("#anyone_remark").hide(); + $("#staff_remark").hide(); } else if( public==0 ) { // If we move to Private (without shares), show Anyone remark // 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 { // public==1 $("#anyone_remark").hide(); + $("#staff_remark").hide(); } } [% IF op == 'view' %] --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt +++ a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt @@ -50,8 +50,11 @@ [% IF shelf.allow_change_from_others %][% ELSE %][% END %] + [% IF staffuser == 1 %][% IF shelf.allow_change_from_staff %][% ELSE %][% END %][% END %] +   +   [% END %] @@ -1042,16 +1045,25 @@ function AdjustRemark() { if( perms < 2 ) { $("#anyone_remark").hide(); + $("#staff_remark").hide(); } else if( public==0 ) { // If we move to Private (without shares), show Anyone remark // 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 { // public==1 $("#anyone_remark").hide(); + $("#staff_remark").hide(); } } --- a/opac/opac-addbybiblionumber.pl +++ a/opac/opac-addbybiblionumber.pl @@ -117,18 +117,51 @@ if ($newvirtualshelf) { }, { join => 'virtualshelfshares', } ); - my $public_shelves = Koha::Virtualshelves->search( - { public => 1, - -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( + { public => 1, + -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( + { public => 1, + -or => [ + -and => { + allow_change_from_owner => 1, + owner => $loggedinuser, + }, + allow_change_from_others => 1, + ], + }, + {order_by => 'shelfname' } + ); + } + } else { + $public_shelves = Koha::Virtualshelves->search( + { public => 1, + -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, --- a/opac/opac-shelves.pl +++ a/opac/opac-shelves.pl @@ -45,6 +45,7 @@ use Koha::Virtualshelves; use Koha::RecordProcessor; use constant ANYONE => 2; +use constant STAFF => 3; my $query = CGI->new; @@ -116,6 +117,7 @@ if ( $op eq 'add_form' ) { public => $public, 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, } ); @@ -147,6 +149,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->public( $public ); eval { $shelf->store }; @@ -441,7 +444,8 @@ if ( $op eq 'list' ) { ), ); } - +my $staffuser; +$staffuser = Koha::Patrons->find( $loggedinuser )->can_patron_change_staff_only_lists if $loggedinuser; $template->param( op => $op, referer => $referer, @@ -450,6 +454,7 @@ $template->param( public => $public, print => scalar $query->param('print') || 0, listsview => 1, + staffuser => $staffuser, ); my $content_type = $query->param('rss')? 'rss' : 'html'; --- a/t/db_dependent/Koha/Patron.t +++ a/t/db_dependent/Koha/Patron.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 15; +use Test::More tests => 16; use Test::Exception; use Test::Warn; @@ -849,6 +849,36 @@ subtest 'article_requests() tests' => sub { $article_requests = $patron->article_requests; is( $article_requests->count, 4, '4 article requests' ); + $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; }; --- a/t/db_dependent/Virtualshelves.t +++ a/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,35 @@ 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 => 100; 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 $patron4 = $builder->build( { source => 'Borrower', value => { flags => '0' } } ); + my $sth = $dbh->prepare("INSERT INTO user_permissions (borrowernumber, module_bit, code) VALUES (?,?,?)"); + $sth->execute($patron4->{borrowernumber}, 20, 'edit_public_lists'); # $patron4 only has the edit_public_lists sub-permission checked + 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,43 +285,62 @@ subtest 'Shelf permissions' => sub { public => 1, 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( $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( $patron2->{borrowernumber} ), 0, 'Public list should not be deleted by someone else' ); + 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( $patron2->{borrowernumber} ), 0, 'Public list should not be managed by someone else' ); + 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' ); 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_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} ), 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' ); + is( $public_shelf->can_biblios_be_removed( $patron4->{borrowernumber} ), 0, 'Public list should not be modified (removed) by someone with the edit_public_lists sub-permission checked' ); $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( $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( $patron2->{borrowernumber} ), 0, 'Public list should not be deleted by someone else' ); + 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( $patron2->{borrowernumber} ), 0, 'Public list should not be managed by someone else' ); + 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( $patron2->{borrowernumber} ), 0, 'Public list should not be modified (add) by someone else' ); - - 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_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 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' ); my $private_shelf = Koha::Virtualshelf->new( { shelfname => "my first shelf", @@ -310,43 +348,92 @@ subtest 'Shelf permissions' => sub { public => 0, 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( $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( $patron2->{borrowernumber} ), 0, 'Private list should not be deleted by someone else' ); + 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( $patron2->{borrowernumber} ), 0, 'Private list should not be managed by someone else' ); + 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} ), 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_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} ), 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' ); + 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-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 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 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 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 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 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' ); $private_shelf->allow_change_from_owner(1); $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( $patron2->{borrowernumber} ), 0, 'Private list should not be viewed by someone else' ); - - 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_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_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 someone else # individual check done later' ); - - 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 someone else # individual check done later' ); + 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 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 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 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 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' ); teardown(); }; --- a/virtualshelves/shelves.pl +++ a/virtualshelves/shelves.pl @@ -41,6 +41,7 @@ use Koha::Patrons; use Koha::Virtualshelves; use constant ANYONE => 2; +use constant STAFF => 3; my $query = CGI->new; @@ -84,6 +85,7 @@ if ( $op eq 'add_form' ) { public => $public, 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'), } ); @@ -113,6 +115,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->public( scalar $query->param('public') ); eval { $shelf->store }; --