Bug 17100 - On summary print, "Account fines and payments" is displayed even if there is nothing to pay
Summary: On summary print, "Account fines and payments" is displayed even if there is ...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 20656
  Show dependency treegraph
 
Reported: 2016-08-10 08:17 UTC by Sophie MEYNIEUX
Modified: 2018-04-24 21:03 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 17100 : Fix "Account fines and payments" diplay on patron's summary print (782 bytes, patch)
2016-08-10 08:22 UTC, Sophie MEYNIEUX
Details | Diff | Splinter Review
Bug 17100: [Alternative patch] Fix issues for on summary print (3.87 KB, patch)
2016-08-10 09:48 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17100: [Alternative patch] Fix issues for on summary print (4.70 KB, patch)
2016-08-10 13:27 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17100: [Alternative patch] Fix issues for on summary print (4.81 KB, patch)
2016-08-11 13:04 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17100: Do not display payments if patron has nothing to pay (5.41 KB, patch)
2016-08-15 14:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17100: Restore previous logic (2.87 KB, patch)
2016-08-15 14:14 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Sophie MEYNIEUX 2016-08-10 08:17:39 UTC
If a patron got fines but the total is null (lost items then returned for example), on print summary, there is a table "Account fines and payments" with no lines in it.
There should be not table at all
Comment 1 Sophie MEYNIEUX 2016-08-10 08:22:55 UTC Comment hidden (obsolete)
Comment 2 Sophie MEYNIEUX 2016-08-10 08:29:54 UTC
Test plan :

* without patch
  1/ find a patron with no lines in accountlines table : print summary shows no "account fines and payments" => OK
  2/ find a patron with some lines in accountlines table and the total amount > 0 : print summary shows a table "account fines and payments" with fines to recover => OK
  3/ find a patron with some lines in accountlines table but the total amount = 0 : print summary shows a table "account fines and payments" with nothing in it => NOK

* with the patch, same cases as before :
  1/ same as without patch
  2/ same as without patch
  3/ print summary does not show "account fines and payments"
Comment 3 Marc Véron 2016-08-10 09:47:55 UTC
Hi Sophie,

Your patch did not work for me becuase some more logic is needed in the template file. Additionally, I encountred formatting problems.

Alternative patch follows.
Comment 4 Marc Véron 2016-08-10 09:48:53 UTC Comment hidden (obsolete)
Comment 5 Marc Véron 2016-08-10 13:27:18 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2016-08-11 13:04:11 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-08-12 13:59:31 UTC
Comment on attachment 54312 [details] [review]
[SIGNED-OFF] Bug 17100: [Alternative patch] Fix issues for on summary print

Review of attachment 54312 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-print.tt
@@ +105,4 @@
>              </tr>
>  
>              [% FOREACH account IN accounts %]
> +                [% IF ( account.amountoutstanding < 0 ) || ( account.amountoutstanding > 0 ) %]

Marc, why did you replace
  next if amount == 0
with this lines?
Comment 8 Marc Véron 2016-08-14 09:51:35 UTC
(In reply to Jonathan Druart from comment #7)
> Comment on attachment 54312 [details] [review] [review]
> [SIGNED-OFF] Bug 17100: [Alternative patch] Fix issues for on summary print
> 
> Review of attachment 54312 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-print.tt
> @@ +105,4 @@
> >              </tr>
> >  
> >              [% FOREACH account IN accounts %]
> > +                [% IF ( account.amountoutstanding < 0 ) || ( account.amountoutstanding > 0 ) %]
> 
> Marc, why did you replace
>   next if amount == 0
> with this lines?

It did not work with amount == 0 because the amount comes as '0.00' (not sure how much decimals tough)
Comment 9 Jonathan Druart 2016-08-15 14:14:05 UTC
Created attachment 54478 [details] [review]
Bug 17100: Do not display payments if patron has nothing to pay

This alternative patch moves logic and formatting to the template file.

To test:
* without patch
  1/ find a patron with no lines in accountlines table : print summary shows no "account fines and payments" => OK
  2/ find a patron with some lines in accountlines table and the total amount > 0 : print summary shows a table "account fines and payments" with fines to recover => OK
  3/ find a patron with some lines in accountlines table but the total amount = 0 : print summary shows a table "account fines and payments" with nothing in it => NOK

* with the patch, same cases as before :
  1/ same as without patch
  2/ same as without patch
  3/ print summary does not show "account fines and payments"

- Additionally, verify that formatting follows syspref 'CurrencyFormat'
- Verify that amount column is right-aligned

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Jonathan Druart 2016-08-15 14:14:11 UTC
Created attachment 54479 [details] [review]
Bug 17100: Restore previous logic

There is no need to change the previous logic here, so let's restore it.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Jonathan Druart 2016-08-15 14:14:53 UTC
(In reply to Marc Véron from comment #8)
> (In reply to Jonathan Druart from comment #7)
> > Comment on attachment 54312 [details] [review] [review] [review]
> > [SIGNED-OFF] Bug 17100: [Alternative patch] Fix issues for on summary print
> > 
> > Review of attachment 54312 [details] [review] [review] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-print.tt
> > @@ +105,4 @@
> > >              </tr>
> > >  
> > >              [% FOREACH account IN accounts %]
> > > +                [% IF ( account.amountoutstanding < 0 ) || ( account.amountoutstanding > 0 ) %]
> > 
> > Marc, why did you replace
> >   next if amount == 0
> > with this lines?
> 
> It did not work with amount == 0 because the amount comes as '0.00' (not
> sure how much decimals tough)

It's because you compared string "0.00", instead of integer 0.
Comment 12 Kyle M Hall 2016-08-18 16:14:30 UTC
Pushed to master for 16.11, thanks Marc, Jonathan!
Comment 13 Frédéric Demians 2016-08-23 10:44:16 UTC
Pushed in 16.05. Will be in 16.05.03.
Comment 14 Julian Maurice 2016-08-24 10:51:17 UTC
Pushed to 3.22.x, will be in 3.22.10