Attachment 142248 Details for Bug 29012 – Bug 29012: Fix existing update to prevent inserting incorrect data

[patch] Bug 29012: Fix existing update to prevent inserting incorrect data

Bug-29012-Fix-existing-update-to-prevent-inserting.patch (text/plain), 4.17 KB, created by Tomás Cohen Arazi (tcohen) on 2022-10-20 14:20:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2022-10-20 14:20:19 UTC

Size: 4.17 KB

patch

obsolete

>From 254f72132046aaa89c866784bb572ae42732054f Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>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 <nick@bywatersolutions.com>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> 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 26aac4b6864..50c01046c91 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.34.1

Actions: View | Diff | Splinter Review

Attachments on bug 29012: 128205 | 128206 | 131043 | 131053 | 131054 | 135194 | 135998 | 138140 | 138309 | 138310 | 141869 | 141876 | 141877 | 141878 | 141879 | 141951 | 141952 | 142248 | 142249