From 1fe879ed45f9586d15c8f84d4e991f2220f62ac5 Mon Sep 17 00:00:00 2001
From: Owen Leonard <oleonard@myacpl.org>
Date: Wed, 19 May 2021 10:55:17 +0000
Subject: [PATCH] Bug 24019: Patron batch modification based on borrowernumber

This patch adds batch patron modifications based on borrowernumber. The
user can choose to upload a file of borrowernumbers or submit a list of
borrowernumbers in a textarea, just like they can with card numbers.

To test, apply the patch and prepare files containing borrowernumbers
and card numbers. Patron lists should be enabled, and you should have at
least one patron list with patrons on it.

- Go to Tools -> Batch patron modification.
- You should see three tabs: "By card number," "By borrowernumber," and
  "By patron list."
- Test each option for batch patron modifications:
  - By card number file
  - By card number list
  - By borrowernumber file
  - By borrowernumber list
  - By patron list
- In each case the correct batch should be submitted, and modifications
  should finish correctly..
- There should be an "order of operations" for card numbers and
  borrowernumbers:
  - If a file is uploaded AND a list of numbers is entered, the list of
    numbers should be used.
- Batches should only get submitted from the active tab.
  - If you upload a file or enter card numbers in one tab and then
    switch to another tab and submit numbers from there, the original
    tab's batches should be ignored.

Signed-off-by: kelly mcelligott <kelly@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
---
 .../prog/en/modules/tools/modborrowers.tt     | 138 +++++++++++++-----
 tools/modborrowers.pl                         |  59 +++++---
 2 files changed, 143 insertions(+), 54 deletions(-)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt
index 39fbc7c12e..4d22b3e529 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt
@@ -36,45 +36,93 @@
 
                 [% IF ( op == 'show_form' ) %]
                 <h1>Batch patron modification</h1>
-                <form method="post" enctype="multipart/form-data" action="/cgi-bin/koha/tools/modborrowers.pl">
-                    <fieldset class="rows">
-                        <legend>Use a file</legend>
-                        <ol>
-                            <li><label for="uploadfile">File: </label> <input type="file" id="uploadfile" name="uploadfile" /></li>
-                        </ol>
-                    </fieldset>
-
-                    [% IF patron_lists %]
-                    <fieldset class="rows">
-                        <legend>Or use a patron list</legend>
-                        <ol>
+                <form id="patron_batchmod_form" method="post" enctype="multipart/form-data" action="/cgi-bin/koha/tools/modborrowers.pl">
+                    <input type="hidden" name="op" value="show" />
+                    <div id="batch_patron_options" class="toptabs">
+                        <ul>
                             <li>
-                                <label for="patron_list_id">Patron list: </label>
-                                <select id="patron_list_id" name="patron_list_id">
-                                    <option value=""></option>
-                                    [% FOREACH pl IN patron_lists %]
-                                        <option value="[% pl.patron_list_id | html %]">[% pl.name | html %]</option>
-                                    [% END %]
-                                </select>
+                                <a href="#usecardnumber">By card number</a>
                             </li>
-                        </ol>
-                    </fieldset>
-                    [% END %]
-
-                    <fieldset class="rows">
-                        <legend>Or list cardnumbers one by one</legend>
-                        <ol>
                             <li>
-                              <label for="cardnumberlist">Card number list (one cardnumber per line): </label>
-                              <textarea rows="10" cols="30" id="cardnumberlist" name="cardnumberlist">[% cardnumberlist | html %]</textarea>
+                                <a href="#useborrowernumber">By borrowernumber</a>
                             </li>
-                        </ol>
+                            [% IF patron_lists %]
+                                <li>
+                                    <a href="#uselist">By patron list</a>
+                                </li>
+                            [% END %]
+                        </ul>
+                        <div id="usecardnumber">
+                            <fieldset class="rows">
+                                <legend>Use a file of card numbers</legend>
+                                <ol>
+                                    <li>
+                                        <label for="cardnumberuploadfile">File: </label> <input type="file" id="cardnumberuploadfile" name="cardnumberuploadfile" />
+                                        <div class="hint">File must contain one card number per line.</div>
+                                    </li>
+                                </ol>
+                            </fieldset>
+                            <fieldset class="rows">
+                                <legend>Or list card numbers one by one</legend>
+                                <ol>
+                                    <li>
+                                    <label for="cardnumberlist">Card number list (one card number per line): </label>
+                                    <textarea rows="10" cols="30" id="cardnumberlist" name="cardnumberlist">[% cardnumberlist | html %]</textarea>
+                                    </li>
+                                </ol>
+                            </fieldset>
+                    <fieldset class="action">
+                        <input type="submit" value="Continue" class="button" />
+                        <a class="cancel" href="/cgi-bin/koha/tools/tools-home.pl">Cancel</a>
                     </fieldset>
-                    <input type="hidden" name="op" value="show" />
+                        </div>
+                        <div id="useborrowernumber">
+                            <fieldset class="rows">
+                                <legend>Use a file of borrowernumbers</legend>
+                                <ol>
+                                    <li>
+                                        <label for="borrowernumberuploadfile">File: </label> <input type="file" id="borrowernumberuploadfile" name="borrowernumberuploadfile" />
+                                        <div class="hint">File must contain one borrowernumber per line.</div>
+                                    </li>
+                                </ol>
+                            </fieldset>
+                            <fieldset class="rows">
+                                <legend>List borrowernumbers one by one</legend>
+                                <ol>
+                                    <li>
+                                    <label for="borrowernumberlist">Borrowernumber list (one number per line): </label>
+                                    <textarea rows="10" cols="30" id="borrowernumberlist" name="borrowernumberlist">[% borrowernumberlist | html %]</textarea>
+                                    </li>
+                                </ol>
+                            </fieldset>
+                    <fieldset class="action">
+                        <input type="submit" value="Continue" class="button" />
+                        <a class="cancel" href="/cgi-bin/koha/tools/tools-home.pl">Cancel</a>
+                    </fieldset>
+                        </div>
+                        [% IF patron_lists %]
+                            <div id="uselist">
+                                <fieldset class="rows">
+                                    <legend>Use a patron list</legend>
+                                    <ol>
+                                        <li>
+                                            <label for="patron_list_id">Patron list: </label>
+                                            <select id="patron_list_id" name="patron_list_id">
+                                                <option value=""> -- Choose a patron list -- </option>
+                                                [% FOREACH pl IN patron_lists %]
+                                                    <option value="[% pl.patron_list_id | html %]">[% pl.name | html %]</option>
+                                                [% END %]
+                                            </select>
+                                        </li>
+                                    </ol>
+                                </fieldset>
                     <fieldset class="action">
                         <input type="submit" value="Continue" class="button" />
                         <a class="cancel" href="/cgi-bin/koha/tools/tools-home.pl">Cancel</a>
                     </fieldset>
+                            </div>
+                        [% END %]
+                    </div><!-- /#batch_patron_options -->
                 </form>
                 [% END %]
 
@@ -82,7 +130,7 @@
                     [% op = 'noshow' # Change op to prevent display in code below %]
                     <h1>Batch patrons modification</h1>
                     <div class="dialog alert">
-                        <p>No patron card numbers given.</p>
+                        <p>No patron card numbers or borrowernumbers given.</p>
                         <form action="/cgi-bin/koha/tools/modborrowers.pl" method="get">
                          <button type="submit" class="approve"><i class="fa fa-fw fa-check"></i> OK</button>
                         </form>
@@ -96,10 +144,19 @@
                         <h1>Batch patrons results</h1>
                     [% END %]
                     [% IF ( notfoundcardnumbers ) %]
-                        <div class="dialog alert"><p>Warning, the following cardnumbers were not found:</p></div>
+                        [% IF ( useborrowernumbers ) -%]
+                            <div class="dialog alert"><p>Warning, the following borrowernumbers were not found:</p></div>
+                        [% ELSE -%]
+                            <div class="dialog alert"><p>Warning, the following card numbers were not found:</p></div>
+                        [% END %]
+
                         <table style="margin:auto;">
                             <thead>
-                                <tr><th>Cardnumbers not found</th></tr>
+                                [% IF ( useborrowernumbers ) -%]
+                                    <tr><th>Borrowernumbers not found</th></tr>
+                                [% ELSE -%]
+                                    <tr><th>Card numbers not found</th></tr>
+                                [% END %]
                             </thead>
                             <tbody>
                                 [% FOREACH notfoundcardnumber IN notfoundcardnumbers %]
@@ -117,7 +174,7 @@
                             [% FOREACH error IN errors %]
                                 [% IF ( error.error == 'can_not_update' ) %]
                                     <li>Can not update patron.
-                                    [% IF ( error.cardnumber ) %] Cardnumber:  [% error.cardnumber | html %] [% END %]
+                                    [% IF ( error.cardnumber ) %] Card number:  [% error.cardnumber | html %] [% END %]
                                     (Borrowernumber: [% error.borrowernumber | html %])
                                     </li>
                                 [% ELSE %]
@@ -387,6 +444,8 @@
                     });
                     return false;
                 });
+            [% ELSE %]
+                $("#batch_patron_options").tabs();
             [% END %]
 
             var values = new Array();
@@ -421,6 +480,17 @@
                 e.preventDefault();
                 del_attributes(this);
             });
+            $("#patron_batchmod_form").on("submit", function(){
+                /* Reset form fields on inactive tabs */
+                var tab = $(this).find('.ui-tabs-active:first a').attr('href');
+                if ( tab == '#usecardnumber' ) {
+                    $("#borrowernumberuploadfile, #patron_list_id, #borrowernumberlist").val("");
+                } else if ( tab == '#useborrowernumber' ) {
+                    $("#cardnumberuploadfile, #cardnumberlist, #patron_list_id").val("");
+                } else { // uselist
+                    $("#borrowernumberuploadfile, #cardnumberuploadfile, #borrowernumberlist, #cardnumberlist").val("");
+                }
+            });
         });
 
         function updateAttrValues (select_attr) {
diff --git a/tools/modborrowers.pl b/tools/modborrowers.pl
index a12c81d715..4ff7dfe6db 100755
--- a/tools/modborrowers.pl
+++ b/tools/modborrowers.pl
@@ -56,35 +56,53 @@ my $dbh       = C4::Context->dbh;
 
 # Show borrower informations
 if ( $op eq 'show' ) {
-    my $filefh         = $input->upload('uploadfile');
-    my $filecontent    = $input->param('filecontent');
-    my $patron_list_id = $input->param('patron_list_id');
     my @borrowers;
-    my @cardnumbers;
+    my @patronidnumbers;
     my @notfoundcardnumbers;
+    my $useborrowernumbers = 0;
 
     # Get cardnumbers from a file or the input area
-    if ($filefh) {
-        while ( my $content = <$filefh> ) {
-            $content =~ s/[\r\n]*$//g;
-            push @cardnumbers, $content if $content;
+    if( my $cardnumberlist = $input->param('cardnumberlist') ){
+        # User submitted a list of card numbers
+        push @patronidnumbers, split( /\s\n/, $cardnumberlist );
+    } elsif ( my $cardnumberuploadfile = $input->param('cardnumberuploadfile') ){
+        # User uploaded a file of card numbers
+        binmode $cardnumberuploadfile, ':encoding(UTF-8)';
+        while ( my $content = <$cardnumberuploadfile> ) {
+            next unless $content;
+            $content =~ s/[\r\n]*$//;
+            push @patronidnumbers, $content if $content;
         }
-    } elsif ( $patron_list_id ) {
+    } elsif ( my $borrowernumberlist = $input->param('borrowernumberlist') ){
+        # User submitted a list of borrowernumbers
+        $useborrowernumbers = 1;
+        push @patronidnumbers, split( /\s\n/, $borrowernumberlist );
+    } elsif ( my $borrowernumberuploadfile = $input->param('borrowernumberuploadfile') ){
+        # User uploaded a file of borrowernumbers
+        $useborrowernumbers = 1;
+        binmode $borrowernumberuploadfile, ':encoding(UTF-8)';
+        while ( my $content = <$borrowernumberuploadfile> ) {
+            next unless $content;
+            $content =~ s/[\r\n]*$//;
+            push @patronidnumbers, $content if $content;
+        }
+    } elsif ( my $patron_list_id = $input->param('patron_list_id') ){
+        # User selected a patron list
         my ($list) = GetPatronLists( { patron_list_id => $patron_list_id } );
-
-        @cardnumbers =
+        @patronidnumbers =
           $list->patron_list_patrons()->search_related('borrowernumber')
           ->get_column('cardnumber')->all();
-
-    } else {
-        if ( my $list = $input->param('cardnumberlist') ) {
-            push @cardnumbers, split( /\s\n/, $list );
-        }
     }
 
     my $max_nb_attr = 0;
-    for my $cardnumber ( @cardnumbers ) {
-        my $patron = Koha::Patrons->find( { cardnumber => $cardnumber } );
+
+    for my $patronidnumber ( @patronidnumbers ) {
+        my $patron;
+        if( $useborrowernumbers == 1 ){
+            $patron = Koha::Patrons->find( { borrowernumber => $patronidnumber } );
+        } else {
+            $patron = Koha::Patrons->find( { cardnumber => $patronidnumber } );
+        }
         if ( $patron ) {
             if ( $logged_in_user->can_see_patron_infos( $patron ) ) {
                 my $borrower = $patron->unblessed;
@@ -94,10 +112,10 @@ if ( $op eq 'show' ) {
                 $max_nb_attr = $borrower->{patron_attributes_count} if $borrower->{patron_attributes_count} > $max_nb_attr;
                 push @borrowers, $borrower;
             } else {
-                push @notfoundcardnumbers, $cardnumber;
+                push @notfoundcardnumbers, $patronidnumber;
             }
         } else {
-            push @notfoundcardnumbers, $cardnumber;
+            push @notfoundcardnumbers, $patronidnumber;
         }
     }
 
@@ -149,6 +167,7 @@ if ( $op eq 'show' ) {
     @notfoundcardnumbers = map { { cardnumber => $_ } } @notfoundcardnumbers;
     $template->param( notfoundcardnumbers => \@notfoundcardnumbers )
         if @notfoundcardnumbers;
+    $template->param( useborrowernumbers => $useborrowernumbers );
 
     # Construct drop-down list values
     my $branches = Koha::Libraries->search({}, { order_by => ['branchname'] })->unblessed;
-- 
2.30.2