maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix collection order with cross posted articles (#6768) [deploy]

Browse source
This commit is contained in:
rhymes 2020-03-23 16:05:32 +01:00 • committed by GitHub
parent 99da2a0863
commit ff739bb8f8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 113 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
app/controllers/stories_controller.rb
View file

@ -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?

39
app/views/articles/_collection.html.erb
View file

@ -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 %>
<div class="article-collection-wrapper article-collection-wrapper-<%= position %>">
<% if @collection.slug.present? %>
<p><%= @collection.slug %> (<%= @collection_size %> Part Series)</p>
<% if collection.slug.present? %>
<p><%= collection.slug %> (<%= collection_size %> Part Series)</p>
<% else %>
<p><%= @collection_size %> Part Series</p>
<p><%= collection_size %> Part Series</p>
<% end %>
<div class="article-collection">
<% @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 %>
<a
href="<%= article.path if article.published %>"
class="<%= "current-article" if @collection.articles.published.order("published_at ASC").pluck(:slug)[2..@collection_size - 3].include?(@article.slug) %> 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 %>
</a>
<% end %>
<a
href="<%= article.path if article.published %>"
class="<%= collection_link_class(@article, article) %> <%= "collection-link-hidden" if @collection_size > 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 %>
</a>
<a
href="<%= article.path %>"
class="<%= collection_link_class(rendered_article, article) %> <%= "collection-link-hidden" if collection_size > 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 %>
</a>
<% end %>
</div>
</div>

16
app/views/articles/show.html.erb
View file

@ -172,14 +172,22 @@
</div>
<% end %>
</header>
<% 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 %>
<div class="body" data-article-id="<%= @article.id %>" id="article-body" itemprop="articleBody">
<%= @article.processed_html.html_safe %>
</div>
<% 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" %>
</article>

69
spec/system/articles/user_visits_an_article_spec.rb
View file

@ -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