From e2e0c327019d43554bc6ec8a69ee5835c94f2e0e Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Thu, 7 Jun 2018 12:16:18 +0000
Subject: [PATCH] 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                           | 10 +++++-
 .../en/modules/patron_lists/add-modify.tt     |  9 ++++++
 .../prog/en/modules/patron_lists/lists.tt     | 17 ++++++++--
 patron_lists/add-modify.pl                    |  5 +--
 patron_lists/lists.pl                         |  5 ++-
 t/db_dependent/PatronLists.t                  | 31 ++++++++++++-------
 6 files changed, 60 insertions(+), 17 deletions(-)

diff --git a/Koha/List/Patron.pm b/Koha/List/Patron.pm
index 1847973bdd..c6f43848a3 100644
--- a/Koha/List/Patron.pm
+++ b/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();
 
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/add-modify.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/add-modify.tt
index 570b309bc5..23ee885598 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/add-modify.tt
+++ b/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>
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt
index f2cd32df5c..dcf3ca549f 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt
+++ b/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>
diff --git a/patron_lists/add-modify.pl b/patron_lists/add-modify.pl
index 27060a6191..956d882448 100755
--- a/patron_lists/add-modify.pl
+++ b/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() );
     }
diff --git a/patron_lists/lists.pl b/patron_lists/lists.pl
index a5cdf2b74c..8b2a97fa98 100755
--- a/patron_lists/lists.pl
+++ b/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 );
diff --git a/t/db_dependent/PatronLists.t b/t/db_dependent/PatronLists.t
index 63fbc23e8b..205538b494 100755
--- a/t/db_dependent/PatronLists.t
+++ b/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;
 
-- 
2.20.1