Renaming method for clarity (#16791)

In looking at forem/forem#16787, it felt like we would benefit from a
similar approach as we adopted for nullifying blank strings.

This refactor helps pave the way for a "normalize_text_for" method (or
some such thing; naming things is hard).

This change also involved introducing a `describe` block into the
associated spec.

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.nullify_blanks_for(:attribute1, :attribute2)
-  #   include StringAttributeCleaner.for(:attribute1, :attribute2, on: :before_save)
+  #   include StringAttributeCleaner.nullify_blanks_for(:attribute1, :attribute2, on: :before_save)
-  def self.for(*attributes, on: :before_validation)
+  def self.nullify_blanks_for(*attributes, on: :before_validation)
     Module.new do
       define_singleton_method(:included) do |klass|
         return unless klass < ActiveRecord::Base

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

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

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/
 

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/

spec/lib/string_attribute_cleaner_spec.rb

@@ -1,87 +1,89 @@
 require "rails_helper"
 
 RSpec.describe StringAttributeCleaner, type: :lib do
-  with_model :TestClass do
+  describe ".nullify_blanks_for" do
-    table do |t|
+    with_model :TestClass do
-      t.string :string_attribute
+      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
+    with_model :AlternateTestClass do
-      # rubocop:disable RSpec/DescribedClass
+      table do |t|
-      include StringAttributeCleaner.for(:string_attribute)
+        t.string :string_attribute
-      # rubocop:enable RSpec/DescribedClass
+      end
-    end
-  end
 
-  with_model :AlternateTestClass do
+      model do
-    table do |t|
+        # rubocop:disable RSpec/DescribedClass
-      t.string :string_attribute
+        include StringAttributeCleaner.nullify_blanks_for(:string_attribute, on: :before_save)
+        # rubocop:enable RSpec/DescribedClass
+      end
     end
 
-    model do
+    context "when specifying an :on callback" do
-      # rubocop:disable RSpec/DescribedClass
+      it "registers that callback on the model", :aggregate_failures do
-      include StringAttributeCleaner.for(:string_attribute, on: :before_save)
+        before_validation_cbs = AlternateTestClass._validation_callbacks.filter_map do |cb|
-      # rubocop:enable RSpec/DescribedClass
+          cb.filter if cb.kind == :before
-    end
+        end
-  end
 
-  context "when specifying an :on callback" do
+        before_save_cbs = AlternateTestClass._save_callbacks.filter_map do |cb|
-    it "registers that callback on the model", :aggregate_failures do
+          cb.filter if cb.kind == :before
-      before_validation_cbs = AlternateTestClass._validation_callbacks.filter_map do |cb|
+        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|
+      expect(before_validation_cbs).to eq([:nullify_blank_attributes])
-        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
+    it "replaces empty strings with nil" do
-    before_validation_cbs = TestClass._validation_callbacks.filter_map do |cb|
+      test_instance = TestClass.new(string_attribute: "")
-      cb.filter if cb.kind == :before
+
+      expect { test_instance.validate }
+        .to change(test_instance, :string_attribute).from("").to(nil)
     end
 
-    expect(before_validation_cbs).to eq([:nullify_blank_attributes])
+    it "replaces blank strings with nil" do
-    expect(TestClass.new).to respond_to(:nullify_blank_attributes)
+      test_instance = TestClass.new(string_attribute: " ")
-  end
 
-  it "replaces empty strings with nil" do
+      expect { test_instance.validate }
-    test_instance = TestClass.new(string_attribute: "")
+        .to change(test_instance, :string_attribute).from(" ").to(nil)
+    end
 
-    expect { test_instance.validate }
+    it "leaves non-blank attributes unchanged" do
-      .to change(test_instance, :string_attribute).from("").to(nil)
+      test_instance = TestClass.new(string_attribute: "Test")
-  end
 
-  it "replaces blank strings with nil" do
+      expect { test_instance.validate }.not_to change(test_instance, :string_attribute)
-    test_instance = TestClass.new(string_attribute: " ")
+    end
 
-    expect { test_instance.validate }
+    it "ignores obsolete attributes" do
-      .to change(test_instance, :string_attribute).from(" ").to(nil)
+      TestClass.include(described_class.nullify_blanks_for(:non_existing))
-  end
 
-  it "leaves non-blank attributes unchanged" do
+      expect { TestClass.new.validate }.not_to raise_error
-    test_instance = TestClass.new(string_attribute: "Test")
+    end
 
-    expect { test_instance.validate }.not_to change(test_instance, :string_attribute)
+    it "works with non-AR classes" do
-  end
+      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
+      expect { test_instance.nullify_blank_attributes }
-    TestClass.include(described_class.for(:non_existing))
+        .to change(test_instance, :test).from(" ").to(nil)
-
+    end
-    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)
   end
 end