From 93ab95754c110e084bfef426ee3b43ee52884d46 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 --- Koha/Cash/Register.pm | 10 ++ .../bug_40445_reconciliation_note_required.pl | 31 +++++ 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, 203 insertions(+), 6 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_40445_reconciliation_note_required.pl 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_reconciliation_note_required.pl b/installer/data/mysql/atomicupdate/bug_40445_reconciliation_note_required.pl new file mode 100644 index 00000000000..a93301720dd --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_40445_reconciliation_note_required.pl @@ -0,0 +1,31 @@ +use Modern::Perl; +use Koha::Installer::Output qw(say_warning say_success say_info); + +return { + bug_number => "40445", + description => "Add system preference for required cashup reconciliation notes", + up => sub { + my ($args) = @_; + my ( $dbh, $out ) = @$args{qw(dbh out)}; + + # 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 b47f27df977..0d4abc85c55 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -150,6 +150,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('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'), +('CashupReconciliationNoteRequired', '0', NULL, 'Require a reconciliation note when completing cashup with discrepancies between expected and actual amounts', 'YesNo'), ('CatalogConcerns', '0', NULL, 'Allow users to report catalog concerns', 'YesNo'), ('CatalogerEmails', '', '', 'Notify these catalogers by email when a catalog concern is submitted', 'free'), ('CatalogModuleRelink','0',NULL,'If OFF the linker will never replace the authids that are set in the cataloging module.','YesNo'), 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.51.1