From f269c033bbcce3cc375dfa8b0928cbd63f1b8d75 Mon Sep 17 00:00:00 2001
From: Aleisha Amohia <aleishaamohia@hotmail.com>
Date: Mon, 29 Apr 2024 04:37:50 +0000
Subject: [PATCH] Bug 32256: Handle batch error messages better

This enhancement modifies the self-checkout script to better handle 'impossible' and 'needs confirmation' checkout messages.

Test with a combination of barcodes that would invoke these messages, for example:
- impossible: barcode doesn't exist
- impossible: barcode is for a 'notforloan' item type
- impossible: debt
- needs confirmation: barcode already checked out, i.e. confirm renewal

Include barcodes that should have successful checkouts too.

Also, test with the following sysprefs:
- BatchCheckouts: enabled
- BatchCheckoutsValidCategories: none selected

Confirm SCO does not have a multi-line text area for more than one barcode, and doesn't accept more than one barcode.
---
 .../bootstrap/en/modules/sco/sco-main.tt      | 80 ++++++++---------
 opac/sco/sco-main.pl                          | 87 +++++++++----------
 2 files changed, 73 insertions(+), 94 deletions(-)

diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/sco/sco-main.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/sco/sco-main.tt
index 13e510ce7b0..cd369b559dc 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/sco/sco-main.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/sco/sco-main.tt
@@ -56,59 +56,59 @@
 
                     <div id="masthead"><h1>[% LibraryName | html %] Self checkout system</h1></div>
 
-                    [% IF ( impossible ) %]<!-- We tried to issue, but failed. -->
+                    [% FOREACH item IN impossible %]<!-- We tried to issue, but failed. -->
                         <div class="alert alert-warning">
                             <span class="sco-alert-warning noissue"></span>
                             <h2>Item cannot be checked out.</h2>
                             <p>Sorry, this item cannot be checked out at this station.</p>
-                            [% IF ( title ) %]
-                                <p>Title: <em>[% title | html %]</em> </p>
+                            [% IF ( item.title ) %]
+                                <p>Title: <em>[% item.title | html %]</em> </p>
                             [% END %]
 
                             <p>
-                                [% IF ( circ_error_UNKNOWN_BARCODE ) %]
+                                [% IF ( item.circ_error_UNKNOWN_BARCODE ) %]
                                     <span id="ce_unknown_barcode">The system does not recognize this barcode.</span>
-                                [% ELSIF ( circ_error_max_loans_allowed ) %]
+                                [% ELSIF ( item.circ_error_max_loans_allowed ) %]
                                     <span id="ce_max_loans_allowed">You have reached the maximum limit of items checked out.</span>
-                                [% ELSIF ( circ_error_ISSUED_TO_ANOTHER ) %]
+                                [% ELSIF ( item.circ_error_ISSUED_TO_ANOTHER ) %]
                                     <span id="ce_issued_to_another">This item is checked out to someone else.</span>
-                                [% ELSIF ( circ_error_NO_MORE_RENEWALS ) %]
+                                [% ELSIF ( item.circ_error_NO_MORE_RENEWALS ) %]
                                     <span id="ce_no_more_renewals">You cannot renew this item again.</span>
-                                [% ELSIF ( circ_error_NOT_FOR_LOAN ) %]
+                                [% ELSIF ( item.circ_error_NOT_FOR_LOAN ) %]
                                     <span id="ce_not_for_loan">This item is not for loan.</span>
-                                [% ELSIF ( circ_error_DEBT ) %]
-                                    <span id="ce_too_much_debt">You owe the library [% DEBT | $Price with_symbol => 1%] and cannot check out.</span>
-                                [% ELSIF ( circ_error_WTHDRAWN ) %]
+                                [% ELSIF ( item.circ_error_DEBT ) %]
+                                    <span id="ce_too_much_debt">You owe the library [% item.DEBT | $Price with_symbol => 1%] and cannot check out.</span>
+                                [% ELSIF ( item.circ_error_WTHDRAWN ) %]
                                     <span id="ce_wthdrawn">This item has been withdrawn from the collection.</span>
-                                [% ELSIF ( circ_error_RESTRICTED ) %]
+                                [% ELSIF ( item.circ_error_RESTRICTED ) %]
                                     <span id="ce_restricted">This item is restricted.</span>
-                                [% ELSIF ( circ_error_RESERVED ) %]
+                                [% ELSIF ( item.circ_error_RESERVED ) %]
                                     <span id="ce_reserved">This item is on hold for another patron.</span>
-                                [% ELSIF ( circ_error_ITEMNOTSAMEBRANCH ) %]
+                                [% ELSIF ( item.circ_error_ITEMNOTSAMEBRANCH ) %]
                                     <span id="ce_itemnotsamebranch">This item belongs to another library.</span>
-                                [% ELSIF ( circ_error_EXPIRED ) %]
+                                [% ELSIF ( item.circ_error_EXPIRED ) %]
                                     <span id="ce_expired">Your account has expired.</span>
-                                [% ELSIF ( circ_error_DEBARRED ) %]
+                                [% ELSIF ( item.circ_error_DEBARRED ) %]
                                     <span id="ce_debarred">Your account has been suspended.</span>
-                                [% ELSIF ( circ_error_CARD_LOST ) %]
+                                [% ELSIF ( item.circ_error_CARD_LOST ) %]
                                     <span id="ce_card_lost">This library card has been declared lost.</span>
-                                [% ELSIF ( circ_error_GNA ) %]
+                                [% ELSIF ( item.circ_error_GNA ) %]
                                     <span id="ce_gna">Your contact information seems to be incomplete.</span>
-                                [% ELSIF ( circ_error_INVALID_DATE ) %]
+                                [% ELSIF ( item.circ_error_INVALID_DATE ) %]
                                     <span id="ce_invalid_date">Due date is not valid.</span>
-                                [% ELSIF ( circ_error_ADDITIONAL_MATERIALS ) %]
+                                [% ELSIF ( item.circ_error_ADDITIONAL_MATERIALS ) %]
                                     <span id="ce_addtional_materials">Item must be checked out at a circulation desk.</span>
                                 [% END %]
                                 <span id ="ce_see_staff">Please see a member of the library staff.</span>
                             </p>
 
-                            [% IF ( returnitem && Koha.Preference('SCOAllowCheckin') ) %]
+                            [% IF ( item.returnitem && Koha.Preference('SCOAllowCheckin') ) %]
                                 <form action="/cgi-bin/koha/sco/sco-main.pl" name="errorForm" class="inline" method="post">
                                     [% INCLUDE 'csrf-token.inc' %]
                                     <legend class="sr-only">Return</legend>
                                     <input type="hidden" name="op" value="cud-returnbook" />
                                     <input type="hidden" name="patronid" value="[% patronid | html %]" />
-                                    <input type="hidden" name="barcode" value="[% barcode | html %]" />
+                                    <input type="hidden" name="barcode" value="[% item.barcode | html %]" />
                                     <input type="hidden" name="load_checkouts" value="[% load_checkouts | html %]" />
                                     <input type="hidden" name="newissues" value="[% newissues | html %]" />
                                     <button type="submit" name="returnbook" class="btn btn-primary"><i class="fa fa-undo" aria-hidden="true"></i> Return this item</button>
@@ -119,19 +119,19 @@
                                 <legend class="sr-only">Error</legend>
                                 <input type="hidden" name="op" value="" />
                                 <input type="hidden" name="patronid" value="[% patronid | html %]" />
-                                <input type="hidden" name="barcode" value="[% barcode | html %]" />
+                                <input type="hidden" name="barcode" value="[% item.barcode | html %]" />
                                 <input type="hidden" name="load_checkouts" value="[% load_checkouts | html %]" />
                                 <input type="hidden" name="newissues" value="[% newissues | html %]" />
                                 <input type="submit" name= "confirm" value="Return to account summary" class="btn btn-info back focus" />
                             </form>
                         </div> <!-- / .alert -->
-                    [% END # / IF ( impossible %]
+                    [% END # / FOREACH impossible %]
 
-                    [% IF ( confirm ) %]<!-- We need to confirm the issue.. -->
+                    [% FOREACH c IN confirm %]<!-- We need to confirm the issue.. -->
                         <div class="alert alert-warning"><h2>Please confirm the checkout:</h2>
                             <span class="sco-alert-warning confirm"></span>
-                            [% IF ( confirm_renew_issue ) %]
-                                <p>This item is already checked out to you: <b>[% confirm | html %]</b> <i>([% barcode | html %])</i></p>
+                            [% IF ( c.confirm_renew_issue ) %]
+                                <p>This item is already checked out to you: <b>[% c.confirm | html %]</b> <i>([% c.barcode | html %])</i></p>
                             [% END %]
 
                             [% IF ( renew && Koha.Preference('SCOAllowCheckin') ) %]
@@ -140,7 +140,7 @@
                                     <legend class="sr-only">Return</legend>
                                     <input type="hidden" name="op" value="cud-returnbook" />
                                     <input type="hidden" name="patronid" value="[% patronid | html %]" />
-                                    <input type="hidden" name="barcode" value="[% barcode | html %]" />
+                                    <input type="hidden" name="barcode" value="[% c.barcode | html %]" />
                                     <input type="hidden" name="load_checkouts" value="[% load_checkouts | html %]" />
                                     <input type="hidden" name="confirmed" value="" />
                                     <input type="hidden" name="newissues" value="[% newissues | html %]" />
@@ -148,13 +148,13 @@
                                 </form>
                             [% END %]
 
-                            [% UNLESS ( renew ) %]
+                            [% UNLESS ( c.renew ) %]
                                 <form action="/cgi-bin/koha/sco/sco-main.pl" name="confirmForm" class="inline" method="post">
                                     [% INCLUDE 'csrf-token.inc' %]
                                     <legend class="sr-only">Renew</legend>
                                     <input type="hidden" name="op" value="cud-renew" />
                                     <input type="hidden" name="patronid" value="[% patronid | html %]" />
-                                    <input type="hidden" name="barcode" value="[% barcode | html %]" />
+                                    <input type="hidden" name="barcode" value="[% c.barcode | html %]" />
                                     <input type="hidden" name="load_checkouts" value="[% load_checkouts | html %]" />
                                     <input type="hidden" name="confirmed" value="1" />
                                     <input type="hidden" name="newissues" value="[% newissues | html %]" />
@@ -166,7 +166,7 @@
                                     <legend class="sr-only">Renew</legend>
                                     <input type="hidden" name="op" value="cud-renew" />
                                     <input type="hidden" name="patronid" value="[% patronid | html %]" />
-                                    <input type="hidden" name="barcode" value="[% barcode | html %]" />
+                                    <input type="hidden" name="barcode" value="[% c.barcode | html %]" />
                                     <input type="hidden" name="load_checkouts" value="[% load_checkouts | html %]" />
                                     <input type="hidden" name="confirmed" value="1" />
                                     <input type="hidden" name="newissues" value="[% newissues | html %]" />
@@ -183,7 +183,7 @@
                                 <button type="submit" class="btn btn-primary"><i class="fa fa-cancel" aria-hidden="true"></i> Cancel</button>
                             </form>
                         </div>
-                    [% END # / IF confirm %]
+                    [% END # / FOREACH confirm %]
 
                     [% IF ( nopermission ) %]
                         <!-- This is what is displayed if user doesn't have permission -->
@@ -241,18 +241,6 @@
                         </div>
                     [% END %]
 
-                    [% IF ( circ_error_BATCH_CHECKOUT ) %]
-                        <div class="alert alert-warning">
-                            <h3>Item cannot be checked out in a batch</h3>
-                            <p>Check out the following item(s) individually:</p>
-                            <ul>
-                            [% FOREACH barcode IN circ_error_BATCH_CHECKOUT %]
-                                <li><i>[% barcode | html %]</i></li>
-                            [% END %]
-                            </ul>
-                        </div>
-                    [% END %]
-
                     [% UNLESS ( hide_main ) %]
 
                         [% IF ( patronid ) %]
@@ -285,10 +273,10 @@
                                         </h2></legend>
                                     <div class="row">
                                         <div class="col">
-                                            <label for="barcode">Scan a new item or enter its barcode[% IF Koha.Preference('BatchCheckouts') %] (or for <b>multiple checkouts only,</b> enter a barcode list, one barcode per line)[% END %]:</label>
+                                            <label for="barcode">Scan a new item or enter its barcode[% IF batch_checkouts_allowed %] (or for <b>multiple checkouts only,</b> enter a barcode list, one barcode per line)[% END %]:</label>
                                         </div>
                                         <div class="col-3">
-                                            [% IF Koha.Preference('BatchCheckouts') %]
+                                            [% IF batch_checkouts_allowed %]
                                                 <textarea id="barcode" name="barcode" rows="10" cols="30" class="focus form-control"></textarea>
                                             [% ELSE %]
                                                 <input id="barcode" name="barcode" size="20" type="text" class="focus form-control" autocomplete="off" />
diff --git a/opac/sco/sco-main.pl b/opac/sco/sco-main.pl
index c049f2bb01b..26c76c2f763 100755
--- a/opac/sco/sco-main.pl
+++ b/opac/sco/sco-main.pl
@@ -138,34 +138,13 @@ my $branch = $issuer->{branchcode};
 my $confirm_required = 0;
 my $return_only = 0;
 
+my $batch_checkouts_allowed;
 if ( C4::Context->preference('BatchCheckouts') and $patron ) {
     my @batch_category_codes = split ',', C4::Context->preference('BatchCheckoutsValidCategories');
     my $categorycode = $patron->categorycode;
     if ( $categorycode && grep { $_ eq $categorycode } @batch_category_codes ) {
-        # do nothing - logged in patron is allowed to do batch checkouts
-    } else {
-        # patron category not allowed to do batch checkouts, only allow first barcode
-        my @error_barcodes;
-        while ( scalar @$barcodes > 1 ) {
-            my $error_barcode = pop @$barcodes;
-            push @error_barcodes, $error_barcode;
-        }
-
-        $template->param(
-            "circ_error_BATCH_CHECKOUT" => \@error_barcodes,
-        ) if @$barcodes;
-    }
-} else {
-    # batch checkouts not enabled, only allow first barcode
-    my @error_barcodes;
-    while ( scalar @$barcodes > 1 ) {
-        my $error_barcode = pop @$barcodes;
-        push @error_barcodes, $error_barcode;
+        $batch_checkouts_allowed = 1;
     }
-
-    $template->param(
-        "circ_error_BATCH_CHECKOUT" => \@error_barcodes,
-    ) if @$barcodes;
 }
 
 if ( $patron && $op eq "cud-returnbook" && $allowselfcheckreturns ) {
@@ -199,6 +178,8 @@ if ( $patron && $op eq "cud-returnbook" && $allowselfcheckreturns ) {
 }
 
 elsif ( $patron && ( $op eq 'cud-checkout' ) ) {
+  my @failed_checkouts;
+  my @confirm_checkouts;
   foreach my $barcode ( @$barcodes ) {
     my $item = Koha::Items->find( { barcode => $barcode } );
     my $impossible  = {};
@@ -222,46 +203,47 @@ elsif ( $patron && ( $op eq 'cud-checkout' ) ) {
     }
 
     if (scalar keys %$impossible) {
-
         my $issue_error = (keys %$impossible)[0]; # FIXME This is wrong, we assume only one error and keys are not ordered
         my $title = ( $item ) ? $item->biblio->title : '';
 
-        $template->param(
-            impossible                => $issue_error,
+        my $failed_checkout = {
             "circ_error_$issue_error" => 1,
             title                     => $title,
-            hide_main                 => 1,
-        );
+        };
+
         if ($issue_error eq 'DEBT') {
-            $template->param(DEBT => $impossible->{DEBT});
+            $failed_checkout->{DEBT} = $impossible->{DEBT};
         }
         if ( $issue_error eq "NO_MORE_RENEWALS" ) {
             $return_only = 1;
-            $template->param(
-                returnitem => 1,
-                barcode    => $barcode,
-            );
+            $failed_checkout->{barcode} = $barcode;
+            $failed_checkout->{returnitem} = 1;
         }
-        last;
-    } elsif ( $needconfirm->{RENEW_ISSUE} ){
+
+        push @failed_checkouts, $failed_checkout;
+
         $template->param(
-                renew               => 1,
-                barcode             => $barcode,
-                confirm             => $item->biblio->title,
-                confirm_renew_issue => 1,
-                hide_main           => 1,
+            hide_main => 1,
         );
-        last;
-    } elsif ( $confirm_required && !$confirmed ) {
+    } elsif ( $needconfirm->{RENEW_ISSUE} ) {
+        my $confirm_checkout = {
+            renew               => 1,
+            barcode             => $barcode,
+            confirm             => $item->biblio->title,
+            confirm_renew_issue => 1,
+        };
+        push @confirm_checkouts, $confirm_checkout;
         $template->param(
-            impossible                => 1,
-            "circ_error_$issue_error" => 1,
-            hide_main                 => 1,
+            hide_main => 1,
         );
+    } elsif ( $confirm_required && !$confirmed ) {
+        my $failed_checkout = {
+            "circ_error_$issue_error" => 1,
+        };
         if ($issue_error eq 'DEBT') {
-            $template->param(DEBT => $needconfirm->{DEBT});
+            $failed_checkout->{DEBT} = $needconfirm->{DEBT};
         }
-        last;
+        push @failed_checkouts, $failed_checkout;
     } else {
         if ( $confirmed || $issuenoconfirm ) {    # we'll want to call getpatroninfo again to get updated issues.
             my ( $hold_existed, $item );
@@ -303,14 +285,22 @@ elsif ( $patron && ( $op eq 'cud-checkout' ) ) {
             }
         } else {
             $confirm_required = 1;
-            $template->param(
+            my $confirm_checkout = {
                 confirm    => "Issuing title: " . $item->biblio->title,
                 barcode    => $barcode,
+            };
+            push @confirm_checkouts, $confirm_checkout;
+            $template->param(
                 hide_main  => 1,
             );
         }
     }
   } # foreach barcode in barcodes
+
+  $template->param(
+    impossible => \@failed_checkouts,
+    confirm    => \@confirm_checkouts,
+  );
 } # $op
 
 if ( $patron && ( $op eq 'cud-renew' ) ) {
@@ -388,6 +378,7 @@ if ( $patron) {
         howpriority   => $show_priority,
         ACCOUNT_LINES => $accountlines,
         total => $total,
+        batch_checkouts_allowed => $batch_checkouts_allowed,
     );
 
     my $patron_messages = Koha::Patron::Messages->search(
-- 
2.39.2