Add boolean attribute main_image_from_frontmatter to indicate if cove… (#16075)

* Add boolean attribute main_image_from_frontmatter to indicate if cover image was set via frontmatter

* use article model spec for main_image_from_frontmatter test cases

* Add data migration script and spec for main_image_from_frontmatter for articles

* Update app/models/article.rb

Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>

Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>
Co-authored-by: Michael Kohl <me@citizen428.net>

app/models/article.rb

@@ -632,7 +632,7 @@ class Article < ApplicationRecord
     set_tag_list(front_matter["tags"]) if front_matter["tags"].present?
     self.published = front_matter["published"] if %w[true false].include?(front_matter["published"].to_s)
     self.published_at = parse_date(front_matter["date"]) if published
-    self.main_image = determine_image(front_matter)
+    set_main_image(front_matter)
     self.canonical_url = front_matter["canonical_url"] if front_matter["canonical_url"].present?
 
     update_description = front_matter["description"].present? || front_matter["title"].present?
@@ -642,16 +642,14 @@ class Article < ApplicationRecord
     self.collection_id = Collection.find_series(front_matter["series"], user).id if front_matter["series"].present?
   end
 
-  def determine_image(front_matter)
-    # In order to clear out the cover_image, we check for the key in the front_matter.
-    # If the key exists, we use the value from it (a url or `nil`).
-    # Otherwise, we fall back to the main_image on the article.
-    has_cover_image = front_matter.include?("cover_image")
-
-    if has_cover_image && (front_matter["cover_image"].present? || main_image)
-      front_matter["cover_image"]
-    else
-      main_image
+  def set_main_image(front_matter)
+    # At one point, we have set the main_image based on the front matter. Forever will that now dictate the behavior.
+    if main_image_from_frontmatter?
+      self.main_image = front_matter["cover_image"]
+    elsif front_matter.key?("cover_image")
+      # They've chosen the set cover image in the front matter, so we'll proceed with that assumption.
+      self.main_image = front_matter["cover_image"]
+      self.main_image_from_frontmatter = true
     end
   end
 

db/migrate/20220111140539_add_main_image_from_frontmatter_to_articles.rb

@@ -0,0 +1,5 @@
+class AddMainImageFromFrontmatterToArticles < ActiveRecord::Migration[6.1]
+  def change
+    add_column :articles, :main_image_from_frontmatter, :boolean, default: false
+  end
+end

db/schema.rb

@@ -112,6 +112,7 @@ ActiveRecord::Schema.define(version: 2022_01_26_205052) do
     t.datetime "last_experience_level_rating_at"
     t.string "main_image"
     t.string "main_image_background_hex_color", default: "#dddddd"
+    t.boolean "main_image_from_frontmatter", default: false
     t.integer "nth_published_by_author", default: 0
     t.integer "organic_page_views_count", default: 0
     t.integer "organic_page_views_past_month_count", default: 0

lib/data_update_scripts/20220120052641_backfill_column_main_image_from_frontmatter.rb

@@ -0,0 +1,11 @@
+module DataUpdateScripts
+  class BackfillColumnMainImageFromFrontmatter
+    def run
+      Article.where(main_image_from_frontmatter: false).find_each do |article|
+        fixed_body_markdown = MarkdownProcessor::Fixer::FixAll.call(article.body_markdown || "")
+        parsed = FrontMatterParser::Parser.new(:md).call(fixed_body_markdown)
+        article.update!(main_image_from_frontmatter: true) if parsed.front_matter&.key?("cover_image")
+      end
+    end
+  end
+end

spec/factories/articles.rb

@@ -34,6 +34,7 @@ FactoryBot.define do
         date: #{date if with_date}
         series: #{with_collection&.slug if with_collection}
         canonical_url: #{canonical_url if with_canonical_url}
+        #{"cover_image: #{Faker::Avatar.image}" if with_main_image && main_image_from_frontmatter}
         ---
 
         #{Faker::Hipster.paragraph(sentence_count: 2)}

spec/lib/data_update_scripts/backfill_column_main_image_from_frontmatter_spec.rb

@@ -0,0 +1,26 @@
+require "rails_helper"
+require Rails.root.join(
+  "lib/data_update_scripts/20220120052641_backfill_column_main_image_from_frontmatter.rb",
+)
+
+describe DataUpdateScripts::BackfillColumnMainImageFromFrontmatter do
+  let(:article_without_image) { create(:article, with_main_image: false) }
+  let(:article_main_image) { create(:article, with_main_image: true) }
+  let(:article_markdown_image) do
+    create(:article,
+           body_markdown: "---\ntitle: hey hey hahuu\npublished: false\ncover_image: \n---\nYo ho ho#{rand(100)}")
+  end
+
+  before do
+    article_without_image
+    article_main_image
+    article_markdown_image
+  end
+
+  it "set main_image_from_frontmatter to true only for articles with cover_image in body_markdown" do
+    described_class.new.run
+    expect(article_without_image.reload.main_image_from_frontmatter).to be false
+    expect(article_main_image.reload.main_image_from_frontmatter).to be false
+    expect(article_markdown_image.reload.main_image_from_frontmatter).to be true
+  end
+end

spec/models/article_spec.rb

@@ -712,6 +712,43 @@ RSpec.describe Article, type: :model do
     end
   end
 
+  describe "#main_image_from_frontmatter" do
+    let(:article) { create(:article, user: user, main_image_from_frontmatter: false) }
+
+    it "set to true if markdown has cover_image" do
+      article = create(
+        :article,
+        user: user,
+        body_markdown: "---\ntitle: hey\npublished: false\ncover_image: #{Faker::Avatar.image}\n---\nYo",
+      )
+      expect(article.main_image_from_frontmatter).to eq(true)
+    end
+
+    context "when false" do
+      it "does not remove main image if cover image not passed in markdown" do
+        expect(article.main_image).not_to be_nil
+        article.update! body_markdown: "---\ntitle: hey\npublished: false\n---\nYo ho ho#{rand(100)}"
+        expect(article.reload.main_image).not_to be_nil
+      end
+
+      it "does remove main image if cover image is passed empty in markdown" do
+        expect(article.main_image).not_to be_nil
+        article.update! body_markdown: "---\ntitle: hey\npublished: false\ncover_image: \n---\nYo ho ho#{rand(100)}"
+        expect(article.reload.main_image).to be_nil
+      end
+    end
+
+    context "when true" do
+      let(:article) { create(:article, main_image_from_frontmatter: true, user: user) }
+
+      it "removes main image when cover_image not provided" do
+        expect(article.main_image).not_to be_nil
+        article.update! body_markdown: "---\ntitle: hey\npublished: false\n---\nYo ho ho#{rand(100)}"
+        expect(article.reload.main_image).to be_nil
+      end
+    end
+  end
+
   describe ".active_help" do
     it "returns properly filtered articles under the 'help' tag" do
       filtered_article = create(:article, user: user, tags: "help",