diff --git a/app/controllers/user_feedbacks_controller.rb b/app/controllers/user_feedbacks_controller.rb index 9eede89c0..da811f23e 100644 --- a/app/controllers/user_feedbacks_controller.rb +++ b/app/controllers/user_feedbacks_controller.rb @@ -1,7 +1,7 @@ class UserFeedbacksController < ApplicationController - before_action :gold_only, :only => [:new, :edit, :create, :update, :destroy] - before_action :check_no_feedback, only: [:new, :edit, :create, :update, :destroy] - respond_to :html, :xml, :json + before_action :gold_only, :only => [:new, :edit, :create, :update] + before_action :check_no_feedback, only: [:new, :edit, :create, :update] + respond_to :html, :xml, :json, :js def new @user_feedback = UserFeedback.new(user_feedback_params(:create)) @@ -20,7 +20,7 @@ class UserFeedbacksController < ApplicationController end def index - @user_feedbacks = UserFeedback.visible.includes(:user, :creator).paginated_search(params, count_pages: true) + @user_feedbacks = UserFeedback.includes(:user, :creator).paginated_search(params, count_pages: true) respond_with(@user_feedbacks) end @@ -32,14 +32,7 @@ class UserFeedbacksController < ApplicationController def update @user_feedback = UserFeedback.visible.find(params[:id]) check_privilege(@user_feedback) - @user_feedback.update(user_feedback_params(:update)) - respond_with(@user_feedback) - end - - def destroy - @user_feedback = UserFeedback.visible.find(params[:id]) - check_privilege(@user_feedback) - @user_feedback.destroy + @user_feedback.update(user_feedback_params(:update, @user_feedback)) respond_with(@user_feedback) end @@ -55,9 +48,10 @@ class UserFeedbacksController < ApplicationController end end - def user_feedback_params(context) + def user_feedback_params(context, user_feedback = nil) permitted_params = %i[body category] permitted_params += %i[user_id user_name] if context == :create + permitted_params += %i[is_deleted] if context == :update && user_feedback.deletable_by?(CurrentUser.user) params.fetch(:user_feedback, {}).permit(permitted_params) end diff --git a/app/models/user_feedback.rb b/app/models/user_feedback.rb index 85f302b34..2f2e859cd 100644 --- a/app/models/user_feedback.rb +++ b/app/models/user_feedback.rb @@ -15,26 +15,14 @@ class UserFeedback < ApplicationRecord ModAction.log(%{#{CurrentUser.name} deleted user feedback for "#{rec.user.name}":/users/#{rec.user_id}}, :user_feedback_delete) end + scope :positive, -> { where(category: "positive") } + scope :neutral, -> { where(category: "neutral") } + scope :negative, -> { where(category: "negative") } + scope :undeleted, -> { where(is_deleted: false) } + module SearchMethods - def positive - where("category = ?", "positive") - end - - def neutral - where("category = ?", "neutral") - end - - def negative - where("category = ?", "negative") - end - def visible(viewer = CurrentUser.user) - if viewer.is_admin? - all - else - # joins(:user).merge(User.undeleted).or(where("body !~ 'Name changed from [^\s:]+ to [^\s:]+'")) - joins(:user).where.not("users.name ~ 'user_[0-9]+~*' AND user_feedback.body ~ 'Name changed from [^\s:]+ to [^\s:]+'") - end + viewer.is_moderator? ? all : undeleted end def default_order @@ -44,7 +32,8 @@ class UserFeedback < ApplicationRecord def search(params) q = super - q = q.search_attributes(params, :user, :creator, :category, :body) + q = q.visible + q = q.search_attributes(params, :user, :creator, :category, :body, :is_deleted) q = q.text_attribute_matches(:body, params[:body_matches]) q.apply_default_order(params) @@ -84,7 +73,11 @@ class UserFeedback < ApplicationRecord end end + def deletable_by?(deleter) + deleter.is_moderator? && deleter != user + end + def editable_by?(editor) - (editor.is_moderator? && editor != user) || creator == editor + (editor.is_moderator? && editor != user) || (creator == editor && !is_deleted?) end end diff --git a/app/views/user_feedbacks/edit.html.erb b/app/views/user_feedbacks/edit.html.erb index 1f10065be..476c84c3d 100644 --- a/app/views/user_feedbacks/edit.html.erb +++ b/app/views/user_feedbacks/edit.html.erb @@ -10,6 +10,9 @@ <%= simple_form_for(@user_feedback) do |f| %> <%= f.input :category, :collection => ["positive", "neutral", "negative"], :include_blank => false %> <%= dtext_field "user_feedback", "body" %> + <% if @user_feedback.deletable_by?(CurrentUser.user) %> + <%= f.input :is_deleted, label: "Deleted" %> + <% end %> <%= f.button :submit, "Submit" %> <%= dtext_preview_button "user_feedback", "body" %> <% end %> diff --git a/app/views/user_feedbacks/index.html.erb b/app/views/user_feedbacks/index.html.erb index c39e58633..522600e13 100644 --- a/app/views/user_feedbacks/index.html.erb +++ b/app/views/user_feedbacks/index.html.erb @@ -7,6 +7,9 @@ <%= f.input :creator_name, input_html: { value: params.dig(:search, :creator_name), "data-autocomplete": "user" } %> <%= f.input :body_matches, label: "Message", input_html: { value: params.dig(:search, :body_matches) } %> <%= f.input :category, collection: %w[positive negative neutral], include_blank: true, selected: params.dig(:search, :category) %> + <% if CurrentUser.is_moderator? %> + <%= f.input :is_deleted, label: "Deleted", collection: %w[Yes No], include_blank: true, selected: params.dig(:search, :is_deleted) %> + <% end %> <%= f.submit "Search" %> <% end %> @@ -15,6 +18,9 @@ User Message + <% if CurrentUser.is_moderator? %> + Status + <% end %> Category Creator @@ -33,6 +39,9 @@ <%= render "application/update_notice", record: feedback, interval: 0.minutes %> + <% if CurrentUser.is_moderator? %> + <%= "deleted" if feedback.is_deleted? %> + <% end %> <%= link_to feedback.category.capitalize, user_feedbacks_path(search: { category: feedback.category }) %> @@ -44,7 +53,11 @@ <% if feedback.editable_by?(CurrentUser.user) %> <%= link_to "edit", edit_user_feedback_path(feedback) %> - | <%= link_to "delete", user_feedback_path(feedback), :method => :delete, :data => {:confirm => "Are you sure you want to delete this user feedback?"} %> + <% if feedback.deletable_by?(CurrentUser.user) && !feedback.is_deleted? %> + | <%= link_to "delete", user_feedback_path(feedback), method: :put, remote: true, "data-params": "user_feedback[is_deleted]=true", "data-confirm": "Are you sure you want to delete this user feedback?" %> + <% elsif feedback.deletable_by?(CurrentUser.user) && feedback.is_deleted? %> + | <%= link_to "undelete", user_feedback_path(feedback), method: :put, remote: true, "data-params": "user_feedback[is_deleted]=false" %> + <% end %> <% end %> diff --git a/app/views/user_feedbacks/update.js b/app/views/user_feedbacks/update.js new file mode 100644 index 000000000..345366b9b --- /dev/null +++ b/app/views/user_feedbacks/update.js @@ -0,0 +1 @@ +location.reload(); diff --git a/config/routes.rb b/config/routes.rb index a86547c29..0b4abdcc2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -283,7 +283,7 @@ Rails.application.routes.draw do end end resource :user_upgrade, :only => [:new, :create, :show] - resources :user_feedbacks + resources :user_feedbacks, except: [:destroy] resources :user_name_change_requests, only: [:new, :create, :show, :index] resources :wiki_pages, id: /.+?(?=\.json|\.xml|\.html)|.+/ do put :revert, on: :member diff --git a/db/migrate/20191223032633_add_is_deleted_to_user_feedback.rb b/db/migrate/20191223032633_add_is_deleted_to_user_feedback.rb new file mode 100644 index 000000000..f7763903b --- /dev/null +++ b/db/migrate/20191223032633_add_is_deleted_to_user_feedback.rb @@ -0,0 +1,5 @@ +class AddIsDeletedToUserFeedback < ActiveRecord::Migration[6.0] + def change + add_column :user_feedback, :is_deleted, :boolean, default: false, null: false + end +end diff --git a/db/structure.sql b/db/structure.sql index 94a4f3b75..c34a734b1 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -3055,7 +3055,8 @@ CREATE TABLE public.user_feedback ( category character varying NOT NULL, body text NOT NULL, created_at timestamp without time zone NOT NULL, - updated_at timestamp without time zone NOT NULL + updated_at timestamp without time zone NOT NULL, + is_deleted boolean DEFAULT false NOT NULL ); @@ -7395,6 +7396,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20191117080647'), ('20191117081229'), ('20191117200404'), -('20191119061018'); +('20191119061018'), +('20191223032633'); diff --git a/test/functional/user_feedbacks_controller_test.rb b/test/functional/user_feedbacks_controller_test.rb index 5b909b963..f3049cbc5 100644 --- a/test/functional/user_feedbacks_controller_test.rb +++ b/test/functional/user_feedbacks_controller_test.rb @@ -6,6 +6,7 @@ class UserFeedbacksControllerTest < ActionDispatch::IntegrationTest @user = create(:user) @critic = create(:gold_user) @mod = create(:moderator_user) + @user_feedback = as(@critic) { create(:user_feedback, user: @user) } end context "new action" do @@ -16,12 +17,6 @@ class UserFeedbacksControllerTest < ActionDispatch::IntegrationTest end context "edit action" do - setup do - as(@critic) do - @user_feedback = create(:user_feedback, user: @user) - end - end - should "render" do get_auth edit_user_feedback_path(@user_feedback), @critic assert_response :success @@ -29,12 +24,6 @@ class UserFeedbacksControllerTest < ActionDispatch::IntegrationTest end context "index action" do - setup do - as(@critic) do - @user_feedback = create(:user_feedback, user: @user) - end - end - should "render" do get_auth user_feedbacks_path, @user assert_response :success @@ -58,44 +47,26 @@ class UserFeedbacksControllerTest < ActionDispatch::IntegrationTest context "update action" do should "update the feedback" do - as(@critic) do - @feedback = create(:user_feedback, user: @user, category: "negative") - end - put_auth user_feedback_path(@feedback), @critic, params: { id: @feedback.id, user_feedback: { category: "positive" }} + put_auth user_feedback_path(@user_feedback), @critic, params: { user_feedback: { category: "positive" }} - assert_redirected_to(@feedback) - assert("positive", @feedback.reload.category) - end - end - - context "destroy action" do - setup do - as(@critic) do - @user_feedback = create(:user_feedback, user: @user) - end - end - - should "delete a feedback" do - assert_difference "UserFeedback.count", -1 do - delete_auth user_feedback_path(@user_feedback), @critic - end + assert_redirected_to(@user_feedback) + assert("positive", @user_feedback.reload.category) end context "by a moderator" do should "allow deleting feedbacks given to other users" do - assert_difference "UserFeedback.count", -1 do - delete_auth user_feedback_path(@user_feedback), @mod - end + put_auth user_feedback_path(@user_feedback), @mod, params: { user_feedback: { is_deleted: "true" }} + + assert_redirected_to @user_feedback + assert(@user_feedback.reload.is_deleted?) end should "not allow deleting feedbacks given to themselves" do - as(@critic) do - @user_feedback = create(:user_feedback, user: @mod) - end + @user_feedback = as(@critic) { create(:user_feedback, user: @mod) } + put_auth user_feedback_path(@user_feedback), @mod, params: { id: @user_feedback.id, user_feedback: { is_deleted: "true" }} - assert_difference "UserFeedback.count", 0 do - delete_auth user_feedback_path(@user_feedback), @mod - end + assert_response 403 + refute(@user_feedback.reload.is_deleted?) end end end