Bug 19524: Share patron lists between staff

Some libraries would like to allow arbitrary lists to be used by all
librarians with the 'manage_patron_lists' permission.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Create or find two patrons with the manage_patron_lists permission
4) Using the first patron, create two new lists, mark one of them as
   shared
5) Log in as the second patron, browse to the patron lists page
6) Note the second patron can view, add and remove patrons from
   the shared list owned by the first patron

Signed-off-by: George Williams <george@nekls.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Koha/List/Patron.pm

@@ -63,7 +63,15 @@ sub GetPatronLists {
         return;
     }
 
-    delete( $params->{owner} ) if ( C4::Context->IsSuperLibrarian() );
+    delete $params->{owner} if C4::Context->IsSuperLibrarian();
+
+    if ( my $owner = $params->{owner} ) {
+        delete $params->{owner};
+        $params->{'-or'} = [
+            owner => $owner,
+            shared => 1,
+        ];
+    }
 
     my $schema = Koha::Database->new()->schema();
 

koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/add-modify.tt

@@ -51,6 +51,15 @@
                         <input id="list-name" name="name" type="text" class="required" required="required" value="[% list.name %]" />
                     </li>
 
+                    <li>
+                        <label for="list-shared">Shared:</label>
+                        [% IF list.shared %]
+                            <input id="list-shared" name="shared" type="checkbox" checked="checked" />
+                        [% ELSE %]
+                            <input id="list-shared" name="shared" type="checkbox" />
+                        [% END %]
+                    </li>
+
                     <li>
                         <span class="label">Owner: </span>[% loggedinusername %]
                     </li>

koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt

@@ -32,15 +32,26 @@
                 <tr>
                     <th>Name</th>
                     <th>Patrons in list</th>
+                    <th>Shared</th>
                     <th class="NoSort">&nbsp;</th>
                 </tr>
             </thead>
 
             <tbody>
                 [% FOREACH l IN lists %]
+                    [% SET shared_by_other = l.owner.id != logged_in_user %]
                     <tr>
                         <td><a href="/cgi-bin/koha/patron_lists/list.pl?patron_list_id=[% l.patron_list_id %]">[% l.name |html%]</a></td>
                         <td>[% l.patron_list_patrons_rs.count || 0 %]</td>
+                        <td>
+                            [% IF l.shared %]
+                                [% IF shared_by_other %]
+                                    by <a href=/cgi-bin/koha/members/moremember.pl?borrowernumber=[% l.owner.id %]">[% INCLUDE 'patron-title.inc' patron=l.owner %]</a>
+                                [% ELSE %]
+                                    by you
+                                [% END %]
+                            [% END %]
+                        </td>
                         <td>
                             <div class="dropdown">
                                 <a class="btn btn-default btn-xs dropdown-toggle" id="listactions[% l.patron_list_id %]" role="button" data-toggle="dropdown" href="#">
@@ -48,8 +59,10 @@
                                 </a>
                                 <ul class="dropdown-menu pull-right" role="menu" aria-labelledby="listactions[% l.patron_list_id %]">
                                     <li><a href="/cgi-bin/koha/patron_lists/list.pl?patron_list_id=[% l.patron_list_id %]"><i class="fa fa-user"></i> Add patrons</a></li>
-                                    <li><a href="/cgi-bin/koha/patron_lists/add-modify.pl?patron_list_id=[% l.patron_list_id %]"><i class="fa fa-pencil"></i> Edit list</a></li>
-                                    <li><a class="delete_patron" href="/cgi-bin/koha/patron_lists/delete.pl?patron_list_id=[% l.patron_list_id %]" data-list-name="[% l.name %]"><i class="fa fa-trash"></i> Delete list</a></li>
+                                    [% UNLESS shared_by_other %]
+                                        <li><a href="/cgi-bin/koha/patron_lists/add-modify.pl?patron_list_id=[% l.patron_list_id %]"><i class="fa fa-pencil"></i> Edit list</a></li>
+                                        <li><a class="delete_patron" href="/cgi-bin/koha/patron_lists/delete.pl?patron_list_id=[% l.patron_list_id %]" data-list-name="[% l.name %]"><i class="fa fa-trash"></i> Delete list</a></li>
+                                    [% END %]
                                     [% IF ( l.patron_list_patrons_rs.count ) %]
                                         <li class="divider"></li>
                                         <li>

patron_lists/add-modify.pl

@@ -39,6 +39,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 
 my $id   = $cgi->param('patron_list_id');
 my $name = $cgi->param('name');
+my $shared = $cgi->param('shared') ? 1 : 0;
 
 if ($id) {
     my ($list) = GetPatronLists( { patron_list_id => $id } );
@@ -47,11 +48,11 @@ if ($id) {
 
 if ($name) {
     if ($id) {
-        ModPatronList( { patron_list_id => $id, name => $name } );
+        ModPatronList( { patron_list_id => $id, name => $name, shared => $shared } );
         print $cgi->redirect('lists.pl');
     }
     else {
-        my $list = AddPatronList( { name => $name } );
+        my $list = AddPatronList( { name => $name, shared => $shared } );
         print $cgi->redirect(
             "list.pl?patron_list_id=" . $list->patron_list_id() );
     }

patron_lists/lists.pl

@@ -39,6 +39,9 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 
 my @lists = GetPatronLists();
 
-$template->param( lists => \@lists );
+$template->param(
+    lists          => \@lists,
+    logged_in_user => $loggedinuser,
+);
 
 output_html_with_http_headers( $cgi, $cookie, $template->output );

t/db_dependent/PatronLists.t

@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 7;
+use Test::More tests => 9;
 use t::lib::TestBuilder;
 
 use Koha::Database;
@@ -38,13 +38,14 @@ foreach (1..10) {
     push @borrowers, $builder->build({ source => 'Borrower' });
 }
 
-my $owner = $borrowers[0]->{borrowernumber};
+my $owner  = $borrowers[0]->{borrowernumber};
+my $owner2 = $borrowers[1]->{borrowernumber};
 
 my @lists = GetPatronLists( { owner => $owner } );
 my $list_count_original = @lists;
 
 my $list1 = AddPatronList( { name => 'Test List 1', owner => $owner } );
-ok( $list1->name() eq 'Test List 1', 'AddPatronList works' );
+is( $list1->name(), 'Test List 1', 'AddPatronList works' );
 
 my $list2 = AddPatronList( { name => 'Test List 2', owner => $owner } );
 
@@ -56,13 +57,13 @@ ModPatronList(
     }
 );
 $list2->discard_changes();
-ok( $list2->name() eq 'Test List 3', 'ModPatronList works' );
+is( $list2->name(), 'Test List 3', 'ModPatronList works' );
 
 AddPatronsToList(
     { list => $list1, cardnumbers => [ map { $_->{cardnumber} } @borrowers ] }
 );
-ok(
-    scalar @borrowers ==
+is(
+    scalar @borrowers,
       $list1->patron_list_patrons()->search_related('borrowernumber')->all(),
     'AddPatronsToList works for cardnumbers'
 );
@@ -73,8 +74,8 @@ AddPatronsToList(
         borrowernumbers => [ map { $_->{borrowernumber} } @borrowers ]
     }
 );
-ok(
-    scalar @borrowers ==
+is(
+    scalar @borrowers,
       $list2->patron_list_patrons()->search_related('borrowernumber')->all(),
     'AddPatronsToList works for borrowernumbers'
 );
@@ -88,17 +89,25 @@ DelPatronsFromList(
     }
 );
 $list1->discard_changes();
-ok( !$list1->patron_list_patrons()->count(), 'DelPatronsFromList works.' );
+is( $list1->patron_list_patrons()->count(), 0, 'DelPatronsFromList works.' );
 
 @lists = GetPatronLists( { owner => $owner } );
-ok( @lists == $list_count_original + 2, 'GetPatronLists works' );
+is( scalar @lists, $list_count_original + 2, 'GetPatronLists works' );
+
+my $list3 = AddPatronList( { name => 'Test List 3', owner => $owner2, shared => 0 } );
+@lists = GetPatronLists( { owner => $owner } );
+is( scalar @lists, $list_count_original + 2, 'GetPatronLists does not return non-shared list' );
+
+my $list4 = AddPatronList( { name => 'Test List 4', owner => $owner2, shared => 1 } );
+@lists = GetPatronLists( { owner => $owner } );
+is( scalar @lists, $list_count_original + 3, 'GetPatronLists does return shared list' );
 
 DelPatronList( { patron_list_id => $list1->patron_list_id(), owner => $owner } );
 DelPatronList( { patron_list_id => $list2->patron_list_id(), owner => $owner } );
 
 @lists =
   GetPatronLists( { patron_list_id => $list1->patron_list_id(), owner => $owner } );
-ok( !@lists, 'DelPatronList works' );
+is( scalar @lists, 0, 'DelPatronList works' );
 
 $schema->storage->txn_rollback;