From 0d19d9fa44dfddcaef06ea6e59feab5ed8dbb01e Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 10 Feb 2020 14:55:13 +0000 Subject: [PATCH] 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 Signed-off-by: Katrin Fischer Signed-off-by: Jonathan Druart --- .../prog/en/modules/members/maninvoice.tt | 52 ++--- members/maninvoice.pl | 184 ++++++++++++------ 2 files changed, 152 insertions(+), 84 deletions(-) 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 864429e425..dfbc5e2338 100644 --- 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 @@
- [% IF ( ERROR ) %] - [% IF ( ITEMNUMBER ) %] - ERROR an invalid itemnumber was entered, please hit back and try again + [% IF error == 'itemnumber' %] +
+ Error: Invalid barcode entered, please try again +
+ [% ELSIF error %] +
+ An error occured, please try again: [% error %] +
[% END %] - [% ELSE %] -
+ + -
- Manual invoice -
    -
  1. - - -
    2. -
    3. -
    4. -
    5. -
  5. Example: 5.00
    6. -
+
+ Manual invoice +
    +
  1. + + +
    2. +
    3. +
    4. +
    5. +
  5. Example: 5.00
    6. +
@@ -67,7 +76,6 @@
- [% END %]
diff --git a/members/maninvoice.pl b/members/maninvoice.pl index c39a323cae..3b4736d8d3 100755 --- a/members/maninvoice.pl +++ b/members/maninvoice.pl @@ -23,6 +23,7 @@ # along with Koha; if not, see . 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; + 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 ); + } + } } } -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 - } - ); +my @debit_types = Koha::Account::DebitTypes->search_with_library_limits( + { can_be_invoiced => 1, archived => 0 }, + {}, $library_id ); - 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; -} +$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;