Attachment 120954 Details for Bug 26346 – Bug 26346: Add option to make public lists editable by all staff

[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), 36.80 KB, created by Andrew Fuerste-Henry on 2021-05-14 01:05:42 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Fuerste-Henry

Created: 2021-05-14 01:05:42 UTC

Size: 36.80 KB

patch

obsolete

>From 91d63b1dfd6fd82071c16f116678f3c8ebd03f3e 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 patch 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>
>---
> 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 %]<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>
>         &emsp; <span id="anyone_remark" style="display:none;color:red;">The Anyone permission has no actual effect while this list is strictly private.</span>
>+        &emsp; <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>
>@@ -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 %]<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>
>         &emsp; <span id="anyone_remark" style="display:none;color:red;">The "Anyone" permission has no actual effect while this list is strictly private.</span>
>+        &emsp; <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 %]
> 
>@@ -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();
>     }
> }
> </script>
>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

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