user feedbacks: replace permanent deletions with soft deletions.

* Add is_deleted flag.
* Allow mods to delete and undelete user feedbacks.
* Don't hide old name change feedbacks (these will be deleted instead).

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 :gold_only, :only => [:new, :edit, :create, :update]
-  before_action :check_no_feedback, only: [:new, :edit, :create, :update, :destroy]
+  before_action :check_no_feedback, only: [:new, :edit, :create, :update]
-  respond_to :html, :xml, :json
+  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))
+    @user_feedback.update(user_feedback_params(:update, @user_feedback))
-    respond_with(@user_feedback)
-  end
-
-  def destroy
-    @user_feedback = UserFeedback.visible.find(params[:id])
-    check_privilege(@user_feedback)
-    @user_feedback.destroy
     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

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?
+      viewer.is_moderator? ? all : undeleted
-        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
     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

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 %>

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 @@
         <tr>
           <th>User</th>
           <th>Message</th>
+          <% if CurrentUser.is_moderator? %>
+            <th>Status</th>
+          <% end %>
           <th>Category</th>
           <th>Creator</th>
           <th></th>
@@ -33,6 +39,9 @@
               </div>
               <%= render "application/update_notice", record: feedback, interval: 0.minutes %>
             </td>
+            <% if CurrentUser.is_moderator? %>
+              <td><%= "deleted" if feedback.is_deleted? %></td>
+            <% end %>
             <td>
               <%= link_to feedback.category.capitalize, user_feedbacks_path(search: { category: feedback.category }) %>
             </td>
@@ -44,7 +53,11 @@
             <td>
               <% 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 %>
             </td>
           </tr>

app/views/user_feedbacks/update.js

@@ -0,0 +1 @@
+location.reload();

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

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

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');
 
 

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
+        put_auth user_feedback_path(@user_feedback), @critic, params: { user_feedback: { category: "positive" }}
-          @feedback = create(:user_feedback, user: @user, category: "negative")
-        end
-        put_auth user_feedback_path(@feedback), @critic, params: { id: @feedback.id, user_feedback: { category: "positive" }}
 
-        assert_redirected_to(@feedback)
+        assert_redirected_to(@user_feedback)
-        assert("positive", @feedback.reload.category)
+        assert("positive", @user_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
       end
 
       context "by a moderator" do
         should "allow deleting feedbacks given to other users" do
-          assert_difference "UserFeedback.count", -1 do
+          put_auth user_feedback_path(@user_feedback), @mod, params: { user_feedback: { is_deleted: "true" }}
-            delete_auth user_feedback_path(@user_feedback), @mod
+
-          end
+          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 = as(@critic) { create(:user_feedback, user: @mod) }
-            @user_feedback = create(:user_feedback, user: @mod)
+          put_auth user_feedback_path(@user_feedback), @mod, params: { id: @user_feedback.id, user_feedback: { is_deleted: "true" }}
-          end
 
-          assert_difference "UserFeedback.count", 0 do
+          assert_response 403
-            delete_auth user_feedback_path(@user_feedback), @mod
+          refute(@user_feedback.reload.is_deleted?)
-          end
         end
       end
     end