Bug 13888: 'Lists' permission should allow/disallow using the lists module in staff

This patch adds two new system preferences, use public lists, and create public
lists. Use public lists determines if a librarian is permitted to see public
lists, whilst create public lists determines if a librarian can create new public
lists.

It also fixes erroneously allowing staff to add items to existing lists, by
honouring the existing edit_public_list_contents

To test:
a)  notice the new my lists link on the account pulldown
    1)  ensure it goes to the lists module
b)  create new public list, add at least one item, make it editable by everyone
    1) note the name of the list
c)  create a new patron with full access to the staff client minus lists permissions
d)  log in as the newly created patron
e)  notice the lists button is missing from the staff client mainpage
f)  set any of the lists permissions except create public lists, use public lists, edit public list contents
g)  notice how the lists button is no longer missing from the staff client mainpage
h)  go to the lists module
    1) notice that public lists are now missing from the datatable
i)  click add list
    1)  notice that the public drop-down is now a fixed label set to private
j)  create list and confirm it is not public
k)  turn use public lists permission on
l)  return to the lists module
    1)  notice that the datatable now shows private and public lists tabs
m)  turn create public lists permission on
n)  repeat steps h-i
    1)  notice that the public drop-down is now visible again
o)  create list and confirm it is public
p)  go to the list you created in step b
q)  notice that add items button, and remove selected button, is missing
r)  turn edit public list contents permission on
s)  repeat steps p-q
    1) notice that add items button is now visible
t)  click add items
u)  enter an item barcode or biblio number, click save
    1) notice that the items are now added to the list

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Koha/Virtualshelf.pm

@@ -256,16 +256,21 @@ sub can_be_managed {
 sub can_biblios_be_added {
     my ( $self, $borrowernumber ) = @_;
 
-    my $patron = Koha::Patrons->find( $borrowernumber ) or return 0;
+    my $patron = Koha::Patrons->find($borrowernumber) or return 0;
     return 1
-      if $borrowernumber
+        if $borrowernumber
-      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_permitted_staff && $patron->can_patron_change_permitted_staff_lists ) 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_permitted_staff && $patron->can_patron_change_permitted_staff_lists )
+        or $self->allow_change_from_others )
+        and ( ( $self->public && C4::Auth::haspermission( $patron->userid, { lists => 'edit_public_list_contents' } ) )
+        or !$self->public );
     return 0;
 }
 
 sub can_biblios_be_removed {
     my ( $self, $borrowernumber ) = @_;
-    return $self->can_biblios_be_added( $borrowernumber );
+    return $self->can_biblios_be_added($borrowernumber);
     # Same answer since bug 18228
 }
 

installer/data/mysql/atomicupdate/bug_13888-Add-extra-list-perms.pl

@@ -0,0 +1,21 @@
+use Modern::Perl;
+
+return {
+    bug_number  => "13888",
+    description => "'Lists' permission should allow/disallow using the lists module in staff",
+    up          => sub {
+        my ($args) = @_;
+        my ( $dbh, $out ) = @$args{qw(dbh out)};
+
+        $dbh->do(
+            q{ INSERT IGNORE INTO permissions (module_bit,code,description) VALUES (20, 'use_public_lists', 'Use public lists') }
+        );
+        say $out "Added permission 'use_public_lists'";
+
+        $dbh->do(
+            q{ INSERT IGNORE INTO permissions (module_bit,code,description) VALUES (20, 'create_public_lists', 'Create public lists') }
+        );
+        say $out "Added permission 'create_public_lists'";
+
+    },
+};

installer/data/mysql/mandatory/userpermissions.sql

@@ -146,9 +146,11 @@ INSERT INTO permissions (module_bit, code, description) VALUES
    (19, 'report', 'Use report plugins'),
    (19, 'admin', 'Use administrative plugins'),
    (19, 'configure', 'Configure plugins'),
+   (20, 'create_public_lists', 'Create public lists'),
    (20, 'delete_public_lists', 'Delete public lists'),
    (20, 'edit_public_lists', 'Edit public lists'),
    (20, 'edit_public_list_contents', 'Edit public list contents'),
+   (20, 'use_public_lists', 'Use public lists'),
    (21, 'edit_templates', 'Create and update club templates'),
    (21, 'edit_clubs', 'Create and update clubs'),
    (21, 'enroll', 'Enroll patrons in clubs'),

koha-tmpl/intranet-tmpl/prog/en/includes/header.inc

@@ -228,6 +228,9 @@
                         <li class="toplinks-mycheckouts">
                             <a class="toplinks" href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% loggedinusernumber | html %]">My checkouts</a>
                         </li>
+                        <li class="toplinks-mylists">
+                            <a class="toplinks" href="/cgi-bin/koha/virtualshelves/shelves.pl">My lists</a>
+                        </li>
                     [% END %]
                     [% IF Koha.Preference( 'CookieConsent' ) %]
                         <li class="toplinks-myconsents">

koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc

@@ -767,6 +767,11 @@
             Use administrative plugins
         </span>
         <span class="permissioncode">([% name | html %])</span>
+    [%- CASE 'create_public_lists' -%]
+        <span class="sub_permission create_public_lists_subpermission">
+            Create public lists
+        </span>
+        <span class="permissioncode">([% name | html %])</span>
     [%- CASE 'delete_public_lists' -%]
         <span class="sub_permission delete_public_lists_subpermission">
             Delete public lists
@@ -782,6 +787,11 @@
             Edit public lists contents
         </span>
         <span class="permissioncode">([% name | html %])</span>
+    [%- CASE 'use_public_lists' -%]
+        <span class="sub_permission use_public_lists_subpermission">
+            Use public lists
+        </span>
+        <span class="permissioncode">([% name | html %])</span>
     [%- CASE 'upload_general_files' -%]
         <span class="sub_permission upload_general_files_subpermission">
             Upload any file

koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt

@@ -91,9 +91,11 @@
                             </li>
                             [% END %]
 
+                            [% IF ( CAN_user_lists ) %]
                             <li>
                                 <a class="icon_general icon_lists" href="/cgi-bin/koha/virtualshelves/shelves.pl"><i class="fa fa-fw fa-list-alt"></i>Lists</a>
                             </li>
+                            [% END %]
 
                             [% IF ( UseCourseReserves ) %]
                                 <li>

koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt

@@ -455,19 +455,25 @@
                                 </select>
                             </li>
                             <li>
-                                <label for="public">Public: </label>
+                                [% IF ( CAN_user_lists_create_public_lists ) %]
-                                <select id="public" name="public" onchange="AdjustRemark()">
+                                    <label for="public">Public: </label>
-                                    [% IF shelf.is_private %]
+                                    <select id="public" name="public" onchange="AdjustRemark()">
-                                        <option value="0" selected="selected">Private</option>
+                                        [% IF shelf.is_private %]
-                                    [% ELSE %]
+                                            <option value="0" selected="selected">Private</option>
-                                        <option value="0">Private</option>
+                                        [% ELSE %]
-                                    [% END %]
+                                            <option value="0">Private</option>
-                                    [% IF shelf.is_public %]
+                                        [% END %]
-                                        <option value="1" selected="selected">Public</option>
+                                        [% IF shelf.is_public %]
-                                    [% ELSE %]
+                                            <option value="1" selected="selected">Public</option>
-                                        <option value="1">Public</option>
+                                        [% ELSE %]
-                                    [% END %]
+                                            <option value="1">Public</option>
-                                </select>
+                                        [% END %]
+                                    </select>
+                                [% ELSE %]
+                                    <label>Public: </label>
+                                    <span>Private</span>
+                                    <input id="public" name="public" value="0" type="hidden" />
+                                [% END %]
                             </li>
                             [% INCLUDE list_permissions %]
                         </ol>
@@ -494,7 +500,9 @@
                 [% WRAPPER tabs id= "tabs" %]
                     [% WRAPPER tabs_nav %]
                         [% WRAPPER tab_item tabname= "privateshelves_tab" bt_active= 1 %] <span>Your lists</span> [% END %]
-                        [% WRAPPER tab_item tabname= "publicshelves_tab" %] <span>Public lists</span> [% END %]
+                        [% IF ( CAN_user_lists_use_public_lists ) %]
+                            [% WRAPPER tab_item tabname= "publicshelves_tab" %] <span>Public lists</span> [% END %]
+                        [% END %]
                     [% END # /WRAPPER tabs_nav %]
 
                     [% WRAPPER tab_panels %]

t/Koha/Auth/Permissions.t

@@ -167,9 +167,11 @@ subtest 'superlibrarian tests' => sub {
         'CAN_user_editcatalogue_set_record_sources'                 => 1,
         'CAN_user_editcatalogue'                                    => 1,
         'CAN_user_ill'                                              => 1,
+        'CAN_user_lists_create_public_lists'                        => 1,
         'CAN_user_lists_delete_public_lists'                        => 1,
         'CAN_user_lists_edit_public_lists'                          => 1,
         'CAN_user_lists_edit_public_list_contents'                  => 1,
+        'CAN_user_lists_use_public_lists'                           => 1,
         'CAN_user_lists'                                            => 1,
         'CAN_user_parameters_manage_accounts'                       => 1,
         'CAN_user_parameters_manage_additional_fields'              => 1,

virtualshelves/shelves.pl

@@ -57,8 +57,14 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 my $op      = $query->param('op')      || 'list';
 my $referer = $query->param('referer') || $op;
 my $page    = int( $query->param('page') || 1 );
-my $public  = $query->param('public') ? 1 : 0;
+my ( $public, $shelf, $shelfnumber, @messages, $allow_transfer, $allow_create_public_lists );
-my ( $shelf, $shelfnumber, @messages, $allow_transfer );
+
+# work out permissions once
+# this check is for the create list permission
+$allow_create_public_lists = haspermission( $loggedinuser, { lists => 'create_public_lists' } ) ? 1 : 0;
+
+# we want the user to be able to pick if public or private only if they are allowed
+$public = ( $query->param('public') == 1 && $allow_create_public_lists == 1 ) ? 1 : 0;
 
 # PART1: Perform a few actions
 if ( $op eq 'add_form' ) {
@@ -397,6 +403,7 @@ $template->param(
     print    => scalar $query->param('print') || 0,
     csv_profiles => [ Koha::CsvProfiles->search({ type => 'marc', used_for => 'export_records' })->as_list ],
     allow_transfer => $allow_transfer,
+    allow_create_public_lists => $allow_create_public_lists,
 );
 
 output_html_with_http_headers $query, $cookie, $template->output;