Bugzilla – Attachment 193896 Details for
Bug 10190
Overdue notice triggers based on item type
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 10190: rewrite atomicupdate with correct 1:1 rule migration
9e682d7.patch (text/plain), 18.47 KB, created by
Martin Renvoize (ashimema)
on 2026-02-25 15:33:05 UTC
(
hide
)
Description:
Bug 10190: rewrite atomicupdate with correct 1:1 rule migration
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-02-25 15:33:05 UTC
Size:
18.47 KB
patch
obsolete
>From 9e682d7b20606e6c6a3386d4965c0fbd516cc759 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Wed, 25 Feb 2026 09:33:05 +0000 >Subject: [PATCH] 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. >--- > .../data/mysql/atomicupdate/bug_10190.pl | 307 +++--------------- > 1 file changed, 46 insertions(+), 261 deletions(-) > >diff --git a/installer/data/mysql/atomicupdate/bug_10190.pl b/installer/data/mysql/atomicupdate/bug_10190.pl >index cdce92695d6..39e0cf5d8c8 100755 >--- a/installer/data/mysql/atomicupdate/bug_10190.pl >+++ b/installer/data/mysql/atomicupdate/bug_10190.pl >@@ -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 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; >- } >- } >+ $insert->execute( $branchcode, $categorycode, $itemtype, "overdue_${i}_delay", $delay ); > >- # 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; >- } >+ if ($notice) { >+ $insert->execute( $branchcode, $categorycode, $itemtype, "overdue_${i}_notice", $notice ); > } > >- # 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; >- } >+ if ($mtt) { >+ $insert->execute( $branchcode, $categorycode, $itemtype, "overdue_${i}_mtt", $mtt ); > } > >- # 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 >- ); >+ # 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 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 >- ); >- >- 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 >- ); >+ # 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 ); > >+ $migrated++; > } > } > >- $dbh->do(q|DROP TABLE overduerules_transport_types|) if TableExists('overduerules_transport_types'); >- $dbh->do(q|DROP TABLE overduerules|) if TableExists('overduerules'); >+ say_success( $out, "Migrated $migrated trigger(s) from overduerules to circulation_rules" ); >+ >+ $dbh->do(q|DROP TABLE overduerules_transport_types|); >+ $dbh->do(q|DROP TABLE overduerules|); > } > }; >-- >2.53.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 10190
:
170357
|
170358
|
170359
|
170360
|
170361
|
170362
|
170363
|
170364
|
170365
|
170366
|
170367
|
170368
|
170369
|
170370
|
170371
|
170372
|
170373
|
170374
|
170375
|
170376
|
170380
|
170449
|
171727
|
171728
|
171729
|
171730
|
171731
|
171732
|
171733
|
171734
|
171735
|
171736
|
171737
|
171738
|
171739
|
171740
|
171741
|
171742
|
171743
|
171744
|
171745
|
171746
|
171747
|
171748
|
171749
|
171750
|
171751
|
171752
|
171753
|
171754
|
171755
|
172588
|
172589
|
172590
|
172591
|
172592
|
172593
|
172594
|
172595
|
172596
|
172597
|
172598
|
172599
|
172600
|
172601
|
172602
|
172603
|
172604
|
172605
|
172606
|
172607
|
172608
|
172609
|
172612
|
172614
|
172617
|
172619
|
172621
|
172622
|
172623
|
172624
|
172625
|
172626
|
172630
|
172631
|
172632
|
172633
|
172634
|
172635
|
172636
|
172637
|
172638
|
172639
|
172640
|
172641
|
172642
|
172643
|
172644
|
172645
|
172646
|
172647
|
172648
|
172649
|
172650
|
172651
|
172652
|
172653
|
172654
|
172655
|
172656
|
172657
|
172658
|
172659
|
172660
|
172661
|
172662
|
172663
|
178643
|
178644
|
178645
|
178646
|
178647
|
178648
|
178649
|
178650
|
178651
|
178652
|
178653
|
178654
|
178655
|
178656
|
178657
|
178658
|
178659
|
178660
|
178661
|
178662
|
178663
|
178664
|
178665
|
178666
|
178667
|
178668
|
178669
|
178670
|
178671
|
178672
|
178673
|
178674
|
178675
|
188075
|
188076
|
188077
|
188078
|
188079
|
188080
|
188081
|
188082
|
188083
|
188084
|
188085
|
188086
|
188087
|
188088
|
188476
|
189345
|
189863
|
189864
|
189865
|
189866
|
189867
|
189868
|
191749
|
191750
|
191751
|
191752
|
191753
|
191754
|
191755
|
191756
|
191757
|
191758
|
191759
|
191760
|
191761
|
191762
|
191763
|
191795
|
191796
|
191805
|
191854
|
191873
|
192871
|
192872
|
192873
|
192874
|
192875
|
192876
|
192877
|
192878
|
192879
|
192880
|
192881
|
192882
|
192883
|
192884
|
192885
|
192886
|
192887
|
192888
|
192889
|
192890
|
192891
|
192892
|
192893
|
192894
|
192895
|
192896
|
192897
|
192898
|
192899
|
193864
|
193866
|
193867
|
193868
|
193869
|
193870
|
193871
|
193872
|
193873
|
193874
|
193875
|
193876
|
193877
|
193878
|
193879
|
193880
|
193881
|
193882
|
193883
|
193884
|
193885
|
193886
|
193887
|
193888
|
193889
|
193890
|
193891
|
193892
|
193893
|
193894
|
193895
|
193896
|
193897
|
193898
|
193899
|
193900
|
193901
|
193902
|
193903
|
193904
|
193905
|
193979
|
194045
|
194046
|
194047
|
194048
|
194049
|
194050
|
194051
|
194052
|
194053
|
194054
|
194055
|
194056
|
194057
|
194058
|
194059
|
194060
|
194061
|
194062
|
194063
|
194064
|
194065
|
194066
|
194067
|
194068
|
194069
|
194070
|
194071
|
194072
|
194073
|
194074
|
194075
|
194076
|
194077
|
194078
|
194079
|
194080
|
194081
|
194082
|
194083
|
194084
|
194085
|
194086
|
194087
|
194088
|
194090
|
194094