From d5b932a1ad41719b496ec0355cf1f45272fe596c Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Tue, 11 Nov 2025 07:02:39 +0000 Subject: [PATCH] Bug 40445: Add optional required reconciliation note validation This patch adds the ability to require reconciliation notes when completing cashup with discrepancies between expected and actual amounts. A new system preference `CashupReconciliationNoteRequired`` controls whether notes are mandatory when surplus or deficit exists. Test plan: 1. Enable CashupReconciliationNoteRequired system preference 2. Perform cashup with discrepancy without note - should fail 3. Perform cashup with discrepancy with note - should succeed 4. Perform cashup without discrepancy and no note - should succeed 5. Disable preference - cashup with discrepancy should work without note 6. Run tests: prove t/db_dependent/Koha/Cash/Register.t Sponsored-by: OpenFifth --- Koha/Cash/Register.pm | 10 ++ .../data/mysql/atomicupdate/bug_40445.pl | 20 +++ installer/data/mysql/mandatory/sysprefs.sql | 1 + .../modules/admin/preferences/accounting.pref | 6 + .../prog/en/modules/pos/register.tt | 19 ++- pos/register.pl | 15 ++- t/db_dependent/Koha/Cash/Register.t | 127 +++++++++++++++++- 7 files changed, 192 insertions(+), 6 deletions(-) diff --git a/Koha/Cash/Register.pm b/Koha/Cash/Register.pm index 3261ca1ff11..15250aac94b 100644 --- a/Koha/Cash/Register.pm +++ b/Koha/Cash/Register.pm @@ -395,6 +395,16 @@ sub add_cashup { # Calculate difference (actual - expected) my $difference = $amount - $expected_amount; + # Validate reconciliation note requirement if there's a discrepancy + if ( $difference != 0 ) { + my $note_required = C4::Context->preference('CashupReconciliationNoteRequired') // 0; + + if ( $note_required && !defined $reconciliation_note ) { + Koha::Exceptions::MissingParameter->throw( + error => "Reconciliation note is required when cashup amount differs from expected amount" ); + } + } + # Use database transaction to ensure consistency my $schema = $self->_result->result_source->schema; my $cashup; diff --git a/installer/data/mysql/atomicupdate/bug_40445.pl b/installer/data/mysql/atomicupdate/bug_40445.pl index f6226eb4512..dce18a8c979 100755 --- a/installer/data/mysql/atomicupdate/bug_40445.pl +++ b/installer/data/mysql/atomicupdate/bug_40445.pl @@ -33,5 +33,25 @@ return { $out, "Staff can now record actual cash amounts during cashup with automatic surplus/deficit tracking" ); + + # Add CashupReconciliationNoteRequired preference + $dbh->do( + q{ + INSERT IGNORE INTO systempreferences (variable, value, options, explanation, type) + VALUES ( + 'CashupReconciliationNoteRequired', + '0', + '', + 'Require a reconciliation note when completing cashup with discrepancies between expected and actual amounts', + 'YesNo' + ) + } + ); + + say_success( $out, "Added CashupReconciliationNoteRequired system preference" ); + say_info( + $out, + "CashupReconciliationNoteRequired: Controls whether reconciliation notes are required during cashup with discrepancies" + ); }, }; diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index 034f48ed3f2..9125d6bcbac 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -147,6 +147,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('CardnumberLength', '', '', 'Set a length for card numbers with a maximum of 32 characters.', 'Free'), ('CardnumberLog','1',NULL,'If ON, log edit actions on patron cardnumbers','YesNo'), ('casAuthentication','0','','Enable or disable CAS authentication','YesNo'), +('CashupReconciliationNoteRequired', '0', NULL, 'Require a reconciliation note when completing cashup with discrepancies between expected and actual amounts', 'YesNo'), ('casLogout','0','','Does a logout from Koha should also log the user out of CAS?','YesNo'), ('casServerUrl','https://localhost:8443/cas','','URL of the cas server','Free'), ('casServerVersion','2', '2|3','Version of the CAS server Koha will connect to.','Choice'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/accounting.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/accounting.pref index 55bcd590cfb..37046e4aa75 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/accounting.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/accounting.pref @@ -57,3 +57,9 @@ Accounting: branchyyyymmincr: 'Automatically generate credit numbers in the form yyyymm0001' incremental: 'Automatically generate credit numbers in the form 1, 2, 3' - Automatic generation also has to be enabled for each credit type (Configure credit types). + - + - pref: CashupReconciliationNoteRequired + choices: + 1: "Require" + 0: "Don't require" + - a reconciliation note when completing cashup with discrepancies between expected and actual amounts. 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 3b91d2d1042..00392774f33 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt @@ -80,6 +80,10 @@
The cashup amount must be a positive number greater than zero.
[% END %] + [% IF ( error_reconciliation_note_required ) %] +
Reconciliation note is required when cashup amount differs from expected amount.
+ [% END %] + [% IF ( error_cashup_complete ) %]
Failed to complete cashup. Please try again. @@ -465,8 +469,11 @@ @@ -743,9 +750,19 @@ // Show/hide note field based on whether there's a discrepancy if (hasDiscrepancy) { $("#reconciliation_note_field").show(); + // Update required attribute and label based on system preference + [% IF reconciliation_note_required %] + $("#reconciliation_note").attr('required', 'required'); + $("#reconciliation_note_label").addClass("required"); + [% ELSE %] + $("#reconciliation_note").removeAttr('required'); + $("#reconciliation_note_label").removeClass("required"); + [% END %] } else { $("#reconciliation_note_field").hide(); $("#reconciliation_note").val(''); // Clear note when balanced + $("#reconciliation_note").removeAttr('required'); + $("#reconciliation_note_label").removeClass("required"); } } else { $("#reconciliation_display").hide(); diff --git a/pos/register.pl b/pos/register.pl index 3a9524adfe4..183d9b27d52 100755 --- a/pos/register.pl +++ b/pos/register.pl @@ -68,9 +68,10 @@ if ( !$registers->count ) { my $cashup_in_progress = $cash_register->cashup_in_progress(); $template->param( - register => $cash_register, - accountlines => $accountlines, - cashup_in_progress => $cashup_in_progress, + register => $cash_register, + accountlines => $accountlines, + cashup_in_progress => $cashup_in_progress, + reconciliation_note_required => C4::Context->preference('CashupReconciliationNoteRequired'), ); my $transactions_range_from = $input->param('trange_f'); @@ -160,7 +161,13 @@ if ( !$registers->count ) { } elsif ( $@->isa('Koha::Exceptions::Object::DuplicateID') ) { $template->param( error_cashup_already_completed => 1 ); } elsif ( $@->isa('Koha::Exceptions::MissingParameter') ) { - $template->param( error_cashup_missing_param => 1, error_message => $@ ); + + # Check if this is a reconciliation note error specifically + if ( $@->error =~ /Reconciliation note is required/ ) { + $template->param( error_reconciliation_note_required => 1 ); + } else { + $template->param( error_cashup_missing_param => 1, error_message => $@ ); + } } elsif ( $@->isa('Koha::Exceptions::Account::AmountNotPositive') ) { $template->param( error_cashup_amount_invalid => 1 ); } else { diff --git a/t/db_dependent/Koha/Cash/Register.t b/t/db_dependent/Koha/Cash/Register.t index 98dac5fc135..875a1bc9c8c 100755 --- a/t/db_dependent/Koha/Cash/Register.t +++ b/t/db_dependent/Koha/Cash/Register.t @@ -20,7 +20,7 @@ use Modern::Perl; use Test::NoWarnings; -use Test::More tests => 10; +use Test::More tests => 11; use Test::Exception; @@ -29,6 +29,7 @@ use Koha::Account; use Koha::Account::CreditTypes; use Koha::Account::DebitTypes; +use t::lib::Mocks; use t::lib::TestBuilder; my $builder = t::lib::TestBuilder->new; @@ -1580,3 +1581,127 @@ subtest 'add_cashup' => sub { $schema->storage->txn_rollback; }; + +subtest 'required_reconciliation_note' => sub { + plan tests => 4; + + $schema->storage->txn_begin; + + my $register = $builder->build_object( { class => 'Koha::Cash::Registers' } ); + my $manager = $builder->build_object( { class => 'Koha::Patrons' } ); + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $account = $patron->account; + + # Create a cash transaction + my $fine = $account->add_debit( + { + amount => '10.00', + type => 'OVERDUE', + interface => 'cron' + } + ); + + my $payment = $account->pay( + { + cash_register => $register->id, + amount => '10.00', + credit_type => 'PAYMENT', + payment_type => 'CASH', + lines => [$fine] + } + ); + + # Enable the required note preference + t::lib::Mocks::mock_preference( 'CashupReconciliationNoteRequired', 1 ); + + # Test 1: Missing note with discrepancy throws exception + throws_ok { + $register->add_cashup( + { + manager_id => $manager->id, + amount => '15.00' # Creates discrepancy + } + ); + } + 'Koha::Exceptions::MissingParameter', + 'Missing reconciliation note with discrepancy throws MissingParameter exception when preference enabled'; + + # Test 2: Note provided with discrepancy succeeds + my $cashup1; + lives_ok { + $cashup1 = $register->add_cashup( + { + manager_id => $manager->id, + amount => '15.00', + reconciliation_note => 'Found extra money' + } + ); + } + 'Cashup with note and discrepancy succeeds when preference enabled'; + + # Test 3: No note with no discrepancy succeeds (note only required for discrepancies) + my $register2 = $builder->build_object( { class => 'Koha::Cash::Registers' } ); + my $fine2 = $account->add_debit( + { + amount => '20.00', + type => 'OVERDUE', + interface => 'cron' + } + ); + + my $payment2 = $account->pay( + { + cash_register => $register2->id, + amount => '20.00', + credit_type => 'PAYMENT', + payment_type => 'CASH', + lines => [$fine2] + } + ); + + my $cashup2; + lives_ok { + $cashup2 = $register2->add_cashup( + { + manager_id => $manager->id, + amount => '20.00' # Exact amount, no discrepancy + } + ); + } + 'Cashup without note succeeds when there is no discrepancy'; + + # Test 4: Preference disabled allows missing note even with discrepancy + t::lib::Mocks::mock_preference( 'CashupReconciliationNoteRequired', 0 ); + + my $register3 = $builder->build_object( { class => 'Koha::Cash::Registers' } ); + my $fine3 = $account->add_debit( + { + amount => '10.00', + type => 'OVERDUE', + interface => 'cron' + } + ); + + my $payment3 = $account->pay( + { + cash_register => $register3->id, + amount => '10.00', + credit_type => 'PAYMENT', + payment_type => 'CASH', + lines => [$fine3] + } + ); + + my $cashup3; + lives_ok { + $cashup3 = $register3->add_cashup( + { + manager_id => $manager->id, + amount => '15.00' # Creates discrepancy + } + ); + } + 'Missing note with discrepancy succeeds when preference disabled'; + + $schema->storage->txn_rollback; +}; -- 2.52.0