dmails: replace hard deletions with soft deletions.

Turn deletions into soft deletions (set the is_deleted flag) instead of
hard deletions (remove from database). The is_deleted flag actually
already existed, but it was never used before.
This commit is contained in:
evazion
2020-01-30 21:49:55 -06:00
parent 5df8d08aae
commit f8db577c25
7 changed files with 26 additions and 24 deletions

View File

@@ -1,6 +1,6 @@
class DmailsController < ApplicationController class DmailsController < ApplicationController
respond_to :html, :xml, :js, :json respond_to :html, :xml, :js, :json
before_action :member_only, except: [:index, :show, :update, :destroy, :mark_all_as_read] before_action :member_only, except: [:index, :show, :update, :mark_all_as_read]
def new def new
if params[:respond_to_id] if params[:respond_to_id]
@@ -18,7 +18,8 @@ class DmailsController < ApplicationController
if params[:folder] && params[:set_default_folder] if params[:folder] && params[:set_default_folder]
cookies.permanent[:dmail_folder] = params[:folder] cookies.permanent[:dmail_folder] = params[:folder]
end end
@dmails = Dmail.active.visible.paginated_search(params, count_pages: true)
@dmails = Dmail.visible.paginated_search(params, defaults: { is_spam: false, is_deleted: false }, count_pages: true)
respond_with(@dmails) respond_with(@dmails)
end end
@@ -43,14 +44,6 @@ class DmailsController < ApplicationController
respond_with(@dmail) respond_with(@dmail)
end end
def destroy
@dmail = Dmail.find(params[:id])
check_privilege(@dmail)
@dmail.mark_as_read!
@dmail.destroy
redirect_to dmails_path, :notice => "Message destroyed"
end
def mark_all_as_read def mark_all_as_read
Dmail.visible.unread.each do |x| Dmail.visible.unread.each do |x|
x.update_column(:is_read, true) x.update_column(:is_read, true)

View File

@@ -7,8 +7,10 @@ class ApplicationRecord < ActiveRecord::Base
extending(PaginationExtension).paginate(*options) extending(PaginationExtension).paginate(*options)
end end
def paginated_search(params, count_pages: params[:search].present?) def paginated_search(params, defaults: {}, count_pages: params[:search].present?)
search_params = params.fetch(:search, {}).permit! search_params = params.fetch(:search, {}).permit!
search_params = defaults.merge(search_params).with_indifferent_access
search(search_params).paginate(params[:page], limit: params[:limit], search_count: count_pages) search(search_params).paginate(params[:page], limit: params[:limit], search_count: count_pages)
end end
end end
@@ -93,7 +95,7 @@ class ApplicationRecord < ActiveRecord::Base
end end
def search_boolean_attribute(attribute, params) def search_boolean_attribute(attribute, params)
return all unless params[attribute] return all unless params.key?(attribute)
value = params[attribute].to_s value = params[attribute].to_s
if value.truthy? if value.truthy?

View File

@@ -136,8 +136,6 @@ class Dmail < ApplicationRecord
q = q.text_attribute_matches(:title, params[:title_matches]) q = q.text_attribute_matches(:title, params[:title_matches])
q = q.text_attribute_matches(:body, params[:message_matches], index_column: :message_index) q = q.text_attribute_matches(:body, params[:message_matches], index_column: :message_index)
params[:is_spam] = false unless params[:is_spam].present?
q = q.read if params[:read].to_s.truthy? q = q.read if params[:read].to_s.truthy?
q = q.unread if params[:read].to_s.falsy? q = q.unread if params[:read].to_s.falsy?

View File

@@ -4,6 +4,7 @@
<%= subnav_link_to "Received", received_dmails_path(set_default_folder: true) %> <%= subnav_link_to "Received", received_dmails_path(set_default_folder: true) %>
<%= subnav_link_to "Sent", sent_dmails_path(set_default_folder: true) %> <%= subnav_link_to "Sent", sent_dmails_path(set_default_folder: true) %>
<%= subnav_link_to "Spam", spam_dmails_path %> <%= subnav_link_to "Spam", spam_dmails_path %>
<%= subnav_link_to "Deleted", dmails_path(search: { is_deleted: true }) %>
<li>|</li> <li>|</li>
<%= subnav_link_to "New", new_dmail_path %> <%= subnav_link_to "New", new_dmail_path %>
<%= subnav_link_to "Mark all as read", {:controller => "dmails", :action => "mark_all_as_read"}, :method => :post, :remote => true %></li> <%= subnav_link_to "Mark all as read", {:controller => "dmails", :action => "mark_all_as_read"}, :method => :post, :remote => true %></li>

View File

@@ -27,7 +27,14 @@
<%= link_to dmail.title, dmail_path(dmail) %> <%= link_to dmail.title, dmail_path(dmail) %>
<% end %> <% end %>
<% t.column do |dmail| %> <% t.column do |dmail| %>
<%= link_to "delete", dmail_path(dmail), :method => :delete, :data => {:confirm => "Are you sure you want to delete this Dmail?"} %> <% if dmail.is_deleted? %>
<%= link_to "Undelete", dmail_path(dmail, format: :js), remote: true, method: :put, "data-params": "dmail[is_deleted]=false" %>
<% else %>
<%= link_to "Delete", dmail_path(dmail, format: :js), remote: true, method: :put, "data-params": "dmail[is_deleted]=true", "data-confirm": "Are you sure you want to delete this dmail?" %>
<% end %>
<% if dmail.reportable_by?(CurrentUser.user) %>
| <%= link_to "Report", new_moderation_report_path(moderation_report: { model_type: "Dmail", model_id: dmail.id }), remote: true, title: "Report this dmail to the moderators" %>
<% end %>
<% end %> <% end %>
<% end %> <% end %>

View File

@@ -110,7 +110,7 @@ Rails.application.routes.draw do
put :cancel put :cancel
end end
end end
resources :dmails, :only => [:new, :create, :update, :index, :show, :destroy] do resources :dmails, :only => [:new, :create, :update, :index, :show] do
collection do collection do
post :mark_all_as_read post :mark_all_as_read
end end

View File

@@ -94,18 +94,19 @@ class DmailsControllerTest < ActionDispatch::IntegrationTest
end end
end end
context "destroy action" do context "update action" do
should "allow deletion if the dmail is owned by the current user" do should "allow deletion if the dmail is owned by the current user" do
assert_difference("Dmail.count", -1) do put_auth dmail_path(@dmail), @user, params: { dmail: { is_deleted: true } }
delete_auth dmail_path(@dmail), @user
assert_redirected_to dmails_path assert_redirected_to dmail_path(@dmail)
end assert_equal(true, @dmail.reload.is_deleted)
end end
should "not allow deletion if the dmail is not owned by the current user" do should "not allow deletion if the dmail is not owned by the current user" do
assert_difference("Dmail.count", 0) do put_auth dmail_path(@dmail), @unrelated_user, params: { dmail: { is_deleted: true } }
delete_auth dmail_path(@dmail), @unrelated_user
end assert_response 403
assert_equal(false, @dmail.reload.is_deleted)
end end
end end
end end