From bca1f122d0e941cdf471f65c88f91633983f71c7 Mon Sep 17 00:00:00 2001 From: evazion Date: Mon, 3 Aug 2020 19:28:10 -0500 Subject: [PATCH] posts: rework post deletion to use dialog box. Rework post deletion from using a separate page to using a dialog box, like flagging. * Add `DELETE /posts/:id` endpoint. * Remove `POST /moderator/post/posts/:id/delete` endpoint. --- .../moderator/post/posts_controller.rb | 12 ------ app/controllers/posts_controller.rb | 12 ++++++ app/policies/post_policy.rb | 4 ++ .../post/posts/confirm_delete.html.erb | 26 ------------- app/views/posts/destroy.js.erb | 5 +++ .../partials/show/_delete_dialog.html.erb | 9 +++++ .../posts/partials/show/_options.html.erb | 2 +- config/routes.rb | 4 +- .../moderator/post/posts_controller_test.rb | 20 ---------- test/functional/posts_controller_test.rb | 37 +++++++++++++++++++ 10 files changed, 69 insertions(+), 62 deletions(-) delete mode 100644 app/views/moderator/post/posts/confirm_delete.html.erb create mode 100644 app/views/posts/destroy.js.erb create mode 100644 app/views/posts/partials/show/_delete_dialog.html.erb diff --git a/app/controllers/moderator/post/posts_controller.rb b/app/controllers/moderator/post/posts_controller.rb index 5fac8e5a2..c12ee368b 100644 --- a/app/controllers/moderator/post/posts_controller.rb +++ b/app/controllers/moderator/post/posts_controller.rb @@ -4,18 +4,6 @@ module Moderator skip_before_action :api_check respond_to :html, :json, :xml, :js - def confirm_delete - @post = ::Post.find(params[:id]) - end - - def delete - @post = authorize ::Post.find(params[:id]) - if params[:commit] == "Delete" - @post.delete!(params[:reason], :move_favorites => params[:move_favorites].present?) - end - redirect_to(post_path(@post)) - end - def confirm_move_favorites @post = ::Post.find(params[:id]) end diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 9829c954f..03bb877f9 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -56,6 +56,18 @@ class PostsController < ApplicationController respond_with_post_after_update(@post) end + def destroy + @post = authorize Post.find(params[:id]) + + if params[:commit] == "Delete" + move_favorites = params.dig(:post, :move_favorites).to_s.truthy? + @post.delete!(params.dig(:post, :reason), move_favorites: move_favorites) + flash[:notice] = "Post deleted" + end + + respond_with_post_after_update(@post) + end + def revert @post = authorize Post.find(params[:id]) @version = @post.versions.find(params[:version_id]) diff --git a/app/policies/post_policy.rb b/app/policies/post_policy.rb index 413ce7876..25046263f 100644 --- a/app/policies/post_policy.rb +++ b/app/policies/post_policy.rb @@ -31,6 +31,10 @@ class PostPolicy < ApplicationPolicy user.is_approver? && !record.is_deleted? end + def destroy? + delete? + end + def ban? user.is_approver? && !record.is_banned? end diff --git a/app/views/moderator/post/posts/confirm_delete.html.erb b/app/views/moderator/post/posts/confirm_delete.html.erb deleted file mode 100644 index c97ab66ec..000000000 --- a/app/views/moderator/post/posts/confirm_delete.html.erb +++ /dev/null @@ -1,26 +0,0 @@ -

Delete Post

- -
- <%= PostPresenter.preview(@post, show_deleted: true) %> -
- -<%= form_tag(delete_moderator_post_post_path, :style => "clear: both;", :class => "simple_form") do %> - <% if @post.parent_id %> -
- -
- <% end %> - -

Note: If the reason you are planning to delete this post is because it is from a banned artist, please <%= link_to "ban", confirm_ban_moderator_post_post_path(@post) %> this post instead of deleting it.

- -
- - <%= text_area_tag "reason" %> -
- - <%= submit_tag "Delete" %> - <%= submit_tag "Cancel" %> -<% end %> diff --git a/app/views/posts/destroy.js.erb b/app/views/posts/destroy.js.erb new file mode 100644 index 000000000..b8221d996 --- /dev/null +++ b/app/views/posts/destroy.js.erb @@ -0,0 +1,5 @@ +<% if params[:commit] == "Delete" %> + location.reload(); +<% else %> + Danbooru.Utility.dialog("Delete Post", "<%= j render "posts/partials/show/delete_dialog", post: @post %>"); +<% end %> diff --git a/app/views/posts/partials/show/_delete_dialog.html.erb b/app/views/posts/partials/show/_delete_dialog.html.erb new file mode 100644 index 000000000..68456af24 --- /dev/null +++ b/app/views/posts/partials/show/_delete_dialog.html.erb @@ -0,0 +1,9 @@ +
+ <%= edit_form_for(post, method: :delete, remote: true) do |f| %> + + <%= f.input :reason, as: :dtext, inline: true, input_html: { value: "" } %> + <% if post.parent_id.present? %> + <%= f.input :move_favorites, label: "Move favorites to parent", as: :boolean, input_html: { checked: false } %> + <% end %> + <% end %> +
diff --git a/app/views/posts/partials/show/_options.html.erb b/app/views/posts/partials/show/_options.html.erb index 18c8a340f..e55116e2f 100644 --- a/app/views/posts/partials/show/_options.html.erb +++ b/app/views/posts/partials/show/_options.html.erb @@ -64,7 +64,7 @@
  • <%= link_to "Move favorites", confirm_move_favorites_moderator_post_post_path(post_id: post.id) %>
    • <% end %> <% elsif policy(post).delete? %> -
  • <%= link_to "Delete", confirm_delete_moderator_post_post_path(post_id: post.id) %>
    • +
  • <%= link_to "Delete", post, method: :delete, remote: true %>
    • <% end %> <% if post.is_approvable? && !post.is_deleted? %> diff --git a/config/routes.rb b/config/routes.rb index 94b6878d6..ce3a273c1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -13,9 +13,7 @@ Rails.application.routes.draw do namespace :post do resources :posts, :only => [:delete, :expunge, :confirm_delete] do member do - get :confirm_delete post :expunge - post :delete get :confirm_move_favorites post :move_favorites get :confirm_ban @@ -175,7 +173,7 @@ Rails.application.routes.draw do end resources :post_replacements, :only => [:index, :new, :create, :update] resources :post_votes, only: [:index] - resources :posts, only: [:index, :show, :update] do + resources :posts, only: [:index, :show, :update, :destroy] do resources :events, :only => [:index], :controller => "post_events" resources :replacements, :only => [:index, :new, :create], :controller => "post_replacements" resource :artist_commentary, :only => [:index, :show] do diff --git a/test/functional/moderator/post/posts_controller_test.rb b/test/functional/moderator/post/posts_controller_test.rb index 8914baf72..9e5ead019 100644 --- a/test/functional/moderator/post/posts_controller_test.rb +++ b/test/functional/moderator/post/posts_controller_test.rb @@ -15,26 +15,6 @@ module Moderator end end - context "confirm_delete action" do - should "render" do - get_auth confirm_delete_moderator_post_post_path(@post), @admin - assert_response :success - end - end - - context "delete action" do - should "render" do - post_auth delete_moderator_post_post_path(@post), @admin, params: {:reason => "xxx", :format => "js", :commit => "Delete"} - assert(@post.reload.is_deleted?) - end - - should "work even if the deleter has flagged the post previously" do - create(:post_flag, post: @post, creator: @admin) - post_auth delete_moderator_post_post_path(@post), @admin, params: {:reason => "xxx", :format => "js", :commit => "Delete"} - assert(@post.reload.is_deleted?) - end - end - context "confirm_move_favorites action" do should "render" do get_auth confirm_move_favorites_moderator_post_post_path(@post), @admin diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb index 67b03454f..f3aeb2826 100644 --- a/test/functional/posts_controller_test.rb +++ b/test/functional/posts_controller_test.rb @@ -633,6 +633,43 @@ class PostsControllerTest < ActionDispatch::IntegrationTest end end + context "destroy action" do + setup do + @approver = create(:approver) + end + + should "delete the post" do + delete_auth post_path(@post), @approver, params: { commit: "Delete", post: { reason: "test" } } + + assert_redirected_to @post + assert_equal(true, @post.reload.is_deleted?) + assert_equal("test", @post.flags.last.reason) + end + + should "delete the post even if the deleter has flagged the post previously" do + create(:post_flag, post: @post, creator: @approver) + delete_auth post_path(@post), @approver, params: { commit: "Delete", post: { reason: "test" } } + + assert_redirected_to @post + assert_equal(true, @post.reload.is_deleted?) + end + + should "not delete the post if the user is unauthorized" do + delete_auth post_path(@post), @user, params: { commit: "Delete" } + + assert_response 403 + assert_equal(false, @post.is_deleted?) + end + + should "render the delete post dialog for an xhr request" do + delete_auth post_path(@post), @approver, xhr: true + + assert_response :success + assert_equal(false, @post.is_deleted?) + end + + end + context "revert action" do setup do PostVersion.sqs_service.stubs(:merge?).returns(false)