From 2091ef0fead8f90246d1c9aa4c77110c3e22e033 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Tue, 21 Apr 2026 10:57:28 +0100 Subject: [PATCH] Bug 34000: Unit tests Adds a new subtest to t/db_dependent/Koha/Patron.t covering fixup_cardnumber and the autoMemberNumValue counter: - Normal increment bumps the counter - Renaming a patron's cardnumber does not free it for reuse - Deleting a patron does not free their cardnumber for reuse - Counter wins when ahead of db_max - db_max wins when ahead of counter - Legacy fallback when the syspref row is absent Also adds autoMemberNumValue to the exceptions list in t/db_dependent/check_sysprefs.t. The preference is managed internally by Koha::Patron::fixup_cardnumber -- it is not user-configurable and has no entry in the admin preference YAML files. Test plan: 1. prove t/db_dependent/Koha/Patron.t 2. prove t/db_dependent/check_sysprefs.t 3. prove t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t (confirm the patron_barcode_transform hook is still called twice during store when autoMemberNum is enabled). Signed-off-by: Andrew Fuerste Henry Signed-off-by: Jacob O'Mara --- t/db_dependent/Koha/Patron.t | 97 ++++++++++++++++++++++++++++++++- t/db_dependent/check_sysprefs.t | 1 + 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index 44802ce4ba..9e976f2cdb 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 46; +use Test::More tests => 47; use Test::NoWarnings; use Test::Exception; use Test::Warn; @@ -3562,3 +3562,98 @@ subtest "create_hold_group, hold_groups, visual_hold_group_id tests" => sub { $schema->storage->txn_rollback; }; + +subtest 'fixup_cardnumber and autoMemberNumValue counter' => sub { + plan tests => 10; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'autoMemberNum', 1 ); + + my $pref_rs = $schema->resultset('Systempreference'); + + my $set_counter = sub { + my $value = shift; + my $row = $pref_rs->find('autoMemberNumValue'); + if ($row) { $row->update( { value => $value } ) } + else { $pref_rs->create( { variable => 'autoMemberNumValue', value => $value } ) } + }; + my $get_counter = sub { + my $row = $pref_rs->find('autoMemberNumValue'); + return $row ? $row->value : undef; + }; + + my $category = $builder->build_object( { class => 'Koha::Patron::Categories' } ); + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + + my $new_patron = sub { + return Koha::Patron->new( + { + surname => 'fixup-test', + categorycode => $category->categorycode, + branchcode => $library->branchcode, + cardnumber => undef, + } + ); + }; + + # Anchor well above any existing fixture cardnumbers for deterministic assertions. + my $baseline = Koha::Patrons->search( + { cardnumber => { -regexp => '^-?[0-9]+$' } }, + { + select => \'CAST(cardnumber AS SIGNED)', + as => ['m'], + } + )->_resultset->get_column('m')->max + || 0; + my $start = $baseline + 1_000; + $set_counter->($start); + + my $p1 = $new_patron->()->store; + is( $p1->cardnumber + 0, $start + 1, 'First auto-generated cardnumber is counter + 1' ); + is( $get_counter->() + 0, $start + 1, 'Counter persisted after auto-generation' ); + + # The bug fix: renaming a patron's cardnumber must not free the old number. + my $original = $p1->cardnumber; + $p1->cardnumber('MANUAL-XYZ')->store; + my $p2 = $new_patron->()->store; + isnt( $p2->cardnumber, $original, q{Original auto-generated number not reused after rename} ); + is( $p2->cardnumber + 0, $start + 2, 'Next patron uses counter + 1, skipping the freed slot' ); + + # Deleting a patron must not free their cardnumber either. + my $p2_cardnumber = $p2->cardnumber; + $p2->delete; + my $p3 = $new_patron->()->store; + isnt( $p3->cardnumber, $p2_cardnumber, q{Deleted patron's cardnumber not reused} ); + + # Counter ahead of DB max: counter wins. + $set_counter->( $start + 1_000_000 ); + my $p4 = $new_patron->()->store; + is( $p4->cardnumber + 0, $start + 1_000_001, 'Counter ahead of DB max wins' ); + + # DB max ahead of counter (e.g. admin imported high numbers directly): DB max wins. + my $high = $start + 2_000_000; + $set_counter->(5); + Koha::Patron->new( + { + surname => 'manual-high', + categorycode => $category->categorycode, + branchcode => $library->branchcode, + cardnumber => "$high", + } + )->store; + my $p5 = $new_patron->()->store; + is( $p5->cardnumber + 0, $high + 1, 'DB max ahead of counter wins' ); + is( $get_counter->() + 0, $high + 1, 'Counter catches up to DB max + 1' ); + + # Pre-upgrade fallback: syspref row absent -> behaves like pre-34000 code. + $pref_rs->find('autoMemberNumValue')->delete; + my $p6 = $new_patron->()->store; + is( $p6->cardnumber + 0, $high + 2, 'Legacy fallback when syspref row is absent' ); + is( + $pref_rs->find('autoMemberNumValue'), undef, + 'Syspref row stays absent when fallback path is taken' + ); + + $schema->storage->txn_rollback; +}; diff --git a/t/db_dependent/check_sysprefs.t b/t/db_dependent/check_sysprefs.t index afe4eccc50..47e14e7b96 100755 --- a/t/db_dependent/check_sysprefs.t +++ b/t/db_dependent/check_sysprefs.t @@ -56,6 +56,7 @@ subtest 'Compare sysprefs.sql with YAML files' => sub { @syspref_names_in_file = grep !/UsageStatsID/, @syspref_names_in_file; # Separate configuration page @syspref_names_in_file = grep !/UsageStatsLastUpdateTime/, @syspref_names_in_file; # Separate configuration page @syspref_names_in_file = grep !/UsageStatsPublicID/, @syspref_names_in_file; # Separate configuration page + @syspref_names_in_file = grep !/autoMemberNumValue/, @syspref_names_in_file; my @missing_yaml = array_minus( @syspref_names_in_file, @yaml_mod ); is( scalar @missing_yaml, 0, "No system preference entries missing from sysprefs.sql" );