diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index 3158d45e6..f15edbe79 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -240,12 +240,28 @@ class StoriesController < ApplicationController end def assign_article_show_variables + not_found if permission_denied? + not_found unless @article.user + @article_show = true @variant_number = params[:variant_version] || (user_signed_in? ? 0 : rand(2)) - assign_user_and_org + + @user = @article.user + @organization = @article.organization + + if @article.collection + @collection = @article.collection + + # we need to make sure that articles that were cross posted after their + # original publication date appear in the correct order in the collection, + # considering non cross posted articles with a more recent publication date + @collection_articles = @article.collection.articles. + published. + order("COALESCE(crossposted_at, published_at) ASC") + end + @comments_to_show_count = @article.cached_tag_list_array.include?("discuss") ? 50 : 30 assign_second_and_third_user - not_found if permission_denied? @comment = Comment.new(body_markdown: @article&.comment_template) end @@ -253,11 +269,6 @@ class StoriesController < ApplicationController !@article.published && params[:preview] != @article.password end - def assign_user_and_org - @user = @article.user || not_found - @organization = @article.organization if @article.organization_id.present? - end - def assign_second_and_third_user return if @article.second_user_id.blank? diff --git a/app/views/articles/_collection.html.erb b/app/views/articles/_collection.html.erb index 49af04c03..0c7d79d01 100644 --- a/app/views/articles/_collection.html.erb +++ b/app/views/articles/_collection.html.erb @@ -1,33 +1,34 @@ -<% @collection = @article.collection %> -<% @collection_size = @collection.articles.published.size %> -<% if @collection.present? && @collection_size > 1 %> +<% collection_size = articles.size %> +<% slugs = articles.pluck(:slug) %> + +<% if collection && collection_size > 1 %>
- <% if @collection.slug.present? %> -

<%= @collection.slug %> (<%= @collection_size %> Part Series)

+ <% if collection.slug.present? %> +

<%= collection.slug %> (<%= collection_size %> Part Series)

<% else %> -

<%= @collection_size %> Part Series

+

<%= collection_size %> Part Series

<% end %>
- <% @collection.articles.published.order("published_at ASC").each_with_index do |article, i| %> - <% if @collection_size > 5 && i == 2 %> + <% articles.each_with_index do |article, i| %> + <% if collection_size > 5 && i == 2 %> collection-link-inbetween" + href="<%= article.path %>" + class="<%= "current-article" if slugs[2..collection_size - 3].include?(rendered_article.slug) %> collection-link-inbetween" data-preload-image="<%= cloud_cover_url(article.main_image) %>" id="collection-link-inbetween" data-no-instant title="View more"> - 3 ... <%= @collection_size - 2 %> + 3 ... <%= collection_size - 2 %> - <% end %> - 5 && (i > 1 && i < @collection_size - 2) %>" - data-preload-image="<%= cloud_cover_url(article.main_image) %>" - title="Published <%= article.readable_publish_date %>"> - <%= i + 1 %>) <%= article.title %> - + + 5 && (i > 1 && i < collection_size - 2) %>" + data-preload-image="<%= cloud_cover_url(article.main_image) %>" + title="Published <%= article.readable_publish_date %>"> + <%= i + 1 %>) <%= article.title %> + <% end %>
diff --git a/app/views/articles/show.html.erb b/app/views/articles/show.html.erb index 6c3676def..a539735c7 100644 --- a/app/views/articles/show.html.erb +++ b/app/views/articles/show.html.erb @@ -172,14 +172,22 @@ <% end %> - <% if @article.collection_id %> - <%= render "articles/collection", position: "top" %> + <% if @collection %> + <%= render "articles/collection", + rendered_article: @article, + collection: @collection, + articles: @collection_articles, + position: :top %> <% end %>
<%= @article.processed_html.html_safe %>
- <% if @article.body_markdown && @article.body_markdown.size > 900 && @article.collection_id %> - <%= render "articles/collection", position: "bottom" %> + <% if @article.body_markdown && @article.body_markdown.size > 900 && @collection %> + <%= render "articles/collection", + rendered_article: @article, + collection: @collection, + articles: @collection_articles, + position: :bottom %> <% end %> <%= render "articles/actions" %> diff --git a/spec/system/articles/user_visits_an_article_spec.rb b/spec/system/articles/user_visits_an_article_spec.rb index 70d533faf..0e348e731 100644 --- a/spec/system/articles/user_visits_an_article_spec.rb +++ b/spec/system/articles/user_visits_an_article_spec.rb @@ -2,21 +2,24 @@ require "rails_helper" RSpec.describe "Views an article", type: :system do let_it_be(:user) { create(:user) } - let_it_be(:article, reload: true) { create(:article, :with_notification_subscription, user: user) } + let_it_be_changeable(:article) do + create(:article, :with_notification_subscription, user: user) + end let(:timestamp) { "2019-03-04T10:00:00Z" } before do sign_in user - visit "/#{user.username}/#{article.slug}" end it "shows an article" do + visit article.path expect(page).to have_content(article.title) end it "shows comments", js: true do create_list(:comment, 3, commentable: article) - visit "/#{user.username}/#{article.slug}" + + visit article.path expect(page).to have_selector(".single-comment-node", visible: true, count: 3) end @@ -24,19 +27,73 @@ RSpec.describe "Views an article", type: :system do expect { visit("/#{user.username}/#{article.slug}/mod") }.to raise_error(Pundit::NotAuthorizedError) end - context "when showing the date" do + describe "when showing the date" do before do - article.update_column(:published_at, Time.zone.parse(timestamp)) - visit "/#{user.username}/#{article.slug}" + article.update_columns(published_at: Time.zone.parse(timestamp)) end it "shows the readable publish date", js: true do + visit article.path expect(page).to have_selector("article time", text: "Mar 4") end it "embeds the published timestamp" do + visit article.path + selector = "article time[datetime='#{timestamp}']" expect(page).to have_selector(selector) end end + + describe "when articles belong to a collection" do + let_it_be_readonly(:collection) { create(:collection) } + let(:articles_selector) { "//div[@class='article-collection']//a" } + + context "with regular articles" do + it "lists the articles in ascending published_at order" do + articles = create_list(:article, 2) + articles.each { |a| a.update_columns(collection_id: collection.id) } + + visit articles.first.path + + elements = page.all(:xpath, articles_selector) + paths = elements.map { |e| e[:href] } + expect(paths).to eq([articles.first.path, articles.second.path]) + end + end + + context "when a crossposted article is between two regular articles" do + # rubocop:disable RSpec/ExampleLength + it "lists the articles in ascending order considering crossposted_at" do + article1 = create(:article) + crossposted_article = create(:article) + article2 = create(:article) + + article1.update_columns( + collection_id: collection.id, + published_at: Time.zone.parse("2020-03-15T13:50:09Z"), + ) + + crossposted_article.update_columns( + canonical_url: Faker::Internet.url, + collection_id: collection.id, + crossposted_at: Time.zone.parse("2020-03-21T10:25:00Z"), + feed_source_url: Faker::Internet.url, + published_at: Time.zone.parse("2020-02-21T06:00:00Z"), + published_from_feed: true, + ) + + article2.update_columns(collection_id: collection.id) + + visit article1.path + + expected_paths = [article1.path, crossposted_article.path, article2.path] + + elements = page.all(:xpath, articles_selector) + paths = elements.map { |e| e[:href] } + expect(paths).to eq(expected_paths) + end + # rubocop:enable RSpec/ExampleLength + end + end end