Bug 34000: Prevent re-issuance of auto-generated cardnumbers
When autoMemberNum is enabled, Koha picks the next cardnumber as
MAX(numeric cardnumber in borrowers) + 1. If patron A's auto-generated
cardnumber is later changed (e.g. they get a physical card) or the
patron is deleted, the original number becomes "free" and the next
self-registrant is assigned it -- inheriting anything third-party
services (OverDrive, Hoopla, unauthenticated self-check) have keyed on
the cardnumber.
This patch makes Koha::Patron::fixup_cardnumber use a monotonic counter
(the autoMemberNumValue syspref) so that once a number has been
auto-generated for any patron, it is never auto-generated again.
The syspref row is locked with SELECT ... FOR UPDATE inside the
existing txn_do, which serialises concurrent callers via InnoDB
row-level locking and addresses the race condition raised against
earlier revisions of this patch.
The formula is: next = MAX(counter, db_max) + 1. When the counter is 0
(pre-upgrade or missing syspref), this degrades gracefully to
db_max + 1 -- the legacy behaviour -- so no feature toggle is needed.
The counter is only bumped when Koha actually auto-generates a
cardnumber; plugin- or caller-supplied values do not touch it.
Test plan:
1. Enable autoMemberNum.
2. Self-register patron A via OPAC, note cardnumber N.
3. Edit patron A in the staff interface, change cardnumber to
something non-numeric like "MANUAL-XYZ".
4. Self-register patron B.
5. Confirm B gets N+1, NOT N.
6. Delete patron A.
7. Self-register patron C.
8. Confirm C gets N+2, NOT A's original number.
9. Install a plugin implementing the patron_barcode_transform hook;
confirm it still runs and its value is honoured (counter is not
bumped when the plugin supplies a cardnumber).
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
(cherry picked from commit ccfbb35de5)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
parent
b55535e770
commit
f1a8e146ac
|
|
@ -121,24 +121,46 @@ sub new {
|
|||
|
||||
=head3 fixup_cardnumber
|
||||
|
||||
Autogenerate next cardnumber from highest value found in database
|
||||
Autogenerate next cardnumber from highest value found in database.
|
||||
|
||||
Uses the C<autoMemberNumValue> system preference as a monotonic floor so a
|
||||
cardnumber is never re-issued after the original holder's cardnumber has been
|
||||
changed or their account deleted. The syspref row is selected C<FOR UPDATE>
|
||||
inside the enclosing transaction so that concurrent C<store()> callers are
|
||||
serialised.
|
||||
|
||||
=cut
|
||||
|
||||
sub fixup_cardnumber {
|
||||
my ($self) = @_;
|
||||
|
||||
my $max = $self->cardnumber;
|
||||
Koha::Plugins->call( 'patron_barcode_transform', \$max );
|
||||
my $next = $self->cardnumber;
|
||||
Koha::Plugins->call( 'patron_barcode_transform', \$next );
|
||||
|
||||
$max ||= Koha::Patrons->search(
|
||||
{ cardnumber => { -regexp => '^-?[0-9]+$' } },
|
||||
{
|
||||
select => \'CAST(cardnumber AS SIGNED)',
|
||||
as => ['cast_cardnumber']
|
||||
}
|
||||
)->_resultset->get_column('cast_cardnumber')->max;
|
||||
$self->cardnumber( ( $max || 0 ) + 1 );
|
||||
# Only auto-generate when no cardnumber has been supplied (directly or by a plugin).
|
||||
unless ($next) {
|
||||
my $schema = $self->_result->result_source->schema;
|
||||
my $pref_rs = $schema->resultset('Systempreference')->search(
|
||||
{ variable => 'autoMemberNumValue' },
|
||||
{ for => 'update' },
|
||||
);
|
||||
my $pref = $pref_rs->first;
|
||||
my $counter = ( $pref && $pref->value ) || 0;
|
||||
|
||||
my $db_max = Koha::Patrons->search(
|
||||
{ cardnumber => { -regexp => '^-?[0-9]+$' } },
|
||||
{
|
||||
select => \'CAST(cardnumber AS SIGNED)',
|
||||
as => ['cast_cardnumber'],
|
||||
}
|
||||
)->_resultset->get_column('cast_cardnumber')->max
|
||||
|| 0;
|
||||
|
||||
$next = ( $counter > $db_max ? $counter : $db_max ) + 1;
|
||||
$pref->update( { value => $next } ) if $pref;
|
||||
}
|
||||
|
||||
$self->cardnumber($next);
|
||||
}
|
||||
|
||||
=head3 trim_whitespace
|
||||
|
|
|
|||
Loading…
Reference in New Issue