diff --git a/app/lib/string_attribute_cleaner.rb b/app/lib/string_attribute_cleaner.rb index c51f1d723..f88b5b376 100644 --- a/app/lib/string_attribute_cleaner.rb +++ b/app/lib/string_attribute_cleaner.rb @@ -12,9 +12,9 @@ module StringAttributeCleaner # @return [void] # # @example Defining a list of attributes to be cleaned - # include StringAttributeCleaner.for(:attribute1, :attribute2) - # include StringAttributeCleaner.for(:attribute1, :attribute2, on: :before_save) - def self.for(*attributes, on: :before_validation) + # include StringAttributeCleaner.nullify_blanks_for(:attribute1, :attribute2) + # include StringAttributeCleaner.nullify_blanks_for(:attribute1, :attribute2, on: :before_save) + def self.nullify_blanks_for(*attributes, on: :before_validation) Module.new do define_singleton_method(:included) do |klass| return unless klass < ActiveRecord::Base diff --git a/app/models/article.rb b/app/models/article.rb index eb603d216..ac766f30d 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -8,7 +8,7 @@ class Article < ApplicationRecord acts_as_taggable_on :tags resourcify - include StringAttributeCleaner.for(:canonical_url, on: :before_save) + include StringAttributeCleaner.nullify_blanks_for(:canonical_url, on: :before_save) DEFAULT_FEED_PAGINATION_WINDOW_SIZE = 50 attr_accessor :publish_under_org diff --git a/app/models/discussion_lock.rb b/app/models/discussion_lock.rb index 2e3f815bc..bae167975 100644 --- a/app/models/discussion_lock.rb +++ b/app/models/discussion_lock.rb @@ -9,7 +9,7 @@ class DiscussionLock < ApplicationRecord belongs_to :article belongs_to :locking_user, class_name: "User" - include StringAttributeCleaner.for(:notes, :reason) + include StringAttributeCleaner.nullify_blanks_for(:notes, :reason) validates :article_id, uniqueness: true end diff --git a/app/models/tag.rb b/app/models/tag.rb index b7f9640b0..fa8f5f3f3 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -31,7 +31,7 @@ class Tag < ActsAsTaggableOn::Tag # change will help us achieve that goal. # # @see https://github.com/forem/forem/blob/72bb284ba73c3df8aa11525427b1dfa1ceba39df/lib/data_update_scripts/20211115154021_nullify_invalid_tag_fields.rb - include StringAttributeCleaner.for(:alias_for) + include StringAttributeCleaner.nullify_blanks_for(:alias_for) ALLOWED_CATEGORIES = %w[uncategorized language library tool site_mechanic location subcommunity].freeze HEX_COLOR_REGEXP = /\A#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})\z/ diff --git a/app/models/user.rb b/app/models/user.rb index b77602d9d..d3f003a0c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -22,7 +22,7 @@ class User < ApplicationRecord end end - include StringAttributeCleaner.for(:email) + include StringAttributeCleaner.nullify_blanks_for(:email) USERNAME_MAX_LENGTH = 30 USERNAME_REGEXP = /\A[a-zA-Z0-9_]+\z/ diff --git a/spec/lib/string_attribute_cleaner_spec.rb b/spec/lib/string_attribute_cleaner_spec.rb index 30318ec30..6718d137f 100644 --- a/spec/lib/string_attribute_cleaner_spec.rb +++ b/spec/lib/string_attribute_cleaner_spec.rb @@ -1,87 +1,89 @@ require "rails_helper" RSpec.describe StringAttributeCleaner, type: :lib do - with_model :TestClass do - table do |t| - t.string :string_attribute + describe ".nullify_blanks_for" do + with_model :TestClass do + table do |t| + t.string :string_attribute + end + + model do + # rubocop:disable RSpec/DescribedClass + include StringAttributeCleaner.nullify_blanks_for(:string_attribute) + # rubocop:enable RSpec/DescribedClass + end end - model do - # rubocop:disable RSpec/DescribedClass - include StringAttributeCleaner.for(:string_attribute) - # rubocop:enable RSpec/DescribedClass - end - end + with_model :AlternateTestClass do + table do |t| + t.string :string_attribute + end - with_model :AlternateTestClass do - table do |t| - t.string :string_attribute + model do + # rubocop:disable RSpec/DescribedClass + include StringAttributeCleaner.nullify_blanks_for(:string_attribute, on: :before_save) + # rubocop:enable RSpec/DescribedClass + end end - model do - # rubocop:disable RSpec/DescribedClass - include StringAttributeCleaner.for(:string_attribute, on: :before_save) - # rubocop:enable RSpec/DescribedClass - end - end + context "when specifying an :on callback" do + it "registers that callback on the model", :aggregate_failures do + before_validation_cbs = AlternateTestClass._validation_callbacks.filter_map do |cb| + cb.filter if cb.kind == :before + end - context "when specifying an :on callback" do - it "registers that callback on the model", :aggregate_failures do - before_validation_cbs = AlternateTestClass._validation_callbacks.filter_map do |cb| + before_save_cbs = AlternateTestClass._save_callbacks.filter_map do |cb| + cb.filter if cb.kind == :before + end + + expect(before_validation_cbs).to eq([]) + expect(before_save_cbs).to eq([:nullify_blank_attributes]) + expect(TestClass.new).to respond_to(:nullify_blank_attributes) + end + end + + it "adds a before_validation callback to the including model", :aggregate_failures do + before_validation_cbs = TestClass._validation_callbacks.filter_map do |cb| cb.filter if cb.kind == :before end - before_save_cbs = AlternateTestClass._save_callbacks.filter_map do |cb| - cb.filter if cb.kind == :before - end - - expect(before_validation_cbs).to eq([]) - expect(before_save_cbs).to eq([:nullify_blank_attributes]) + expect(before_validation_cbs).to eq([:nullify_blank_attributes]) expect(TestClass.new).to respond_to(:nullify_blank_attributes) end - end - it "adds a before_validation callback to the including model", :aggregate_failures do - before_validation_cbs = TestClass._validation_callbacks.filter_map do |cb| - cb.filter if cb.kind == :before + it "replaces empty strings with nil" do + test_instance = TestClass.new(string_attribute: "") + + expect { test_instance.validate } + .to change(test_instance, :string_attribute).from("").to(nil) end - expect(before_validation_cbs).to eq([:nullify_blank_attributes]) - expect(TestClass.new).to respond_to(:nullify_blank_attributes) - end + it "replaces blank strings with nil" do + test_instance = TestClass.new(string_attribute: " ") - it "replaces empty strings with nil" do - test_instance = TestClass.new(string_attribute: "") + expect { test_instance.validate } + .to change(test_instance, :string_attribute).from(" ").to(nil) + end - expect { test_instance.validate } - .to change(test_instance, :string_attribute).from("").to(nil) - end + it "leaves non-blank attributes unchanged" do + test_instance = TestClass.new(string_attribute: "Test") - it "replaces blank strings with nil" do - test_instance = TestClass.new(string_attribute: " ") + expect { test_instance.validate }.not_to change(test_instance, :string_attribute) + end - expect { test_instance.validate } - .to change(test_instance, :string_attribute).from(" ").to(nil) - end + it "ignores obsolete attributes" do + TestClass.include(described_class.nullify_blanks_for(:non_existing)) - it "leaves non-blank attributes unchanged" do - test_instance = TestClass.new(string_attribute: "Test") + expect { TestClass.new.validate }.not_to raise_error + end - expect { test_instance.validate }.not_to change(test_instance, :string_attribute) - end + it "works with non-AR classes" do + klass = Struct.new(:test, keyword_init: true) + klass.include(described_class.nullify_blanks_for(:test)) + test_instance = klass.new(test: " ") - it "ignores obsolete attributes" do - TestClass.include(described_class.for(:non_existing)) - - expect { TestClass.new.validate }.not_to raise_error - end - - it "works with non-AR classes" do - klass = Struct.new(:test, keyword_init: true) - klass.include(described_class.for(:test)) - test_instance = klass.new(test: " ") - - expect { test_instance.nullify_blank_attributes } - .to change(test_instance, :test).from(" ").to(nil) + expect { test_instance.nullify_blank_attributes } + .to change(test_instance, :test).from(" ").to(nil) + end end end