From b02d43ca2d113cabca5b278ed5572a29e3133d97 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 10 Jun 2021 11:04:33 -0400 Subject: [PATCH] Create DiscussionLocks (#13905) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Create DiscussionLocks * Fix specs * Update nullify_blank_notes_and_reason * Update before_validation call * Updated DiscussionLockPolicy for clarity * Move permitted_attributes to a constant * Update route * Apply suggestions from code review for frontend Co-authored-by: Suzanne Aitchison * Add title tags * Wrap unlock confirm in main element * Wrap flash messages up in div * Actually fix title tags * Hide comment reply button when discussion is locked * Add E2E tests * Try to fix E2E tests * Cypress...you work locally but not in CI...why!? * PR feedback * Update E2E tests * More E2E updates 😭 Co-authored-by: Suzanne Aitchison --- app/controllers/articles_controller.rb | 19 ++++ .../discussion_locks_controller.rb | 49 ++++++++ app/controllers/profile_pins_controller.rb | 4 +- app/controllers/stories_controller.rb | 1 + app/models/article.rb | 2 + app/models/comment.rb | 7 ++ app/models/discussion_lock.rb | 17 +++ app/models/user.rb | 1 + app/policies/article_policy.rb | 8 ++ app/policies/discussion_lock_policy.rb | 21 ++++ .../articles/_full_comment_area.html.erb | 9 +- .../articles/discussion_lock_confirm.html.erb | 35 ++++++ .../discussion_unlock_confirm.html.erb | 39 +++++++ app/views/articles/manage.html.erb | 14 ++- app/views/comments/_comment_footer.html.erb | 32 +++--- .../comments/_discussion_lock_reason.html.erb | 10 ++ .../dashboards/_dashboard_article.html.erb | 6 + config/routes.rb | 6 +- .../manageFlows/lockDiscussion.spec.js | 107 ++++++++++++++++++ .../manageFlows/unlockDiscussion.spec.js | 88 ++++++++++++++ .../20210531152321_create_discussion_locks.rb | 12 ++ db/schema.rb | 13 +++ spec/factories/articles.rb | 4 + spec/factories/discussion_locks.rb | 9 ++ spec/models/article_spec.rb | 2 + spec/models/comment_spec.rb | 6 + spec/models/discussion_lock_spec.rb | 18 +++ spec/models/user_spec.rb | 1 + spec/policies/discussion_lock_policy_spec.rb | 51 +++++++++ spec/requests/discussion_locks_spec.rb | 67 +++++++++++ .../articles/user_discussion_locks_spec.rb | 61 ++++++++++ 31 files changed, 694 insertions(+), 25 deletions(-) create mode 100644 app/controllers/discussion_locks_controller.rb create mode 100644 app/models/discussion_lock.rb create mode 100644 app/policies/discussion_lock_policy.rb create mode 100644 app/views/articles/discussion_lock_confirm.html.erb create mode 100644 app/views/articles/discussion_unlock_confirm.html.erb create mode 100644 app/views/comments/_discussion_lock_reason.html.erb create mode 100644 cypress/integration/articleFlows/manageFlows/lockDiscussion.spec.js create mode 100644 cypress/integration/articleFlows/manageFlows/unlockDiscussion.spec.js create mode 100644 db/migrate/20210531152321_create_discussion_locks.rb create mode 100644 spec/factories/discussion_locks.rb create mode 100644 spec/models/discussion_lock_spec.rb create mode 100644 spec/policies/discussion_lock_policy_spec.rb create mode 100644 spec/requests/discussion_locks_spec.rb create mode 100644 spec/system/articles/user_discussion_locks_spec.rb diff --git a/app/controllers/articles_controller.rb b/app/controllers/articles_controller.rb index 5afbe5ca3..e3208b088 100644 --- a/app/controllers/articles_controller.rb +++ b/app/controllers/articles_controller.rb @@ -77,6 +77,7 @@ class ArticlesController < ApplicationController authorize @article @article = @article.decorate + @discussion_lock = @article.discussion_lock @user = @article.user @rating_vote = RatingVote.where(article_id: @article.id, user_id: @user.id).first @organizations = @user&.organizations @@ -195,6 +196,24 @@ class ArticlesController < ApplicationController end end + def discussion_lock_confirm + # This allows admins to also use this action vs searching only in the current_user.articles scope + @article = Article.find_by(slug: params[:slug]) + not_found unless @article + authorize @article + + @discussion_lock = DiscussionLock.new + end + + def discussion_unlock_confirm + # This allows admins to also use this action vs searching only in the current_user.articles scope + @article = Article.find_by(slug: params[:slug]) + not_found unless @article + authorize @article + + @discussion_lock = @article.discussion_lock + end + private def base_editor_assigments diff --git a/app/controllers/discussion_locks_controller.rb b/app/controllers/discussion_locks_controller.rb new file mode 100644 index 000000000..8842bb9a1 --- /dev/null +++ b/app/controllers/discussion_locks_controller.rb @@ -0,0 +1,49 @@ +class DiscussionLocksController < ApplicationController + before_action :authenticate_user! + + DISCUSSION_LOCK_PARAMS = %i[article_id notes reason].freeze + + def create + @discussion_lock = DiscussionLock.new(discussion_lock_params) + + authorize @discussion_lock + article = Article.find(discussion_lock_params[:article_id]) + + if @discussion_lock.save + bust_article_cache(article) + + flash[:success] = "Discussion was successfully locked!" + else + flash[:error] = "Error: #{@discussion_lock.errors_as_sentence}" + end + + redirect_to "#{article.path}/manage" + end + + def destroy + discussion_lock = DiscussionLock.find(params[:id]) + + authorize discussion_lock + article = discussion_lock.article + + if discussion_lock.destroy + bust_article_cache(article) + + flash[:success] = "Discussion was successfully unlocked!" + else + flash[:error] = "Error: #{discussion_lock.errors_as_sentence}" + end + + redirect_to "#{article.path}/manage" + end + + private + + def discussion_lock_params + params.require(:discussion_lock).permit(DISCUSSION_LOCK_PARAMS).merge(locking_user_id: current_user.id) + end + + def bust_article_cache(article) + EdgeCache::BustArticle.call(article) + end +end diff --git a/app/controllers/profile_pins_controller.rb b/app/controllers/profile_pins_controller.rb index e3a68e183..17da3cdee 100644 --- a/app/controllers/profile_pins_controller.rb +++ b/app/controllers/profile_pins_controller.rb @@ -8,9 +8,9 @@ class ProfilePinsController < ApplicationController @profile_pin.pinnable_id = profile_pin_params[:pinnable_id].to_i @profile_pin.pinnable_type = "Article" if @profile_pin.save - flash[:pins_success] = "📌 Pinned! (pinned posts display chronologically, 5 max)" + flash[:success] = "📌 Pinned! (pinned posts display chronologically, 5 max)" else - flash[:pins_error] = "You can only have five pins" + flash[:error] = "You can only have five pins" end redirect_back(fallback_location: "/dashboard") bust_user_profile diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index 59c77941d..0bcaa6d76 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -259,6 +259,7 @@ class StoriesController < ApplicationController @article_show = true + @discussion_lock = @article.discussion_lock @user = @article.user @organization = @article.organization diff --git a/app/models/article.rb b/app/models/article.rb index ef41dc3a9..c5173917f 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -28,6 +28,8 @@ class Article < ApplicationRecord # The date that we began limiting the number of user mentions in an article. MAX_USER_MENTION_LIVE_AT = Time.utc(2021, 4, 7).freeze + has_one :discussion_lock, dependent: :destroy + has_many :mentions, as: :mentionable, inverse_of: :mentionable, dependent: :destroy has_many :comments, as: :commentable, inverse_of: :commentable, dependent: :nullify has_many :html_variant_successes, dependent: :nullify diff --git a/app/models/comment.rb b/app/models/comment.rb index 19c1a990d..f165b5bba 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -39,6 +39,7 @@ class Comment < ApplicationRecord after_save :synchronous_bust after_save :bust_cache + validate :discussion_not_locked, if: :commentable, on: :create validate :published_article, if: :commentable validate :user_mentions_in_markdown validates :body_markdown, presence: true, length: { in: BODY_MARKDOWN_SIZE_RANGE } @@ -316,6 +317,12 @@ class Comment < ApplicationRecord self.markdown_character_count = body_markdown.size end + def discussion_not_locked + return unless commentable_type == "Article" && commentable.discussion_lock + + errors.add(:commentable_id, "the discussion is locked on this Post") + end + def published_article errors.add(:commentable_id, "is not valid.") if commentable_type == "Article" && !commentable.published end diff --git a/app/models/discussion_lock.rb b/app/models/discussion_lock.rb new file mode 100644 index 000000000..26fe6c207 --- /dev/null +++ b/app/models/discussion_lock.rb @@ -0,0 +1,17 @@ +class DiscussionLock < ApplicationRecord + belongs_to :article + belongs_to :locking_user, class_name: "User" + + before_validation :nullify_blank_notes_and_reason + + validates :article_id, presence: true, uniqueness: true + validates :locking_user_id, presence: true + + private + + def nullify_blank_notes_and_reason + # Prevent blank strings from beings saved to the DB + self.notes = nil if notes.blank? + self.reason = nil if reason.blank? + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 4708d6825..301751b73 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -180,6 +180,7 @@ class User < ApplicationRecord has_many :comments, dependent: :destroy has_many :created_podcasts, class_name: "Podcast", foreign_key: :creator_id, inverse_of: :creator, dependent: :nullify has_many :credits, dependent: :destroy + has_many :discussion_locks, dependent: :destroy, inverse_of: :locking_user, foreign_key: :locking_user_id has_many :display_ad_events, dependent: :destroy has_many :email_authorizations, dependent: :delete_all has_many :email_messages, class_name: "Ahoy::Message", dependent: :destroy diff --git a/app/policies/article_policy.rb b/app/policies/article_policy.rb index cf1c9ced3..0ddb9e76b 100644 --- a/app/policies/article_policy.rb +++ b/app/policies/article_policy.rb @@ -19,6 +19,14 @@ class ArticlePolicy < ApplicationPolicy update? end + def discussion_lock_confirm? + update? + end + + def discussion_unlock_confirm? + update? + end + def destroy? update? end diff --git a/app/policies/discussion_lock_policy.rb b/app/policies/discussion_lock_policy.rb new file mode 100644 index 000000000..df03b1fe3 --- /dev/null +++ b/app/policies/discussion_lock_policy.rb @@ -0,0 +1,21 @@ +class DiscussionLockPolicy < ApplicationPolicy + PERMITTED_ATTRIBUTES = %i[article_id notes reason].freeze + + def create? + (user_author? || minimal_admin?) && !user_suspended? + end + + def destroy? + create? + end + + def permitted_attributes + PERMITTED_ATTRIBUTES + end + + private + + def user_author? + record.locking_user_id == user.id + end +end diff --git a/app/views/articles/_full_comment_area.html.erb b/app/views/articles/_full_comment_area.html.erb index d863eafeb..6d5c6bcc4 100644 --- a/app/views/articles/_full_comment_area.html.erb +++ b/app/views/articles/_full_comment_area.html.erb @@ -1,4 +1,4 @@ -<% cache("whole-comment-area-#{@article.id}-#{@article.last_comment_at}-#{@article.show_comments}", expires_in: 2.hours) do %> +<% cache("whole-comment-area-#{@article.id}-#{@article.last_comment_at}-#{@article.show_comments}-#{@discussion_lock&.updated_at}", expires_in: 2.hours) do %>
<% if @article.show_comments %>
@@ -16,7 +16,12 @@ data-commentable-id="<%= @article.id %>" data-commentable-type="Article" data-has-recent-comment-activity="<%= @article.has_recent_comment_activity? %>"> - <%= render "/comments/form", commentable: @article, commentable_type: "Article" %> + + <% if @discussion_lock %> + <%= render "/comments/discussion_lock_reason" %> + <% else %> + <%= render "/comments/form", commentable: @article, commentable_type: "Article" %> + <% end %>
<% if @article.comments_count > 0 %> diff --git a/app/views/articles/discussion_lock_confirm.html.erb b/app/views/articles/discussion_lock_confirm.html.erb new file mode 100644 index 000000000..cf92eff38 --- /dev/null +++ b/app/views/articles/discussion_lock_confirm.html.erb @@ -0,0 +1,35 @@ +<% title("Confirm Discussion Lock") %> +
+
+ <%= @article.title %> +
+
+

Are you sure you want to lock the discussion on this post?

+

+ Users will not be able to add new comments. This will not remove + existing comments. You can unlock the discussion at any time to allow users to leave comments + again. +

+ + <%= form_for @discussion_lock, method: :post do |f| %> +
+ <%= f.hidden_field :article_id, value: @article.id %> +
+ <%= f.label :reason, "Reason for locking the discussion (optional) - this will be publicly displayed", class: "crayons-field__label" %> + <%= f.text_field :reason, + placeholder: "The reason why you're locking this discussion.", + required: false, + class: "crayons-textfield" %> +
+ <%= f.label :notes, "Notes (optional) - this is only visible to you and admins", class: "crayons-field__label" %> + <%= f.text_field :notes, + placeholder: "More details as to why you locked the discussion.", + required: false, + class: "crayons-textfield" %> +
+
+ <%= f.submit "Lock discussion", class: "crayons-btn crayons-btn--danger" %> + Cancel + <% end %> +
+
diff --git a/app/views/articles/discussion_unlock_confirm.html.erb b/app/views/articles/discussion_unlock_confirm.html.erb new file mode 100644 index 000000000..ef9430d91 --- /dev/null +++ b/app/views/articles/discussion_unlock_confirm.html.erb @@ -0,0 +1,39 @@ +<% title("Confirm Discussion Unlock") %> +
+
+ <%= @article.title %> +
+
+

Are you sure you want to unlock the discussion on this post?

+

+ Users will be able to add new comments. You can lock the discussion at + any time to prevent users from leaving comments again. +

+ +
+
+ Reason this discussion was locked (publicly visible): +
+ + <%= @discussion_lock.reason %> + +
+ +
+
+ Notes from when this discussion was locked (only visible to you and admins): +
+ + <%= @discussion_lock.notes %> + +
+ + <%= form_for @discussion_lock, method: :delete do |f| %> +
+ <%= f.hidden_field :article_id, value: @article.id %> +
+ <%= f.submit "Unlock discussion", class: "crayons-btn crayons-btn--danger" %> + Cancel + <% end %> +
+
diff --git a/app/views/articles/manage.html.erb b/app/views/articles/manage.html.erb index 568af57da..200cf2a79 100644 --- a/app/views/articles/manage.html.erb +++ b/app/views/articles/manage.html.erb @@ -5,11 +5,15 @@
Dashboard 👉 Manage Your Post
- <% if flash[:pins_error] %> -

Uh Oh! <%= flash[:pins_error] %> 😬

+ <% if flash[:error] %> +
+

Uh Oh! <%= flash[:error] %> 😬

+
<% end %> - <% if flash[:pins_success] %> -

<%= flash[:pins_success] %>

+ <% if flash[:success] %> +
+

<%= flash[:success] %>

+
<% end %>

Tools:

Experience Level of Post: Is your post written more for a beginner or an advanced audience? Adjust this level as a gentle indicator which will help show the post to the people who will benefit the most. @@ -29,7 +33,7 @@

- <%= render "dashboards/dashboard_article", article: @article, organization: @article.organization, org_admin: @user.org_admin?(@article.organization), manage_view: true %> + <%= render "dashboards/dashboard_article", article: @article, discussion_lock: @discussion_lock, organization: @article.organization, org_admin: @user.org_admin?(@article.organization), manage_view: true %>
<%= form_for(RatingVote.new) do |f| %>

Experience Level of Post

diff --git a/app/views/comments/_comment_footer.html.erb b/app/views/comments/_comment_footer.html.erb index e4e3ace4e..4a1a9d3a1 100644 --- a/app/views/comments/_comment_footer.html.erb +++ b/app/views/comments/_comment_footer.html.erb @@ -14,21 +14,23 @@ - <% if comment.depth < 2 || is_childless %> - - <%= inline_svg_tag("small-comment.svg", aria: true, class: "crayons-icon reaction-icon not-reacted", title: "Comment button") %> - - - <% else %> - - <%= inline_svg_tag("small-thread.svg", aria: true, class: "crayons-icon", title: "Thread") %> - Thread - + <% unless @discussion_lock %> + <% if (comment.depth < 2 || is_childless) %> + + <%= inline_svg_tag("small-comment.svg", aria: true, class: "crayons-icon reaction-icon not-reacted", title: "Comment button") %> + + + <% else %> + + <%= inline_svg_tag("small-thread.svg", aria: true, class: "crayons-icon", title: "Thread") %> + Thread + + <% end %> <% end %> diff --git a/app/views/comments/_discussion_lock_reason.html.erb b/app/views/comments/_discussion_lock_reason.html.erb new file mode 100644 index 000000000..e93249e25 --- /dev/null +++ b/app/views/comments/_discussion_lock_reason.html.erb @@ -0,0 +1,10 @@ +
+ The discussion has been locked. New comments can't be added. + <% if @discussion_lock&.reason %> +
+ + <%= @discussion_lock.reason %> + +
+ <% end %> +
diff --git a/app/views/dashboards/_dashboard_article.html.erb b/app/views/dashboards/_dashboard_article.html.erb index 966141418..7953027dd 100644 --- a/app/views/dashboards/_dashboard_article.html.erb +++ b/app/views/dashboards/_dashboard_article.html.erb @@ -67,6 +67,12 @@ Stats + <% if discussion_lock %> + Unlock discussion + <% else %> + Lock discussion + <% end %> + <% if article.published %> diff --git a/config/routes.rb b/config/routes.rb index 138f2c277..ed3f349c2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -203,6 +203,8 @@ Rails.application.routes.draw do resources :liquid_tags, only: %i[index], defaults: { format: :json } + resources :discussion_locks, only: %i[create destroy] + get "/verify_email_ownership", to: "email_authorizations#verify", as: :verify_email_authorizations get "/search/tags", to: "search#tags" get "/search/chat_channels", to: "search#chat_channels" @@ -421,9 +423,11 @@ Rails.application.routes.draw do constraints: { view: /moderate/ } get "/:username/:slug/mod", to: "moderations#article" get "/:username/:slug/actions_panel", to: "moderations#actions_panel" - get "/:username/:slug/manage", to: "articles#manage" + get "/:username/:slug/manage", to: "articles#manage", as: :article_manage get "/:username/:slug/edit", to: "articles#edit" get "/:username/:slug/delete_confirm", to: "articles#delete_confirm" + get "/:username/:slug/discussion_lock_confirm", to: "articles#discussion_lock_confirm" + get "/:username/:slug/discussion_unlock_confirm", to: "articles#discussion_unlock_confirm" get "/:username/:slug/stats", to: "articles#stats" get "/:username/:view", to: "stories#index", constraints: { view: /comments|moderate|admin/ } diff --git a/cypress/integration/articleFlows/manageFlows/lockDiscussion.spec.js b/cypress/integration/articleFlows/manageFlows/lockDiscussion.spec.js new file mode 100644 index 000000000..ccf44afa3 --- /dev/null +++ b/cypress/integration/articleFlows/manageFlows/lockDiscussion.spec.js @@ -0,0 +1,107 @@ +describe('Lock discussion', () => { + const getDiscussionLockButton = () => + cy.findByRole('link', { + name: 'Lock discussion', + }); + + const getDiscussionUnlockButton = () => + cy.findByRole('link', { + name: 'Unlock discussion', + }); + + const getDiscussionLockSubmitButton = () => + cy.findByRole('button', { + name: 'Lock discussion', + }); + + const exampleReason = 'Discussion lock example reason!'; + const exampleNotes = 'Discussion lock example notes!'; + + describe('locking the discussion', () => { + beforeEach(() => { + cy.testSetup(); + cy.fixture('users/articleEditorV1User.json').as('user'); + + cy.get('@user').then((user) => { + cy.loginUser(user).then(() => { + cy.createArticle({ + title: 'Test Article', + tags: ['beginner', 'ruby', 'go'], + content: `This is a test article's contents.`, + published: true, + }).then((response) => { + cy.visit(`${response.body.current_state_path}/manage`); + }); + }); + }); + }); + + it('should allow a user to lock a discussion', () => { + getDiscussionLockButton().click(); + + cy.findByRole('heading', { + name: 'Are you sure you want to lock the discussion on this post?', + }).should('exist'); + + getDiscussionLockSubmitButton().should('exist').click(); + getDiscussionUnlockButton().should('exist'); + }); + + it('should allow a user to supply a reason and notes', () => { + getDiscussionLockButton().click(); + cy.findByRole('textbox', { + name: 'Reason for locking the discussion (optional) - this will be publicly displayed', + }).should('exist'); + + cy.findByRole('textbox', { + name: 'Notes (optional) - this is only visible to you and admins', + }).should('exist'); + }); + }); + + describe('when an article has its discussion locked', () => { + beforeEach(() => { + cy.testSetup(); + cy.fixture('users/articleEditorV1User.json').as('user'); + + cy.get('@user').then((user) => { + cy.loginUser(user).then(() => { + cy.createArticle({ + title: 'Test Article', + tags: ['beginner', 'ruby', 'go'], + content: `This is a test article's contents.`, + published: true, + }).then((response) => { + const articlePath = response.body.current_state_path; + cy.visit(`${articlePath}/manage`); + getDiscussionLockButton().click(); + cy.get('#discussion_lock_reason') + .should('be.visible') + .type(exampleReason); + cy.get('#discussion_lock_notes') + .should('be.visible') + .type(exampleNotes); + getDiscussionLockSubmitButton().click(); + cy.visit(articlePath); + }); + }); + }); + }); + + it('should show the discussion lock reason on an article', () => { + cy.get('#discussion-lock').should(($discussionLock) => { + expect($discussionLock).to.contain.text(exampleReason); + }); + }); + + it('should not show the discussion lock notes on an article', () => { + cy.get('#discussion-lock').should(($discussionLock) => { + expect($discussionLock).not.to.contain.text(exampleNotes); + }); + }); + + it('should not show the new comment box', () => { + cy.get('#new_comment').should('not.exist'); + }); + }); +}); diff --git a/cypress/integration/articleFlows/manageFlows/unlockDiscussion.spec.js b/cypress/integration/articleFlows/manageFlows/unlockDiscussion.spec.js new file mode 100644 index 000000000..55e25ab1b --- /dev/null +++ b/cypress/integration/articleFlows/manageFlows/unlockDiscussion.spec.js @@ -0,0 +1,88 @@ +describe('Unlock discussion', () => { + const getDiscussionLockButton = () => + cy.findByRole('link', { + name: 'Lock discussion', + }); + + const getDiscussionUnlockButton = () => + cy.findByRole('link', { + name: 'Unlock discussion', + }); + + const getDiscussionLockSubmitButton = () => + cy.findByRole('button', { + name: 'Lock discussion', + }); + + const getDiscussionUnlockSubmitButton = () => + cy.findByRole('button', { + name: 'Unlock discussion', + }); + + describe('Unlocking the discussion', () => { + beforeEach(() => { + cy.testSetup(); + cy.fixture('users/articleEditorV1User.json').as('user'); + + cy.get('@user').then((user) => { + cy.loginUser(user).then(() => { + cy.createArticle({ + title: 'Test Article', + tags: ['beginner', 'ruby', 'go'], + content: `This is a test article's contents.`, + published: true, + }).then((response) => { + cy.visit(`${response.body.current_state_path}/manage`); + getDiscussionLockButton().click(); + getDiscussionLockSubmitButton().click(); + }); + }); + }); + }); + + it('should allow a user to unlock a discussion', () => { + getDiscussionUnlockButton().click(); + getDiscussionUnlockSubmitButton().click(); + + getDiscussionLockButton().should('exist'); + }); + + it('should ask the user to confirm the discussion unlock', () => { + getDiscussionUnlockButton().click(); + + cy.findByRole('heading', { + name: 'Are you sure you want to unlock the discussion on this post?', + }).should('exist'); + + getDiscussionUnlockSubmitButton().should('exist'); + }); + }); + + describe('when an article has its discussion unlocked', () => { + beforeEach(() => { + cy.testSetup(); + cy.fixture('users/articleEditorV1User.json').as('user'); + + cy.get('@user').then((user) => { + cy.loginUser(user).then(() => { + cy.createArticle({ + title: 'Test Article', + tags: ['beginner', 'ruby', 'go'], + content: `This is a test article's contents.`, + published: true, + }).then((response) => { + cy.visit(response.body.current_state_path); + }); + }); + }); + }); + + it('should show the new comment box', () => { + cy.get('#new_comment').should('exist'); + }); + + it('should not show a discussion lock', () => { + cy.get('#dicussion-lock').should('not.exist'); + }); + }); +}); diff --git a/db/migrate/20210531152321_create_discussion_locks.rb b/db/migrate/20210531152321_create_discussion_locks.rb new file mode 100644 index 000000000..9cb04b0d9 --- /dev/null +++ b/db/migrate/20210531152321_create_discussion_locks.rb @@ -0,0 +1,12 @@ +class CreateDiscussionLocks < ActiveRecord::Migration[6.1] + def change + create_table :discussion_locks do |t| + t.references :article, null: false, foreign_key: true, index: { unique: true } + t.references :locking_user, references: :users, foreign_key: { to_table: :users }, null: false + t.text :reason + t.text :notes + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 08d72d974..b7c671be4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -461,6 +461,17 @@ ActiveRecord::Schema.define(version: 2021_06_09_103939) do t.index ["user_id", "token", "platform", "consumer_app_id"], name: "index_devices_on_user_id_and_token_and_platform_and_app", unique: true end + create_table "discussion_locks", force: :cascade do |t| + t.bigint "article_id", null: false + t.datetime "created_at", precision: 6, null: false + t.bigint "locking_user_id", null: false + t.text "notes" + t.text "reason" + t.datetime "updated_at", precision: 6, null: false + t.index ["article_id"], name: "index_discussion_locks_on_article_id", unique: true + t.index ["locking_user_id"], name: "index_discussion_locks_on_locking_user_id" + end + create_table "display_ad_events", force: :cascade do |t| t.string "category" t.string "context_type" @@ -1527,6 +1538,8 @@ ActiveRecord::Schema.define(version: 2021_06_09_103939) do add_foreign_key "custom_profile_fields", "profiles", on_delete: :cascade add_foreign_key "devices", "consumer_apps" add_foreign_key "devices", "users" + add_foreign_key "discussion_locks", "articles" + add_foreign_key "discussion_locks", "users", column: "locking_user_id" add_foreign_key "display_ad_events", "display_ads", on_delete: :cascade add_foreign_key "display_ad_events", "users", on_delete: :cascade add_foreign_key "display_ads", "organizations", on_delete: :cascade diff --git a/spec/factories/articles.rb b/spec/factories/articles.rb index 5a99b56e2..921631965 100644 --- a/spec/factories/articles.rb +++ b/spec/factories/articles.rb @@ -61,4 +61,8 @@ FactoryBot.define do trait :with_user_subscription_tag_role_user do after(:build) { |article| article.user.add_role(:restricted_liquid_tag, LiquidTags::UserSubscriptionTag) } end + + trait :with_discussion_lock do + after(:create) { |article| create(:discussion_lock, locking_user_id: article.user_id, article_id: article.id) } + end end diff --git a/spec/factories/discussion_locks.rb b/spec/factories/discussion_locks.rb new file mode 100644 index 000000000..0681c617b --- /dev/null +++ b/spec/factories/discussion_locks.rb @@ -0,0 +1,9 @@ +FactoryBot.define do + factory :discussion_lock do + association :article, factory: :article, strategy: :create + association :locking_user, factory: :user, strategy: :create + + reason { "This post has too many off-topic comments" } + notes { "Private notes" } + end +end diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index 226256129..2dbc6370d 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -18,6 +18,8 @@ RSpec.describe Article, type: :model do it { is_expected.to belong_to(:organization).optional } it { is_expected.to belong_to(:user) } + it { is_expected.to have_one(:discussion_lock).dependent(:destroy) } + it { is_expected.to have_many(:comments).dependent(:nullify) } it { is_expected.to have_many(:mentions).dependent(:destroy) } it { is_expected.to have_many(:html_variant_successes).dependent(:nullify) } diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 46182755f..84508991f 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -45,6 +45,12 @@ RSpec.describe Comment, type: :model do expect(subject).not_to be_valid end + it "is invalid if commentable is an article and the discussion is locked" do + subject.commentable = build(:article, :with_discussion_lock) + + expect(subject).not_to be_valid + end + it "is valid without a commentable" do subject.commentable = nil diff --git a/spec/models/discussion_lock_spec.rb b/spec/models/discussion_lock_spec.rb new file mode 100644 index 000000000..3b79ccbfb --- /dev/null +++ b/spec/models/discussion_lock_spec.rb @@ -0,0 +1,18 @@ +require "rails_helper" + +RSpec.describe DiscussionLock, type: :model do + let(:discussion_lock) { create(:discussion_lock) } + + describe "relationships" do + it { is_expected.to belong_to(:article) } + it { is_expected.to belong_to(:locking_user) } + end + + describe "validations" do + subject { discussion_lock } + + it { is_expected.to validate_presence_of(:article_id) } + it { is_expected.to validate_presence_of(:locking_user_id) } + it { is_expected.to validate_uniqueness_of(:article_id) } + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a25774a01..af4f939cd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -55,6 +55,7 @@ RSpec.describe User, type: :model do it { is_expected.to have_many(:collections).dependent(:destroy) } it { is_expected.to have_many(:comments).dependent(:destroy) } it { is_expected.to have_many(:credits).dependent(:destroy) } + it { is_expected.to have_many(:discussion_locks).dependent(:destroy) } it { is_expected.to have_many(:display_ad_events).dependent(:destroy) } it { is_expected.to have_many(:email_authorizations).dependent(:delete_all) } it { is_expected.to have_many(:email_messages).class_name("Ahoy::Message").dependent(:destroy) } diff --git a/spec/policies/discussion_lock_policy_spec.rb b/spec/policies/discussion_lock_policy_spec.rb new file mode 100644 index 000000000..98870a75f --- /dev/null +++ b/spec/policies/discussion_lock_policy_spec.rb @@ -0,0 +1,51 @@ +require "rails_helper" + +RSpec.describe DiscussionLockPolicy do + subject { described_class.new(locking_user, discussion_lock) } + + let!(:locking_user) { create(:user) } + let(:article) { build_stubbed(:article, user: locking_user) } + let(:discussion_lock) { build_stubbed(:discussion_lock, locking_user: locking_user, article: article) } + let(:valid_attributes) { %i[article_id notes reason] } + + context "when user is not signed-in" do + let(:locking_user) { nil } + + it { within_block_is_expected.to raise_error(Pundit::NotAuthorizedError) } + end + + context "when user is not the author" do + let(:user) { build_stubbed(:user) } + let(:discussion_lock) { build_stubbed(:discussion_lock, locking_user: user, article: article) } + + it { is_expected.to forbid_actions(%i[create destroy]) } + end + + context "when user is the author" do + let(:article) { build_stubbed(:article, user: locking_user) } + + it { is_expected.to permit_actions(%i[create destroy]) } + it { is_expected.to permit_mass_assignment_of(valid_attributes) } + end + + context "when user is suspended" do + let(:user) { build_stubbed(:user, :suspended) } + let(:discussion_lock) { build_stubbed(:discussion_lock, locking_user: user, article: article) } + + it { is_expected.to forbid_actions(%i[create destroy]) } + end + + context "when user is an admin" do + let(:locking_user) { build(:user, :admin) } + + it { is_expected.to permit_actions(%i[create destroy]) } + it { is_expected.to permit_mass_assignment_of(valid_attributes) } + end + + context "when user is a super_admin" do + let(:locking_user) { build(:user, :super_admin) } + + it { is_expected.to permit_actions(%i[create destroy]) } + it { is_expected.to permit_mass_assignment_of(valid_attributes) } + end +end diff --git a/spec/requests/discussion_locks_spec.rb b/spec/requests/discussion_locks_spec.rb new file mode 100644 index 000000000..760de7835 --- /dev/null +++ b/spec/requests/discussion_locks_spec.rb @@ -0,0 +1,67 @@ +require "rails_helper" + +RSpec.describe "DiscussionLocks", type: :request do + let(:user) { create(:user) } + let(:cache_buster) { EdgeCache::BustArticle } + + before { sign_in user } + + describe "POST /discussion_locks - DiscussionLocks#create" do + it "creates a DiscussionLock" do + article = create(:article, user: user) + reason = "Unproductice comments." + notes = "Hostile comment from user @user" + valid_attributes = { article_id: article.id, locking_user_id: user.id, notes: notes, reason: reason } + expect do + post discussion_locks_path, params: { discussion_lock: valid_attributes } + end.to change(DiscussionLock, :count).by(1) + + expect(request.flash[:success]).to include("Discussion was successfully locked!") + discussion_lock = DiscussionLock.last + expect(discussion_lock.article_id).to eq article.id + expect(discussion_lock.locking_user_id).to eq user.id + expect(discussion_lock.notes).to eq notes + expect(discussion_lock.reason).to eq reason + end + + it "returns an error for an Article that already has a DiscussionLock" do + article = create(:article, :with_discussion_lock, user: user) + invalid_article_attributes = { article_id: article.id, locking_user_id: user.id } + expect do + post discussion_locks_path, params: { discussion_lock: invalid_article_attributes } + end.to change(DiscussionLock, :count).by(0) + + expect(request.flash[:error]).to include("Error: Article has already been taken") + end + + it "busts the cache for the article" do + article = create(:article, user: user) + allow(cache_buster).to receive(:call).with(article) + valid_attributes = { article_id: article.id, locking_user_id: user.id } + + post discussion_locks_path, params: { discussion_lock: valid_attributes } + + expect(cache_buster).to have_received(:call).with(article).once + end + end + + describe "DELETE /discussion_locks/:id - DiscussionLocks#destroy" do + it "destroys a DiscussionLock" do + article = create(:article, user: user) + discussion_lock = create(:discussion_lock, article_id: article.id, locking_user_id: user.id) + expect do + delete discussion_lock_path(discussion_lock.id) + end.to change(DiscussionLock, :count).by(-1) + end + + it "busts the cache for the article" do + article = create(:article, user: user) + allow(cache_buster).to receive(:call).with(article) + discussion_lock = create(:discussion_lock, article_id: article.id, locking_user_id: user.id) + + delete discussion_lock_path(discussion_lock.id) + + expect(cache_buster).to have_received(:call).with(article).once + end + end +end diff --git a/spec/system/articles/user_discussion_locks_spec.rb b/spec/system/articles/user_discussion_locks_spec.rb new file mode 100644 index 000000000..927700247 --- /dev/null +++ b/spec/system/articles/user_discussion_locks_spec.rb @@ -0,0 +1,61 @@ +require "rails_helper" + +RSpec.describe "User discussion locks", type: :system, js: true do + let(:user) { create(:user) } + let(:article) { create(:article, user: user) } + let(:discussion_lock) { create(:discussion_lock, article: article, locking_user: user) } + + before do + article + sign_in user + end + + context "when an Article does not have a discussion lock" do + it "shows the discussion lock button in manage" do + visit dashboard_path + click_on "Manage" + + within "div.dashboard-actions" do + expect(page).to have_link("Lock discussion", href: "#{article.path}/discussion_lock_confirm") + end + end + + it "doesn't show a DiscussionLock on the Article" do + visit article.path + expect(page).not_to have_selector("#discussion-lock") + end + + it "doesn't hide new comment box on the Article" do + visit article.path + expect(page).to have_selector("#new_comment") + end + end + + context "when an Article has a discussion lock" do + before { discussion_lock } + + it "shows the unlock discussion lock button in manage" do + visit dashboard_path + click_on "Manage" + + within "div.dashboard-actions" do + expect(page).to have_link("Unlock discussion", href: "#{article.path}/discussion_unlock_confirm") + end + end + + it "shows a DiscussionLock on the Article" do + visit article.path + expect(page).to have_selector("#discussion-lock") + + within "#discussion-lock" do + expect(page).to have_text("The discussion has been locked. New comments can't be added.") + expect(page).to have_text(discussion_lock.reason) + end + end + + it "hides new comment box on the Article" do + visit article.path + expect(page).not_to have_selector("#new_comment") + end + end +end