Bugzilla – Attachment 190388 Details for
Bug 40445
Point of Sale reconciliation input during daily summaries
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40445: (follow-up) Support negative cashup amounts for float deficits
9bbcc81.patch (text/plain), 20.36 KB, created by
Martin Renvoize (ashimema)
on 2025-12-10 08:04:00 UTC
(
hide
)
Description:
Bug 40445: (follow-up) Support negative cashup amounts for float deficits
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-12-10 08:04:00 UTC
Size:
20.36 KB
patch
obsolete
>From 9bbcc8135bad1606ef96fc1fb09607dda62050f6 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Tue, 9 Dec 2025 17:20:17 +0000 >Subject: [PATCH] Bug 40445: (follow-up) Support negative cashup amounts for > float deficits >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >When cash refunds exceed takings in a session, the register's float >is depleted, resulting in negative bankable amounts. This patch adds >full support for this scenario. > >Backend changes: >- Removed abs() from start_cashup to preserve sign of amounts >- Updated validation to allow negative amounts (while preventing zero) >- Changed expected_amount calculation to: total * -1 (consistent) > >Frontend changes (registers.tt): >- Dynamic modal messages based on amount sign >- Positive: "Remove £X cash from register to bank" >- Negative: "Top up the register with £X to restore the float" >- Updated input pattern to accept negative numbers: ^-?\d+(\.\d{2})?$ >- Labels change based on context (add vs. remove) > >Frontend changes (register.tt): >- Added Template Toolkit conditionals for negative amounts >- Updated input pattern to allow negative numbers >- Consistent labeling with registers page > >All user-facing strings use _() for proper translation support. > >Test plan for librarians: > >Setup - Create negative cashup scenario: >1. Start with a register at its float amount (e.g., £100) >2. Process a large cash refund that exceeds takings > Example: £80 in takings, £150 cash refund = -£70 bankable > >Test "Record cashup" modal (Quick cashup): >3. Go to registers page (pos/registers.pl) >4. Click "Record cashup" for the register with negative balance >5. Verify the modal shows: > - Quick cashup: "Top up the register with £70.00 to restore the float" > - Float reminder: "This will bring the register back to the expected float of £100.00" >6. Click "Quick cashup" >7. Enter -70.00 in the amount field (negative number) >8. Verify cashup completes successfully > >Test "Start cashup" modal (Two-phase): >9. Create another negative balance scenario >10. Click "Record cashup" >11. Verify Start cashup instructions show: > - "Count cash in the register" > - "The register can continue operating during counting" > - "Complete the cashup by adding cash to restore the float" >12. Click "Start cashup" >13. Click "Complete cashup" for the register >14. Verify modal shows: > - "Expected amount to add: £70.00" > - Label: "Actual amount added to register:" >15. Enter -70.00 (the negative amount you're adding) >16. Verify cashup completes successfully > >Test reconciliation with negative amounts: >17. Start cashup on register with -£70.00 expected >18. Complete cashup with -£68.00 actual (£2 less added than expected) >19. Verify reconciliation shows correct deficit/surplus calculation >20. Add a reconciliation note and complete > >Test positive amounts still work (regression): >21. Create a normal positive cashup scenario >22. Verify all original messages appear correctly >23. Complete cashup with positive amount >24. Verify everything works as before > >Test individual register page consistency: >25. Navigate to individual register page (pos/register.pl) >26. Verify negative amount handling matches registers page >27. Test both modals show consistent messaging > >Expected results: >- Negative amounts are accepted in all cashup workflows >- Messages clearly indicate adding money vs. removing money >- Reconciliation calculations work correctly for negative amounts >- All translations display properly >- Positive amount workflows unchanged >--- > Koha/Cash/Register.pm | 10 +- > .../prog/en/modules/pos/register.tt | 94 ++++++++++++++++--- > .../prog/en/modules/pos/registers.tt | 93 ++++++++++++++---- > 3 files changed, 163 insertions(+), 34 deletions(-) > >diff --git a/Koha/Cash/Register.pm b/Koha/Cash/Register.pm >index 15250aac94b..ba2a9e5aa5c 100644 >--- a/Koha/Cash/Register.pm >+++ b/Koha/Cash/Register.pm >@@ -289,10 +289,10 @@ sub start_cashup { > Koha::Exceptions::Object::DuplicateID->throw( error => "A cashup is already in progress for this register" ); > } > >- my $expected_amount = abs( $self->outstanding_accountlines->total( { payment_type => [ 'CASH', 'SIP00' ] } ) ); >+ my $expected_amount = $self->outstanding_accountlines->total( { payment_type => [ 'CASH', 'SIP00' ] } ) * -1; > > # Prevent starting a cashup when there are no cash transactions >- unless ( $expected_amount > 0 ) { >+ unless ( $expected_amount != 0 ) { > Koha::Exceptions::Object::BadValue->throw( > error => "Cannot start cashup with zero cash transactions", > type => 'amount', >@@ -345,10 +345,10 @@ sub add_cashup { > } > my $manager_id = $params->{manager_id}; > >- # Validate amount should always be a positive value >+ # Validate amount is a non-zero number > my $amount = $params->{amount}; >- unless ( looks_like_number($amount) && $amount > 0 ) { >- Koha::Exceptions::Account::AmountNotPositive->throw( error => 'Cashup amount passed is not positive number' ); >+ unless ( looks_like_number($amount) && $amount != 0 ) { >+ Koha::Exceptions::Account::AmountNotPositive->throw( error => 'Cashup amount must be a non-zero number' ); > } > > # Sanitize reconciliation note - treat empty/whitespace-only as undef >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt >index be22824bad9..70ff0ae0133 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt >@@ -447,7 +447,11 @@ > <li> > <span class="label"> > [% IF cashup_in_progress %] >- Expected cashup amount: >+ [% IF cashup_in_progress.amount < 0 %] >+ Expected amount to add: >+ [% ELSE %] >+ Expected cashup amount: >+ [% END %] > [% ELSE %] > Expected amount to remove: > [% END %] >@@ -457,12 +461,16 @@ > <li> > <label class="required" for="amount"> > [% IF cashup_in_progress %] >- Actual cashup amount counted: >+ [% IF cashup_in_progress.amount < 0 %] >+ Actual amount added to register: >+ [% ELSE %] >+ Actual cashup amount counted: >+ [% END %] > [% ELSE %] > Actual amount removed from register: > [% END %] > </label> >- <input type="text" inputmode="decimal" pattern="^\d+(\.\d{2})?$" id="amount" name="amount" required="required" /> >+ <input type="text" inputmode="decimal" pattern="^-?\d+(\.\d{2})?$" id="amount" name="amount" required="required" /> > <span class="required">Required</span> > </li> > <li id="reconciliation_display" style="display: none;"> >@@ -564,25 +572,24 @@ > <div class="modal-body"> > <p><strong>Choose how to proceed with the cashup:</strong></p> > <p><strong>Start cashup</strong></p> >- <ul> >- <li>Remove cash from the register for counting</li> >- <li>The register can continue operating during counting</li> >- <li>Complete the cashup once counted</li> >+ <ul id="start_cashup_instructions_individual"> >+ <!-- JavaScript will populate this based on positive/negative amount --> > </ul> > <p><strong>Quick cashup</strong></p> >- <ul> >- <li>Confirm you have removed [% accountlines.total( payment_type => [ 'CASH', 'SIP00' ]) * -1 | $Price %] cash from the register to bank immediately</li> >+ <ul id="quick_cashup_instructions_individual"> >+ <!-- JavaScript will populate this based on positive/negative amount --> > </ul> >- <p>Remember to leave the float amount of <strong>[% register.starting_float | $Price %]</strong> in the register.</p> >+ <p id="float_reminder_text_individual"> >+ <!-- JavaScript will populate this based on positive/negative amount --> >+ </p> > </div> > <div class="modal-footer"> > <input type="hidden" name="registerid" value="[% register.id | html %]" /> > <input type="hidden" name="op" value="cud-cashup_start" /> > <input type="hidden" name="amount" value="" /> >+ <input type="hidden" id="register_bankable_amount" value="[% accountlines.total( payment_type => [ 'CASH', 'SIP00' ]) * -1 | html %]" /> > <button type="submit" class="btn btn-primary">Start cashup</button> >- <button type="button" class="btn btn-success" onclick="this.form.op.value='cud-cashup'; this.form.amount.value='[% accountlines.total( payment_type => [ 'CASH', 'SIP00' ]) * -1 | html %]'; this.form.submit();" >- >Quick cashup</button >- > >+ <button type="button" class="btn btn-success" id="quick_cashup_btn_individual">Quick cashup</button> > <button type="button" class="btn btn-default" data-bs-dismiss="modal">Cancel</button> > </div> > </div> >@@ -801,6 +808,67 @@ > $("#reconciliation_note_field").hide(); > $("#reconciliation_note").val(''); > }); >+ >+ // Handle triggerCashupModal for individual register page >+ $("#triggerCashupModal").on("shown.bs.modal", function(e){ >+ var bankableAmount = parseFloat($("#register_bankable_amount").val()); >+ var isNegative = bankableAmount < 0; >+ var formattedAmount = Math.abs(bankableAmount).format_price(); >+ var floatAmount = parseFloat([% register.starting_float | html %]).format_price(); >+ >+ // Update Start cashup instructions >+ if (isNegative) { >+ $("#start_cashup_instructions_individual").html( >+ '<li>' + _("Count cash in the register") + '</li>' + >+ '<li>' + _("The register can continue operating during counting") + '</li>' + >+ '<li>' + _("Complete the cashup by adding cash to restore the float") + '</li>' >+ ); >+ } else { >+ $("#start_cashup_instructions_individual").html( >+ '<li>' + _("Remove cash from the register for counting") + '</li>' + >+ '<li>' + _("The register can continue operating during counting") + '</li>' + >+ '<li>' + _("Complete the cashup once counted") + '</li>' >+ ); >+ } >+ >+ // Update Quick cashup instructions >+ if (isNegative) { >+ $("#quick_cashup_instructions_individual").html( >+ '<li>' + _("Top up the register with %s to restore the float").format(formattedAmount) + '</li>' >+ ); >+ } else { >+ $("#quick_cashup_instructions_individual").html( >+ '<li>' + _("Confirm you have removed %s cash from the register to bank immediately").format(formattedAmount) + '</li>' >+ ); >+ } >+ >+ // Update float reminder >+ if (isNegative) { >+ $("#float_reminder_text_individual").html( >+ _("This will bring the register back to the expected float of <strong>%s</strong>").format(floatAmount) >+ ); >+ } else { >+ $("#float_reminder_text_individual").html( >+ _("Remember to leave the float amount of <strong>%s</strong> in the register.").format(floatAmount) >+ ); >+ } >+ }); >+ >+ // Handle Quick cashup button click for individual register >+ $("#quick_cashup_btn_individual").on("click", function(e){ >+ e.preventDefault(); >+ var form = $(this).closest('form'); >+ var bankableAmount = $("#register_bankable_amount").val(); >+ >+ // Change operation to cud-cashup (quick cashup) >+ form.find('input[name="op"]').val('cud-cashup'); >+ >+ // Set the amount to the expected bankable amount >+ form.find('input[name="amount"]').val(bankableAmount); >+ >+ // Submit the form >+ form.submit(); >+ }); > </script> > [% END %] > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/pos/registers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/pos/registers.tt >index 2f74d397eff..e983c8ed7c2 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/pos/registers.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/pos/registers.tt >@@ -272,18 +272,16 @@ > <div class="modal-body"> > <p><strong>Choose how to proceed with the cashup:</strong></p> > <p><strong>Start cashup</strong></p> >- <ul> >- <li>Remove cash from the register for counting</li> >- <li>The register can continue operating during counting</li> >- <li>Complete the cashup once counted</li> >+ <ul id="start_cashup_instructions"> >+ <!-- JavaScript will populate this based on positive/negative amount --> > </ul> > <p><strong>Quick cashup</strong></p> >- <ul> >- <li>Confirm you have removed <span id="expected_amount_display"></span> cash from the register to bank immediately</li> >+ <ul id="quick_cashup_instructions"> >+ <!-- JavaScript will populate this based on positive/negative amount --> > </ul> >- <p >- >Remember to leave the float amount of <strong><span id="float_amount_display"></span></strong> in the register.</p >- > >+ <p id="float_reminder_text"> >+ <!-- JavaScript will populate this based on positive/negative amount --> >+ </p> > </div> > <div class="modal-footer"> > <input type="hidden" name="registerid" id="register_id_field" value="" /> >@@ -377,12 +375,12 @@ > <fieldset class="rows"> > <ol> > <li> >- <span class="label"> Expected cashup amount: </span> >+ <span class="label" id="expected_amount_label">Expected cashup amount:</span> > <span id="cashc" class="expected-amount"></span> > </li> > <li> >- <label class="required" for="amount"> Actual cashup amount counted: </label> >- <input type="text" inputmode="decimal" pattern="^\d+(\.\d{2})?$" id="amount" name="amount" required="required" /> >+ <label class="required" for="amount" id="actual_amount_label">Actual cashup amount counted:</label> >+ <input type="text" inputmode="decimal" pattern="^-?\d+(\.\d{2})?$" id="amount" name="amount" required="required" /> > <span class="required">Required</span> > </li> > <li id="reconciliation_display" style="display: none;"> >@@ -518,6 +516,20 @@ > $("#registerc").text(register); > var expected = button.data('expected'); > $("#cashc").text(expected); >+ >+ // Parse expected amount to check if negative >+ var expectedAmount = expected.replace(/[^0-9.-]/g, ''); >+ var isNegative = parseFloat(expectedAmount) < 0; >+ >+ // Update labels based on sign >+ if (isNegative) { >+ $("#expected_amount_label").text(_("Expected amount to add:")); >+ $("#actual_amount_label").text(_("Actual amount added to register:")); >+ } else { >+ $("#expected_amount_label").text(_("Expected cashup amount:")); >+ $("#actual_amount_label").text(_("Actual cashup amount counted:")); >+ } >+ > var rfloat = button.data('float'); > $('#floatc').text(rfloat); > var rid = button.data('registerid'); >@@ -534,14 +546,63 @@ > var register = button.data('register'); > $("#register_desc").text(register); > var bankable = button.data('bankable'); >- $("#expected_amount_display").text(bankable); > var rfloat = button.data('float'); >- $('#float_amount_display').text(rfloat); > var rid = button.data('registerid'); > $('#register_id_field').val(rid); > >- // Store bankable amount for quick cashup >- $('#triggerCashupModalRegister').data('bankable-amount', bankable.replace(/[^0-9.-]/g, '')); >+ // Guard against undefined/null bankable value >+ if (!bankable) { >+ console.error("Bankable amount is undefined"); >+ return; >+ } >+ >+ // Parse bankable amount (remove currency formatting, keep minus sign) >+ var bankableAmount = String(bankable).replace(/[^0-9.-]/g, ''); >+ var numericAmount = parseFloat(bankableAmount); >+ var isNegative = numericAmount < 0; >+ >+ // Format the absolute amount for display >+ var absAmountFormatted = Math.abs(numericAmount).format_price(); >+ >+ // Update Start cashup instructions >+ if (isNegative) { >+ $("#start_cashup_instructions").html( >+ '<li>' + _("Count cash in the register") + '</li>' + >+ '<li>' + _("The register can continue operating during counting") + '</li>' + >+ '<li>' + _("Complete the cashup by adding cash to restore the float") + '</li>' >+ ); >+ } else { >+ $("#start_cashup_instructions").html( >+ '<li>' + _("Remove cash from the register for counting") + '</li>' + >+ '<li>' + _("The register can continue operating during counting") + '</li>' + >+ '<li>' + _("Complete the cashup once counted") + '</li>' >+ ); >+ } >+ >+ // Update Quick cashup instructions >+ if (isNegative) { >+ $("#quick_cashup_instructions").html( >+ '<li>' + _("Top up the register with %s to restore the float").format(absAmountFormatted) + '</li>' >+ ); >+ } else { >+ $("#quick_cashup_instructions").html( >+ '<li>' + _("Confirm you have removed %s cash from the register to bank immediately").format(absAmountFormatted) + '</li>' >+ ); >+ } >+ >+ // Update float reminder >+ if (isNegative) { >+ $("#float_reminder_text").html( >+ _("This will bring the register back to the expected float of <strong>%s</strong>").format(rfloat) >+ ); >+ } else { >+ $("#float_reminder_text").html( >+ _("Remember to leave the float amount of <strong>%s</strong> in the register.").format(rfloat) >+ ); >+ } >+ >+ // Store bankable amount for quick cashup (with sign) >+ $('#triggerCashupModalRegister').data('bankable-amount', bankableAmount); > }); > > // Handle Quick cashup button click >-- >2.52.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40445
:
185289
|
185290
|
185291
|
185292
|
185809
|
186636
|
186637
|
186638
|
186639
|
186640
|
186641
|
186642
|
186643
|
189333
|
189334
|
189335
|
189336
|
189337
|
189338
|
189339
|
189340
|
189341
|
189353
|
189354
|
189355
|
189356
|
189357
|
189358
|
189359
|
189360
|
189361
|
189362
|
189363
|
189364
|
189410
|
189411
|
189412
|
189413
|
189414
|
189415
|
189420
|
189421
|
189422
|
189423
|
189424
|
189425
|
189426
|
189427
|
189428
|
189429
|
189430
|
189476
|
189477
|
189478
|
189479
|
189480
|
189481
|
189482
|
190361
|
190362
|
190363
|
190364
|
190365
|
190366
|
190367
|
190368
|
190369
|
190370
|
190371
|
190372
|
190373
|
190374
|
190375
|
190376
|
190379
|
190380
|
190388
|
190389
|
190404
|
190405
|
190406
|
190407
|
190408
|
190409
|
190410
|
190411
|
190412
|
190413
|
190414
|
190415