gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 10190: rewrite atomicupdate with correct 1:1 rule migration 

The previous migration had two fundamental errors:

1. It manufactured phantom empty rows for all patron categories that
   had no rule, which corrupted the "most frequent" calculation.

2. It created categorycode=NULL (all-categories) default rules for
   every branchcode seen, silently extending overdue notice coverage
   to patron categories that had no configured rule in the old system.
   The backend (overdue_notices.pl) does not consult has_rules, so
   these invented defaults would cause notices to be sent to patrons
   that were deliberately uncovered.

The rewrite migrates only explicit (branchcode, categorycode) pairs
that had a positive delay configured. No default rules are created.
branchcode='' maps to NULL as before. exit is replaced with return
to avoid killing subsequent atomicupdate scripts.

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Martin Renvoize 2026-02-25 09:33:05 +00:00 committed by Pedro Amorim
parent 8c8160761d
commit d539637c5e
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
1 changed files with 47 additions and 262 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

309
installer/data/mysql/atomicupdate/bug_10190.pl
View File

@ -8,9 +8,11 @@ return {
my ($args) = @_;
my ( $dbh, $out ) = @$args{qw(dbh out)};
# This script first pulls data from these tables, then deletes them. -> Do not run if they do not exist.
# This script pulls data from these tables then drops them.
# Skip silently if they no longer exist (e.g. fresh install or already migrated).
if ( !TableExists('overduerules_transport_types') || !TableExists('overduerules') ) {
exit;
say_info( $out, "Tables overduerules / overduerules_transport_types not found, skipping migration" );
return;
}
# Remove the tools sub-permission for notice triggers; access is now
@ -19,49 +21,38 @@ return {
$dbh->do(q|DELETE FROM permissions WHERE code = 'edit_notice_status_triggers'|);
say_success( $out, "Removed deprecated permission 'edit_notice_status_triggers'" );
# Populate empty overduerules table for missing categories
$dbh->do(
q|
INSERT IGNORE INTO overduerules (branchcode, categorycode)
SELECT '', categorycode
FROM categories
WHERE categorycode NOT IN (SELECT categorycode FROM overduerules);
|
);
# Fetch all overdue rules
# Fetch all explicit overdue rules with their transport types.
# No phantom rows are added - only rules that were explicitly configured are migrated.
my $rules = $dbh->selectall_arrayref(
q|
SELECT
o.overduerules_id,
o.branchcode,
o.categorycode,
o.delay1,
o.letter1,
o.debarred1,
o.delay2,
o.debarred2,
o.letter2,
o.debarred2,
o.delay3,
o.letter3,
o.debarred3,
GROUP_CONCAT(
CASE WHEN ott.letternumber = 1 THEN ott.message_transport_type END
ORDER BY ott.message_transport_type ASC
) AS message_transport_type_1,
) AS mtt_1,
GROUP_CONCAT(
CASE WHEN ott.letternumber = 2 THEN ott.message_transport_type END
ORDER BY ott.message_transport_type ASC
) AS message_transport_type_2,
) AS mtt_2,
GROUP_CONCAT(
CASE WHEN ott.letternumber = 3 THEN ott.message_transport_type END
ORDER BY ott.message_transport_type ASC
) AS message_transport_type_3
) AS mtt_3
FROM
overduerules o
LEFT JOIN
overduerules_transport_types ott
ON o.overduerules_id = ott.overduerules_id
overduerules_transport_types ott ON o.overduerules_id = ott.overduerules_id
GROUP BY
o.overduerules_id,
o.branchcode,
@ -70,8 +61,8 @@ return {
o.letter1,
o.debarred1,
o.delay2,
o.debarred2,
o.letter2,
o.debarred2,
o.delay3,
o.letter3,
o.debarred3
@ -79,266 +70,60 @@ return {
{ Slice => {} }
);
# Initialize hashes to store the frequency of delays, notices, and mtt per group (1, 2, 3)
my %branchcodes;
my %delay_count;
my %notice_count;
my %mtt_count;
my %restrict_count;
# Populate the hashes with the frequency of each value by group
for my $rule ( @{$rules} ) {
my $branchcode = $rule->{'branchcode'};
$branchcodes{$branchcode} = 1;
foreach my $i ( 1 .. 3 ) {
my $delay = $rule->{"delay$i"} // '';
my $notice = $rule->{"letter$i"} // '';
my $mtt = $rule->{"message_transport_type_$i"} // '';
my $restrict = $rule->{"debarred$i"} // '';
# Increment counts only if values are defined
$delay_count{$branchcode}{$i}{$delay}++ if defined $delay;
$notice_count{$branchcode}{$i}{$notice}++ if defined $notice;
$mtt_count{$branchcode}{$i}{$mtt}++ if defined $mtt;
$restrict_count{$branchcode}{$i}{$restrict}++ if defined $restrict;
}
}
# Find the most frequent delay, notice, and mtt for each branchcode and group
my ( %most_frequent_delay, %most_frequent_notice, %most_frequent_mtt, %most_frequent_restrict );
for my $branchcode ( keys %branchcodes ) {
foreach my $i ( 1 .. 3 ) {
# Find the most frequent delay in group $i
my $max_delay_count = 0;
for my $delay ( keys %{ $delay_count{$branchcode}{$i} } ) {
if ( $delay_count{$branchcode}{$i}{$delay} > $max_delay_count ) {
$max_delay_count = $delay_count{$branchcode}{$i}{$delay};
$most_frequent_delay{$branchcode}{$i} = $delay;
}
}
# Find the most frequent notice in group $i
my $max_notice_count = 0;
for my $notice ( keys %{ $notice_count{$branchcode}{$i} } ) {
if ( $notice_count{$branchcode}{$i}{$notice} > $max_notice_count ) {
$max_notice_count = $notice_count{$branchcode}{$i}{$notice};
$most_frequent_notice{$branchcode}{$i} = $notice;
}
}
# Find the most frequent mtt in group $i
my $max_mtt_count = 0;
for my $mtt ( keys %{ $mtt_count{$branchcode}{$i} } ) {
if ( $mtt_count{$branchcode}{$i}{$mtt} > $max_mtt_count ) {
$max_mtt_count = $mtt_count{$branchcode}{$i}{$mtt};
$most_frequent_mtt{$branchcode}{$i} = $mtt;
}
}
# Find the most frequent mtt in group $i
my $max_restrict_count = 0;
for my $restrict ( keys %{ $restrict_count{$branchcode}{$i} } ) {
if ( $restrict_count{$branchcode}{$i}{$restrict} > $max_restrict_count ) {
$max_restrict_count = $restrict_count{$branchcode}{$i}{$restrict};
$most_frequent_restrict{$branchcode}{$i} = $restrict;
}
}
}
}
# Migrate rules from overduerules to circulation_rules, skipping most frequent values as those will be our defaults
my $insert = $dbh->prepare(
"INSERT IGNORE INTO circulation_rules (branchcode, categorycode, itemtype, rule_name, rule_value) VALUES (?, ?, ?, ?, ?)"
);
my $itemtype = undef;
my $migrated = 0;
for my $rule ( @{$rules} ) {
my $branchcode = $rule->{'branchcode'} || undef;
my $categorycode = $rule->{'categorycode'} || undef;
my $branchcode_key = $branchcode // '';
# In the old system branchcode='' is the global/default library.
# In circulation_rules this is represented as branchcode=NULL.
my $branchcode = $rule->{branchcode} || undef;
my $categorycode = $rule->{categorycode} || undef;
my $itemtype = undef;
my $context_label = ( $branchcode // '*' ) . ':' . ( $categorycode // '*' );
foreach my $i ( 1 .. 3 ) {
my $delay = $rule->{"delay$i"};
my $notice = $rule->{"letter$i"};
my $mtt = $rule->{"mtt_$i"};
my $restrict = $rule->{"debarred$i"};
my $has_rules = 0;
# A zero or null delay means this trigger was not configured in the old system.
# Only migrate explicitly configured triggers.
next unless defined $delay && $delay > 0;
# Insert the delay rule for group $i, skipping if it matches the most frequent delay
if ( my $delay = $rule->{"delay$i"} ) {
$delay ||= '';
unless ( $delay eq $most_frequent_delay{$branchcode_key}{$i} ) {
$delay ||= undef;
say $out "Inserting "
. ( $branchcode // '' ) . ":"
. ( $categorycode // '' ) . ':'
. ( $itemtype // '' )
. " overdue_$i"
. '_delay: '
. ( $delay // '' );
$insert->execute(
$branchcode,
$categorycode,
$itemtype,
"overdue_$i" . '_delay',
$delay
);
$has_rules = 1;
}
say $out "Migrating trigger $i for $context_label (delay=$delay)";
$insert->execute( $branchcode, $categorycode, $itemtype, "overdue_${i}_delay", $delay );
if ($notice) {
$insert->execute( $branchcode, $categorycode, $itemtype, "overdue_${i}_notice", $notice );
}
# Insert the notice rule for group $i, skipping if it matches the most frequent notice
if ( my $notice = $rule->{"letter$i"} ) {
unless ( $notice eq $most_frequent_notice{$branchcode_key}{$i} ) {
say $out "Inserting "
. ( $branchcode // '' ) . ":"
. ( $categorycode // '' ) . ':'
. ( $itemtype // '' )
. " overdue_$i"
. '_notice: '
. ( $notice // '' );
$insert->execute(
$branchcode,
$categorycode,
$itemtype,
"overdue_$i" . '_notice',
$notice
);
$has_rules = 1;
}
if ($mtt) {
$insert->execute( $branchcode, $categorycode, $itemtype, "overdue_${i}_mtt", $mtt );
}
# Insert the message transport type rule for group $i, skipping if it matches the most frequent mtt
if ( my $mtt = $rule->{"message_transport_type_$i"} ) {
unless ( $mtt eq $most_frequent_mtt{$branchcode_key}{$i} ) {
say $out "Inserting "
. ( $branchcode // '' ) . ":"
. ( $categorycode // '' ) . ':'
. ( $itemtype // '' )
. " overdue_$i"
. '_mtt: '
. ( $mtt // '' );
$insert->execute(
$branchcode,
$categorycode,
$itemtype,
"overdue_$i" . '_mtt',
$mtt
);
$has_rules = 1;
}
# Only insert restrict when explicitly enabled (=1); absence defaults to no restriction.
if ($restrict) {
$insert->execute( $branchcode, $categorycode, $itemtype, "overdue_${i}_restrict", $restrict );
}
# Insert the restrict rule for group $i
if ( my $restrict = $rule->{"debarred$i"} ) {
unless ( $restrict eq $most_frequent_restrict{$branchcode_key}{$i} ) {
say $out "Inserting "
. ( $branchcode // '' ) . ":"
. ( $categorycode // '' ) . ':'
. ( $itemtype // '' )
. " overdue_$i"
. '_restrict: '
. ( $restrict // '' );
$insert->execute(
$branchcode,
$categorycode,
$itemtype,
"overdue_$i" . '_restrict',
$restrict
);
$has_rules = 1;
}
}
# has_rules is a UI sentinel that marks an explicit ruleset for this context/trigger.
# It is not used by the backend (overdue_notices.pl).
$insert->execute( $branchcode, $categorycode, $itemtype, "overdue_${i}_has_rules", 1 );
# Insert the has_rules rule for group $i if and only if a rule set has been added into the db for this context.
if ( $has_rules == 1 ) {
say $out "Inserting "
. ( $branchcode // '' ) . ":"
. ( $categorycode // '' ) . ':'
. ( $itemtype // '' )
. " overdue_$i"
. "has_rules: 1";
$insert->execute(
$branchcode,
$categorycode,
$itemtype,
"overdue_$i" . '_has_rules',
1
);
}
$migrated++;
}
}
# Insert the default rules for each group
for my $branchcode ( keys %branchcodes ) {
my $branchcode_value = $branchcode || undef;
foreach my $i ( 1 .. 3 ) {
my $most_frequent_delay = $most_frequent_delay{$branchcode}{$i};
say $out "Inserting "
. ( $branchcode_value // '' ) . ":all:"
. ( $itemtype // '' )
. " default most frequent delay for overdue_$i: "
. $most_frequent_delay;
$insert->execute(
$branchcode_value,
undef,
$itemtype,
"overdue_${i}_delay",
$most_frequent_delay
);
say_success( $out, "Migrated $migrated trigger(s) from overduerules to circulation_rules" );
my $most_frequent_notice = $most_frequent_notice{$branchcode}{$i};
say $out "Inserting "
. ( $branchcode_value // '' )
. ":all:all default most frequent notice for overdue_$i: "
. $most_frequent_notice;
$insert->execute(
$branchcode_value,
undef,
undef,
"overdue_${i}_notice",
$most_frequent_notice
);
my $most_frequent_mtt = $most_frequent_mtt{$branchcode}{$i};
say $out "Inserting "
. ( $branchcode_value // '' )
. ":all:all default most frequent mtt for overdue_$i: "
. $most_frequent_mtt;
$insert->execute(
$branchcode_value,
undef,
undef,
"overdue_${i}_mtt",
$most_frequent_mtt
);
my $most_frequent_restrict = $most_frequent_restrict{$branchcode}{$i};
say $out "Inserting "
. ( $branchcode_value // '' )
. ":all:all default most frequent restrict for overdue_$i: "
. $most_frequent_restrict;
$insert->execute(
$branchcode_value,
undef,
undef,
"overdue_${i}_restrict",
$most_frequent_restrict
);
# Insert the has_rules rule for group $i's default context
say $out "Inserting " . ( $branchcode_value // '' ) . ":all:all default has_rules for overdue_$i: 1";
$insert->execute(
$branchcode_value,
undef,
undef,
"overdue_${i}_has_rules",
1
);
}
}
$dbh->do(q|DROP TABLE overduerules_transport_types|) if TableExists('overduerules_transport_types');
$dbh->do(q|DROP TABLE overduerules|) if TableExists('overduerules');
$dbh->do(q|DROP TABLE overduerules_transport_types|);
$dbh->do(q|DROP TABLE overduerules|);
}
};