Attachment 141601 Details for Bug 31727 – Bug 31727: Fix existing update to prevent inserting incorrect data

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

Bug-31727-Fix-existing-update-to-prevent-inserting.patch (text/plain), 4.11 KB, created by Nick Clemens (kidclamp) on 2022-10-11 12:56:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2022-10-11 12:56:51 UTC

Size: 4.11 KB

patch

obsolete

>From 448e60723c9c02b4fe64884ebae6307c5c4eafbd 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 31727: 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>
>---
> 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.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 31727: 141595 | 141600 | 141601 | 141618 | 141725