Bug 22393: Remove last remaining use of C4::Accounts::manualinvoice

This patch re-arranges the manualinvoice controller script to clarify
code flow, replaces the last call to C4::Accounts::manualinvoice with a
call to Koha::Accounts->add_debit wrapped in a try catch block and also
adds a check on passed barcodes when the invoice type is 'LOST' so it
can link the subsequently created accountline to the item and issue.

Test plan
1/ Add a manual invoice (without entering a barcode)
2/ Add a manual invoice with a valid barcode (Not a LOST type)
3/ Add a manual invoice with a valid, but old, barcode (Not a LOST type)
4/ Add a manual invoice with an invalid barcode, note that an error is
displayed
5/ Add a manual invoice with type 'LOST' and a valid barcode for a
checkout your user has had checked out
6/ Add a manual invoice with type 'LOST' and a valid barcode, but not
one that will match a checkout for your user. Note an error is displayed
7/ When errors are displayed, note the form contains data from the
previous submission so you can just correct the error rather than
re-enter all data.
8/ Signoff

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt

@@ -36,12 +36,17 @@
                 </ul>
                 <div class="tabs-container">
 
-                [% IF ( ERROR ) %]
+                [% IF error == 'itemnumber' %]
-                [% IF ( ITEMNUMBER ) %]
+                    <div id="error_message" class="dialog alert">
-                  ERROR an invalid itemnumber was entered, please hit back and try again
+                        Error: Invalid barcode entered, please try again
+                    </div>
+                [% ELSIF error %]
+                    <div id="error_message" class="dialog alert">
+                        An error occured, please try again: [% error %]
+                    </div>
                 [% END %]
-                [% ELSE %]
+                <form action="/cgi-bin/koha/members/maninvoice.pl" method="post" id="maninvoice">
-                <form action="/cgi-bin/koha/members/maninvoice.pl" method="post" id="maninvoice"><input type="hidden" name="borrowernumber" id="borrowernumber" value="[% patron.borrowernumber | html %]" />
+                    <input type="hidden" name="borrowernumber" id="borrowernumber" value="[% patron.borrowernumber | html %]" />
                     <input type="hidden" name="csrf_token" value="[% csrf_token | html %]" />
                     <fieldset class="rows">
                         <legend>Manual invoice</legend>
@@ -50,14 +55,18 @@
                                 <label for="type">Type: </label>
                                 <select name="type" id="type">
                                     [% FOREACH debit_type IN debit_types %]
+                                    [% IF debit_type.code == type %]
+                                    <option value="[% debit_type.code | html %]" selected="selected">[% debit_type.description | html %]</option>
+                                    [% ELSE %]
                                     <option value="[% debit_type.code | html %]">[% debit_type.description | html %]</option>
                                     [% END %]
+                                    [% END %]
                                 </select>
                             </li>
-              <li><label for="barcode">Barcode: </label><input type="text" name="barcode" id="barcode" /></li>
+                            <li><label for="barcode">Barcode: </label><input type="text" name="barcode" id="barcode" value="[% barcode %]" /></li>
-                      <li><label for="desc">Description: </label><input type="text" name="desc" id="desc" size="50" /></li>
+                            <li><label for="desc">Description: </label><input type="text" name="desc" id="desc" size="50" value="[% desc %]" /></li>
-                      <li><label for="note">Note: </label><input type="text" name="note" size="50" id="note" /></li>
+                            <li><label for="note">Note: </label><input type="text" name="note" size="50" id="note" value="[% note %]" /></li>
-                      <li><label for="amount">Amount: </label><input type="number" name="amount" id="amount" required="required" value="" step="any" min="0" /> Example: 5.00</li>
+                            <li><label for="amount">Amount: </label><input type="number" name="amount" id="amount" required="required" step="any" min="0" value="[% amount %]" /> Example: 5.00</li>
                         </ol>
                     </fieldset>
                     <fieldset class="action">
@@ -67,7 +76,6 @@
                     </fieldset>
                 </form>
 
-                [% END %]
                 </div>
             </div>
 

members/maninvoice.pl

@@ -23,6 +23,7 @@
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
 use Modern::Perl;
+use Try::Tiny;
 
 use C4::Auth;
 use C4::Output;
@@ -32,8 +33,11 @@ use C4::Accounts;
 use C4::Items;
 use Koha::Token;
 
-use Koha::Items;
 use Koha::Patrons;
+use Koha::Items;
+use Koha::Old::Items;
+use Koha::Checkouts;
+use Koha::Old::Checkouts;
 
 use Koha::Patron::Categories;
 use Koha::Account::DebitTypes;
@@ -60,7 +64,30 @@ unless ($patron) {
     exit;
 }
 
+my $logged_in_user = Koha::Patrons->find($loggedinuser);
+output_and_exit_if_error(
+    $input, $cookie,
+    $template,
+    {
+        module         => 'members',
+        logged_in_user => $logged_in_user,
+        current_patron => $patron
+    }
+);
+
 my $library_id = C4::Context->userenv->{'branch'};
+my $desc       = $input->param('desc');
+my $amount     = $input->param('amount');
+my $note       = $input->param('note');
+my $debit_type = $input->param('type');
+my $barcode    = $input->param('barcode');
+$template->param(
+    desc    => $desc,
+    amount  => $amount,
+    note    => $note,
+    type    => $debit_type,
+    barcode => $barcode
+);
 
 my $add = $input->param('add');
 if ($add) {
@@ -72,44 +99,80 @@ if ($add) {
         }
       );
 
-# Note: If the logged in user is not allowed to see this patron an invoice can be forced
+    # Note: If the logged in user is not allowed to see this patron an invoice can be forced
-# Here we are trusting librarians not to hack the system
+    # Here we are trusting librarians not to hack the system
-    my $barcode = $input->param('barcode');
-    my $itemnum;
-    if ($barcode) {
-        my $item = Koha::Items->find( { barcode => $barcode } );
-        $itemnum = $item->itemnumber if $item;
-    }
     my $desc       = $input->param('desc');
     my $amount     = $input->param('amount');
-    my $type   = $input->param('type');
     my $note       = $input->param('note');
-    my $error =
+    my $debit_type = $input->param('type');
-      C4::Accounts::manualinvoice( $borrowernumber, $itemnum, $desc, $type,
+
-        $amount, $note );
+    # If barcode is passed, attempt to find the associated item
-    if ($error) {
+    my $failed;
-        if ( $error =~ /FOREIGN KEY/ && $error =~ /itemnumber/ ) {
+    my $item_id;
-            $template->param( 'ITEMNUMBER' => 1 );
+    my $issue_id;
-        }
+    if ($barcode) {
-        $template->param(
+        my $item = Koha::Items->find( { barcode => $barcode } );
-            csrf_token => Koha::Token->new->generate_csrf(
+        if ($item) {
-                { session_id => scalar $input->cookie('CGISESSID') }
+            $item_id = $item->itemnumber;
-            )
-        );
-        $template->param( 'ERROR' => $error );
-        output_html_with_http_headers $input, $cookie, $template->output;
         }
         else {
+            $item = Koha::Old::Items->find( { barcode => $barcode } );
+            if ($item) {
+                $item_id = $item->itemnumber;
+            }
+            else {
+                $template->param( error => 'itemnumber' );
+                $failed = 1;
+            }
+        }
+
+        if ( ( $debit_type eq 'LOST' ) && $item_id ) {
+            my $checkouts = Koha::Checkouts->search(
+                {
+                    itemnumber     => $item_id,
+                    borrowernumber => $borrowernumber
+                }
+            );
+            my $checkout =
+                $checkouts->count
+              ? $checkouts->next
+              : Koha::Old::Checkouts->search(
+                {
+                    itemnumber     => $item_id,
+                    borrowernumber => $borrowernumber
+                },
+                { order_by => { -desc => 'returndate' }, rows => 1 }
+            )->next;
+            $issue_id = $checkout ? $checkout->issue_id : undef;
+        }
+    }
+
+    unless ($failed) {
+        try {
+            $patron->account->add_debit(
+                {
+                    amount      => $amount,
+                    description => $desc,
+                    note        => $note,
+                    user_id     => $logged_in_user->borrowernumber,
+                    interface   => 'intranet',
+                    library_id  => $library_id,
+                    type        => $debit_type,
+                    item_id     => $item_id,
+                    issue_id    => $issue_id
+                }
+            );
 
             if ( C4::Context->preference('AccountAutoReconcile') ) {
                 $patron->account->reconcile_balance;
             }
 
-        if ($add eq 'save and pay') {
+            if ( $add eq 'save and pay' ) {
                 print $input->redirect(
                     "/cgi-bin/koha/members/pay.pl?borrowernumber=$borrowernumber"
                 );
-        } else {
+            }
+            else {
                 print $input->redirect(
                     "/cgi-bin/koha/members/boraccount.pl?borrowernumber=$borrowernumber"
                 );
@@ -117,31 +180,28 @@ if ($add) {
 
             exit;
         }
-}
+        catch {
-else {
+            my $error = $_;
-
+            if ( ref($error) eq 'Koha::Exceptions::Object::FKConstraint' ) {
-    my $logged_in_user = Koha::Patrons->find($loggedinuser)
+                $template->param( error => $error->broken_fk );
-      or die "Not logged in";
-    output_and_exit_if_error(
-        $input, $cookie,
-        $template,
-        {
-            module         => 'members',
-            logged_in_user => $logged_in_user,
-            current_patron => $patron
             }
-    );
+            else {
+                $template->param( error => $error );
+            }
+        }
+    }
+}
 
-    my @debit_types = Koha::Account::DebitTypes->search_with_library_limits(
+my @debit_types = Koha::Account::DebitTypes->search_with_library_limits(
   { can_be_invoiced => 1, archived => 0 },
   {}, $library_id );
-    $template->param( debit_types => \@debit_types );
+
-    $template->param(
+$template->param(
+  debit_types => \@debit_types,
   csrf_token  => Koha::Token->new->generate_csrf(
       { session_id => scalar $input->cookie('CGISESSID') }
   ),
   patron    => $patron,
   finesview => 1,
   );
-    output_html_with_http_headers $input, $cookie, $template->output;
+output_html_with_http_headers $input, $cookie, $template->output;
-}