diff --git a/app/controllers/settings/keyword_mutes_controller.rb b/app/controllers/settings/keyword_mutes_controller.rb index 699b8a3ef..cf364a903 100644 --- a/app/controllers/settings/keyword_mutes_controller.rb +++ b/app/controllers/settings/keyword_mutes_controller.rb @@ -52,7 +52,7 @@ class Settings::KeywordMutesController < Settings::BaseController end def keyword_mute_params - params.require(:keyword_mute).permit(:keyword, :whole_word) + params.require(:keyword_mute).permit(:keyword, :whole_word, :apply_to_mentions) end def paginated_keyword_mutes_for_account diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 3a2dcac68..6eb278871 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -153,7 +153,7 @@ class FeedManager def filter_from_home?(status, receiver_id) return false if receiver_id == status.account_id return true if status.reply? && (status.in_reply_to_id.nil? || status.in_reply_to_account_id.nil?) - return true if keyword_filter?(status, receiver_id) + return true if keyword_filter_from_home?(status, receiver_id) check_for_mutes = [status.account_id] check_for_mutes.concat(status.mentions.pluck(:account_id)) @@ -182,8 +182,24 @@ class FeedManager false end - def keyword_filter?(status, receiver_id) - Glitch::KeywordMuteHelper.new(receiver_id).matches?(status) + def keyword_filter_from_home?(status, receiver_id) + # If this status mentions the receiver, use the mentions scope: it's + # possible that the status will show up in the receiver's mentions, which + # means it ought to show up in the home feed as well. + # + # If it doesn't mention the receiver but is still headed for the home feed, + # use the home feed scope. + scope = if status.mentions.pluck(:account_id).include?(receiver_id) + Glitch::KeywordMute::Scopes::Mentions + else + Glitch::KeywordMute::Scopes::HomeFeed + end + + return true if keyword_filter?(status, receiver_id, scope) + end + + def keyword_filter?(status, receiver_id, scope) + Glitch::KeywordMuteHelper.new(receiver_id).matches?(status, scope) end def filter_from_mentions?(status, receiver_id) @@ -197,7 +213,7 @@ class FeedManager should_filter = blocks_or_mutes?(receiver_id, check_for_blocks, :mentions) # Filter if it's from someone I blocked, in reply to someone I blocked, or mentioning someone I blocked (or muted) should_filter ||= (status.account.silenced? && !Follow.where(account_id: receiver_id, target_account_id: status.account_id).exists?) # of if the account is silenced and I'm not following them - should_filter ||= keyword_filter?(status, receiver_id) # or if the mention contains a muted keyword + should_filter ||= keyword_filter?(status, receiver_id, Glitch::KeywordMute::Scopes::Mentions) # or if the mention contains a muted keyword should_filter end diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb index 01dc48ee7..916261a17 100644 --- a/app/models/bookmark.rb +++ b/app/models/bookmark.rb @@ -3,11 +3,11 @@ # # Table name: bookmarks # -# id :integer not null, primary key +# id :bigint(8) not null, primary key +# account_id :bigint(8) not null +# status_id :bigint(8) not null # created_at :datetime not null # updated_at :datetime not null -# account_id :integer not null -# status_id :integer not null # class Bookmark < ApplicationRecord diff --git a/app/models/glitch/keyword_mute.rb b/app/models/glitch/keyword_mute.rb index 7d0002afd..49769cb73 100644 --- a/app/models/glitch/keyword_mute.rb +++ b/app/models/glitch/keyword_mute.rb @@ -3,12 +3,13 @@ # # Table name: glitch_keyword_mutes # -# id :integer not null, primary key -# account_id :integer not null -# keyword :string not null -# whole_word :boolean default(TRUE), not null -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint(8) not null, primary key +# account_id :bigint(8) not null +# keyword :string not null +# whole_word :boolean default(TRUE), not null +# created_at :datetime not null +# updated_at :datetime not null +# apply_to_mentions :boolean default(TRUE), not null # class Glitch::KeywordMute < ApplicationRecord @@ -18,6 +19,12 @@ class Glitch::KeywordMute < ApplicationRecord after_commit :invalidate_cached_matchers + module Scopes + Unscoped = 0b00 + HomeFeed = 0b01 + Mentions = 0b10 + end + def self.text_matcher_for(account_id) TextMatcher.new(account_id) end @@ -26,6 +33,13 @@ class Glitch::KeywordMute < ApplicationRecord TagMatcher.new(account_id) end + def scope + s = Scopes::Unscoped + s |= Scopes::HomeFeed + s |= Scopes::Mentions if apply_to_mentions? + s + end + private def invalidate_cached_matchers @@ -36,10 +50,12 @@ class Glitch::KeywordMute < ApplicationRecord class CachedKeywordMute attr_reader :keyword attr_reader :whole_word + attr_reader :scope - def initialize(keyword, whole_word) + def initialize(keyword, whole_word, scope) @keyword = keyword @whole_word = whole_word + @scope = scope end def boundary_regex_for_keyword @@ -49,26 +65,27 @@ class Glitch::KeywordMute < ApplicationRecord /(?mix:#{sb}#{Regexp.escape(keyword)}#{eb})/ end - def matches?(str) - str =~ (whole_word ? boundary_regex_for_keyword : /#{Regexp.escape(keyword)}/i) + def matches?(str, required_scope) + ((required_scope & scope) == required_scope) && \ + str =~ (whole_word ? boundary_regex_for_keyword : /#{Regexp.escape(keyword)}/i) end end class Matcher attr_reader :account_id - attr_reader :words + attr_reader :keywords def initialize(account_id) @account_id = account_id - @words = Rails.cache.fetch(self.class.cache_key(account_id)) { fetch_keywords } + @keywords = Rails.cache.fetch(self.class.cache_key(account_id)) { fetch_keywords } end protected def fetch_keywords - Glitch::KeywordMute.where(account_id: account_id).pluck(:whole_word, :keyword).map do |whole_word, keyword| - CachedKeywordMute.new(transform_keyword(keyword), whole_word) - end + Glitch::KeywordMute.select(:whole_word, :keyword, :apply_to_mentions) + .where(account_id: account_id) + .map { |kw| CachedKeywordMute.new(transform_keyword(kw.keyword), kw.whole_word, kw.scope) } end def transform_keyword(keyword) @@ -81,8 +98,8 @@ class Glitch::KeywordMute < ApplicationRecord format('keyword_mutes:regex:text:%s', account_id) end - def matches?(str) - words.any? { |w| w.matches?(str) } + def matches?(str, scope) + keywords.any? { |kw| kw.matches?(str, scope) } end end @@ -91,9 +108,9 @@ class Glitch::KeywordMute < ApplicationRecord format('keyword_mutes:regex:tag:%s', account_id) end - def matches?(tags) + def matches?(tags, scope) tags.pluck(:name).any? do |n| - words.any? { |w| w.matches?(n) } + keywords.any? { |kw| kw.matches?(n, scope) } end end diff --git a/app/models/glitch/keyword_mute_helper.rb b/app/models/glitch/keyword_mute_helper.rb index 6d067947f..955c3b1f3 100644 --- a/app/models/glitch/keyword_mute_helper.rb +++ b/app/models/glitch/keyword_mute_helper.rb @@ -9,16 +9,16 @@ class Glitch::KeywordMuteHelper @tag_matcher = Glitch::KeywordMute.tag_matcher_for(receiver_id) end - def matches?(status) - matchers_match?(status) || (status.reblog? && matchers_match?(status.reblog)) + def matches?(status, scope) + matchers_match?(status, scope) || (status.reblog? && matchers_match?(status.reblog, scope)) end private - def matchers_match?(status) - text_matcher.matches?(prepare_text(status.text)) || - text_matcher.matches?(prepare_text(status.spoiler_text)) || - tag_matcher.matches?(status.tags) + def matchers_match?(status, scope) + text_matcher.matches?(prepare_text(status.text), scope) || + text_matcher.matches?(prepare_text(status.spoiler_text), scope) || + tag_matcher.matches?(status.tags, scope) end def prepare_text(text) diff --git a/app/views/settings/keyword_mutes/_fields.html.haml b/app/views/settings/keyword_mutes/_fields.html.haml index 892676f18..843e9366a 100644 --- a/app/views/settings/keyword_mutes/_fields.html.haml +++ b/app/views/settings/keyword_mutes/_fields.html.haml @@ -2,6 +2,9 @@ = f.input :keyword = f.check_box :whole_word = f.label :whole_word, t('keyword_mutes.match_whole_word') + %br + = f.check_box :apply_to_mentions + = f.label :apply_to_mentions, t('keyword_mutes.apply_to_mentions') .actions - if f.object.persisted? diff --git a/app/views/settings/keyword_mutes/_keyword_mute.html.haml b/app/views/settings/keyword_mutes/_keyword_mute.html.haml index c45cc64fb..f9f6e3eb6 100644 --- a/app/views/settings/keyword_mutes/_keyword_mute.html.haml +++ b/app/views/settings/keyword_mutes/_keyword_mute.html.haml @@ -4,6 +4,9 @@ %td - if keyword_mute.whole_word %i.fa.fa-check + %td + - if keyword_mute.apply_to_mentions + %i.fa.fa-check %td = table_link_to 'edit', t('keyword_mutes.edit'), edit_settings_keyword_mute_path(keyword_mute) %td diff --git a/app/views/settings/keyword_mutes/index.html.haml b/app/views/settings/keyword_mutes/index.html.haml index 9ef8d55bc..7255f57b3 100644 --- a/app/views/settings/keyword_mutes/index.html.haml +++ b/app/views/settings/keyword_mutes/index.html.haml @@ -7,6 +7,7 @@ %tr %th= t('keyword_mutes.keyword') %th= t('keyword_mutes.match_whole_word') + %th= t('keyword_mutes.apply_to_mentions') %th %th %tbody diff --git a/config/locales/en.yml b/config/locales/en.yml index 66d181851..fdc64fd93 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -526,6 +526,7 @@ en: title: Invite people keyword_mutes: add_keyword: Add keyword + apply_to_mentions: Apply to mentions edit: Edit edit_keyword: Edit keyword keyword: Keyword diff --git a/db/migrate/20180604000556_add_apply_to_mentions_flag_to_keyword_mutes.rb b/db/migrate/20180604000556_add_apply_to_mentions_flag_to_keyword_mutes.rb new file mode 100644 index 000000000..cd97d0f20 --- /dev/null +++ b/db/migrate/20180604000556_add_apply_to_mentions_flag_to_keyword_mutes.rb @@ -0,0 +1,17 @@ +require 'mastodon/migration_helpers' + +class AddApplyToMentionsFlagToKeywordMutes < ActiveRecord::Migration[5.2] + include Mastodon::MigrationHelpers + + disable_ddl_transaction! + + def up + safety_assured do + add_column_with_default :glitch_keyword_mutes, :apply_to_mentions, :boolean, allow_null: false, default: true + end + end + + def down + remove_column :glitch_keyword_mutes, :apply_to_mentions + end +end diff --git a/db/schema.rb b/db/schema.rb index 709859ab8..b1e586944 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,8 +10,9 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2018_06_09_104432) do +ActiveRecord::Schema.define(version: 2018_06_09_104432) do + # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -206,6 +207,7 @@ ActiveRecord::Schema.define(version: 2018_06_09_104432) do t.boolean "whole_word", default: true, null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.boolean "apply_to_mentions", default: true, null: false t.index ["account_id"], name: "index_glitch_keyword_mutes_on_account_id" end @@ -466,7 +468,6 @@ ActiveRecord::Schema.define(version: 2018_06_09_104432) do t.bigint "application_id" t.bigint "in_reply_to_account_id" t.boolean "local_only" - t.text "full_status_text", default: "", null: false t.index ["account_id", "id", "visibility", "updated_at"], name: "index_statuses_20180106", order: { id: :desc } t.index ["conversation_id"], name: "index_statuses_on_conversation_id" t.index ["in_reply_to_id"], name: "index_statuses_on_in_reply_to_id" diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb index 3f91bba4e..db9bf12d1 100644 --- a/spec/lib/feed_manager_spec.rb +++ b/spec/lib/feed_manager_spec.rb @@ -175,7 +175,7 @@ RSpec.describe FeedManager do it 'returns true for a status with a tag that matches a muted keyword' do Fabricate('Glitch::KeywordMute', account: alice, keyword: 'jorts') status = Fabricate(:status, account: bob) - status.tags << Fabricate(:tag, name: 'jorts') + status.tags << Fabricate(:tag, name: 'jorts') expect(FeedManager.instance.filter?(:home, status, alice.id)).to be true end @@ -183,10 +183,18 @@ RSpec.describe FeedManager do it 'returns true for a status with a tag that matches an octothorpe-prefixed muted keyword' do Fabricate('Glitch::KeywordMute', account: alice, keyword: '#jorts') status = Fabricate(:status, account: bob) - status.tags << Fabricate(:tag, name: 'jorts') + status.tags << Fabricate(:tag, name: 'jorts') expect(FeedManager.instance.filter?(:home, status, alice.id)).to be true end + + it 'returns false if the status is muted by a keyword mute that does not apply to mentions' do + Fabricate('Glitch::KeywordMute', account: alice, keyword: 'take', apply_to_mentions: false) + status = Fabricate(:status, spoiler_text: 'This is a hot take', account: bob) + status.mentions.create!(account_id: alice.id) + + expect(FeedManager.instance.filter?(:home, status, alice.id)).to be false + end end context 'for mentions feed' do @@ -222,6 +230,13 @@ RSpec.describe FeedManager do bob.follow!(alice) expect(FeedManager.instance.filter?(:mentions, status, bob.id)).to be true end + + it 'returns false for a mention that contains a word muted by a keyword that does not apply to mentions' do + Fabricate('Glitch::KeywordMute', account: bob, keyword: 'take', apply_to_mentions: false) + status = Fabricate(:status, text: 'This is a hot take', account: alice) + bob.follow!(alice) + expect(FeedManager.instance.filter?(:mentions, status, bob.id)).to be false + end end end diff --git a/spec/models/glitch/keyword_mute_helper_spec.rb b/spec/models/glitch/keyword_mute_helper_spec.rb index b3f991d5b..f9408d62d 100644 --- a/spec/models/glitch/keyword_mute_helper_spec.rb +++ b/spec/models/glitch/keyword_mute_helper_spec.rb @@ -2,6 +2,8 @@ require 'rails_helper' RSpec.describe Glitch::KeywordMuteHelper do describe '#matches?' do + Unscoped = Glitch::KeywordMute::Scopes::Unscoped + let(:alice) { Fabricate(:account, username: 'alice').tap(&:save!) } let(:helper) { Glitch::KeywordMuteHelper.new(alice) } @@ -9,42 +11,42 @@ RSpec.describe Glitch::KeywordMuteHelper do status = Fabricate(:status, text: 'uh example') Glitch::KeywordMute.create!(account: alice, keyword: 'addr') - expect(helper.matches?(status)).to be false + expect(helper.matches?(status, Unscoped)).to be false end it 'ignores properties of HTML tags in status text' do status = Fabricate(:status, text: 'uh example') Glitch::KeywordMute.create!(account: alice, keyword: 'href') - expect(helper.matches?(status)).to be false + expect(helper.matches?(status, Unscoped)).to be false end it 'matches text inside HTML tags' do status = Fabricate(:status, text: '

HEY THIS IS SOMETHING ANNOYING

') Glitch::KeywordMute.create!(account: alice, keyword: 'annoying') - expect(helper.matches?(status)).to be true + expect(helper.matches?(status, Unscoped)).to be true end it 'matches < in HTML-stripped text' do status = Fabricate(:status, text: '

I <3 oats

') Glitch::KeywordMute.create!(account: alice, keyword: '<3') - expect(helper.matches?(status)).to be true + expect(helper.matches?(status, Unscoped)).to be true end it 'matches < in HTML text' do status = Fabricate(:status, text: '

I <3 oats

') Glitch::KeywordMute.create!(account: alice, keyword: '<3') - expect(helper.matches?(status)).to be true + expect(helper.matches?(status, Unscoped)).to be true end it 'matches link hrefs in HTML text' do status = Fabricate(:status, text: '

yep

') Glitch::KeywordMute.create!(account: alice, keyword: 'milk') - expect(helper.matches?(status)).to be true + expect(helper.matches?(status, Unscoped)).to be true end end end diff --git a/spec/models/glitch/keyword_mute_spec.rb b/spec/models/glitch/keyword_mute_spec.rb index 443832ac7..12c354738 100644 --- a/spec/models/glitch/keyword_mute_spec.rb +++ b/spec/models/glitch/keyword_mute_spec.rb @@ -4,6 +4,8 @@ RSpec.describe Glitch::KeywordMute, type: :model do let(:alice) { Fabricate(:account, username: 'alice').tap(&:save!) } let(:bob) { Fabricate(:account, username: 'bob').tap(&:save!) } + Unscoped = Glitch::KeywordMute::Scopes::Unscoped + describe '.text_matcher_for' do let(:matcher) { Glitch::KeywordMute.text_matcher_for(alice.id) } @@ -13,7 +15,7 @@ RSpec.describe Glitch::KeywordMute, type: :model do end it 'does not match' do - expect(matcher.matches?('This is a hot take')).to be_falsy + expect(matcher.matches?('This is a hot take', Unscoped)).to be_falsy end end @@ -21,87 +23,87 @@ RSpec.describe Glitch::KeywordMute, type: :model do it 'does not match keywords set by a different account' do Glitch::KeywordMute.create!(account: bob, keyword: 'take') - expect(matcher.matches?('This is a hot take')).to be_falsy + expect(matcher.matches?('This is a hot take', Unscoped)).to be_falsy end it 'does not match if no keywords match the status text' do Glitch::KeywordMute.create!(account: alice, keyword: 'cold') - expect(matcher.matches?('This is a hot take')).to be_falsy + expect(matcher.matches?('This is a hot take', Unscoped)).to be_falsy end it 'considers word boundaries when matching' do Glitch::KeywordMute.create!(account: alice, keyword: 'bob', whole_word: true) - expect(matcher.matches?('bobcats')).to be_falsy + expect(matcher.matches?('bobcats', Unscoped)).to be_falsy end it 'matches substrings if whole_word is false' do Glitch::KeywordMute.create!(account: alice, keyword: 'take', whole_word: false) - expect(matcher.matches?('This is a shiitake mushroom')).to be_truthy + expect(matcher.matches?('This is a shiitake mushroom', Unscoped)).to be_truthy end it 'matches keywords at the beginning of the text' do Glitch::KeywordMute.create!(account: alice, keyword: 'take') - expect(matcher.matches?('Take this')).to be_truthy + expect(matcher.matches?('Take this', Unscoped)).to be_truthy end it 'matches keywords at the end of the text' do Glitch::KeywordMute.create!(account: alice, keyword: 'take') - expect(matcher.matches?('This is a hot take')).to be_truthy + expect(matcher.matches?('This is a hot take', Unscoped)).to be_truthy end it 'matches if at least one keyword case-insensitively matches the text' do Glitch::KeywordMute.create!(account: alice, keyword: 'hot') - expect(matcher.matches?('This is a HOT take')).to be_truthy + expect(matcher.matches?('This is a HOT take', Unscoped)).to be_truthy end it 'matches if at least one non-whole-word keyword case-insensitively matches the text' do Glitch::KeywordMute.create!(account: alice, keyword: 'hot', whole_word: false) - expect(matcher.matches?('This is a HOTTY take')).to be_truthy + expect(matcher.matches?('This is a HOTTY take', Unscoped)).to be_truthy end it 'maintains case-insensitivity when combining keywords into a single matcher' do Glitch::KeywordMute.create!(account: alice, keyword: 'hot') Glitch::KeywordMute.create!(account: alice, keyword: 'cold') - expect(matcher.matches?('This is a HOT take')).to be_truthy + expect(matcher.matches?('This is a HOT take', Unscoped)).to be_truthy end it 'matches keywords surrounded by non-alphanumeric ornamentation' do Glitch::KeywordMute.create!(account: alice, keyword: 'hot') - expect(matcher.matches?('(hot take)')).to be_truthy + expect(matcher.matches?('(hot take)', Unscoped)).to be_truthy end it 'escapes metacharacters in whole-word keywords' do Glitch::KeywordMute.create!(account: alice, keyword: '(hot take)') - expect(matcher.matches?('(hot take)')).to be_truthy + expect(matcher.matches?('(hot take)', Unscoped)).to be_truthy end it 'escapes metacharacters in non-whole-word keywords' do Glitch::KeywordMute.create!(account: alice, keyword: '(-', whole_word: false) - expect(matcher.matches?('bad (-)')).to be_truthy + expect(matcher.matches?('bad (-)', Unscoped)).to be_truthy end it 'uses case-folding rules appropriate for more than just English' do Glitch::KeywordMute.create!(account: alice, keyword: 'großeltern') - expect(matcher.matches?('besuch der grosseltern')).to be_truthy + expect(matcher.matches?('besuch der grosseltern', Unscoped)).to be_truthy end it 'matches keywords that are composed of multiple words' do Glitch::KeywordMute.create!(account: alice, keyword: 'a shiitake') - expect(matcher.matches?('This is a shiitake')).to be_truthy - expect(matcher.matches?('This is shiitake')).to_not be_truthy + expect(matcher.matches?('This is a shiitake', Unscoped)).to be_truthy + expect(matcher.matches?('This is shiitake', Unscoped)).to_not be_truthy end end end @@ -118,7 +120,7 @@ RSpec.describe Glitch::KeywordMute, type: :model do it 'does not match' do status.tags << Fabricate(:tag, name: 'xyzzy') - expect(matcher.matches?(status.tags)).to be false + expect(matcher.matches?(status.tags, Unscoped)).to be false end end @@ -127,42 +129,42 @@ RSpec.describe Glitch::KeywordMute, type: :model do status.tags << Fabricate(:tag, name: 'xyzzy') Glitch::KeywordMute.create!(account: bob, keyword: 'take') - expect(matcher.matches?(status.tags)).to be false + expect(matcher.matches?(status.tags, Unscoped)).to be false end it 'matches #xyzzy when given the mute "#xyzzy"' do status.tags << Fabricate(:tag, name: 'xyzzy') Glitch::KeywordMute.create!(account: alice, keyword: '#xyzzy') - expect(matcher.matches?(status.tags)).to be true + expect(matcher.matches?(status.tags, Unscoped)).to be true end it 'matches #thingiverse when given the non-whole-word mute "#thing"' do status.tags << Fabricate(:tag, name: 'thingiverse') Glitch::KeywordMute.create!(account: alice, keyword: '#thing', whole_word: false) - expect(matcher.matches?(status.tags)).to be true + expect(matcher.matches?(status.tags, Unscoped)).to be true end it 'matches #hashtag when given the mute "##hashtag""' do status.tags << Fabricate(:tag, name: 'hashtag') Glitch::KeywordMute.create!(account: alice, keyword: '##hashtag') - expect(matcher.matches?(status.tags)).to be true + expect(matcher.matches?(status.tags, Unscoped)).to be true end it 'matches #oatmeal when given the non-whole-word mute "oat"' do status.tags << Fabricate(:tag, name: 'oatmeal') Glitch::KeywordMute.create!(account: alice, keyword: 'oat', whole_word: false) - expect(matcher.matches?(status.tags)).to be true + expect(matcher.matches?(status.tags, Unscoped)).to be true end it 'does not match #oatmeal when given the mute "#oat"' do status.tags << Fabricate(:tag, name: 'oatmeal') Glitch::KeywordMute.create!(account: alice, keyword: 'oat') - expect(matcher.matches?(status.tags)).to be false + expect(matcher.matches?(status.tags, Unscoped)).to be false end end end