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 <andrew@bywatersolutions.com>
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>

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;
+};

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" );