Bug 40305 - Collected and change variables are inconsistent in controllers and notice templates
Summary: Collected and change variables are inconsistent in controllers and notice tem...
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Martin Renvoize (ashimema)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 24381
Blocks: 36846
  Show dependency treegraph
 
Reported: 2025-07-04 09:07 UTC by Martin Renvoize (ashimema)
Modified: 2025-08-14 15:36 UTC (History)
11 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This standardizes the terminology used throughout the payment system to consistently use 'tendered' instead of the mixed usage of 'collected' and 'tendered' that has caused confusion over the years. It changes the variable names used in the code, HTML forms, and notices for the point of sale module and patron accounting - there is no change to the terms staff see on the pages in the staff interfaces. Note: It changes the variable names used in the RECEIPT and ACCOUNT_CREDIT notices. If you have not made any change to the default notices, they will automatically be updated. If you have customized these notices, you will need to manually update them.
Version(s) released in:
25.11.00
Circulation function:


Attachments
Bug 40305: Fix 'tendered' vs 'collected' variable name inconsistency (13.94 KB, patch)
2025-07-04 09:08 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 40305: Add database update to fix existing notice templates (4.52 KB, patch)
2025-07-04 09:08 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 40305: Fix 'tendered' vs 'collected' variable name inconsistency (13.98 KB, patch)
2025-07-05 21:49 UTC, David Nind
Details | Diff | Splinter Review
Bug 40305: Add database update to fix existing notice templates (4.57 KB, patch)
2025-07-05 21:50 UTC, David Nind
Details | Diff | Splinter Review
Bug 40305: Fix 'tendered' vs 'collected' variable name inconsistency (13.99 KB, patch)
2025-07-22 16:41 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 40305: Add database update to fix existing notice templates (4.57 KB, patch)
2025-07-22 16:41 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 40305: Fix 'tendered' vs 'collected' variable name inconsistency (14.08 KB, patch)
2025-07-25 07:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 40305: Add database update to fix existing notice templates (4.66 KB, patch)
2025-07-25 07:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2025-07-04 09:07:23 UTC
We've tried fixing this a few times but never fully achieved it.

We need to get these consistent across the board to ensure the feature is working correctly out of the box everywhere.

collected => tendered
change_given => change
Comment 1 Martin Renvoize (ashimema) 2025-07-04 09:08:39 UTC
Created attachment 183787 [details] [review]
Bug 40305: Fix 'tendered' vs 'collected' variable name inconsistency

"One Ring to rule them all, one Ring to find them, One Ring to bring
them all and in the darkness bind them"

We've been battling against this for years, with a mixed use of
terminology and thus confusion in templates vs controllers and notice
templates.  I've decided to just try and make sure all the terminology
matches throughout, and use the more popular 'tendered' term over the
'collected' term.

This patch standardizes the terminology used throughout the payment
  system to consistently use 'tendered' instead of the mixed usage of
  'collected' and 'tendered' that has caused confusion over the years.

Changes include:
  - Update all payment form templates to use 'tendered' input names
  - Update JavaScript validation and change calculation logic
  - Update controller parameter handling in pos/pay.pl, pos/printreceipt.pl,
    and members/paycollect.pl
  - Update sample notice templates (RECEIPT) to use 'tendered' variable
  - Add null check for $total_tendered to prevent perl warnings

Test plan:
  1. Test Point of Sale payments with tendered amounts and change calculation
  2. Test member account payments with tendered amounts
  3. Test receipt generation (both print and email)
  4. Verify JavaScript validation works correctly
  5. Confirm no perl warnings when tendered amount is undefined
Comment 2 Martin Renvoize (ashimema) 2025-07-04 09:08:42 UTC
Created attachment 183788 [details] [review]
Bug 40305: Add database update to fix existing notice templates

This database update fixes existing RECEIPT and ACCOUNT_CREDIT notice
templates that may still use the old 'collected' variable instead of
'tendered'. It handles various formatting patterns and provides
verification that the conversion was successful.
Comment 3 David Nind 2025-07-05 21:49:57 UTC
Created attachment 183806 [details] [review]
Bug 40305: Fix 'tendered' vs 'collected' variable name inconsistency

"One Ring to rule them all, one Ring to find them, One Ring to bring
them all and in the darkness bind them"

We've been battling against this for years, with a mixed use of
terminology and thus confusion in templates vs controllers and notice
templates.  I've decided to just try and make sure all the terminology
matches throughout, and use the more popular 'tendered' term over the
'collected' term.

This patch standardizes the terminology used throughout the payment
  system to consistently use 'tendered' instead of the mixed usage of
  'collected' and 'tendered' that has caused confusion over the years.

Changes include:
  - Update all payment form templates to use 'tendered' input names
  - Update JavaScript validation and change calculation logic
  - Update controller parameter handling in pos/pay.pl, pos/printreceipt.pl,
    and members/paycollect.pl
  - Update sample notice templates (RECEIPT) to use 'tendered' variable
  - Add null check for $total_tendered to prevent perl warnings

Test plan:
  1. Test Point of Sale payments with tendered amounts and change calculation
  2. Test member account payments with tendered amounts
  3. Test receipt generation (both print and email)
  4. Verify JavaScript validation works correctly
  5. Confirm no perl warnings when tendered amount is undefined

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2025-07-05 21:50:00 UTC
Created attachment 183807 [details] [review]
Bug 40305: Add database update to fix existing notice templates

This database update fixes existing RECEIPT and ACCOUNT_CREDIT notice
templates that may still use the old 'collected' variable instead of
'tendered'. It handles various formatting patterns and provides
verification that the conversion was successful.

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2025-07-05 22:10:55 UTC
I've signed off, as everything seems to work the same as it did before the patches.

I have noted some errors and warning in the logs, but there is no change from before the patches.

Testing notes (using KTD):
1. Enable the EnablePointOfSale and UseCashRegisters system preferences.
2. If you want to test the actual notices sent, set up your system so that email can be sent [1].
3. Have the developer tools open on the console when browsing so that you can see if there are any JavaScript errors.
4. Check the logs to see if there are any warnings or errors recorded in the logs. I had another terminal window and shell open tailing the logs: tail -f /var/log/koha/kohadev/*.log
5. Add a cash register:
   5.1 Administration > Accounting > Cash registers > + New cash register
       - Name: BZ40305
       - Description: Cash register for bug 40305 testing
6. Add some debit and credit types:
   6.1 Administration > Accounting > Debit types > + New debit type
       - Debit type code: PRODUCT1
       - Default amount: 5.00
       - Description: Product 1
       - Can be manually invoice: ticked
       - Can be sold: ticked
       (Repeat if you want to add more products for sale)
   6.2 Administration > Accounting > Credit types > + New credit type
       - Credit type code: OVERPAYMENT
       - Description: Record overpayment for refund or offset
       - Can be manually added: ticked
       - Enabled credit number: ticked
       - Can be sold: ticked
       (Repeat if you want to add more credit types)
7. Test the Point of Sale system:
   7.1 Add some sales:
       - Add one or product to a sale
       - Note the terminology used in the collect payment section:
         . Amount being paid:
         . Amount tendered
         . Change to give
       - Complete the collect payment section
       - Check the "Print receipt" and "Email receipt" options:
         . Print receipt PDF:
           . Tendered
           . Change
         . Email receipt:
           . Tendered
           . Change given
8. Check the text in the default notices (Tools > Patrons and circulation > Notices and slips):
   - RECEIPT
   - ACCOUNT_CREDIT
9. Check the cash summary reports and cashup processes:
   9.1 Point of Sale > Cash management > Cash summary for Centerville
       . Record a cashup for the BZ40305 cash register
       . Check a summary for the BZ40305 cash register
         . Click the register name in the table listing cash registers
         . On the transaction history page check click "Summary" and also check the print output for the summary
         . In the older transactions section of the page, change the dates to show transactions, then check the "Print receipt" and "Issue refund" actions for a transaction you created. Note the terms used:
           . Print receipt: Tendered and Change
           . Issue refund: Amount paid, Returned to patron
           . If you issue a refund, in the transactions table it has "Payment from library to patron (Cash)"
           . If you attempt to print a receipt for that transaction, you get a "500 error" (both before and after the patch)
10. Test the patron account pages:
    10.1 Patrons > [choose any patron] > Accounting
    10.2 Create a manual invoice:
         . Type: Manual fee
         . Amount: 20.00
         . Buttons are "Save" and "Save and pay"
    10.3 Make a payment (either from the tab or use the "Pay" action). Terms used:
         . Total amount payable:
         . Amount being paid:
         . Amount tendered
         . Change to give
         Note the terms used on the printable receipt ("Print") and the other options ("Details" and "Void payment")
    10.4 Check some of the other options:
         . Apply discount: has "Amount charged" and "Discount to apply"
         . Issue refund: has "Amount paid" and "Returned to patron"
    10.5 Check creating a manual credit:
         . Terms are "Amount"
         . For the "Issue payout" action: "Outstanding credit", "Returned to patron"
11. Apply the patch, update the database (updatedatabase), restart everything (restart_all), and clear your browser cache
12. Retest as above - things should all work as expected. There is no change to the UI terminology visible, no JavaScript errors in the developer tools console, and no unexpected errors or warnings in the logs.


[1] To setup KTD so you can send emails using a Google account
--------------------------------------------------------------
1. Set up an App password for your Google Account
2. Configure a new 'SMTP Server' under 'Administration > SMTP servers' using the following settings, where `User name` = your Google email address and `Password` = your APP password, not your Google account password
3. Set this server as default
   - Host: smtp.gmail.com
   - Port: 587
   - Timeout: 5
   - SSL: STARTTLS
   - User name: GOOGLEACCOUNTUSER
   - Password: GOOGLEAPPPASSWORD

[2] JavaScript Errors noticed before and after the patches
----------------------------------------------------------
1. Add a sale, the email receipt - console error before you click "Confirm". This only happens if you select a previous amount entered when the field history for "Amount tendered" is shown:
  Uncaught ReferenceError: code is not defined
    at HTMLDocument.func (shortcut_25.0600002.js:46:49)

[3] When you press confirm to send the email there is this warning in the logs, before the email output (both before and after the patches)
-------------------------------------------------------------------------------------------------------------------------------------------

 ==> /var/log/koha/kohadev/plack-intranet-error.log <==
[2025/07/05 20:59:43] [WARN] Argument "" isn't numeric in multiplication (*) at input text line 62.

[4] In the older transactions section of the transactions history for a cash register page
------------------------------------------------------------------------------------------

1. Change the dates for older transactions.
2. If you issue a refund for a transaction, in the transactions table it has "Payment from library to patron (Cash)"
3. If you attempt to print a receipt for that transaction, you get a "500 error" (both before and after the patches)
4. The error in the logs (after the patches - I didn't check before):

==> /var/log/koha/kohadev/plack-error.log <==
ERROR PROCESSING TEMPLATE: undef error - Exception 'Koha::Exceptions::Account::IsNotCredit' thrown 'Account line 5 is not a credit'
 at /kohadevbox/koha/pos/printreceipt.pl line 54.

[5] Several of these GET errors/404.pl lines in the /var/log/koha/kohadev/plack.log
-----------------------------------------------------------------------------------

I don't think these are an issue:

172.18.0.1 - - [05/Jul/2025:21:03:03 +0000] "GET errors/404.pl HTTP/1.1" 404 36404 "-" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/137.0.0.0 Safari/537.36"
Comment 6 Martin Renvoize (ashimema) 2025-07-22 16:41:22 UTC
Created attachment 184491 [details] [review]
Bug 40305: Fix 'tendered' vs 'collected' variable name inconsistency

"One Ring to rule them all, one Ring to find them, One Ring to bring
them all and in the darkness bind them"

We've been battling against this for years, with a mixed use of
terminology and thus confusion in templates vs controllers and notice
templates.  I've decided to just try and make sure all the terminology
matches throughout, and use the more popular 'tendered' term over the
'collected' term.

This patch standardizes the terminology used throughout the payment
  system to consistently use 'tendered' instead of the mixed usage of
  'collected' and 'tendered' that has caused confusion over the years.

Changes include:
  - Update all payment form templates to use 'tendered' input names
  - Update JavaScript validation and change calculation logic
  - Update controller parameter handling in pos/pay.pl, pos/printreceipt.pl,
    and members/paycollect.pl
  - Update sample notice templates (RECEIPT) to use 'tendered' variable
  - Add null check for $total_tendered to prevent perl warnings

Test plan:
  1. Test Point of Sale payments with tendered amounts and change calculation
  2. Test member account payments with tendered amounts
  3. Test receipt generation (both print and email)
  4. Verify JavaScript validation works correctly
  5. Confirm no perl warnings when tendered amount is undefined

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 Martin Renvoize (ashimema) 2025-07-22 16:41:24 UTC
Created attachment 184492 [details] [review]
Bug 40305: Add database update to fix existing notice templates

This database update fixes existing RECEIPT and ACCOUNT_CREDIT notice
templates that may still use the old 'collected' variable instead of
'tendered'. It handles various formatting patterns and provides
verification that the conversion was successful.

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 Martin Renvoize (ashimema) 2025-07-22 16:43:18 UTC
Thanks for the testing David, thorough as always :)

Just a quick minor rebase to keep it applying for the QA team.
Comment 9 Marcel de Rooy 2025-07-25 07:44:59 UTC
Created attachment 184616 [details] [review]
Bug 40305: Fix 'tendered' vs 'collected' variable name inconsistency

"One Ring to rule them all, one Ring to find them, One Ring to bring
them all and in the darkness bind them"

We've been battling against this for years, with a mixed use of
terminology and thus confusion in templates vs controllers and notice
templates.  I've decided to just try and make sure all the terminology
matches throughout, and use the more popular 'tendered' term over the
'collected' term.

This patch standardizes the terminology used throughout the payment
  system to consistently use 'tendered' instead of the mixed usage of
  'collected' and 'tendered' that has caused confusion over the years.

Changes include:
  - Update all payment form templates to use 'tendered' input names
  - Update JavaScript validation and change calculation logic
  - Update controller parameter handling in pos/pay.pl, pos/printreceipt.pl,
    and members/paycollect.pl
  - Update sample notice templates (RECEIPT) to use 'tendered' variable
  - Add null check for $total_tendered to prevent perl warnings

Test plan:
  1. Test Point of Sale payments with tendered amounts and change calculation
  2. Test member account payments with tendered amounts
  3. Test receipt generation (both print and email)
  4. Verify JavaScript validation works correctly
  5. Confirm no perl warnings when tendered amount is undefined

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2025-07-25 07:45:02 UTC
Created attachment 184617 [details] [review]
Bug 40305: Add database update to fix existing notice templates

This database update fixes existing RECEIPT and ACCOUNT_CREDIT notice
templates that may still use the old 'collected' variable instead of
'tendered'. It handles various formatting patterns and provides
verification that the conversion was successful.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2025-07-25 07:46:05 UTC
What about paycollect ;)
Not sure if this is really a bugfix. Most probably not. Leaving to RM.
Comment 12 Lucas Gass (lukeg) 2025-07-25 20:40:14 UTC
Nice work everyone!

Pushed to main for 25.11