Allow an article's main_image to be removed (#5687) [deploy]

Currently, we don't allow users to remove the `main_image` of an article once it has been set. It can be changed, but never removed.
This change allows the `main_image` to be removed by checking for the `cover_image` key in the frontmatter that is sent along in the article's markdown. This means that a user should be able to remove the image associated with an article by removing the value of `cover_image:` from their frontmatter.

app/models/article.rb

@@ -525,7 +525,7 @@ class Article < ApplicationRecord
     end
     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 = front_matter["cover_image"] if front_matter["cover_image"].present?
+    self.main_image = determine_image(front_matter)
     self.canonical_url = front_matter["canonical_url"] if front_matter["canonical_url"].present?
     self.description = front_matter["description"] if front_matter["description"].present? || front_matter["title"].present? # Do this if frontmatte exists at all
     self.collection_id = nil if front_matter["title"].present?
@@ -533,6 +533,19 @@ class Article < ApplicationRecord
     self.automatically_renew = front_matter["automatically_renew"] if front_matter["automatically_renew"].present? && tag_list.include?("hiring")
   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
+    end
+  end
+
   def parse_date(date)
     # once published_at exist, it can not be adjusted
     published_at || date || Time.current

spec/factories/articles.rb

@@ -9,6 +9,7 @@ FactoryBot.define do
       tags { Faker::Hipster.words(number: 4).join(", ") }
       canonical_url { Faker::Internet.url }
       with_canonical_url { false }
+      with_main_image { true }
       with_date { false }
       with_tags { true }
       with_hr_issue { false }
@@ -18,7 +19,7 @@ FactoryBot.define do
     end
     association :user, factory: :user, strategy: :create
     description { Faker::Hipster.paragraph(sentence_count: 1)[0..100] }
-    main_image    { Faker::Avatar.image }
+    main_image    { with_main_image ? Faker::Avatar.image : nil }
     language { "en" }
     experience_level_rating { rand(4..6) }
     body_markdown do

spec/fixtures/files/article_published_empty_cover_image.txt

@@ -0,0 +1,9 @@
+---
+title: Sample Article
+published: true
+description: this is a sample article
+tags: test
+cover_image:
+---
+
+Suspendisse ac lobortis velit, a feugiat sapien. Aenean condimentum, nulla at fermentum sagittis, tellus nisi suscipit velit, vel sollicitudin odio ligula a odio. Integer eget efficitur massa, in sodales velit. Nunc fermentum consequat scelerisque. Morbi elementum tristique faucibus. Nulla vel lectus non justo euismod varius. Vivamus id nisl sit amet odio tincidunt fringilla. Pellentesque odio odio, vulputate in risus eu, lacinia porttitor lorem. Nunc posuere tempus est, imperdiet suscipit odio maximus id. Nam eget feugiat magna.

spec/models/article_spec.rb

@@ -263,6 +263,52 @@ RSpec.describe Article, type: :model do
         expect(test_article.body_text).to eq(sanitized_html)
       end
     end
+
+    context "when a main_image does not already exist" do
+      let!(:article_without_main_image) { build(:article, with_main_image: false) }
+      let(:image) { Faker::Avatar.image }
+
+      before { article_without_main_image.validate }
+
+      it "can parse the main_image" do
+        expect(article_without_main_image.main_image).to eq(nil)
+      end
+
+      it "can parse the main_image when added" do
+        article_without_main_image.main_image = image
+        article_without_main_image.validate
+
+        expect(article_without_main_image.main_image).to eq(image)
+      end
+    end
+
+    context "when a main_image exists" do
+      # The `with_main_image` flag is the factory default, but we're being explicit here.
+      let!(:article_with_main_image) { build(:article, with_main_image: true) }
+      let(:image) { article_with_main_image.main_image }
+
+      before { article_with_main_image.validate }
+
+      it "can parse the main_image" do
+        expect(article_with_main_image.main_image).to eq(image)
+      end
+
+      it "can parse the main_image when removed" do
+        article_with_main_image.main_image = nil
+        article_with_main_image.validate
+
+        expect(article_with_main_image.main_image).to eq(nil)
+      end
+
+      it "can parse the main_image when changed" do
+        expect(article_with_main_image.main_image).to eq(image)
+
+        other_image = Faker::Avatar.image
+        article_with_main_image.main_image = other_image
+        article_with_main_image.validate
+        expect(article_with_main_image.main_image).to eq(other_image)
+      end
+    end
   end
 
   describe "#class_name" do

spec/requests/api/v0/articles_spec.rb

@@ -806,6 +806,34 @@ RSpec.describe "Api::V0::Articles", type: :request do
         expect(article.body_markdown).to eq(body_markdown)
       end
 
+      it "updates the main_image to be empty if given an empty cover_image" do
+        image = Faker::Avatar.image
+        article.update(main_image: image)
+        expect(article.main_image).to eq(image)
+
+        body_markdown = file_fixture("article_published_empty_cover_image.txt").read
+        put_article(
+          title: Faker::Book.title,
+          body_markdown: body_markdown,
+        )
+        expect(response).to have_http_status(:ok)
+        expect(article.reload.main_image).to eq(nil)
+      end
+
+      it "updates the main_image to be empty if given a different cover_image" do
+        image = Faker::Avatar.image
+        article.update(main_image: image)
+        expect(article.main_image).to eq(image)
+
+        body_markdown = file_fixture("article_published_cover_image.txt").read
+        put_article(
+          title: Faker::Book.title,
+          body_markdown: body_markdown,
+        )
+        expect(response).to have_http_status(:ok)
+        expect(article.reload.main_image).to eq("https://dummyimage.com/100x100")
+      end
+
       it "updates the tags" do
         expect do
           put_article(