From 4c69cbe88c6b9a026080fa419e0d33e0962f3374 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 | 98 ++++++++++++++++++++++++++++++++- t/db_dependent/check_sysprefs.t | 1 + 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index 2ce1a19488..dceb1786a4 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -19,7 +19,8 @@ use Modern::Perl; -use Test::More tests => 38; +use Test::More tests => 39; +use Test::NoWarnings; use Test::Exception; use Test::Warn; use Time::Fake; @@ -2962,3 +2963,98 @@ subtest 'preferred_name' => sub { is( $patron->preferred_name, "Preferred again", "Preferred name set on update when passed" ); }; + +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 3d194e4a56..1391c11b37 100755 --- a/t/db_dependent/check_sysprefs.t +++ b/t/db_dependent/check_sysprefs.t @@ -66,6 +66,7 @@ subtest 'Compare sysprefs.sql with YAML files' => sub { @sysprefs_mod = grep !/UsageStatsID/, @sysprefs_mod; # Separate configuration page @sysprefs_mod = grep !/UsageStatsLastUpdateTime/, @sysprefs_mod; # Separate configuration page @sysprefs_mod = grep !/UsageStatsPublicID/, @sysprefs_mod; # Separate configuration page + @sysprefs_mod = grep !/autoMemberNumValue/, @sysprefs_mod; # Internal counter, not exposed in YAML my @missing_yaml = array_minus( @sysprefs_mod, @yaml_mod ); is( scalar @missing_yaml, 0, "No system preference entries missing from sysprefs.sql" );