From 02b1a5eb75c5f3c9c139a0aa084c04613ad897bd Mon Sep 17 00:00:00 2001 From: Daniel Uber Date: Thu, 9 Dec 2021 14:10:59 -0600 Subject: [PATCH] Revisit previous update script (#15373) The data update script "nullify empty tag colors" failed to update because of the way it was written - in two passes, updating the background color for tags where the background color was empty, then updating the foreground color for tags where the foreground color was empty. Because both attributes have a validation regex, neither pass updated any tags (the problematic cases observed had empty string values in both columns). Perform in a single loop, over the union of the two sets (expected to include only the same rows as before), an update for both columns at once. Additionally, use update_columns which bypasses _other_ validation issues (the only one I could suspect is the alias_for check). Nullify empty strings in alias_for (simplifies a few things, now it's either valid or null). When a tag is detected with a "danlging alias" repair it by setting alias for nil. --- ...211115154021_nullify_invalid_tag_fields.rb | 34 ++++++++++++ .../nullify_invalid_tag_fields_spec.rb | 53 +++++++++++++++++++ 2 files changed, 87 insertions(+) create mode 100644 lib/data_update_scripts/20211115154021_nullify_invalid_tag_fields.rb create mode 100644 spec/lib/data_update_scripts/nullify_invalid_tag_fields_spec.rb diff --git a/lib/data_update_scripts/20211115154021_nullify_invalid_tag_fields.rb b/lib/data_update_scripts/20211115154021_nullify_invalid_tag_fields.rb new file mode 100644 index 000000000..ecc08dbef --- /dev/null +++ b/lib/data_update_scripts/20211115154021_nullify_invalid_tag_fields.rb @@ -0,0 +1,34 @@ +module DataUpdateScripts + class NullifyInvalidTagFields + def run + Tag.where(bg_color_hex: "").or(Tag.where(text_color_hex: "")).find_each do |tag| + # presence here selects non-empty strings if set (avoiding unsetting valid values) + # but nullifies empty tag colors + tag.update_columns( + bg_color_hex: tag.bg_color_hex.presence, + text_color_hex: tag.text_color_hex.presence, + ) + end + + # normalize empty string to nil (alias_for? behaved correctly already), this is just cleanup + Tag.where(alias_for: "").update(alias_for: nil) + + Tag.where.not(alias_for: nil).find_each do |aliased_tag| + # this is expected to not do anything - based on this query in blazer giving 0 rows + # (there are 64 with aliases, all valid) + + # SELECT tags.alias_for, t2.name FROM tags + # LEFT JOIN tags AS t2 ON tags.alias_for = t2.name + # WHERE tags.alias_for IS NOT NULL + # AND tags.alias_for != '' + # AND t2.name IS NULL; + + aliased_tag.validate + + if aliased_tag.errors.any? { |e| e.type == "alias_for must refer to an existing tag" } + aliased_tag.update_column(:alias_for, nil) + end + end + end + end +end diff --git a/spec/lib/data_update_scripts/nullify_invalid_tag_fields_spec.rb b/spec/lib/data_update_scripts/nullify_invalid_tag_fields_spec.rb new file mode 100644 index 000000000..f28409c0c --- /dev/null +++ b/spec/lib/data_update_scripts/nullify_invalid_tag_fields_spec.rb @@ -0,0 +1,53 @@ +require "rails_helper" +require Rails.root.join( + "lib/data_update_scripts/20211115154021_nullify_invalid_tag_fields.rb", +) + +describe DataUpdateScripts::NullifyInvalidTagFields do + let(:tag) { create(:tag) } + + it "nullifies empty background color" do + tag.update_column(:bg_color_hex, "") + + described_class.new.run + + expect(tag.reload.bg_color_hex).to be_nil + expect(tag.valid?).to be true + end + + it "nullifies empty text foreground color" do + tag.update_column(:text_color_hex, "") + + described_class.new.run + + expect(tag.reload.text_color_hex).to be_nil + expect(tag.valid?).to be true + end + + it "nullifies when both colors empty" do + tag.update_columns(bg_color_hex: "", text_color_hex: "") + + described_class.new.run + + expect(tag.reload.text_color_hex).to be_nil + expect(tag.valid?).to be true + end + + it "nullifies invalid alias_for values" do + tag.update_columns(alias_for: "this-is-not-a-tag-name") + + described_class.new.run + + expect(tag.reload.alias_for).to be_nil + expect(tag.valid?).to be true + end + + it "nullifies empty string alias_for values" do + tag.update(alias_for: "") + + described_class.new.run + + expect(tag.reload.alias_for).to be_nil + expect(tag.valid?).to be true + end +end