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>

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt
index 864429e..dfbc5e2 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt
@@ -36,29 +36,38 @@
                 </ul>
                 <div class="tabs-container">
 
-                [% IF ( ERROR ) %]
-                [% IF ( ITEMNUMBER ) %]
-                  ERROR an invalid itemnumber was entered, please hit back and try again
+                [% IF error == 'itemnumber' %]
+                    <div id="error_message" class="dialog alert">
+                        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"><input type="hidden" name="borrowernumber" id="borrowernumber" value="[% patron.borrowernumber | html %]" />
+                <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="csrf_token" value="[% csrf_token | html %]" />
-            <fieldset class="rows">
-            <legend>Manual invoice</legend>
-            <ol>
-                      <li>
-                        <label for="type">Type: </label>
-                        <select name="type" id="type">
-                          [% FOREACH debit_type IN debit_types %]
-                            <option value="[% debit_type.code | html %]">[% debit_type.description | html %]</option>
-                          [% END %]
-                        </select>
-                      </li>
-              <li><label for="barcode">Barcode: </label><input type="text" name="barcode" id="barcode" /></li>
-                      <li><label for="desc">Description: </label><input type="text" name="desc" id="desc" size="50" /></li>
-                      <li><label for="note">Note: </label><input type="text" name="note" size="50" id="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>
-            </ol>
+                    <fieldset class="rows">
+                        <legend>Manual invoice</legend>
+                        <ol>
+                            <li>
+                                <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" value="[% barcode %]" /></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" value="[% note %]" /></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">
                         <button type="submit" name="add" value="save">Save</button>
@@ -67,7 +76,6 @@
                     </fieldset>
                 </form>
 
-                [% END %]
                 </div>
             </div>
 

diff --git a/members/maninvoice.pl b/members/maninvoice.pl
index c39a323..3b4736d 100755 (executable)
--- a/members/maninvoice.pl
+++ b/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,76 +99,109 @@ if ($add) {
         }
       );
 
-# 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
-    my $barcode = $input->param('barcode');
-    my $itemnum;
+    # 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
+    my $desc       = $input->param('desc');
+    my $amount     = $input->param('amount');
+    my $note       = $input->param('note');
+    my $debit_type = $input->param('type');
+
+    # If barcode is passed, attempt to find the associated item
+    my $failed;
+    my $item_id;
+    my $issue_id;
     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 =
-      C4::Accounts::manualinvoice( $borrowernumber, $itemnum, $desc, $type,
-        $amount, $note );
-    if ($error) {
-        if ( $error =~ /FOREIGN KEY/ && $error =~ /itemnumber/ ) {
-            $template->param( 'ITEMNUMBER' => 1 );
+        if ($item) {
+            $item_id = $item->itemnumber;
         }
-        $template->param(
-            csrf_token => Koha::Token->new->generate_csrf(
-                { session_id => scalar $input->cookie('CGISESSID') }
-            )
-        );
-        $template->param( 'ERROR' => $error );
-        output_html_with_http_headers $input, $cookie, $template->output;
-    }
-    else {
-
-        if ( C4::Context->preference('AccountAutoReconcile') ) {
-            $patron->account->reconcile_balance;
+        else {
+            $item = Koha::Old::Items->find( { barcode => $barcode } );
+            if ($item) {
+                $item_id = $item->itemnumber;
+            }
+            else {
+                $template->param( error => 'itemnumber' );
+                $failed = 1;
+            }
         }
 
-        if ($add eq 'save and pay') {
-            print $input->redirect(
-                "/cgi-bin/koha/members/pay.pl?borrowernumber=$borrowernumber"
-            );
-        } else {
-            print $input->redirect(
-                "/cgi-bin/koha/members/boraccount.pl?borrowernumber=$borrowernumber"
+        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;
         }
-
-        exit;
     }
-}
-else {
 
-    my $logged_in_user = Koha::Patrons->find($loggedinuser)
-      or die "Not logged in";
-    output_and_exit_if_error(
-        $input, $cookie,
-        $template,
-        {
-            module         => 'members',
-            logged_in_user => $logged_in_user,
-            current_patron => $patron
+    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' ) {
+                print $input->redirect(
+                    "/cgi-bin/koha/members/pay.pl?borrowernumber=$borrowernumber"
+                );
+            }
+            else {
+                print $input->redirect(
+                    "/cgi-bin/koha/members/boraccount.pl?borrowernumber=$borrowernumber"
+                );
+            }
+
+            exit;
+        }
+        catch {
+            my $error = $_;
+            if ( ref($error) eq 'Koha::Exceptions::Object::FKConstraint' ) {
+                $template->param( error => $error->broken_fk );
+            }
+            else {
+                $template->param( error => $error );
+            }
         }
-    );
-
-    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(
-        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;
+    }
 }
+
+my @debit_types = Koha::Account::DebitTypes->search_with_library_limits(
+  { can_be_invoiced => 1, archived => 0 },
+  {}, $library_id );
+
+$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;