From 42b39da1a8b1c51b65b2adba0ac9475a782be4e6 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Wed, 9 Feb 2022 11:28:58 -0500 Subject: [PATCH] Ensuring an admin can confirm delete a user's post (#16485) Prior to this commit, the admin could delete the post but were blocked by the "delete confirm". The blocker was because we scoped the finding of an article to the current user. With this commit, the admin should be able to confirm the deletion of an article. Closes #16461 --- app/controllers/articles_controller.rb | 2 +- .../articles/articles_destroy_spec.rb | 90 ++++++++++++++----- spec/requests/articles/articles_spec.rb | 13 --- 3 files changed, 71 insertions(+), 34 deletions(-) diff --git a/app/controllers/articles_controller.rb b/app/controllers/articles_controller.rb index 6afa7a4b3..356a2c488 100644 --- a/app/controllers/articles_controller.rb +++ b/app/controllers/articles_controller.rb @@ -150,7 +150,7 @@ class ArticlesController < ApplicationController end def delete_confirm - @article = current_user.articles.find_by(slug: params[:slug]) + @article = Article.find_by(slug: params[:slug]) not_found unless @article authorize @article end diff --git a/spec/requests/articles/articles_destroy_spec.rb b/spec/requests/articles/articles_destroy_spec.rb index fc3fd8d2e..28c475c40 100644 --- a/spec/requests/articles/articles_destroy_spec.rb +++ b/spec/requests/articles/articles_destroy_spec.rb @@ -8,30 +8,80 @@ RSpec.describe "ArticlesDestroy", type: :request do sign_in user end - it "destroyed an article" do - delete "/articles/#{article.id}" - destroyed_article = Article.find_by(id: article.id) - expect(destroyed_article).to be_nil - end - - it "schedules a RemoveAllWorker if there are comments" do - create(:comment, commentable: article, user: user) - sidekiq_assert_enqueued_with(job: Notifications::RemoveAllWorker) do + context "when DELETE /articles/:slug" do + it "destroyed an article" do delete "/articles/#{article.id}" + destroyed_article = Article.find_by(id: article.id) + expect(destroyed_article).to be_nil + end + + it "schedules a RemoveAllWorker if there are comments" do + create(:comment, commentable: article, user: user) + sidekiq_assert_enqueued_with(job: Notifications::RemoveAllWorker) do + delete "/articles/#{article.id}" + end + end + + it "removes all previous published notifications" do + create(:notification, notifiable: article, action: "Published", user: user) + expect do + delete "/articles/#{article.id}" + end.to change(Notification, :count).by(-1) + end + + it "doesn't destroy another person's article" do + article2 = create(:article, user_id: create(:user).id) + expect do + delete "/articles/#{article2.id}" + end.to raise_error(Pundit::NotAuthorizedError) end end - it "removes all previous published notifications" do - create(:notification, notifiable: article, action: "Published", user: user) - expect do - delete "/articles/#{article.id}" - end.to change(Notification, :count).by(-1) - end + describe "when GET /delete_confirm" do + context "without an article" do + before { sign_in user } - it "doesn't destroy another person's article" do - article2 = create(:article, user_id: create(:user).id) - expect do - delete "/articles/#{article2.id}" - end.to raise_error(Pundit::NotAuthorizedError) + it "renders not_found" do + article = create(:article, user: user) + expect do + get "#{article.path}_1/delete_confirm" + end.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context "with an article the current user wrote" do + before { sign_in user } + + it "renders success" do + article = create(:article, user: user) + get "#{article.path}/delete_confirm" + expect(response).to be_successful + end + end + + context "when an admin attempts to delete an article" do + let(:admin) { create(:user, :admin) } + + before { sign_in admin } + + it "renders success" do + article = create(:article, user: user) + get "#{article.path}/delete_confirm" + expect(response).to be_successful + end + end + + context "when another user attempts to delete someone's article" do + let(:other_user) { create(:user) } + + before { sign_in other_user } + + it "raises a policy error" do + article = create(:article, user: user) + expect do + get "#{article.path}/delete_confirm" + end.to raise_error(Pundit::NotAuthorizedError) + end + end end end diff --git a/spec/requests/articles/articles_spec.rb b/spec/requests/articles/articles_spec.rb index 5098e9b20..473b349dc 100644 --- a/spec/requests/articles/articles_spec.rb +++ b/spec/requests/articles/articles_spec.rb @@ -335,17 +335,4 @@ RSpec.describe "Articles", type: :request do expect(response.body).to include("Stats for Your Article") end end - - describe "GET /delete_confirm" do - before { sign_in user } - - context "without an article" do - it "renders not_found" do - article = create(:article, user: user) - expect do - get "#{article.path}_1/delete_confirm" - end.to raise_error(ActiveRecord::RecordNotFound) - end - end - end end