[deploy] Add co_author_ids to Articles (#10339)

* Replaces second_user_id and third_user_id with co_author_ids on the Articles table

* Adjusts migration that adds co_author_ids to also add array: true

* Replaces second_user_id and third_user_id with co_author_ids in all applicable files
  - Use co_author_ids in _individual_article view
  - Use co_author_ids in articles and stories controllers
  - Use co_author_ids in article and podcast_episode models
  - Use co_author_ids in article_dashboard
  - Use co_author_ids in comments_helper
  - Use co_author_ids in articles_spec

* Replaces second_user_id and third_user_id with co_author_ids in article.rb model

* Remove unused #assign_co_authors method from the Stories::Controller

* Add #co_authors and #co_author_name_and_path to article_decorator.rb

* Adds a descriptive form field for co_author_ids for Admins in /admin/articles

* Adds a conditional in article/show.html.erb to render co_author names on an Article in a human-readable way

* Replaces get_co_author_name_and_path with proper method name, co_author_name_and_path in show.html.erb

* Adds latest schema to (hopefully) resolve Travis conflicts

* Adds the safe operator to show.html.erb

* Adjusts articles_spec.rb to use an array in the first test

* Replaces unless &.empty? with if ./present? in articles/show.html.erb

* Replaces second_user_id with co_author_ids in stories_show_spec.rb test

* Replaces elsif with else in articles/show.html.erb

* Cleans up show.html.erb by removing conditional in favor of decorator method

* Refactors splitting of params in Articles::Controller and optimizes query in #co_authors

* Reverts removal of second_user_id and third_user_id and migration file

* Adds a data_update script to update co_author_ids with existing second and third user_ids

* Adds validations to co_authors and flash_messages to indicate whether an update was successful or not
  - Adds to methods to article.rb to validate the IDs of co_authors and authors
  - Adds flash messages to the Admin::Articles::Controller and a redirect to the show page
  - Removes the JS highlighting upon submit when updating an Article in Admin
  - Refactors #update action in Admin::Articles::Controller

* Adds tests to article_spec.rb around co_author_id validations in article.rb

* Adjusts #validate_co_authors_must_not_be_the_same to use .include? instead

* Uses Field::Select.with_options in article_dashboard.rb to properly display co_author_ids

* Reverts removal of assigning co_author_ids in the Stories::Controller

* Adjusts error message and adjusts return logic in article.rb (thanks, Fernando!)

* Fixes failing article_spec.rb test that checks for co_author_ids uniqueness

* Adds a default array to co_author_ids and checks if they are .blank?

* Refactor data_update script to use a single SQL statement (thank you, Michael)

* Preserve array order of co_author_ids in article_decorator.rb

* Add db file for default: []

* Add validation to fix bug related to text inputs and invalid users when adding co_authors

* Adds tests to ensure that co_author_ids are both an integer and an integer > 0

* Updates admin/articles_spec.rb to default [] instead of nil

* Adjusts validations in article.rb to be DRY-er and more consistent

* Consolidates validations further

* Refactors validations in article.rb to use procs

* Refactors data_update script to remove nil values from array
This commit is contained in:
Julianna Tetreault 2020-10-02 09:06:11 -06:00 committed by GitHub
parent 4a9f27163e
commit c87974b7ac
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 109 additions and 45 deletions

View file

@ -45,10 +45,13 @@ module Admin
article.boosted_additional_articles = article_params[:boosted_additional_articles].to_s == "true"
article.boosted_dev_digest_email = article_params[:boosted_dev_digest_email].to_s == "true"
article.user_id = article_params[:user_id].to_i
article.second_user_id = article_params[:second_user_id].to_i
article.third_user_id = article_params[:third_user_id].to_i
article.update!(article_params)
render body: nil
article.co_author_ids = article_params[:co_author_ids].split(",").map(&:strip)
if article.save
flash[:success] = "Article saved!"
else
flash[:danger] = article.errors_as_sentence
end
redirect_to admin_article_path(article.id)
end
private
@ -131,8 +134,7 @@ module Admin
main_image_background_hex_color
featured_number
user_id
second_user_id
third_user_id
co_author_ids
last_buffered]
params.require(:article).permit(allowed_params)
end

View file

@ -294,8 +294,8 @@ class StoriesController < ApplicationController
end
@comments_to_show_count = @article.cached_tag_list_array.include?("discuss") ? 50 : 30
assign_second_and_third_user
set_article_json_ld
assign_co_authors
@comment = Comment.new(body_markdown: @article&.comment_template)
end
@ -303,11 +303,10 @@ class StoriesController < ApplicationController
!@article.published && params[:preview] != @article.password
end
def assign_second_and_third_user
return if @article.second_user_id.blank?
def assign_co_authors
return if @article.co_author_ids.blank?
@second_user = User.find(@article.second_user_id)
@third_user = User.find(@article.third_user_id) if @article.third_user_id.present?
@co_author_ids = User.find(@article.co_author_ids)
end
def assign_user_comments

View file

@ -10,8 +10,7 @@ class ArticleDashboard < Administrate::BaseDashboard
ATTRIBUTE_TYPES = {
user: Field::BelongsTo,
user_id: UserIdField,
second_user_id: Field::Number,
third_user_id: Field::Number,
co_author_ids: Field::Select.with_options(collection: []),
organization: Field::BelongsTo,
id: Field::Number,
title: Field::String,
@ -65,8 +64,6 @@ class ArticleDashboard < Administrate::BaseDashboard
# on the model's form (`new` and `edit`) pages.
FORM_ATTRIBUTES = %i[
user_id
second_user_id
third_user_id
organization
title
search_optimized_title_preamble

View file

@ -86,4 +86,14 @@ class ArticleDecorator < ApplicationDecorator
def long_markdown?
body_markdown.present? && body_markdown.size > LONG_MARKDOWN_THRESHOLD
end
def co_authors
User.select(:name, :username).where(id: co_author_ids).order(created_at: :asc)
end
def co_author_name_and_path
co_authors.map do |user|
"<b><a href=\"#{user.path}\">#{user.name}</a></b>"
end.to_sentence
end
end

View file

@ -15,8 +15,7 @@ module CommentsHelper
commentable &&
[
commentable.user_id,
commentable.second_user_id,
commentable.third_user_id,
commentable.co_author_ids,
].any? { |id| id == comment.user_id }
end

View file

@ -79,6 +79,9 @@ class Article < ApplicationRecord
validate :validate_collection_permission
validate :validate_tag
validate :validate_video
validate :validate_co_authors, unless: -> { co_author_ids.blank? }
validate :validate_co_authors_must_not_be_the_same, unless: -> { co_author_ids.blank? }
validate :validate_co_authors_exist, unless: -> { co_author_ids.blank? }
before_validation :evaluate_markdown, :create_slug
before_save :update_cached_user
@ -150,7 +153,7 @@ class Article < ApplicationRecord
:video_thumbnail_url, :video_closed_caption_track_url, :social_image,
:published_from_feed, :crossposted_at, :published_at, :featured_number,
:last_buffered, :facebook_last_buffered, :created_at, :body_markdown,
:email_digest_eligible, :processed_html, :second_user_id, :third_user_id)
:email_digest_eligible, :processed_html, :co_author_ids)
}
scope :boosted_via_additional_articles, lambda {
@ -558,6 +561,24 @@ class Article < ApplicationRecord
errors.add(:collection_id, "must be one you have permission to post to")
end
def validate_co_authors
return if co_author_ids.exclude?(user_id)
errors.add(:co_author_ids, "must not be the same user as the author")
end
def validate_co_authors_must_not_be_the_same
return if co_author_ids.uniq.count == co_author_ids.count
errors.add(:base, "co-author IDs must be unique")
end
def validate_co_authors_exist
return if User.where(id: co_author_ids).count == co_author_ids.count
errors.add(:co_author_ids, "must be valid user IDs")
end
def past_or_present_date
return unless published_at && published_at > Time.current

View file

@ -91,8 +91,7 @@ class PodcastEpisode < ApplicationRecord
nil
end
alias user_id nil_method
alias second_user_id nil_method
alias third_user_id nil_method
alias co_author_ids nil_method
private

View file

@ -68,7 +68,7 @@
<img src="<%= cloud_cover_url(article.main_image) %>" style="width:100%;max-width:380px;border-radius:8px;background:<%= article.main_image_background_hex_color %>;" alt="cover image" /><br />
</div>
<% end %>
<%= form_with url: admin_article_path(article.id), html: { data: { action: "submit->article#highlightElement" } } do |f| %>
<%= form_with url: admin_article_path(article.id), local: true do |f| %>
<div class="form-group">
<input name="utf8" type="hidden" value="✓">
<input type="hidden" name="authenticity_token" value="<%= form_authenticity_token %>" />
@ -86,14 +86,9 @@
value="<%= article.user_id %>">
</div>
<div class="form-group col">
<label for="second_author_id_<%= article.id %>">Second Author ID:</label>
<input id="second_author_id_<%= article.id %>" class="form-control" size="6" name="article[second_user_id]"
value="<%= article.second_user_id %>">
</div>
<div class="form-group col">
<label for="third_author_id_<%= article.id %>">Third Author ID:</label>
<input id="third_author_id_<%= article.id %>" class="form-control" size="6" name="article[third_user_id]"
value="<%= article.third_user_id %>">
<label for="co_author_ids_<%= article.id %>">Co-Author IDs (comma separated):</label>
<input id="co_author_ids_<%= article.id %>" class="form-control" size="6" name="article[co_author_ids]"
value="<%= article.co_author_ids&.join(", ") %>">
</div>
</div>
<div class="row">

View file

@ -155,11 +155,8 @@
<% if @article.published_timestamp.present? %>
<%= local_date(@article.published_timestamp, show_year: @article.displayable_published_at.year != Time.current.year) %>
<% end %>
<% if @second_user.present? %>
<em>with <b><a href="<%= @second_user.path %>"><%= @second_user.name %></a></b></em>
<% end %>
<% if @third_user.present? %>
<em> and <b><a href="<%= @third_user.path %>"><%= @third_user.name %></a></b></em>
<% if @article.co_author_ids.present? %>
<em>with <%= @article.co_author_name_and_path.html_safe %></em>
<% end %>
<% if should_show_updated_on?(@article) %>

View file

@ -0,0 +1,5 @@
class AddCoAuthorIdsToArticles < ActiveRecord::Migration[6.0]
def change
add_column :articles, :co_author_ids, :bigint, array: true, default: []
end
end

View file

@ -90,6 +90,7 @@ ActiveRecord::Schema.define(version: 2020_10_02_102303) do
t.string "cached_user_name"
t.string "cached_user_username"
t.string "canonical_url"
t.bigint "co_author_ids", default: [], array: true
t.bigint "collection_id"
t.integer "comment_score", default: 0
t.string "comment_template"

View file

@ -0,0 +1,11 @@
module DataUpdateScripts
class BackfillCoAuthorIdsForArticles
def run
articles = Article.where.not(second_user_id: nil).or(Article.where.not(third_user_id: nil))
articles.find_each do |article|
co_author_ids = [article.second_user_id, article.third_user_id].compact
article.update_columns(co_author_ids: co_author_ids)
end
end
end
end

View file

@ -62,6 +62,39 @@ RSpec.describe Article, type: :model do
end
end
describe "#validate co_authors" do
it "is invalid if the co_author is the same as the author" do
article.co_author_ids = [user.id]
expect(article).not_to be_valid
end
it "is invalid if there are duplicate co_authors for the same article" do
co_author1 = create(:user)
article.co_author_ids = [co_author1, co_author1]
expect(article).not_to be_valid
end
it "is invalid if the co_author is entered as a text value rather than an integer" do
article.co_author_ids = [user.id, "abc"]
expect(article).not_to be_valid
end
it "is invalid if the co_author ID is not greater than 0" do
article.co_author_ids = [user.id, 0]
expect(article).not_to be_valid
end
it "is valid if co_author_ids is nil" do
article.co_author_ids = nil
expect(article).to be_valid
end
end
describe "#after_commit" do
it "on update enqueues job to index article to elasticsearch" do
article.save

View file

@ -18,19 +18,14 @@ RSpec.describe "/admin/articles", type: :request do
get request
expect do
article.update_columns(second_user_id: second_user.id)
end.to change(article, :second_user_id).from(nil).to(second_user.id)
article.update_columns(co_author_ids: [1])
end.to change(article, :co_author_ids).from([]).to([1])
end
it "allows an Admin to add multiple co-authors to an individual article" do
article.update_columns(second_user_id: second_user.id, third_user_id: third_user.id)
it "allows an Admin to add co-authors to an individual article" do
article.update_columns(co_author_ids: [2, 3])
get request
article.reload
expect(article.second_user_id).to eq(second_user.id)
expect(article.third_user_id).to eq(third_user.id)
expect(article.co_author_ids).to eq([2, 3])
end
end
end

View file

@ -103,7 +103,7 @@ RSpec.describe "StoriesShow", type: :request do
it "renders second and third users if present" do
# 3rd user doesn't seem to get rendered for some reason
user2 = create(:user)
article.update(second_user_id: user2.id)
article.update(co_author_ids: [user2.id])
get article.path
expect(response.body).to include "<em>with <b><a href=\"#{user2.path}\">"
end