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: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 22809 25998 26689
Blocks: 13985 14825 35064 34332
  Show dependency treegraph
 
Reported: 2020-01-08 21:37 UTC by Katrin Fischer
Modified: 2023-11-02 20:50 UTC (History)
13 users (show)

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


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
Bug 24381: Ratify account slip printing (11.99 KB, patch)
2020-09-16 14:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update sample notices (15.84 KB, patch)
2020-09-16 14:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update notices if defaults used (11.93 KB, patch)
2020-09-16 14:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update notice params (3.36 KB, patch)
2020-09-16 14:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Ratify account slip printing (12.06 KB, patch)
2020-09-21 12:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24381: Update sample notices (15.91 KB, patch)
2020-09-21 12:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24381: Update notices if defaults used (12.00 KB, patch)
2020-09-21 12:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24381: Update notice params (3.43 KB, patch)
2020-09-21 12:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24381: Ratify account slip printing (12.04 KB, patch)
2020-10-13 11:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update sample notices (15.90 KB, patch)
2020-10-13 11:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update notices if defaults used (11.99 KB, patch)
2020-10-13 11:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update notice params (3.42 KB, patch)
2020-10-13 11:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: (follow-up) Notice improvements (13.53 KB, patch)
2020-10-13 11:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Ratify account slip printing (12.04 KB, patch)
2020-10-14 11:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update sample notices (15.90 KB, patch)
2020-10-14 11:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update notices if defaults used (11.99 KB, patch)
2020-10-14 11:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update notice params (3.42 KB, patch)
2020-10-14 11:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: (follow-up) Notice improvements (13.56 KB, patch)
2020-10-14 11:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Convert printfeercpt to use GetPreparedLetter (6.33 KB, patch)
2020-10-14 11:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Add payout_amount method to Koha::Account (3.47 KB, patch)
2021-02-05 14:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24381: Update sample notices (16.31 KB, patch)
2022-09-22 21:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24381: Update notices if defaults used (11.99 KB, patch)
2022-09-22 21:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24381: Update notice params (3.42 KB, patch)
2022-09-22 21:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24381: (follow-up) Notice improvements (13.56 KB, patch)
2022-09-22 21:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24381: Convert printfeercpt to use GetPreparedLetter (2.94 KB, patch)
2022-09-22 21:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24381: (follow-up) Add USE statements (2.32 KB, patch)
2022-09-22 21:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24381: Update sample notices (16.36 KB, patch)
2022-10-11 21:56 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 24381: Update notices if defaults used (12.04 KB, patch)
2022-10-11 21:56 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 24381: Update notice params (3.47 KB, patch)
2022-10-11 21:56 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 24381: (follow-up) Notice improvements (13.61 KB, patch)
2022-10-11 21:57 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 24381: (follow-up) Add USE statements (2.38 KB, patch)
2022-10-11 21:57 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 24381: Update sample notices (16.42 KB, patch)
2022-10-21 13:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24381: Update notices if defaults used (12.10 KB, patch)
2022-10-21 13:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24381: Update notice params (3.52 KB, patch)
2022-10-21 13:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24381: (follow-up) Notice improvements (13.67 KB, patch)
2022-10-21 13:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24381: (follow-up) Add USE statements (2.43 KB, patch)
2022-10-21 13:38 UTC, Kyle M Hall
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
Comment 29 Katrin Fischer 2020-09-16 06:20:45 UTC
Martin, can you please rebase?
Comment 30 Martin Renvoize 2020-09-16 14:33:21 UTC
Created attachment 110210 [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 31 Martin Renvoize 2020-09-16 14:33:27 UTC
Created attachment 110211 [details] [review]
Bug 24381: Update sample notices

This patch update the sample notices for new installs
Comment 32 Martin Renvoize 2020-09-16 14:33:33 UTC
Created attachment 110212 [details] [review]
Bug 24381: Update notices if defaults used
Comment 33 Martin Renvoize 2020-09-16 14:33:42 UTC
Created attachment 110213 [details] [review]
Bug 24381: Update notice params

If the update to catch defaults fails, we need to attempt to update
renamed template variables
Comment 34 Kyle M Hall 2020-09-21 12:56:38 UTC
Created attachment 110461 [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.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 35 Kyle M Hall 2020-09-21 12:56:51 UTC
Created attachment 110462 [details] [review]
Bug 24381: Update sample notices

This patch update the sample notices for new installs

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 36 Kyle M Hall 2020-09-21 12:56:57 UTC
Created attachment 110463 [details] [review]
Bug 24381: Update notices if defaults used

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 37 Kyle M Hall 2020-09-21 12:57:02 UTC
Created attachment 110464 [details] [review]
Bug 24381: Update notice params

If the update to catch defaults fails, we need to attempt to update
renamed template variables

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 38 Joonas Kylmälä 2020-10-06 10:26:07 UTC
Before this patch on koha-testing-docker if I add two manual fees and click on the print button on either one of the fees then it shows that fee as itemized. After applying this patch, running updatedatabase and restart_all I only see the total amount and not details for this fee. I think this is not expected, after this patch all the fee lines are supposed to be shown, right?
Comment 39 Martin Renvoize 2020-10-12 16:44:09 UTC
(In reply to Joonas Kylmälä from comment #38)
> Before this patch on koha-testing-docker if I add two manual fees and click
> on the print button on either one of the fees then it shows that fee as
> itemized. After applying this patch, running updatedatabase and restart_all
> I only see the total amount and not details for this fee. I think this is
> not expected, after this patch all the fee lines are supposed to be shown,
> right?

You are quit right.. seems I didn't test the cases where we haven't actually applied any credits against the debt.. or vica versa.  Those don't looked great right now.

I'm working on a better notice template to replace it with.
Comment 40 Martin Renvoize 2020-10-13 11:31:35 UTC
Created attachment 111530 [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.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 41 Martin Renvoize 2020-10-13 11:31:40 UTC
Created attachment 111531 [details] [review]
Bug 24381: Update sample notices

This patch update the sample notices for new installs

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 42 Martin Renvoize 2020-10-13 11:31:49 UTC
Created attachment 111532 [details] [review]
Bug 24381: Update notices if defaults used

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 43 Martin Renvoize 2020-10-13 11:31:55 UTC
Created attachment 111533 [details] [review]
Bug 24381: Update notice params

If the update to catch defaults fails, we need to attempt to update
renamed template variables

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 44 Martin Renvoize 2020-10-13 11:32:00 UTC
Created attachment 111534 [details] [review]
Bug 24381: (follow-up) Notice improvements
Comment 45 Martin Renvoize 2020-10-13 11:37:42 UTC
To Test

The updated templates are used for a plethora of different cases and I'd like some feedback as to whether they're an improvement for each.

ACCOUNT_DEBIT
* This one is used for accountlines of debit_type_code 'ACCOUNT', 'ACCOUNT_RENEW', 'LOST', 'MANUAL', 'NEW_CARD', 'OVERDUE', 'PROCESSING', 'RENT', 'RENT_DAILY', 'RENT_RENEW', 'RENT_DAILY_RENEW', 'RESERVE', 'RESERVE_EXPIRED', 'PAYOUT' and any other locally added debit types.  Other than the 'PAYOUT' type, which I believe may warrant it's own template entirely I'm reasonably happy.  I am, however interested in feedback regarding how they look prior to and post the debt being paid (partially, fully in one and fully over multiple payments)

ACCOUNT_CREDIT
* This one is used for accountlines of credit_type_code 'PAYMENT', 'WRITEOFF', 'FORGIVEN', 'CREDIT', 'LOST_FOUND', 'REFUND' and any other locally added credit types. Again, I'm interested in feedback regarding how they look before and after they have been applied to debts.

ACCOUNT_PAYMENT
* I can't remember what this one is used for :(
Comment 46 Joonas Kylmälä 2020-10-14 08:12:27 UTC
Martin, the changes you made to the template in the last patch are quite huge in my opinion, if you don't mind I would like to set this to Need Signoff to have someone review the last patch before having the QA team check for final time.
Comment 47 Martin Renvoize 2020-10-14 08:48:10 UTC
As an additional note, whilst working on the templates I noticed that the [% today %] substitution doesn't work in this notice as it does elsewhere..

After a discussion with Jonathan, we noted that these print slips do not go via GetPreparedLetter and rather call the private getletter function directly and then pass the resultant template into a process_tt block in the print template.

We deceded we aught to work out a better way to handle this.
Comment 48 Martin Renvoize 2020-10-14 11:04:25 UTC
Created attachment 111590 [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.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 49 Martin Renvoize 2020-10-14 11:04:31 UTC
Created attachment 111591 [details] [review]
Bug 24381: Update sample notices

This patch update the sample notices for new installs

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 50 Martin Renvoize 2020-10-14 11:04:37 UTC
Created attachment 111592 [details] [review]
Bug 24381: Update notices if defaults used

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 51 Martin Renvoize 2020-10-14 11:04:46 UTC
Created attachment 111593 [details] [review]
Bug 24381: Update notice params

If the update to catch defaults fails, we need to attempt to update
renamed template variables

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 52 Martin Renvoize 2020-10-14 11:04:55 UTC
Created attachment 111594 [details] [review]
Bug 24381: (follow-up) Notice improvements
Comment 53 Martin Renvoize 2020-10-14 11:05:05 UTC
Created attachment 111595 [details] [review]
Bug 24381: Convert printfeercpt to use GetPreparedLetter

This patch updates C4::Letters to allow use of existing koha template
includes from notices, then updates the printfeercpt slip print option
to use GetPreparedLetter rather than calling getletter directly.

As part of this work, we also add credits and debits handling to the
_parseletter_sth and _get_tt_params routines in C4::Letters to allow for
recognisable variable names in the notice template.
Comment 54 Martin Renvoize 2020-10-14 11:06:41 UTC
OK, back to NSO.. that final followup is a proof of concept for converting the first slip to use GetPreparedLetter as aposed to calling getletter directly and then parting the evaluating the tt from inside the template.
Comment 55 Martin Renvoize 2021-02-02 16:16:04 UTC
People seemed to generally lose interest in this.. :(..  the work moved to bug 26734 to split it into sensible parts as the code here ended up very customised for each case instead of using a more generalised patturn.
Comment 56 Martin Renvoize 2021-02-05 14:29:08 UTC
Created attachment 116394 [details] [review]
Bug 24381: Add payout_amount method to Koha::Account
Comment 57 Lucas Gass 2022-06-03 15:27:57 UTC
Patch doesn't apply to current master
Comment 58 Nick Clemens 2022-09-22 21:31:38 UTC
Created attachment 140868 [details] [review]
Bug 24381: Update sample notices

This patch update the sample notices for new installs

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 59 Nick Clemens 2022-09-22 21:31:45 UTC
Created attachment 140869 [details] [review]
Bug 24381: Update notices if defaults used

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 60 Nick Clemens 2022-09-22 21:31:50 UTC
Created attachment 140870 [details] [review]
Bug 24381: Update notice params

If the update to catch defaults fails, we need to attempt to update
renamed template variables

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 61 Nick Clemens 2022-09-22 21:31:57 UTC
Created attachment 140871 [details] [review]
Bug 24381: (follow-up) Notice improvements
Comment 62 Nick Clemens 2022-09-22 21:32:06 UTC
Created attachment 140872 [details] [review]
Bug 24381: Convert printfeercpt to use GetPreparedLetter

This patch updates C4::Letters to allow use of existing koha template
includes from notices, then updates the printfeercpt slip print option
to use GetPreparedLetter rather than calling getletter directly.

As part of this work, we also add credits and debits handling to the
_parseletter_sth and _get_tt_params routines in C4::Letters to allow for
recognisable variable names in the notice template.
Comment 63 Nick Clemens 2022-09-22 21:32:12 UTC
Created attachment 140873 [details] [review]
Bug 24381: (follow-up) Add USE statements
Comment 64 Nick Clemens 2022-09-22 21:33:13 UTC
I dropped the first patch, it seemed to have been done already, updated the others slightly, worked in brief testing
Comment 65 Lucas Gass 2022-09-29 13:57:33 UTC
It appears like there is a problem with the rebase here. I think this block is now being repeated:

    my $htdocs = C4::Context->config('intrahtdocs');
    my ($theme, $lang, $activethemes)= C4::Templates::themelanguage( $htdocs, 'about.tt', 'intranet');
    my @includes;
    foreach (@$activethemes) {
        push @includes, "$htdocs/$_/$lang/includes";
        push @includes, "$htdocs/$_/en/includes" unless $lang eq 'en';
    } 


The QA tool complains about that:

 FAIL	C4/Letters.pm
   FAIL	  valid
		"my" variable $htdocs masks earlier declaration in same scope
		"my" variable $theme masks earlier declaration in same scope
		"my" variable $lang masks earlier declaration in same scope
		"my" variable @includes masks earlier declaration in same scope
		"my" variable $htdocs masks earlier declaration in same scope
		"my" variable $theme masks earlier declaration in same scope
		"my" variable $lang masks earlier declaration in same scope
		"my" variable @includes masks earlier declaration in same scope
Comment 66 Martin Renvoize 2022-10-10 15:07:36 UTC
I've obsoleted another of the patches that Nick rebased as it's already done in bug 26734...

This is another case of a bug loosing momentum due to a distinct lack of testing at the time and I'm struggling to catch back up again..

All we need at this point is the new notices.. all the core code stuff is in...

Also, with bug 26689 we can finally split these notices up so they're specific to their credit_type_code and debit_type_code.. so the notices submitted here with various cases for the different types built into the one notice don't really make much sense now.. it's much clearer to have a notice per type instead.
Comment 67 Martin Renvoize 2022-10-10 15:12:38 UTC
The patchset should now work as it stands.. but personally I'd split the notices up to remove the `[% IF ( credit.credit_type_code == 'PAYMENT' ) %]"` and similar blocks and instead put them in new notices 'CREDIT_PAYMENT' for example.

Either way.. give it a test and see if you like seeing the itemised summaries..
Comment 68 Lucas Gass 2022-10-11 21:56:42 UTC
Created attachment 141619 [details] [review]
Bug 24381: Update sample notices

This patch update the sample notices for new installs

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 69 Lucas Gass 2022-10-11 21:56:49 UTC
Created attachment 141620 [details] [review]
Bug 24381: Update notices if defaults used

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 70 Lucas Gass 2022-10-11 21:56:55 UTC
Created attachment 141621 [details] [review]
Bug 24381: Update notice params

If the update to catch defaults fails, we need to attempt to update
renamed template variables

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 71 Lucas Gass 2022-10-11 21:57:01 UTC
Created attachment 141622 [details] [review]
Bug 24381: (follow-up) Notice improvements

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 72 Lucas Gass 2022-10-11 21:57:07 UTC
Created attachment 141623 [details] [review]
Bug 24381: (follow-up) Add USE statements

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 73 Lucas Gass 2022-10-11 21:57:56 UTC
Thanks for this one Martin. It will make a lot of librarians happy. :)
Comment 74 Kyle M Hall 2022-10-21 13:37:48 UTC
Created attachment 142386 [details] [review]
Bug 24381: Update sample notices

This patch update the sample notices for new installs

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 75 Kyle M Hall 2022-10-21 13:38:02 UTC
Created attachment 142387 [details] [review]
Bug 24381: Update notices if defaults used

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 76 Kyle M Hall 2022-10-21 13:38:09 UTC
Created attachment 142388 [details] [review]
Bug 24381: Update notice params

If the update to catch defaults fails, we need to attempt to update
renamed template variables

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 77 Kyle M Hall 2022-10-21 13:38:15 UTC
Created attachment 142389 [details] [review]
Bug 24381: (follow-up) Notice improvements

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 78 Kyle M Hall 2022-10-21 13:38:20 UTC
Created attachment 142390 [details] [review]
Bug 24381: (follow-up) Add USE statements

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 79 Tomás Cohen Arazi 2022-10-24 21:43:57 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 80 Lucas Gass 2022-12-05 23:13:31 UTC
Patchset won't apply cleanly to 22.05.x, no backport