diff --git a/app/models/article.rb b/app/models/article.rb index ec479cd3f..8801d9d13 100644 --- a/app/models/article.rb +++ b/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 diff --git a/spec/factories/articles.rb b/spec/factories/articles.rb index bc0696e08..af2040c3e 100644 --- a/spec/factories/articles.rb +++ b/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 diff --git a/spec/fixtures/files/article_published_empty_cover_image.txt b/spec/fixtures/files/article_published_empty_cover_image.txt new file mode 100644 index 000000000..362ed7e58 --- /dev/null +++ b/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. diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index f12445f6b..1d0a49f31 100644 --- a/spec/models/article_spec.rb +++ b/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 diff --git a/spec/requests/api/v0/articles_spec.rb b/spec/requests/api/v0/articles_spec.rb index 24cc161ef..b8a2a21e4 100644 --- a/spec/requests/api/v0/articles_spec.rb +++ b/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(