Bugzilla – Attachment 94434 Details for
Bug 23051
Add ability to optionally renew fine accruing items when all fines on item are paid off
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 23051: (follow-up) Fix/improve feedback
Bug-23051-follow-up-Fiximprove-feedback.patch (text/plain), 13.00 KB, created by
Andrew Isherwood
on 2019-10-21 11:11:24 UTC
(
hide
)
Description:
Bug 23051: (follow-up) Fix/improve feedback
Filename:
MIME Type:
Creator:
Andrew Isherwood
Created:
2019-10-21 11:11:24 UTC
Size:
13.00 KB
patch
obsolete
>From 791d4e20578052b156be8acd6f342c765f8b1718 Mon Sep 17 00:00:00 2001 >From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> >Date: Mon, 21 Oct 2019 12:09:13 +0100 >Subject: [PATCH] Bug 23051: (follow-up) Fix/improve feedback > >This follow up patch addresses the following parts of Nick's feedback in >comment #35: > >- it would be nice to get feedback on what was successfully renewed as well >- In general I think I would prefer to see 'ok' and 'not_ok' returned as >a single 'renewal_results' array >- There is no listing of errors if I use the 'pay' button on an >individual fine >--- > Koha/Account.pm | 19 ++++------ > .../prog/en/includes/renew_results.inc | 12 ++++++ > .../{renew_error_strings.inc => renew_strings.inc} | 0 > .../prog/en/modules/members/boraccount.tt | 11 +----- > .../intranet-tmpl/prog/en/modules/members/pay.tt | 9 +---- > members/boraccount.pl | 20 +++++++--- > members/pay.pl | 21 +++++++---- > members/paycollect.pl | 43 ++++++++++++---------- > 8 files changed, 75 insertions(+), 60 deletions(-) > create mode 100644 koha-tmpl/intranet-tmpl/prog/en/includes/renew_results.inc > rename koha-tmpl/intranet-tmpl/prog/en/includes/{renew_error_strings.inc => renew_strings.inc} (100%) > >diff --git a/Koha/Account.pm b/Koha/Account.pm >index 61e3a8ca33..7d64ec7bb9 100644 >--- a/Koha/Account.pm >+++ b/Koha/Account.pm >@@ -758,16 +758,12 @@ and, if the syspref is set, renew them > sub _maybe_renew { > my ($items, $patron_id) = @_; > >- my $results = { >- ok => [], >- not_ok => [] >- }; >+ my @results = (); > > if ( > C4::Context->preference('RenewAccruingItemWhenPaid') && > keys %{$items} > ) { >- > foreach my $itemnumber (keys %{$items}) { > # Only do something if this item has no fines left on it > my $fine = C4::Overdues::GetFine( $itemnumber, $patron_id ); >@@ -784,20 +780,21 @@ sub _maybe_renew { > undef, > 1 > ); >- push @{$results->{ok}}, { >+ push @results, { > itemnumber => $itemnumber, >- due_date => $due_date >+ due_date => $due_date, >+ success => 1 > }; > } else { >- push @{$results->{not_ok}}, { >+ push @results, { > itemnumber => $itemnumber, >- error => $error >+ error => $error, >+ success => 0 > }; > } > } >- return $results; > } >- return $results; >+ return \@results; > } > > 1; >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/renew_results.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/renew_results.inc >new file mode 100644 >index 0000000000..4c4ab3c056 >--- /dev/null >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/renew_results.inc >@@ -0,0 +1,12 @@ >+[% IF renew_results && renew_results.size > 0 %] >+ <div class="alert"> >+ The fines on the following items were paid off, renewal results are displayed below: >+ [% FOREACH result IN renew_results %] >+ [% IF result.success %] >+ <p>[% INCLUDE 'biblio-title.inc' biblio=result.item.biblio %] ( [% result.item.barcode | html %] ): Renewed - due [% result.info | html %]</p> >+ [% ELSE %] >+ <p>[% INCLUDE 'biblio-title.inc' biblio=result.item.biblio %] ( [% result.item.barcode | html %] ): Not renewed - [% INCLUDE 'renew_strings.inc' error=result.info %]</p> >+ [% END %] >+ [% END %] >+ </div> >+[% END %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/renew_error_strings.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/renew_strings.inc >similarity index 100% >rename from koha-tmpl/intranet-tmpl/prog/en/includes/renew_error_strings.inc >rename to koha-tmpl/intranet-tmpl/prog/en/includes/renew_strings.inc >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt >index 5bf5382dab..100044736b 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt >@@ -38,15 +38,8 @@ > <li><a href="/cgi-bin/koha/members/maninvoice.pl?borrowernumber=[% patron.borrowernumber | uri %]" >Create manual invoice</a></li> > <li><a href="/cgi-bin/koha/members/mancredit.pl?borrowernumber=[% patron.borrowernumber | uri %]" >Create manual credit</a></li> > </ul> >-<div class="tabs-container"> >-[% IF renew_errors.size > 0 %] >- <div class="alert"> >- The fines on the following items were paid off, but the items could not be renewed: >- [% FOREACH problem IN renew_errors %] >- <p>[% INCLUDE 'biblio-title.inc' biblio=problem.item.biblio %] ( [% problem.item.barcode | html %] ): [% INCLUDE 'renew_error_strings.inc' error=problem.error %]</p> >- [% END %] >- </div> >-[% END %] >+<div class="tabs-container"> >+[% INCLUDE 'renew_results.inc' renew_results=renew_results %] > <!-- The table with the account items --> > <table id="table_account_fines"> > <thead> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt >index 6fd8317dc2..bc4b55a927 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt >@@ -40,14 +40,7 @@ > <form action="/cgi-bin/koha/members/pay.pl" method="post" id="pay-fines-form"> > <input type="hidden" name="borrowernumber" id="borrowernumber" value="[% patron.borrowernumber | html %]" /> > <p><span class="checkall"><a id="CheckAll" href="#"><i class="fa fa-check"></i> Select all</a></span> | <span class="clearall"><a id="CheckNone" href="#"><i class="fa fa-remove"></i> Clear all</a></span></p> >-[% IF renew_errors.size > 0 %] >- <div class="alert"> >- The fines on the following items were paid off, but the items could not be renewed: >- [% FOREACH problem IN renew_errors %] >- <p>[% INCLUDE 'biblio-title.inc' biblio=problem.item.biblio %] ( [% problem.item.barcode | html %] ): [% INCLUDE 'renew_error_strings.inc' error=problem.error %]</p> >- [% END %] >- </div> >-[% END %] >+[% INCLUDE 'renew_results.inc' renew_results=renew_results %] > <table id="finest"> > <thead> > <tr> >diff --git a/members/boraccount.pl b/members/boraccount.pl >index 91994158b5..85f765218c 100755 >--- a/members/boraccount.pl >+++ b/members/boraccount.pl >@@ -23,6 +23,7 @@ > # along with Koha; if not, see <http://www.gnu.org/licenses>. > > use Modern::Perl; >+use URI::Escape; > > use C4::Auth; > use C4::Output; >@@ -52,7 +53,7 @@ my $borrowernumber = $input->param('borrowernumber'); > my $payment_id = $input->param('payment_id'); > my $change_given = $input->param('change_given'); > my $action = $input->param('action') || ''; >-my @renew_errors = $input->param('renew_error'); >+my @renew_results = $input->param('renew_result'); > > my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in"; > my $patron = Koha::Patrons->find( $borrowernumber ); >@@ -84,11 +85,18 @@ if($total <= 0){ > > # Populate an arrayref with everything we need to display any > # renew errors that occurred based on what we were passed >-my $renew_errors_display = []; >-foreach my $renew_error(@renew_errors) { >- my ($itemnumber, $error) = split(/,/, $renew_error); >+my $renew_results_display = []; >+foreach my $renew_result(@renew_results) { >+ my ($itemnumber, $success, $info) = split(/,/, $renew_result); > my $item = Koha::Items->find($itemnumber); >- push @{$renew_errors_display}, { item => $item, error => $error }; >+ if ($success) { >+ $info = uri_unescape($info); >+ } >+ push @{$renew_results_display}, { >+ item => $item, >+ success => $success, >+ info => $info >+ }; > } > > $template->param( >@@ -99,7 +107,7 @@ $template->param( > accounts => \@accountlines, > payment_id => $payment_id, > change_given => $change_given, >- renew_errors => $renew_errors_display, >+ renew_results => $renew_results_display, > ); > > output_html_with_http_headers $input, $cookie, $template->output; >diff --git a/members/pay.pl b/members/pay.pl >index 505a6360c1..8a1f617a1c 100755 >--- a/members/pay.pl >+++ b/members/pay.pl >@@ -66,7 +66,7 @@ if ( !$borrowernumber ) { > > my $payment_id = $input->param('payment_id'); > our $change_given = $input->param('change_given'); >-our @renew_errors = $input->param('renew_error'); >+our @renew_results = $input->param('renew_result'); > > # get borrower details > my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in"; >@@ -138,19 +138,26 @@ for (@names) { > } > > # Populate an arrayref with everything we need to display any >-# renew errors that occurred based on what we were passed >-my $renew_errors_display = []; >-foreach my $renew_error(@renew_errors) { >- my ($itemnumber, $error) = split(/,/, $renew_error); >+# renew results that occurred based on what we were passed >+my $renew_results_display = []; >+foreach my $renew_result(@renew_results) { >+ my ($itemnumber, $success, $info) = split(/,/, $renew_result); > my $item = Koha::Items->find($itemnumber); >- push @{$renew_errors_display}, { item => $item, error => $error }; >+ if ($success) { >+ $info = uri_unescape($info); >+ } >+ push @{$renew_results_display}, { >+ item => $item, >+ success => $success, >+ info => $info >+ }; > } > > $template->param( > finesview => 1, > payment_id => $payment_id, > change_given => $change_given, >- renew_errors => $renew_errors_display >+ renew_results => $renew_results_display > ); > > add_accounts_to_template(); >diff --git a/members/paycollect.pl b/members/paycollect.pl >index db9587a139..6c03024c3c 100755 >--- a/members/paycollect.pl >+++ b/members/paycollect.pl >@@ -34,6 +34,7 @@ use Koha::Patron::Categories; > use Koha::AuthorisedValues; > use Koha::Account; > use Koha::Token; >+use Koha::DateUtils; > > my $input = CGI->new(); > >@@ -150,9 +151,11 @@ if ( $total_paid and $total_paid ne '0.00' ) { > token => scalar $input->param('csrf_token'), > }); > >+ my $url; >+ my $pay_result; > if ($pay_individual) { > my $line = Koha::Account::Lines->find($accountlines_id); >- my $pay_result = $account->pay( >+ $pay_result = $account->pay( > { > lines => [$line], > amount => $total_paid, >@@ -164,17 +167,9 @@ if ( $total_paid and $total_paid ne '0.00' ) { > } > ); > $payment_id = $pay_result->{payment_id}; >- # It's possible renewals took place, parse any renew results >- # and pass errors on >- my @renew_errors = map { "renew_error=$_->{itemnumber},$_->{error}" } >- @{$pay_result->{renew_result}->{not_ok}}; >- my $renew_result = $pay_result->{renew_result}; >- my $append = scalar @renew_errors ? '&' . join('&', @renew_errors) : ''; >- >- print $input->redirect( >- "/cgi-bin/koha/members/pay.pl?borrowernumber=$borrowernumber&payment_id=$payment_id&change_given=${change_given}${append}"); >+ >+ $url = "/cgi-bin/koha/members/pay.pl"; > } else { >- my $pay_result; > if ($select) { > if ( $select =~ /^([\d,]*).*/ ) { > $select = $1; # ensure passing no junk >@@ -220,15 +215,25 @@ if ( $total_paid and $total_paid ne '0.00' ) { > ); > } > $payment_id = $pay_result->{payment_id}; >- # It's possible renewals took place, parse any renew results >- # and pass errors on >- my @renew_errors = map { "renew_error=$_->{itemnumber},$_->{error}" } >- @{$pay_result->{renew_result}->{not_ok}}; >- my $renew_result = $pay_result->{renew_result}; >- my $append = scalar @renew_errors ? '&' . join('&', @renew_errors) : ''; >- >- print $input->redirect("/cgi-bin/koha/members/boraccount.pl?borrowernumber=$borrowernumber&payment_id=$payment_id&change_given=${change_given}${append}"); >+ >+ $url = "/cgi-bin/koha/members/boraccount.pl"; >+ } >+ # It's possible renewals took place, parse any renew results >+ # and pass on >+ my @renew_result = (); >+ foreach my $ren( @{$pay_result->{renew_result}} ) { >+ my $str = "renew_result=$ren->{itemnumber},$ren->{success},"; >+ my $app = $ren->{success} ? >+ uri_escape( >+ output_pref({ dt => $ren->{due_date}, as_due_date => 1 }) >+ ) : $ren->{error}; >+ push @renew_result, "${str}${app}"; > } >+ my $append = scalar @renew_result ? '&' . join('&', @renew_result) : ''; >+ >+ $url .= "?borrowernumber=$borrowernumber&payment_id=$payment_id&change_given=${change_given}${append}"; >+ >+ print $input->redirect($url); > } > } else { > $total_paid = '0.00'; #TODO not right with pay_individual >-- >2.11.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=94434">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 23051
:
90559
|
90560
|
90561
|
91058
|
91059
|
91060
|
91792
|
91793
|
91794
|
91795
|
92684
|
92685
|
92686
|
92687
|
92688
|
92689
|
93922
|
93923
|
93924
|
93925
|
94020
|
94021
|
94022
|
94023
|
94024
|
94402
|
94434
|
94453
|
94525
|
94604
|
94605
|
94606
|
94607
|
94608
|
94609
|
94610
|
94611
|
94629
|
94649
|
95557
|
95558
|
95559
|
95560
|
100127
|
100128
|
100129
|
100130
|
100131
|
100132
|
100133
|
100134
|
100135
|
100138
|
100172
|
100182
|
100183
|
100184
|
100185
|
100186
|
100187
|
100188
|
100271