From 254f72132046aaa89c866784bb572ae42732054f Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Tue, 11 Oct 2022 12:40:51 +0000 Subject: [PATCH] Bug 29012: Fix existing update to prevent inserting incorrect data Both updates now rely on 'suspension_chargeperiod' as the source of truth for existence of a rule. Spot checking some existing production databases this seems to be acceptable. This alters the update to only add rules where a 'suspension_chargeperiod' rule exists and a value for the rule is not already set. NOTE: While the table has a unique key: UNIQUE KEY `branchcode` (`branchcode`,`categorycode`,`itemtype`,`rule_name`), This key doesn't prevent duplicates on NULLs, i.e.: branchcode | categorycode | itemtype | rule_name | rule_value NULL | NULL | BK | unseen_renewals_allowed | 1 NULL | NULL | BK | unseen_renewals_allowed | 2 NULL | NULL | BK | unseen_renewals_allowed | 3 So we need to both check that any rule is present and that the specific rule is not. The 'INSERT IGNORE' won't IGNORE existing values except for the most specific rules Test plan: 0 - Edit and save existing default rule in DB (sample data does not include suspension_chargeperiod) 1 - Set some rules for "Default checkout, hold and return policy" for "Standard rule" and some specific libraries 2 - cp installer/data/mysql/db_revs/220600024.pl installer/data/mysql/atomicupdate/ 3 - SELECT * FROM circulation rules, note the number of rules that exist 4 - updatedatabase 5 - Check the DB again, you should now have rules as described in the bug i.e. You will have 5 new rules per 'Default checkout, hold and return policy' 6 - apply 1st patch on bug 7 - rm installer/data/mysql/atomicupdate/220600024.pl (so we don't add again) 8 - updatedatabase 9 - Check DB, the extra rules have been deleted 10 - Apply 2nd patch 11 - cp installer/data/mysql/db_revs/220600024.pl installer/data/mysql/atomicupdate/ 12 - rm installer/data/mysql/atomicupdate/bug_31727.pl (so we can confirm update isn't adding rules) 13 - updatedatabase 14 - Confirm no change in rules since step 9 15 - git reset --hard (get deleted update back) 16 - rm installer/data/mysql/atomicupdate/220600024.pl Signed-off-by: Nick Clemens Signed-off-by: Tomas Cohen Arazi --- installer/data/mysql/db_revs/220600024.pl | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/installer/data/mysql/db_revs/220600024.pl b/installer/data/mysql/db_revs/220600024.pl index 26aac4b686..50c01046c9 100755 --- a/installer/data/mysql/db_revs/220600024.pl +++ b/installer/data/mysql/db_revs/220600024.pl @@ -9,7 +9,7 @@ return { my %default_rule_values = ( issuelength => 0, hardduedate => '', - unseenrenewalsallowed => '', + unseen_renewals_allowed => '', rentaldiscount => 0, decreaseloanholds => '', ); @@ -17,7 +17,15 @@ return { $dbh->do(q{ INSERT IGNORE INTO circulation_rules (branchcode, categorycode, itemtype, rule_name, rule_value) SELECT branchcode, categorycode, itemtype, ?, ? FROM circulation_rules cr - WHERE NOT EXISTS ( + WHERE EXISTS ( + SELECT * FROM circulation_rules cr2 + WHERE + cr2.rule_name="suspension_chargeperiod" + AND ( (cr2.branchcode=cr.branchcode) OR ( ISNULL(cr2.branchcode) AND ISNULL(cr.branchcode) ) ) + AND ( (cr2.categorycode=cr.categorycode) OR ( ISNULL(cr2.categorycode) AND ISNULL(cr.categorycode) ) ) + AND ( (cr2.itemtype=cr.itemtype) OR ( ISNULL(cr2.itemtype) AND ISNULL(cr.itemtype) ) ) + ) + AND NOT EXISTS ( SELECT * FROM circulation_rules cr2 WHERE cr2.rule_name=? -- 2.39.5