Break MarkdownFixer into MarkdownProcessor::Fixer services (#12241)

* Create new MarkdownProcessor::Fixer services

* Remove old MarkdownFixer

* Code cleanup

* Capitalize Base in code comments

* Remove comments related to inheritance

* Add fix_methods method to hold METHDOS constant

app/controllers/articles_controller.rb

@@ -85,7 +85,7 @@ class ArticlesController < ApplicationController
     authorize Article
 
     begin
-      fixed_body_markdown = MarkdownFixer.fix_for_preview(params[:article_body])
+      fixed_body_markdown = MarkdownProcessor::Fixer::FixForPreview.call(params[:article_body])
       parsed = FrontMatterParser::Parser.new(:md).call(fixed_body_markdown)
       parsed_markdown = MarkdownProcessor::Parser.new(parsed.content, source: Article.new, user: current_user)
       processed_html = parsed_markdown.finalize

app/controllers/comments_controller.rb

@@ -203,7 +203,7 @@ class CommentsController < ApplicationController
     skip_authorization
     begin
       permitted_body_markdown = permitted_attributes(Comment)[:body_markdown]
-      fixed_body_markdown = MarkdownFixer.fix_for_preview(permitted_body_markdown)
+      fixed_body_markdown = MarkdownProcessor::Fixer::FixForPreview.call(permitted_body_markdown)
       parsed_markdown = MarkdownProcessor::Parser.new(fixed_body_markdown, source: Comment.new, user: current_user)
       processed_html = parsed_markdown.finalize
     rescue StandardError => e

app/labor/markdown_fixer.rb

@@ -1,102 +0,0 @@
-class MarkdownFixer
-  FRONT_MATTER_DETECTOR = /-{3}.*?-{3}/m.freeze
-
-  class << self
-    def fix_all(markdown)
-      methods = %i[
-        add_quotes_to_title add_quotes_to_description lowercase_published
-        modify_hr_tags convert_new_lines split_tags underscores_in_usernames
-      ]
-      methods.reduce(markdown) { |acc, elem| public_send(elem, acc) }
-    end
-
-    def fix_for_preview(markdown)
-      methods = %i[add_quotes_to_title add_quotes_to_description modify_hr_tags underscores_in_usernames]
-      methods.reduce(markdown) { |acc, elem| public_send(elem, acc) }
-    end
-
-    def fix_for_comment(markdown)
-      methods = %I[modify_hr_tags underscores_in_usernames]
-      methods.reduce(markdown) { |acc, elem| public_send(elem, acc) }
-    end
-
-    def add_quotes_to_title(markdown)
-      add_quotes_to_section(markdown, section: "title")
-    end
-
-    def add_quotes_to_description(markdown)
-      add_quotes_to_section(markdown, section: "description")
-    end
-
-    # This turns --- into ------- after the first two,
-    # because --- messes with front matter
-    def modify_hr_tags(markdown)
-      markdown.gsub(/-{3}.*?-{3}/m) do |front_matter|
-        front_matter.gsub(/^---/).with_index { |match, i| i > 1 ? "#{match}-----" : match }
-      end
-    end
-
-    def lowercase_published(markdown)
-      markdown.gsub(/-{3}.*?-{3}/m) do |front_matter|
-        front_matter.gsub(/^published: /i, "published: ")
-      end
-    end
-
-    def convert_new_lines(markdown)
-      markdown.gsub("\r\n", "\n")
-    end
-
-    def split_tags(markdown)
-      markdown.gsub(/\ntags:.*\n/) do |tags|
-        tags.split(" #").join(",").delete("#").gsub(":,", ": ")
-      end
-    end
-
-    def underscores_in_usernames(markdown)
-      return markdown unless markdown.match?(USERNAME_WITH_UNDERSCORE_REGEXP)
-
-      traverser = MarkdownTraverser.new(markdown)
-      traverser.each do |line|
-        next if traverser.in_codeblock?
-
-        escape_underscored_username_in_line!(line)
-      end.join
-    end
-
-    private
-
-    # Match @_username_ that is not preceded by backtick
-    USERNAME_WITH_UNDERSCORE_REGEXP = /(?<!`)@_\w+_/.freeze
-
-    # Escapes underscored username that is not in code
-    def escape_underscored_username_in_line!(line)
-      line.scan(USERNAME_WITH_UNDERSCORE_REGEXP).each do |to_escape|
-        line.sub!(to_escape, to_escape.gsub("_", "\\_"))
-      end
-      line
-    end
-
-    def add_quotes_to_section(markdown, section:)
-      # Only add quotes to front matter, or text between triple dashes
-      markdown.sub(FRONT_MATTER_DETECTOR) do |front_matter|
-        front_matter.gsub(/#{section}: ?(?<content>.*?)(\r\n|\n)/m) do |target|
-          # `content` is the captured group (.*?)
-          captured_text = Regexp.last_match("content")
-          # The query below checks if the whole text is wrapped in
-          # either single or double quotes.
-          match = captured_text.scan(/(^".*"$|^'.*'$)/)
-          if match.empty?
-            # Double quotes that aren't already escaped will get esacped.
-            # Then the whole text get warped in double quotes.
-            parsed_text = captured_text.gsub(/(?<!\\)"/, "\\\"")
-            "#{section}: \"#{parsed_text}\"\n"
-          else
-            # if the text comes pre-warped in either single or double quotes,
-            # no more processing is done
-            target
-          end
-        end
-      end
-    end
-  end
-end

app/models/article.rb

@@ -296,7 +296,7 @@ class Article < ApplicationRecord
   end
 
   def has_frontmatter?
-    fixed_body_markdown = MarkdownFixer.fix_all(body_markdown)
+    fixed_body_markdown = MarkdownProcessor::Fixer::FixAll.call(body_markdown)
     begin
       parsed = FrontMatterParser::Parser.new(:md).call(fixed_body_markdown)
       parsed.front_matter["title"].present?
@@ -414,7 +414,7 @@ class Article < ApplicationRecord
   end
 
   def evaluate_markdown
-    fixed_body_markdown = MarkdownFixer.fix_all(body_markdown || "")
+    fixed_body_markdown = MarkdownProcessor::Fixer::FixAll.call(body_markdown || "")
     parsed = FrontMatterParser::Parser.new(:md).call(fixed_body_markdown)
     parsed_markdown = MarkdownProcessor::Parser.new(parsed.content, source: self, user: user)
     self.reading_time = parsed_markdown.calculate_reading_time

app/models/comment.rb

@@ -162,7 +162,7 @@ class Comment < ApplicationRecord
   end
 
   def evaluate_markdown
-    fixed_body_markdown = MarkdownFixer.fix_for_comment(body_markdown)
+    fixed_body_markdown = MarkdownProcessor::Fixer::FixForComment.call(body_markdown)
     parsed_markdown = MarkdownProcessor::Parser.new(fixed_body_markdown, source: self, user: user)
     self.processed_html = parsed_markdown.finalize(link_attributes: { rel: "nofollow" })
     wrap_timestamps_if_video_present! if commentable

app/services/markdown_processor/fixer/base.rb

@@ -0,0 +1,99 @@
+module MarkdownProcessor
+  module Fixer
+    # Services here in the Fixer module should inherit from this base class.
+    # #call is implemented here so other services only need to define a
+    # METHODS constant that is an Array of symbols referencing other methods
+    # found here.
+    #
+    # For example
+    # METHODS = %i[add_quotes_to_tile add_quotes_to_description]
+    class Base
+      FRONT_MATTER_DETECTOR = /-{3}.*?-{3}/m.freeze
+
+      def self.call(markdown)
+        return unless markdown
+
+        fix_methods.reduce(markdown) { |acc, elem| public_send(elem, acc) }
+      end
+
+      def self.fix_methods
+        self::METHODS
+      end
+
+      def self.add_quotes_to_title(markdown)
+        add_quotes_to_section(markdown, section: "title")
+      end
+
+      def self.add_quotes_to_description(markdown)
+        add_quotes_to_section(markdown, section: "description")
+      end
+
+      # This turns --- into ------- after the first two,
+      # because --- messes with front matter
+      def self.modify_hr_tags(markdown)
+        markdown.gsub(/^---/).with_index { |match, i| i > 1 ? "#{match}-----" : match }
+      end
+
+      def self.lowercase_published(markdown)
+        markdown.gsub(/-{3}.*?-{3}/m) do |front_matter|
+          front_matter.gsub(/^published: /i, "published: ")
+        end
+      end
+
+      def self.convert_new_lines(markdown)
+        markdown.gsub("\r\n", "\n")
+      end
+
+      def self.split_tags(markdown)
+        markdown.gsub(/\ntags:.*\n/) do |tags|
+          tags.split(" #").join(",").delete("#").gsub(":,", ": ")
+        end
+      end
+
+      def self.underscores_in_usernames(markdown)
+        return markdown unless markdown.match?(USERNAME_WITH_UNDERSCORE_REGEXP)
+
+        traverser = MarkdownTraverser.new(markdown)
+        traverser.each do |line|
+          next if traverser.in_codeblock?
+
+          escape_underscored_username_in_line!(line)
+        end.join
+      end
+
+      # Match @_username_ that is not preceded by backtick
+      USERNAME_WITH_UNDERSCORE_REGEXP = /(?<!`)@_\w+_/.freeze
+
+      # Escapes underscored username that is not in code
+      def self.escape_underscored_username_in_line!(line)
+        line.scan(USERNAME_WITH_UNDERSCORE_REGEXP).each do |to_escape|
+          line.sub!(to_escape, to_escape.gsub("_", "\\_"))
+        end
+        line
+      end
+
+      def self.add_quotes_to_section(markdown, section:)
+        # Only add quotes to front matter, or text between triple dashes
+        markdown.sub(FRONT_MATTER_DETECTOR) do |front_matter|
+          front_matter.gsub(/#{section}: ?(?<content>.*?)(\r\n|\n)/m) do |target|
+            # `content` is the captured group (.*?)
+            captured_text = Regexp.last_match("content")
+            # The query below checks if the whole text is wrapped in
+            # either single or double quotes.
+            match = captured_text.scan(/(^".*"$|^'.*'$)/)
+            if match.empty?
+              # Double quotes that aren't already escaped will get esacped.
+              # Then the whole text get warped in double quotes.
+              parsed_text = captured_text.gsub(/(?<!\\)"/, "\\\"")
+              "#{section}: \"#{parsed_text}\"\n"
+            else
+              # if the text comes pre-warped in either single or double quotes,
+              # no more processing is done
+              target
+            end
+          end
+        end
+      end
+    end
+  end
+end

app/services/markdown_processor/fixer/fix_all.rb

@@ -0,0 +1,15 @@
+module MarkdownProcessor
+  module Fixer
+    class FixAll < Base
+      METHODS = %i[
+        add_quotes_to_title
+        add_quotes_to_description
+        lowercase_published
+        modify_hr_tags
+        convert_new_lines
+        split_tags
+        underscores_in_usernames
+      ].freeze
+    end
+  end
+end

app/services/markdown_processor/fixer/fix_for_comment.rb

@@ -0,0 +1,10 @@
+module MarkdownProcessor
+  module Fixer
+    class FixForComment < Base
+      METHODS = %i[
+        modify_hr_tags
+        underscores_in_usernames
+      ].freeze
+    end
+  end
+end

app/services/markdown_processor/fixer/fix_for_preview.rb

@@ -0,0 +1,12 @@
+module MarkdownProcessor
+  module Fixer
+    class FixForPreview < Base
+      METHODS = %i[
+        add_quotes_to_title
+        add_quotes_to_description
+        modify_hr_tags
+        underscores_in_usernames
+      ].freeze
+    end
+  end
+end

spec/services/markdown_processor/fixer/base_spec.rb

@@ -1,6 +1,6 @@
 require "rails_helper"
 
-RSpec.describe MarkdownFixer, type: :labor do
+RSpec.describe MarkdownProcessor::Fixer::Base, type: :service do
   let(:sample_text) { Faker::Book.title }
 
   def front_matter(title: "", description: "")
@@ -109,33 +109,6 @@ RSpec.describe MarkdownFixer, type: :labor do
     end
   end
 
-  describe "::fix_all" do
-    it "escapes title and description" do
-      result = described_class
-        .fix_all(front_matter(title: sample_text, description: sample_text))
-      expected_result = front_matter(title: %("#{sample_text}"), description: %("#{sample_text}"))
-      expect(result).to eq(expected_result)
-    end
-
-    context "when description is empty" do
-      it "escapes title and description" do
-        result = described_class
-          .fix_all("---\ntitle: #{sample_text}\ndescription:\ntags: \n---\n")
-        expected_result = "---\ntitle: \"#{sample_text}\"\ndescription: \"\"\ntags: \n---\n"
-        expect(result).to eq(expected_result)
-      end
-    end
-  end
-
-  describe "::fix_for_preview" do
-    it "escapes title and description" do
-      result = described_class
-        .fix_for_preview(front_matter(title: sample_text, description: sample_text))
-      expected_result = front_matter(title: %("#{sample_text}"), description: %("#{sample_text}"))
-      expect(result).to eq(expected_result)
-    end
-  end
-
   describe "::underscores_in_usernames" do
     it "escapes underscores in a username" do
       test_string1 = "@_xy_"
@@ -170,4 +143,20 @@ RSpec.describe MarkdownFixer, type: :labor do
       expect(result).to include("@_no_escape_codeblock", "@_no_escape_code")
     end
   end
+
+  describe "::modify_hr_tags" do
+    it "modifies hr tags" do
+      markdown =
+        <<~HEREDOC
+          #{front_matter(title: sample_text)}
+          ---
+          These hr tags should be converted to more dashes
+          ---
+        HEREDOC
+
+      result = described_class.modify_hr_tags(markdown)
+      expect(result).to include("-------").twice
+      expect(result).to include("---").at_least(:twice)
+    end
+  end
 end

spec/services/markdown_processor/fixer/fix_all_spec.rb

@@ -0,0 +1,50 @@
+require "rails_helper"
+
+RSpec.describe MarkdownProcessor::Fixer::FixAll, type: :service do
+  let(:sample_text) { Faker::Book.title }
+
+  def front_matter(title: "", description: "")
+    <<~HEREDOC
+      ---
+      title: #{title}
+      published: false
+      description: #{description}
+      ---
+    HEREDOC
+  end
+
+  describe "defining constants" do
+    it "defines METHODS" do
+      methods = %i[
+        add_quotes_to_title add_quotes_to_description lowercase_published
+        modify_hr_tags convert_new_lines split_tags underscores_in_usernames
+      ]
+
+      expect(described_class::METHODS).to eq methods
+    end
+  end
+
+  describe "#call" do
+    it "escapes title and description" do
+      markdown = front_matter(title: sample_text, description: sample_text)
+      result = described_class.call(markdown)
+      expected_result = front_matter(title: %("#{sample_text}"), description: %("#{sample_text}"))
+      expect(result).to eq(expected_result)
+    end
+
+    context "when description is empty" do
+      it "escapes title and description" do
+        markdown = "---\ntitle: #{sample_text}\ndescription:\ntags: \n---\n"
+        result = described_class.call(markdown)
+        expected_result = "---\ntitle: \"#{sample_text}\"\ndescription: \"\"\ntags: \n---\n"
+        expect(result).to eq(expected_result)
+      end
+    end
+
+    context "when markdown is nil" do
+      it "doesn't raise an error" do
+        expect { described_class.call(nil) }.not_to raise_error
+      end
+    end
+  end
+end

spec/services/markdown_processor/fixer/fix_for_comment_spec.rb

@@ -0,0 +1,54 @@
+require "rails_helper"
+
+RSpec.describe MarkdownProcessor::Fixer::FixForComment, type: :service do
+  let(:sample_text) { Faker::Book.title }
+
+  def front_matter(title: "", description: "")
+    <<~HEREDOC
+      ---
+      title: #{title}
+      published: false
+      description: #{description}
+      ---
+    HEREDOC
+  end
+
+  describe "defining constants" do
+    it "defines METHODS" do
+      methods = %i[modify_hr_tags underscores_in_usernames]
+      expect(described_class::METHODS).to eq methods
+    end
+  end
+
+  describe "#call" do
+    it "escapes underscores in a username" do
+      test_string1 = "@_xy_"
+      expected_result1 = "@\\_xy\\_"
+      test_string2 = "@_x_y_"
+      expected_result2 = "@\\_x\\_y\\_"
+
+      expect(described_class.call(test_string1)).to eq(expected_result1)
+      expect(described_class.call(test_string2)).to eq(expected_result2)
+    end
+
+    it "modifies hr tags" do
+      markdown =
+        <<~HEREDOC
+          #{front_matter(title: sample_text)}
+          ---
+          These hr tags should be converted to more dashes
+          ---
+        HEREDOC
+
+      result = described_class.call(markdown)
+      expect(result).to include("-------").twice
+      expect(result).to include("---").at_least(:twice)
+    end
+
+    context "when markdown is nil" do
+      it "doesn't raise an error" do
+        expect { described_class.call(nil) }.not_to raise_error
+      end
+    end
+  end
+end

spec/services/markdown_processor/fixer/fix_for_preview_spec.rb

@@ -0,0 +1,37 @@
+require "rails_helper"
+
+RSpec.describe MarkdownProcessor::Fixer::FixForPreview, type: :service do
+  let(:sample_text) { Faker::Book.title }
+
+  def front_matter(title: "", description: "")
+    <<~HEREDOC
+      ---
+      title: #{title}
+      published: false
+      description: #{description}
+      ---
+    HEREDOC
+  end
+
+  describe "defining constants" do
+    it "defines METHODS" do
+      methods = %i[add_quotes_to_title add_quotes_to_description modify_hr_tags underscores_in_usernames]
+      expect(described_class::METHODS).to eq methods
+    end
+  end
+
+  describe "#call" do
+    it "escapes title and description" do
+      markdown = front_matter(title: sample_text, description: sample_text)
+      result = described_class.call(markdown)
+      expected_result = front_matter(title: %("#{sample_text}"), description: %("#{sample_text}"))
+      expect(result).to eq(expected_result)
+    end
+
+    context "when markdown is nil" do
+      it "doesn't raise an error" do
+        expect { described_class.call(nil) }.not_to raise_error
+      end
+    end
+  end
+end