Bug 20656

Summary: Print summary for patron shows paid fines and formats payments badly
Product: Koha Reporter: Nick Clemens <nick>
Component: PatronsAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact: Josef Moravec <josef.moravec>
Severity: normal    
Priority: P5 - low CC: amitddng135, black23, f.demians, fridolin.somers, gmcharlt, jonathan.druart, josef.moravec, katrin.fischer, kyle.m.hall, martin.renvoize
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Print summary for patron will now show only outstanding fines/payments. To print all fines/payments you can use the 'print' option for the table in the accounts page for the patron.
Version(s) released in:
Bug Depends on: 17100    
Bug Blocks:    
Attachments: Bug 20656: Don't print lines that have no balance on print summary
Bug 20656: Better formatting for print summary
Bug 20656: Don't print lines that have no balance on print summary
Bug 20656: Better formatting for print summary
Bug 20656: Don't print lines that have no balance on print summary
Bug 20656: Better formatting for print summary
Bug 20656: (follow-up) Don't display title for F or FU fines (it is in description)
Bug 20656: Don't print lines that have no balance on print summary
Bug 20656: Better formatting for print summary
Bug 20656: (follow-up) Don't display title for F or FU fines (it is in description)
Bug 20656: Don't print lines that have no balance on print summary
Bug 20656: Better formatting for print summary
Bug 20656: (follow-up) Don't display title for F or FU fines (it is in description)
Bug 20656: (RM follow-up) Add missing filter

Description Nick Clemens 2018-04-24 20:56:55 UTC
To recreate:
1 - Add some fines to a patron
2 - pay them all
3 - print summary - nothing shows - good
4 - Add a new fine
5 - print summary
6 - All fines show
7 - Inspect the payment line - note lots of newlines and ugly characters
Comment 1 Nick Clemens 2018-04-24 21:01:56 UTC
Created attachment 74829 [details] [review]
Bug 20656: Don't print lines that have no balance on print summary

To test:
1 - Add at least two fines to a patron
2 - Pay off one of them
3 - Print summary - all 3 lines show
4 - Apply patch
5 - Print summary - only line with balance shows
Comment 2 Nick Clemens 2018-04-24 21:01:58 UTC
Created attachment 74830 [details] [review]
Bug 20656: Better formatting for print summary

This patch is a slightly edited c&p from boraccount.tt

1 - Manually add a credit with no notes to patron
2 - Print summary
3 - Inspect the line
4 - Note that td has extra newlines and no words
5 - Apply patch
6 - Print summary again
7 - Note better formatting
Comment 3 Frédéric Demians 2018-04-30 17:13:23 UTC
I'm not sure it won't be considered as a regression by some libraries. Before your patch, Print summary shows the fine and its payment. After your patch, the fine and payment info doesn't appear anymore. On Summary page, the table showing information is labelled Account fines and payments. So 'payments' seems to be expected.
Comment 4 Katrin Fischer 2018-07-08 17:17:26 UTC
I think I like the new display better. As it's supposed to be a summary displaying only open credits and fines makes sense to me, but the heading should then be changed to match the new display.

Signing, but waiting for follow-up and maybe another opinion here.
Comment 5 Katrin Fischer 2018-07-08 17:17:52 UTC
Created attachment 76775 [details] [review]
Bug 20656: Don't print lines that have no balance on print summary

To test:
1 - Add at least two fines to a patron
2 - Pay off one of them
3 - Print summary - all 3 lines show
4 - Apply patch
5 - Print summary - only line with balance shows

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Katrin Fischer 2018-07-08 17:17:55 UTC
Created attachment 76776 [details] [review]
Bug 20656: Better formatting for print summary

This patch is a slightly edited c&p from boraccount.tt

1 - Manually add a credit with no notes to patron
2 - Print summary
3 - Inspect the line
4 - Note that td has extra newlines and no words
5 - Apply patch
6 - Print summary again
7 - Note better formatting

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Michal Denar 2018-09-13 12:00:09 UTC
Please rebase on actual master.
Comment 8 Nick Clemens 2018-09-16 21:03:43 UTC
Created attachment 79002 [details] [review]
Bug 20656: Don't print lines that have no balance on print summary

To test:
1 - Add at least two fines to a patron
2 - Pay off one of them
3 - Print summary - all 3 lines show
4 - Apply patch
5 - Print summary - only line with balance shows

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Nick Clemens 2018-09-16 21:03:46 UTC
Created attachment 79003 [details] [review]
Bug 20656: Better formatting for print summary

This patch is a slightly edited c&p from boraccount.tt

1 - Manually add a credit with no notes to patron
2 - Print summary
3 - Inspect the line
4 - Note that td has extra newlines and no words
5 - Apply patch
6 - Print summary again
7 - Note better formatting

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Jonathan Druart 2018-09-16 22:29:11 UTC
Condition is missing to display the biblio's title:
  account.accounttype != 'F' AND account.accounttype != 'FU'
Comment 11 Nick Clemens 2018-09-16 22:30:05 UTC
Created attachment 79024 [details] [review]
Bug 20656: (follow-up) Don't display title for F or FU fines (it is in description)
Comment 12 Josef Moravec 2018-09-19 08:34:07 UTC
Comment on attachment 79003 [details] [review]
Bug 20656: Better formatting for print summary

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

::: koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-print.tt
@@ +97,4 @@
>              <caption>Account fines and payments</caption>
> +            <thead>
> +              <tr>
> +                  <th class="title-string">Date</th>

Why are you adding the class here? It is usually used for sorting in datatables, which is not this case.
Comment 13 Nick Clemens 2018-09-19 12:05:57 UTC
Created attachment 79095 [details] [review]
Bug 20656: Don't print lines that have no balance on print summary

To test:
1 - Add at least two fines to a patron
2 - Pay off one of them
3 - Print summary - all 3 lines show
4 - Apply patch
5 - Print summary - only line with balance shows

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Nick Clemens 2018-09-19 12:06:01 UTC
Created attachment 79096 [details] [review]
Bug 20656: Better formatting for print summary

This patch is a slightly edited c&p from boraccount.tt

1 - Manually add a credit with no notes to patron
2 - Print summary
3 - Inspect the line
4 - Note that td has extra newlines and no words
5 - Apply patch
6 - Print summary again
7 - Note better formatting

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Nick Clemens 2018-09-19 12:06:05 UTC
Created attachment 79097 [details] [review]
Bug 20656: (follow-up) Don't display title for F or FU fines (it is in description)
Comment 16 Nick Clemens 2018-09-19 12:06:53 UTC
(In reply to Josef Moravec from comment #12)
> > +              <tr>
> > +                  <th class="title-string">Date</th>
> 
> Why are you adding the class here? It is usually used for sorting in
> datatables, which is not this case.

Bad copy/paste during rebase
Comment 17 Josef Moravec 2018-09-20 09:36:26 UTC
Created attachment 79138 [details] [review]
Bug 20656: Don't print lines that have no balance on print summary

To test:
1 - Add at least two fines to a patron
2 - Pay off one of them
3 - Print summary - all 3 lines show
4 - Apply patch
5 - Print summary - only line with balance shows

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 18 Josef Moravec 2018-09-20 09:36:30 UTC
Created attachment 79139 [details] [review]
Bug 20656: Better formatting for print summary

This patch is a slightly edited c&p from boraccount.tt

1 - Manually add a credit with no notes to patron
2 - Print summary
3 - Inspect the line
4 - Note that td has extra newlines and no words
5 - Apply patch
6 - Print summary again
7 - Note better formatting

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 19 Josef Moravec 2018-09-20 09:36:34 UTC
Created attachment 79140 [details] [review]
Bug 20656: (follow-up) Don't display title for F or FU fines (it is in description)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 20 Josef Moravec 2018-09-20 09:38:16 UTC
(In reply to Nick Clemens from comment #16)
> (In reply to Josef Moravec from comment #12)
> > > +              <tr>
> > > +                  <th class="title-string">Date</th>
> > 
> > Why are you adding the class here? It is usually used for sorting in
> > datatables, which is not this case.
> 
> Bad copy/paste during rebase

Thanks fir quick fix, passing QA
Comment 21 Nick Clemens 2018-09-20 13:42:23 UTC
Created attachment 79176 [details] [review]
Bug 20656: (RM follow-up) Add missing filter
Comment 22 Nick Clemens 2018-09-20 13:46:25 UTC
Awesome work all!

Pushed to master for 18.11
Comment 23 Nick Clemens 2018-09-20 13:46:59 UTC
(In reply to Katrin Fischer from comment #4)
> I think I like the new display better. As it's supposed to be a summary
> displaying only open credits and fines makes sense to me, but the heading
> should then be changed to match the new display.
> 
> Signing, but waiting for follow-up and maybe another opinion here.

I am happy to fix this on a follow-up bug, can you suggest a new title?
Comment 24 Martin Renvoize 2018-09-25 13:35:01 UTC
Pushed to 18.05.x for 18.05.05.
Comment 25 Fridolin Somers 2018-10-03 11:48:19 UTC
17.11.x is still using GetMemberAccountRecords instead of Koha::Account::Lines so patch does not applies.
Is is relevant for 17.11.x ?