Bug 24381 - ACCOUNT_CREDIT and ACCOUNT_DEBIT slip not printing information about paid fines/fees
Summary: ACCOUNT_CREDIT and ACCOUNT_DEBIT slip not printing information about paid fin...
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 25998 22761 22809
Blocks: 13985
  Show dependency treegraph
 
Reported: 2020-01-08 21:37 UTC by Katrin Fischer
Modified: 2020-07-17 15:37 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 24381: Ratify account slip printing (12.10 KB, patch)
2020-07-16 11:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update sample notices (11.64 KB, patch)
2020-07-16 11:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Ratify account slip printing (12.10 KB, patch)
2020-07-16 13:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update sample notices (15.84 KB, patch)
2020-07-16 13:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23481: Update notices if defaults used (11.88 KB, patch)
2020-07-16 13:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update notices if defaults used (11.93 KB, patch)
2020-07-16 14:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25998: Add 'library' relation to Koha::Account::Line (942 bytes, patch)
2020-07-16 16:03 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Ratify account slip printing (12.10 KB, patch)
2020-07-16 16:03 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update sample notices (15.84 KB, patch)
2020-07-16 16:03 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update notices if defaults used (11.93 KB, patch)
2020-07-16 16:03 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update notice params (3.36 KB, patch)
2020-07-16 16:03 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Ratify account slip printing (12.10 KB, patch)
2020-07-17 07:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update sample notices (15.84 KB, patch)
2020-07-17 07:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update notices if defaults used (11.93 KB, patch)
2020-07-17 07:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update notice params (3.36 KB, patch)
2020-07-17 07:04 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2020-01-08 21:37:09 UTC
While the templates suggests it should print the fines/fees paid/changed by the payment, this doesn't work for me on current master.
Comment 2 Katrin Fischer 2020-01-08 21:52:23 UTC
I think what we should have in the table for each:

ACCOUNT_CREDIT
Fee receipt - list of fines paid (print is on payment line)

ACCOUNT_DEBIT
Invoice - list of payments made (print is on fine/fee)
Comment 3 Jonathan Druart 2020-01-14 11:17:06 UTC
I do not understand, can you explain a bit more please? Or maybe attach a screenshot?

I see the Print button for each line, the letter displays only 1 line. Do you expect all the lines to be displayed? In that case the button should be moved somewhere else?
Comment 4 Katrin Fischer 2020-01-14 11:48:20 UTC
It might get clearer if we start with the payment receipts printed from a 'payment' line.

At the moment, it will have:

- a table with the column heading: Description of charges
- Below always 1 line only with "Payment"
- And of course... there is not much to total

What I think we should have, and we can already see this in the 'details' for the payment and in the sample notice for PAYMENT notices:

- a table with the column heading: Description of charges
- A list of the fines the payment paid off, so something like:
  - Rental fee X
  - Fine for ...
- The total of the payment

I've tried to use the TT syntax from the payment emails, but couldn't get it to work.
Comment 5 Martin Renvoize 2020-01-14 14:52:42 UTC
I'm sure this used to work...
Comment 6 Martin Renvoize 2020-01-14 15:14:30 UTC
Right.. I think I know what you guys want... but I'm pretty confident it wasn't different in 18.11 unless I'm missing a bug somewhere.. the conversion from hard coded to a notices template did not change the output going back to my notes..
Comment 7 Martin Renvoize 2020-01-14 15:17:45 UTC
Now that bug 23354 has been pushed to master, could you take a look at the template included for a RECEIPT slipt for the Point Of Sale receipts and confirm that would conform to the data you're looking for in the default slip here too..

If so, I can create a patch to correct that for new installs.. I think the reason we didn't add such a slip during the change to slips was to preserve existing receipt look and feel during upgrades.
Comment 8 Martin Renvoize 2020-01-14 15:22:25 UTC
Whilst we're here.. can someone explain to me why we need 4 distinct notices for this?

ACCOUNT_CREDIT - printfeercpt
ACCOUNT_DEBIT - printinvoice
ACCOUNT_PAYMENT - What is this for?
ACCOUNT_WRITEOFF - What is this for?

RECEIPT - pos/printreceipt
Comment 9 Katrin Fischer 2020-01-14 21:12:58 UTC
(In reply to Martin Renvoize from comment #8)
> Whilst we're here.. can someone explain to me why we need 4 distinct notices
> for this?

Interesting question!
 
> ACCOUNT_CREDIT - printfeercpt
> ACCOUNT_DEBIT - printinvoice

These are for the slips. I have to say I keep confusing debit and credit, but i think:

CREDIT = Payment
DEBIT = Invoice

> ACCOUNT_PAYMENT - What is this for? 
> ACCOUNT_WRITEOFF - What is this for?

Sending emails to the patron, similar to checkout/checkin slips. It was a major step forward as at that time the slips were still not customizable. - see UseEmailReceipts system preference.

> RECEIPT - pos/printreceipt

I think there is a good precedence to have different templates for printed slips, so that makes sense to me. Also while we could probably combine payment, writeoff notice and invoice notice are different things.

I have not idea about RECEIPT yet - but this one you should know about ;)

We have tested ACCOUNT_ emails and they do what we need on the slips. I'd prefere if they all shared the same names for objects/variables or this will be really messy to train and explain.
Comment 10 Martin Renvoize 2020-02-07 14:19:07 UTC
I need some more of the other accounts bugs to move before I can delegate time to this I'm afraid.
Comment 11 Martin Renvoize 2020-07-15 14:03:59 UTC
OK, coming back to this..

>We have tested ACCOUNT_ emails and they do what we need on the slips. I'd prefere if they all shared the same names for objects/variables or this will be really messy to train and explain.

I agree.. it would be great to unify the syntax's and have the default template all take a similar form... the issue with this to date has been backwards compatibility...

I'm now considering how we can move to a consistent approach without having huge amounts of redundant code for backward compatibility.. any suggestions greatly welcomed.
Comment 12 Martin Renvoize 2020-07-16 11:10:38 UTC
Created attachment 106946 [details] [review]
Bug 24381: Ratify account slip printing

This patch simplifies the members/print*.pl controller scripts to only
pass around the minimum required data for the templates.
Comment 13 Martin Renvoize 2020-07-16 11:10:42 UTC
Created attachment 106947 [details] [review]
Bug 24381: Update sample notices

This patch update the sample notices for new installs
Comment 14 Martin Renvoize 2020-07-16 11:14:56 UTC
So.. this works really nicely for new installs now and the code is significantly cleaner in my opinion.. but I need to now work out how to support backward compatibility with older notices...
Comment 15 Martin Renvoize 2020-07-16 13:37:23 UTC
Created attachment 106950 [details] [review]
Bug 24381: Ratify account slip printing

This patch simplifies the members/print*.pl controller scripts to only
pass around the minimum required data for the templates.
Comment 16 Martin Renvoize 2020-07-16 13:37:27 UTC
Created attachment 106951 [details] [review]
Bug 24381: Update sample notices

This patch update the sample notices for new installs
Comment 17 Martin Renvoize 2020-07-16 13:37:30 UTC
Created attachment 106952 [details] [review]
Bug 23481: Update notices if defaults used
Comment 18 Martin Renvoize 2020-07-16 14:05:42 UTC
Created attachment 106953 [details] [review]
Bug 24381: Update notices if defaults used
Comment 19 Martin Renvoize 2020-07-16 16:03:40 UTC
Created attachment 106956 [details] [review]
Bug 25998: Add 'library' relation to Koha::Account::Line
Comment 20 Martin Renvoize 2020-07-16 16:03:44 UTC
Created attachment 106957 [details] [review]
Bug 24381: Ratify account slip printing

This patch simplifies the members/print*.pl controller scripts to only
pass around the minimum required data for the templates.
Comment 21 Martin Renvoize 2020-07-16 16:03:48 UTC
Created attachment 106958 [details] [review]
Bug 24381: Update sample notices

This patch update the sample notices for new installs
Comment 22 Martin Renvoize 2020-07-16 16:03:51 UTC
Created attachment 106959 [details] [review]
Bug 24381: Update notices if defaults used
Comment 23 Martin Renvoize 2020-07-16 16:03:56 UTC
Created attachment 106960 [details] [review]
Bug 24381: Update notice params

If the update to catch defaults fails, we need to attempt to update
renamed template variables
Comment 24 Martin Renvoize 2020-07-17 07:04:08 UTC
Created attachment 106974 [details] [review]
Bug 24381: Ratify account slip printing

This patch simplifies the members/print*.pl controller scripts to only
pass around the minimum required data for the templates.
Comment 25 Martin Renvoize 2020-07-17 07:04:12 UTC
Created attachment 106975 [details] [review]
Bug 24381: Update sample notices

This patch update the sample notices for new installs
Comment 26 Martin Renvoize 2020-07-17 07:04:16 UTC
Created attachment 106976 [details] [review]
Bug 24381: Update notices if defaults used
Comment 27 Martin Renvoize 2020-07-17 07:04:20 UTC
Created attachment 106977 [details] [review]
Bug 24381: Update notice params

If the update to catch defaults fails, we need to attempt to update
renamed template variables
Comment 28 Martin Renvoize 2020-07-17 07:09:01 UTC
Test plan

Fresh Install
1/ Install a koha fresh and allow the sample notices to be installed
2/ Create some debits on a patrons account
3/ 'Print' the debit and verify the print works and looks reasonable
4/ Create some credits on a patrons account
5/ 'Print' the credit and verify the print works and looks reasonable
6/ Apply a credit against some debits
7/ Print both 'credit' and 'debit' and confirm the receipts look reasonable

Upgrade (With default notices)
1/ Perform an upgrade where the notices were still set to their defaults prior to upgrade
2/ Check that the notices have been upgraded to the new defaults

Upgrade (With modified notices)
1/ Perform an upgrade where the notices have been modified locally
2/ Check that the notice customisations are intact
3/ Check that the notices still print correctly