Bugzilla – Attachment 128019 Details for
Bug 26346
Add option to make a public list editable by library staff only
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 26346: Add option to make public lists editable by all staff
Bug-26346-Add-option-to-make-public-lists-editable.patch (text/plain), 33.78 KB, created by
Alex Buckley
on 2021-11-25 20:47:18 UTC
(
hide
)
Description:
Bug 26346: Add option to make public lists editable by all staff
Filename:
MIME Type:
Creator:
Alex Buckley
Created:
2021-11-25 20:47:18 UTC
Size:
33.78 KB
patch
obsolete
>From fe3e22b538d008f40380a3b009bde7276530ffaf Mon Sep 17 00:00:00 2001 >From: Alex Buckley <alexbuckley@catalyst.net.nz> >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 first 3 patches and run updatedatabase.pl >cd installer/data/mysql >sudo koha-shell <instance> >./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 <instance> >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 > >Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> > >Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> >--- > Koha/Patron.pm | 15 ++++ > Koha/Virtualshelf.pm | 10 ++- > Koha/Virtualshelves.pm | 36 ++++++--- > .../prog/en/modules/virtualshelves/shelves.tt | 14 +++- > .../opac-tmpl/bootstrap/en/modules/opac-shelves.tt | 14 +++- > opac/opac-addbybiblionumber.pl | 55 ++++++++++--- > opac/opac-shelves.pl | 6 +- > t/db_dependent/Koha/Patron.t | 32 +++++++- > t/db_dependent/Virtualshelves.t | 91 +++++++++++++++++----- > virtualshelves/shelves.pl | 3 + > 10 files changed, 229 insertions(+), 47 deletions(-) > >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index 85d9c01bf1..08f0858d1c 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -1910,6 +1910,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/Virtualshelf.pm b/Koha/Virtualshelf.pm >index a7273a95d9..4b96d80ff0 100644 >--- a/Koha/Virtualshelf.pm >+++ b/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,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( > { >@@ -200,6 +202,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, >@@ -228,7 +231,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; > } >@@ -245,7 +248,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; > } > >@@ -265,4 +268,3 @@ sub _type { > return 'Virtualshelve'; > } > >-1; >diff --git a/Koha/Virtualshelves.pm b/Koha/Virtualshelves.pm >index df75c3b59a..8e678e25a3 100644 >--- a/Koha/Virtualshelves.pm >+++ b/Koha/Virtualshelves.pm >@@ -22,6 +22,8 @@ use Koha::Database; > > use Koha::Virtualshelf; > >+use C4::Auth; >+ > use base qw(Koha::Objects); > > =head1 NAME >@@ -87,16 +89,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 ( !$public ) { > push @conditions, { >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 09550fe2b4..1735600a9b 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt >@@ -33,8 +33,11 @@ > > [% IF shelf.allow_change_from_others %]<option value="2" selected="selected">Anyone seeing this list</option>[% ELSE %]<option value="2">Anyone seeing this list</option>[% END %] > >+ [% IF shelf.allow_change_from_staff %]<option value="3" selected="selected">Staff only</option>[% ELSE %]<option value="3">Staff only</option>[% END %] >+ > </select> >   <span id="anyone_remark" style="display:none;color:red;">The Anyone permission has no actual effect while this list is strictly private.</span> >+   <span id="staff_remark" style="display:none;color:red;">The Staff only permission has no actual effect while this list is strictly private.</span> > </li> > [% END %] > </head> >@@ -803,16 +806,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' %] >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 0859d453ec..ffee7b5d4b 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt >@@ -50,8 +50,11 @@ > > [% IF shelf.allow_change_from_others %]<option value="2" selected="selected">Anyone seeing this list</option>[% ELSE %]<option value="2">Anyone seeing this list</option>[% END %] > >+ [% IF staffuser == 1 %][% IF shelf.allow_change_from_staff %]<option value="3" selected="selected">Staff only</option>[% ELSE %]<option value="3">Staff only</option>[% END %][% END %] >+ > </select> >   <span id="anyone_remark" style="display:none;color:red;">The "Anyone" permission has no actual effect while this list is strictly private.</span> >+   <span id="staff_remark" style="display:none;color:red;">The "Staff only" permission has no actual effect while this list is strictly private.</span> > </li> > [% 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(); > } > } > </script> >diff --git a/opac/opac-addbybiblionumber.pl b/opac/opac-addbybiblionumber.pl >index 7dfe42cc14..4edbd133e7 100755 >--- a/opac/opac-addbybiblionumber.pl >+++ b/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, >diff --git a/opac/opac-shelves.pl b/opac/opac-shelves.pl >index 78ce49da0e..7143876816 100755 >--- a/opac/opac-shelves.pl >+++ b/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 }; > >@@ -439,7 +442,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, >@@ -448,6 +451,7 @@ $template->param( > public => $public, > 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 2929349661..b7ea0529bd 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 => 10; >+use Test::More tests => 11; > use Test::Exception; > use Test::Warn; > >@@ -845,4 +845,34 @@ subtest 'article_requests() tests' => sub { > 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; > }; >diff --git a/t/db_dependent/Virtualshelves.t b/t/db_dependent/Virtualshelves.t >index a31d84cc49..d8404e2a07 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 { > 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( $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 { > 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( $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 d1ce868e4b..614bda9a34 100755 >--- a/virtualshelves/shelves.pl >+++ b/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 }; > >-- >2.11.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=128019">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 26346
:
119686
|
120663
|
120664
|
120665
|
120909
|
120910
|
120953
|
120954
|
121511
|
121512
|
122033
|
122034
|
122533
|
122535
|
122536
|
122537
|
122538
|
122539
|
122540
|
122685
|
122686
|
122687
|
122688
|
122689
|
122691
|
124010
|
124011
|
125436
|
125472
|
125914
|
125915
|
125916
|
125917
|
127556
|
127557
|
127558
|
127559
|
127560
|
128017
|
128018
|
128019
|
128020
|
128021
|
128022
|
128023
|
128383
|
128384
|
128385
|
131699
|
131700
|
131757
|
131764
|
132373
|
132475
|
132476
|
132477
|
132478
|
132479
|
132751
|
132752
|
132753
|
132754
|
132755