From cae9a5d7e3bf9f21d46fbb66a97d7c5d469eb521 Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 19 Jan 2020 12:55:58 -0600 Subject: [PATCH] Drop dmail filters. Few people used dmail filters (~900 users in 5 years) and even fewer used them correctly. Most people used them to try to block dmail spam, but usually they either blocked too much (by adding common words that are present in nearly all dmails, causing all mails to them to be filtered) or too little (blocking specific email addresses or urls, which usually are never seen again after the spammer is banned). Nowadays the spam detection system does a better job of filtering spam. --- .../user/dmail_filters_controller.rb | 33 ----------- app/controllers/users_controller.rb | 1 - app/models/dmail.rb | 11 ---- app/models/dmail_filter.rb | 27 --------- app/models/user.rb | 2 - app/views/dmails/index.html.erb | 12 +--- app/views/dmails/show.html.erb | 1 - .../user/dmail_filters/edit.html.erb | 34 ----------- app/views/users/edit.html.erb | 7 --- config/routes.rb | 1 - .../20200119184442_drop_dmail_filters.rb | 7 +++ db/structure.sql | 57 +------------------ .../user/dmail_filters_controller_test.rb | 35 ------------ test/unit/dmail_filter_test.rb | 57 ------------------- test/unit/dmail_test.rb | 43 -------------- 15 files changed, 11 insertions(+), 317 deletions(-) delete mode 100644 app/controllers/maintenance/user/dmail_filters_controller.rb delete mode 100644 app/models/dmail_filter.rb delete mode 100644 app/views/maintenance/user/dmail_filters/edit.html.erb create mode 100644 db/migrate/20200119184442_drop_dmail_filters.rb delete mode 100644 test/functional/maintenance/user/dmail_filters_controller_test.rb delete mode 100644 test/unit/dmail_filter_test.rb diff --git a/app/controllers/maintenance/user/dmail_filters_controller.rb b/app/controllers/maintenance/user/dmail_filters_controller.rb deleted file mode 100644 index 69eb58153..000000000 --- a/app/controllers/maintenance/user/dmail_filters_controller.rb +++ /dev/null @@ -1,33 +0,0 @@ -module Maintenance - module User - class DmailFiltersController < ApplicationController - before_action :ensure_ownership - respond_to :html, :json, :xml - - def edit - @dmail_filter = CurrentUser.dmail_filter || DmailFilter.new - end - - def update - @dmail_filter = CurrentUser.dmail_filter || DmailFilter.new - @dmail_filter.update(dmail_filter_params) - flash[:notice] = "Filter updated" - respond_with(@dmail) - end - - private - - def ensure_ownership - @dmail = Dmail.find(params[:dmail_id]) - - if @dmail.owner_id != CurrentUser.user.id - raise ::User::PrivilegeError.new - end - end - - def dmail_filter_params - params.require(:dmail_filter).permit(:words) - end - end - end -end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index aa1536934..cbe809266 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -118,7 +118,6 @@ class UsersController < ApplicationController enable_recommended_posts opt_out_tracking ] - permitted_params += [dmail_filter_attributes: %i[id words]] permitted_params << :name if context == :create permitted_params << :level if CurrentUser.is_admin? diff --git a/app/models/dmail.rb b/app/models/dmail.rb index 212426c4b..c1a9967b3 100644 --- a/app/models/dmail.rb +++ b/app/models/dmail.rb @@ -14,7 +14,6 @@ class Dmail < ApplicationRecord belongs_to :from, :class_name => "User" after_initialize :initialize_attributes, if: :new_record? - before_create :auto_read_if_filtered after_create :update_recipient after_commit :send_email, on: :create @@ -185,16 +184,6 @@ class Dmail < ApplicationRecord owner == to end - def filtered? - CurrentUser.dmail_filter.try(:filtered?, self) - end - - def auto_read_if_filtered - if is_recipient? && to.dmail_filter.try(:filtered?, self) - self.is_read = true - end - end - def update_recipient if is_recipient? && !is_deleted? && !is_read? to.update(has_mail: true, unread_dmail_count: to.dmails.unread.count) diff --git a/app/models/dmail_filter.rb b/app/models/dmail_filter.rb deleted file mode 100644 index 0c8d05b28..000000000 --- a/app/models/dmail_filter.rb +++ /dev/null @@ -1,27 +0,0 @@ -class DmailFilter < ApplicationRecord - extend Memoist - - belongs_to :user - before_validation :initialize_user - - def initialize_user - unless user_id - self.user_id = CurrentUser.user.id - end - end - - def filtered?(dmail) - dmail.from.level < User::Levels::MODERATOR && has_filter? && (dmail.body.match?(regexp) || dmail.title.match?(regexp) || dmail.from.name.match?(regexp)) - end - - def has_filter? - !words.strip.empty? - end - - def regexp - union = words.split(/[[:space:]]+/).map { |word| Regexp.escape(word) }.join("|") - /\b#{union}\b/i - end - - memoize :regexp -end diff --git a/app/models/user.rb b/app/models/user.rb index 63cf7974e..7510ff332 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -107,7 +107,6 @@ class User < ApplicationRecord has_one :recent_ban, -> {order("bans.id desc")}, :class_name => "Ban" has_one :api_key - has_one :dmail_filter has_one :super_voter has_one :token_bucket has_many :notes, foreign_key: :creator_id @@ -122,7 +121,6 @@ class User < ApplicationRecord has_many :tag_aliases, foreign_key: :creator_id has_many :tag_implications, foreign_key: :creator_id belongs_to :inviter, class_name: "User", optional: true - accepts_nested_attributes_for :dmail_filter enum theme: { light: 0, dark: 100 }, _suffix: true diff --git a/app/views/dmails/index.html.erb b/app/views/dmails/index.html.erb index 2050bd66e..757a34ecf 100644 --- a/app/views/dmails/index.html.erb +++ b/app/views/dmails/index.html.erb @@ -15,21 +15,13 @@ <%= compact_time(dmail.created_at) %> <% end %> <% t.column "From" do |dmail| %> - <% if dmail.filtered? %> - <%= link_to "[filtered]", user_path(dmail.from) %> - <% else %> - <%= link_to_user dmail.from %> - <% end %> + <%= link_to_user dmail.from %> <% end %> <% t.column "To" do |dmail| %> <%= link_to_user dmail.to %> <% end %> <% t.column "Subject", td: { class: "col-expand" } do |dmail| %> - <% if dmail.filtered? %> - <%= link_to "[filtered]", dmail_path(dmail) %> - <% else %> - <%= link_to dmail.title, dmail_path(dmail) %> - <% end %> + <%= link_to dmail.title, dmail_path(dmail) %> <% end %> <% t.column do |dmail| %> <%= link_to "delete", dmail_path(dmail), :method => :delete, :data => {:confirm => "Are you sure you want to delete this Dmail?"} %> diff --git a/app/views/dmails/show.html.erb b/app/views/dmails/show.html.erb index 9d092b02c..d68d45b57 100644 --- a/app/views/dmails/show.html.erb +++ b/app/views/dmails/show.html.erb @@ -24,7 +24,6 @@

<%= link_to "Respond", new_dmail_path(:respond_to_id => @dmail) %> | <%= link_to "Forward", new_dmail_path(:respond_to_id => @dmail, :forward => true) %> - | <%= link_to "Filter messages like these", edit_maintenance_user_dmail_filter_path(:dmail_id => @dmail.id) %> | <%= link_to "Permalink", dmail_path(@dmail, :key => @dmail.key), :title => "Use this URL to privately share with a moderator" %> <% if CurrentUser.is_gold? %> diff --git a/app/views/maintenance/user/dmail_filters/edit.html.erb b/app/views/maintenance/user/dmail_filters/edit.html.erb deleted file mode 100644 index 8a4149790..000000000 --- a/app/views/maintenance/user/dmail_filters/edit.html.erb +++ /dev/null @@ -1,34 +0,0 @@ -

-
-

Edit Message Filters

- -
-

<%= @dmail.title %>

- -
    -
  • Sender: <%= link_to_user @dmail.from %>
    • -
  • Recipient: <%= link_to_user @dmail.to, :raw => true %>
    • -
  • Date: <%= compact_time(@dmail.created_at) %>
    • -
- -

Body

-
- <%= format_text(@dmail.body) %> -
-
- - <%= edit_form_for @dmail_filter, :url => maintenance_user_dmail_filter_path(:dmail_id => @dmail.id) do |f| %> -
- - <%= text_area_tag "dmail_filter[words]", @dmail_filter.words, :id => "dmail_filter_words", :class => "text", :style => "height: 10em;" %> -

A list of banned words or users (space delimited). Any message you receive with a banned word will automatically be deleted. Make sure user names have no spaces in them (replace with underscores).

-
- - <%= f.button :submit, "Submit" %> - <% end %> -
-
- -<% content_for(:page_title) do %> - Edit Message Filters - <%= Danbooru.config.app_name %> -<% end %> diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index 9de157b6c..e7fccae9d 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -65,13 +65,6 @@ <%= f.input :disable_responsive_mode, :as => :select, :collection => [["No", "false"], ["Yes", "true"]], :include_blank => false, :hint => "Disable alternative layout for mobile and tablet" %> <%= f.input :opt_out_tracking, :as => :select, :collection => [["No", "false"], ["Yes", "true"]], :include_blank => false, :hint => "Opt out of tracking" %> -
- - <%= hidden_field_tag "user[dmail_filter_attributes][id]", @user.dmail_filter.try(:id) %> - <%= text_field_tag "user[dmail_filter_attributes][words]", @user.dmail_filter.try(:words), :id => "user_dmail_filter_attributes_words", :class => "text optional", :size => 40 %> - A list of banned words (space delimited). Any dmail you receive with a banned word will automatically be deleted. -
- <%= f.input :favorite_tags, :label => "Frequent tags", :hint => "A list of tags that you use often. They will appear when using the list of Related Tags.", :input_html => { :rows => 5, :data => { :autocomplete => "tag-query" } } %> <%= f.input :custom_style, :label => "Custom CSS style".html_safe, :hint => "CSS rules to apply to the whole site.", :input_html => {:size => "40x5"} %> diff --git a/config/routes.rb b/config/routes.rb index f7a750ca3..7bb4ae82b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -59,7 +59,6 @@ Rails.application.routes.draw do resource :password_reset, :only => [:new, :create, :edit, :update] resource :deletion, :only => [:show, :destroy] resource :email_change, :only => [:new, :create] - resource :dmail_filter, :only => [:edit, :update] resource :api_key, :only => [:show, :view, :update, :destroy] do post :view end diff --git a/db/migrate/20200119184442_drop_dmail_filters.rb b/db/migrate/20200119184442_drop_dmail_filters.rb new file mode 100644 index 000000000..57e6800ba --- /dev/null +++ b/db/migrate/20200119184442_drop_dmail_filters.rb @@ -0,0 +1,7 @@ +require_relative "20141120045943_create_dmail_filters" + +class DropDmailFilters < ActiveRecord::Migration[6.0] + def change + revert CreateDmailFilters + end +end diff --git a/db/structure.sql b/db/structure.sql index b545e73e6..3f524b6e9 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -807,38 +807,6 @@ CREATE SEQUENCE public.delayed_jobs_id_seq ALTER SEQUENCE public.delayed_jobs_id_seq OWNED BY public.delayed_jobs.id; --- --- Name: dmail_filters; Type: TABLE; Schema: public; Owner: - --- - -CREATE TABLE public.dmail_filters ( - id integer NOT NULL, - user_id integer NOT NULL, - words text NOT NULL, - created_at timestamp without time zone NOT NULL, - updated_at timestamp without time zone NOT NULL -); - - --- --- Name: dmail_filters_id_seq; Type: SEQUENCE; Schema: public; Owner: - --- - -CREATE SEQUENCE public.dmail_filters_id_seq - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - - --- --- Name: dmail_filters_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - --- - -ALTER SEQUENCE public.dmail_filters_id_seq OWNED BY public.dmail_filters.id; - - -- -- Name: dmails; Type: TABLE; Schema: public; Owner: - -- @@ -3330,13 +3298,6 @@ ALTER TABLE ONLY public.comments ALTER COLUMN id SET DEFAULT nextval('public.com ALTER TABLE ONLY public.delayed_jobs ALTER COLUMN id SET DEFAULT nextval('public.delayed_jobs_id_seq'::regclass); --- --- Name: dmail_filters id; Type: DEFAULT; Schema: public; Owner: - --- - -ALTER TABLE ONLY public.dmail_filters ALTER COLUMN id SET DEFAULT nextval('public.dmail_filters_id_seq'::regclass); - - -- -- Name: dmails id; Type: DEFAULT; Schema: public; Owner: - -- @@ -4385,14 +4346,6 @@ ALTER TABLE ONLY public.delayed_jobs ADD CONSTRAINT delayed_jobs_pkey PRIMARY KEY (id); --- --- Name: dmail_filters dmail_filters_pkey; Type: CONSTRAINT; Schema: public; Owner: - --- - -ALTER TABLE ONLY public.dmail_filters - ADD CONSTRAINT dmail_filters_pkey PRIMARY KEY (id); - - -- -- Name: dmails dmails_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -4932,13 +4885,6 @@ CREATE INDEX index_delayed_jobs_on_locked_by ON public.delayed_jobs USING btree CREATE INDEX index_delayed_jobs_on_run_at ON public.delayed_jobs USING btree (run_at); --- --- Name: index_dmail_filters_on_user_id; Type: INDEX; Schema: public; Owner: - --- - -CREATE UNIQUE INDEX index_dmail_filters_on_user_id ON public.dmail_filters USING btree (user_id); - - -- -- Name: index_dmails_on_created_at; Type: INDEX; Schema: public; Owner: - -- @@ -7479,6 +7425,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200114204550'), ('20200115010442'), ('20200117220602'), -('20200118015014'); +('20200118015014'), +('20200119184442'); diff --git a/test/functional/maintenance/user/dmail_filters_controller_test.rb b/test/functional/maintenance/user/dmail_filters_controller_test.rb deleted file mode 100644 index 2ca6c8b56..000000000 --- a/test/functional/maintenance/user/dmail_filters_controller_test.rb +++ /dev/null @@ -1,35 +0,0 @@ -require "test_helper" - -module Maintenance - module User - class DmailFiltersControllerTest < ActionDispatch::IntegrationTest - context "The dmail filters controller" do - setup do - @user1 = create(:user) - @user2 = create(:user) - end - - context "update action" do - setup do - as(@user1) do - @dmail = create(:dmail, owner: @user1) - end - end - - should "not allow a user to create a filter belonging to another user" do - params = { - :dmail_id => @dmail.id, - :dmail_filter => { - :words => "owned", - :user_id => @user2.id - } - } - - put_auth maintenance_user_dmail_filter_path, @user1, params: params - assert_not_equal("owned", @user2.reload.dmail_filter.try(&:words)) - end - end - end - end - end -end diff --git a/test/unit/dmail_filter_test.rb b/test/unit/dmail_filter_test.rb deleted file mode 100644 index c476ed6b5..000000000 --- a/test/unit/dmail_filter_test.rb +++ /dev/null @@ -1,57 +0,0 @@ -require 'test_helper' - -class DmailFilterTest < ActiveSupport::TestCase - def setup - super - - @receiver = FactoryBot.create(:user) - @sender = FactoryBot.create(:user) - end - - def create_dmail(body, title) - CurrentUser.scoped(@sender, "127.0.0.1") do - Dmail.create_split(:to_id => @receiver.id, :body => body, :title => title) - end - end - - context "a dmail filter for a word" do - setup do - @dmail_filter = @receiver.create_dmail_filter(:words => "banned") - end - - should "filter on that word in the body" do - create_dmail("banned", "okay") - assert_equal(true, @receiver.dmails.last.is_read?) - end - - should "filter on that word in the title" do - create_dmail("okay", "banned") - assert_equal(true, @receiver.dmails.last.is_read?) - end - - should "be case insensitive" do - create_dmail("Banned.", "okay") - assert_equal(true, @receiver.dmails.last.is_read?) - end - end - - context "a dmail filter for a user name" do - setup do - @dmail_filter = @receiver.create_dmail_filter(:words => @sender.name) - end - - should "filter on the sender" do - create_dmail("okay", "okay") - assert_equal(true, @receiver.dmails.last.is_read?) - end - end - - context "a dmail filter containing multiple words" do - should "filter dmails containing any of the words" do - @receiver.create_dmail_filter(words: "foo bar spam") - create_dmail("this is a test (not *SPAM*)", "hello world") - - assert_equal(true, @receiver.dmails.last.is_read?) - end - end -end diff --git a/test/unit/dmail_test.rb b/test/unit/dmail_test.rb index 98b4a5909..ab7fccc19 100644 --- a/test/unit/dmail_test.rb +++ b/test/unit/dmail_test.rb @@ -43,49 +43,6 @@ class DmailTest < ActiveSupport::TestCase end end - context "filter" do - setup do - @recipient = FactoryBot.create(:user) - @recipient.create_dmail_filter(:words => "banned") - @dmail = FactoryBot.build(:dmail, :title => "xxx", :owner => @recipient, :body => "banned word here", :to => @recipient, :from => @user) - end - - should "detect banned words" do - assert(@recipient.dmail_filter.filtered?(@dmail)) - end - - should "autoread if it has a banned word" do - @dmail.save - assert_equal(true, @dmail.is_read?) - end - - should "not update the recipient's has_mail if filtered" do - @dmail.save - @recipient.reload - assert_equal(false, @recipient.has_mail?) - end - - should "be ignored when sender is a moderator" do - CurrentUser.scoped(FactoryBot.create(:moderator_user), "127.0.0.1") do - @dmail = FactoryBot.create(:dmail, :owner => @recipient, :body => "banned word here", :to => @recipient) - end - - assert_equal(false, @recipient.dmail_filter.filtered?(@dmail)) - assert_equal(false, @dmail.is_read?) - assert_equal(true, @recipient.has_mail?) - end - - context "that is empty" do - setup do - @recipient.dmail_filter.update(words: " ") - end - - should "not filter everything" do - assert(!@recipient.dmail_filter.filtered?(@dmail)) - end - end - end - context "from a banned user" do setup do @user.update_attribute(:is_banned, true)