From b4e7dc933df6d9236456aecdced5f29ded490bf6 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
Content-Type: text/plain; charset=utf-8

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>
---
 Koha/List/Patron.pm                                | 10 ++++++-
 .../prog/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 1847973..c6f4384 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 bf89a4d..62612f6 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 @@
                     </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>
                 </ol>
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 f2cd32d..dcf3ca5 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,24 +32,37 @@
                 <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="#">
                                    Actions <b class="caret"></b>
                                 </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 27060a6..956d882 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 a5cdf2b..8b2a97f 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 63fbc23..205538b 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.1.4