From 3fd01c326d86802233dad528dd822a35318fc74d Mon Sep 17 00:00:00 2001 From: Takuma Date: Wed, 22 Jul 2020 23:34:55 +0900 Subject: [PATCH] Bug Fix: Article Posted Date and Author Card Posted Date do not match (#9272) * Refactor: move has_long_markdown from view to helper, and change it from variable to method * Fix: Article Posted Date and Author Card Posted Date do not match #8789 * Add test case for Article Posted Date and Author Card Posted Date do not match #8789 * Use our custom URL helper module instead of manually constructing URL * Move long_markdown? from ArticlesHelper to ArticleDecorator * Change method name from has_long_markdown? to long_markdown? * Add test suites for long_markdown? method to ArticleDecorator spec * Use << over += * Fix typo * Remove unnecessary cache --- app/decorators/article_decorator.rb | 6 +++++ app/views/articles/_about_author.html.erb | 10 +++++---- app/views/articles/show.html.erb | 10 ++++----- spec/decorators/article_decorator_spec.rb | 18 +++++++++++++++ .../articles/user_visits_an_article_spec.rb | 22 +++++++++++++++++++ 5 files changed, 56 insertions(+), 10 deletions(-) diff --git a/app/decorators/article_decorator.rb b/app/decorators/article_decorator.rb index e8b838ff0..8ddd60cca 100644 --- a/app/decorators/article_decorator.rb +++ b/app/decorators/article_decorator.rb @@ -1,4 +1,6 @@ class ArticleDecorator < ApplicationDecorator + LONG_MARKDOWN_THRESHOLD = 900 + def current_state_path published ? "/#{username}/#{slug}" : "/#{username}/#{slug}?preview=#{password}" end @@ -80,4 +82,8 @@ class ArticleDecorator < ApplicationDecorator video_closed_caption_track_url: video_closed_caption_track_url } end + + def long_markdown? + body_markdown.present? && body_markdown.size > LONG_MARKDOWN_THRESHOLD + end end diff --git a/app/views/articles/_about_author.html.erb b/app/views/articles/_about_author.html.erb index 348acfe58..ec2fa37c0 100644 --- a/app/views/articles/_about_author.html.erb +++ b/app/views/articles/_about_author.html.erb @@ -1,14 +1,16 @@
-

Posted on <%= published %> by:

+

+ Posted on by: +

- + <%= @user.username %> profile
-

<%= @user.name %>

+

<%= @user.name %>

- + @<%= @user.username %>

diff --git a/app/views/articles/show.html.erb b/app/views/articles/show.html.erb index ec6d54f17..e0d50620c 100644 --- a/app/views/articles/show.html.erb +++ b/app/views/articles/show.html.erb @@ -85,8 +85,6 @@ <% end %> <% end %> -<% has_long_markdown = @article.body_markdown && @article.body_markdown.size > 900 %> -
- <% if has_long_markdown && @collection %> + <% if @article.long_markdown? && @collection %> <%= render "articles/collection", rendered_article: @article, collection: @collection, @@ -203,12 +201,12 @@ <% end %> - <% if has_long_markdown || @organization %> + <% if @article.long_markdown? || @organization %>
- <% if has_long_markdown %> + <% if @article.long_markdown? %> <%= render "articles/about_author", published: @article.readable_publish_date %> <% end %> - <%= render "articles/org_branding", organization: @organization, author: has_long_markdown %> + <%= render "articles/org_branding", organization: @organization, author: @article.long_markdown? %>
<% end %> diff --git a/spec/decorators/article_decorator_spec.rb b/spec/decorators/article_decorator_spec.rb index 00a636c68..daaa4f2c5 100644 --- a/spec/decorators/article_decorator_spec.rb +++ b/spec/decorators/article_decorator_spec.rb @@ -203,4 +203,22 @@ RSpec.describe ArticleDecorator, type: :decorator do ) end end + + describe "#long_markdown?" do + it "returns false if body_markdown is nil" do + article.body_markdown = nil + expect(article.decorate.long_markdown?).to eq false + end + + it "returns false if body_markdown has fewer characters than LONG_MARKDOWN_THRESHOLD" do + article.body_markdown = "---\ntitle: Title\n---\n\nHey this is the article" + expect(article.decorate.long_markdown?).to eq false + end + + it "returns true if body_markdown has more characters than LONG_MARKDOWN_THRESHOLD" do + additional_characters_length = (ArticleDecorator::LONG_MARKDOWN_THRESHOLD + 1) - article.body_markdown.length + article.body_markdown << Faker::Hipster.paragraph_by_chars(characters: additional_characters_length) + expect(article.decorate.long_markdown?).to eq true + end + end end diff --git a/spec/system/articles/user_visits_an_article_spec.rb b/spec/system/articles/user_visits_an_article_spec.rb index 623ce4148..4dab7fcf9 100644 --- a/spec/system/articles/user_visits_an_article_spec.rb +++ b/spec/system/articles/user_visits_an_article_spec.rb @@ -42,6 +42,28 @@ RSpec.describe "Views an article", type: :system do selector = "article time[datetime='#{timestamp}']" expect(page).to have_selector(selector) end + + context "when articles have long markdowns and different published dates" do + let(:first_article) { build(:article, published_at: "2019-03-04T10:00:00Z") } + let(:second_article) { build(:article, published_at: "2019-03-05T10:00:00Z") } + + before do + [first_article, second_article].each do |article| + additional_characters_length = (ArticleDecorator::LONG_MARKDOWN_THRESHOLD + 1) - article.body_markdown.length + article.body_markdown << Faker::Hipster.paragraph_by_chars(characters: additional_characters_length) + article.save! + end + end + + it "shows the identical readable publish dates in each page", js: true do + visit first_article.path + expect(page).to have_selector("article time", text: "Mar 4") + expect(page).to have_selector(".crayons-card--secondary time", text: "Mar 4") + visit second_article.path + expect(page).to have_selector("article time", text: "Mar 5") + expect(page).to have_selector(".crayons-card--secondary time", text: "Mar 5") + end + end end describe "when articles belong to a collection" do