FEATURE: Remove ignore feature SiteSetting and enable ignore by default (#7349)

This commit is contained in:
Tarek Khalil 2019-04-10 11:54:59 +01:00 committed by Régis Hanol
parent a723699519
commit 442fb2facb
13 changed files with 87 additions and 129 deletions

View File

@ -6,16 +6,14 @@
</div>
<div class="instructions">{{i18n 'user.muted_users_instructions'}}</div>
{{#if siteSettings.ignore_user_enabled}}
<div class="controls tracking-controls">
<label>{{d-icon "eye-slash" class="icon"}} {{i18n 'user.ignored_users'}}</label>
{{user-selector excludeCurrentUser=true
usernames=model.ignored_usernames
onChangeCallback=(action "ignoredUsernamesChanged")
class="user-selector"}}
</div>
<div class="instructions">{{i18n 'user.ignored_users_instructions'}}</div>
{{/if}}
<div class="controls tracking-controls">
<label>{{d-icon "eye-slash" class="icon"}} {{i18n 'user.ignored_users'}}</label>
{{user-selector excludeCurrentUser=true
usernames=model.ignored_usernames
onChangeCallback=(action "ignoredUsernamesChanged")
class="user-selector"}}
</div>
<div class="instructions">{{i18n 'user.ignored_users_instructions'}}</div>
</div>
{{plugin-outlet name="user-preferences-notifications" args=(hash model=model save=(action "save"))}}

View File

@ -993,7 +993,6 @@ class UsersController < ApplicationController
end
def notification_level
raise Discourse::NotFound unless SiteSetting.ignore_user_enabled
user = fetch_user_from_params
if params[:notification_level] == "ignore"

View File

@ -3,8 +3,6 @@ module Jobs
every 1.day
def execute(args)
return unless SiteSetting.ignore_user_enabled
params = {
threshold: SiteSetting.ignored_users_count_message_threshold,
gap_days: SiteSetting.ignored_users_message_gap_days,

View File

@ -1837,8 +1837,6 @@ en:
log_personal_messages_views: "Log personal message views by Admin for other users/groups."
ignore_user_enabled: "[Beta] Allow ignoring users."
ignored_users_count_message_threshold: "Notify moderators if a particular user is ignored by this many other users."
ignored_users_message_gap_days: "How long to wait before notifying moderators again about a user who has been ignored by many others."

View File

@ -529,9 +529,6 @@ users:
default: false
client: true
log_personal_messages_views: false
ignore_user_enabled:
default: false
client: true
ignored_users_count_message_threshold:
default: 5
client: true

View File

@ -0,0 +1,8 @@
class RemoveIgnoreUserEnabledSiteSetting < ActiveRecord::Migration[5.2]
def up
execute "DELETE FROM site_settings WHERE name = 'ignore_user_enabled'"
end
def down
end
end

View File

@ -407,7 +407,7 @@ class Guardian
def can_ignore_users?
return false if anonymous?
SiteSetting.ignore_user_enabled? && (@user.staff? || @user.trust_level >= TrustLevel.levels[:member])
@user.staff? || @user.trust_level >= TrustLevel.levels[:member]
end
def allow_themes?(theme_ids, include_preview: false)

View File

@ -602,24 +602,21 @@ class TopicView
@contains_gaps = false
@filtered_posts = unfiltered_posts
if SiteSetting.ignore_user_enabled
sql = <<~SQL
SELECT ignored_user_id
FROM ignored_users as ig
JOIN users as u ON u.id = ig.ignored_user_id
WHERE ig.user_id = :current_user_id
AND ig.ignored_user_id <> :current_user_id
AND NOT u.admin
AND NOT u.moderator
SQL
sql = <<~SQL
SELECT ignored_user_id
FROM ignored_users as ig
JOIN users as u ON u.id = ig.ignored_user_id
WHERE ig.user_id = :current_user_id
AND ig.ignored_user_id <> :current_user_id
AND NOT u.admin
AND NOT u.moderator
SQL
ignored_user_ids = DB.query_single(sql, current_user_id: @user&.id)
ignored_user_ids = DB.query_single(sql, current_user_id: @user&.id)
if ignored_user_ids.present?
@filtered_posts = @filtered_posts.where.not("user_id IN (?) AND id <> ?", ignored_user_ids, first_post_id)
@contains_gaps = true
end
if ignored_user_ids.present?
@filtered_posts = @filtered_posts.where.not("user_id IN (?) AND id <> ?", ignored_user_ids, first_post_id)
@contains_gaps = true
end
# Filters

View File

@ -2676,9 +2676,6 @@ describe Guardian do
end
describe '#can_ignore_user?' do
before do
SiteSetting.ignore_user_enabled = true
end
let(:guardian) { Guardian.new(trust_level_2) }

View File

@ -39,64 +39,48 @@ describe TopicView do
let!(:post2) { Fabricate(:post, topic: topic, user: evil_trout) }
let!(:post3) { Fabricate(:post, topic: topic, user: user) }
describe "when SiteSetting.ignore_user_enabled is false" do
it "does not filter out ignored user posts" do
SiteSetting.ignore_user_enabled = false
tv = TopicView.new(topic.id, evil_trout)
expect(tv.filtered_post_ids).to eq([post.id, post2.id, post3.id])
end
it "filters out ignored user posts" do
tv = TopicView.new(topic.id, evil_trout)
expect(tv.filtered_post_ids).to eq([post.id, post2.id])
end
describe "when SiteSetting.ignore_user_enabled is true" do
describe "when an ignored user made the original post" do
let!(:post) { Fabricate(:post, topic: topic, user: user) }
before do
SiteSetting.ignore_user_enabled = true
end
it "filters out ignored user posts" do
it "filters out ignored user posts only" do
tv = TopicView.new(topic.id, evil_trout)
expect(tv.filtered_post_ids).to eq([post.id, post2.id])
end
end
describe "when an ignored user made the original post" do
let!(:post) { Fabricate(:post, topic: topic, user: user) }
describe "when an anonymous user made a post" do
let(:anonymous) { Fabricate(:anonymous) }
let!(:post4) { Fabricate(:post, topic: topic, user: anonymous) }
it "filters out ignored user posts only" do
tv = TopicView.new(topic.id, evil_trout)
expect(tv.filtered_post_ids).to eq([post.id, post2.id])
end
it "filters out ignored user posts only" do
tv = TopicView.new(topic.id, evil_trout)
expect(tv.filtered_post_ids).to eq([post.id, post2.id, post4.id])
end
end
describe "when an anonymous user made a post" do
let(:anonymous) { Fabricate(:anonymous) }
let!(:post4) { Fabricate(:post, topic: topic, user: anonymous) }
describe "when an anonymous (non signed-in) user is viewing a Topic" do
let(:anonymous) { Fabricate(:anonymous) }
let!(:post4) { Fabricate(:post, topic: topic, user: anonymous) }
it "filters out ignored user posts only" do
tv = TopicView.new(topic.id, evil_trout)
expect(tv.filtered_post_ids).to eq([post.id, post2.id, post4.id])
end
it "filters out ignored user posts only" do
tv = TopicView.new(topic.id, nil)
expect(tv.filtered_post_ids).to eq([post.id, post2.id, post3.id, post4.id])
end
end
describe "when an anonymous (non signed-in) user is viewing a Topic" do
let(:anonymous) { Fabricate(:anonymous) }
let!(:post4) { Fabricate(:post, topic: topic, user: anonymous) }
describe "when a staff user is ignored" do
let!(:admin) { Fabricate(:user, admin: true) }
let!(:admin_ignored_user) { Fabricate(:ignored_user, user: evil_trout, ignored_user: admin) }
let!(:post4) { Fabricate(:post, topic: topic, user: admin) }
it "filters out ignored user posts only" do
tv = TopicView.new(topic.id, nil)
expect(tv.filtered_post_ids).to eq([post.id, post2.id, post3.id, post4.id])
end
end
describe "when a staff user is ignored" do
let!(:admin) { Fabricate(:user, admin: true) }
let!(:admin_ignored_user) { Fabricate(:ignored_user, user: evil_trout, ignored_user: admin) }
let!(:post4) { Fabricate(:post, topic: topic, user: admin) }
it "filters out ignored user excluding the staff user" do
tv = TopicView.new(topic.id, evil_trout)
expect(tv.filtered_post_ids).to eq([post.id, post2.id, post4.id])
end
it "filters out ignored user excluding the staff user" do
tv = TopicView.new(topic.id, evil_trout)
expect(tv.filtered_post_ids).to eq([post.id, post2.id, post4.id])
end
end
end

View File

@ -4,7 +4,6 @@ require_dependency 'jobs/scheduled/ignored_users_summary'
describe Jobs::IgnoredUsersSummary do
before do
SiteSetting.ignore_user_enabled = true
SiteSetting.ignored_users_count_message_threshold = 1
SiteSetting.ignored_users_message_gap_days = 365
end

View File

@ -2033,55 +2033,42 @@ describe UsersController do
sign_in(user)
end
context 'when ignore_user_enable is OFF' do
it 'raises an error when not logged in' do
put "/u/#{another_user.username}/notification_level.json", params: { notification_level: "" }
expect(response.status).to eq(404)
let!(:ignored_user) { Fabricate(:ignored_user, user: user, ignored_user: another_user) }
let!(:muted_user) { Fabricate(:muted_user, user: user, muted_user: another_user) }
context 'when changing notification level to normal' do
it 'changes notification level to normal' do
put "/u/#{another_user.username}/notification_level.json", params: { notification_level: "normal" }
expect(IgnoredUser.count).to eq(0)
expect(MutedUser.count).to eq(0)
end
end
context 'when ignore_user_enable is ON' do
before do
SiteSetting.ignore_user_enabled = true
context 'when changing notification level to mute' do
it 'changes notification level to mute' do
put "/u/#{another_user.username}/notification_level.json", params: { notification_level: "mute" }
expect(IgnoredUser.count).to eq(0)
expect(MutedUser.find_by(user_id: user.id, muted_user_id: another_user.id)).to be_present
end
end
context 'when changing notification level to ignore' do
it 'changes notification level to ignore' do
put "/u/#{another_user.username}/notification_level.json", params: { notification_level: "ignore" }
expect(MutedUser.count).to eq(0)
expect(IgnoredUser.find_by(user_id: user.id, ignored_user_id: another_user.id)).to be_present
end
let!(:ignored_user) { Fabricate(:ignored_user, user: user, ignored_user: another_user) }
let!(:muted_user) { Fabricate(:muted_user, user: user, muted_user: another_user) }
context 'when changing notification level to normal' do
it 'changes notification level to normal' do
put "/u/#{another_user.username}/notification_level.json", params: { notification_level: "normal" }
expect(IgnoredUser.count).to eq(0)
expect(MutedUser.count).to eq(0)
end
end
context 'when changing notification level to mute' do
it 'changes notification level to mute' do
put "/u/#{another_user.username}/notification_level.json", params: { notification_level: "mute" }
expect(IgnoredUser.count).to eq(0)
expect(MutedUser.find_by(user_id: user.id, muted_user_id: another_user.id)).to be_present
end
end
context 'when changing notification level to ignore' do
context 'when expiring_at param is set' do
it 'changes notification level to ignore' do
put "/u/#{another_user.username}/notification_level.json", params: { notification_level: "ignore" }
expect(MutedUser.count).to eq(0)
expect(IgnoredUser.find_by(user_id: user.id, ignored_user_id: another_user.id)).to be_present
end
freeze_time(Time.now) do
expiring_at = 3.days.from_now
put "/u/#{another_user.username}/notification_level.json", params: { notification_level: "ignore", expiring_at: expiring_at }
context 'when expiring_at param is set' do
it 'changes notification level to ignore' do
freeze_time(Time.now) do
expiring_at = 3.days.from_now
put "/u/#{another_user.username}/notification_level.json", params: { notification_level: "ignore", expiring_at: expiring_at }
ignored_user = IgnoredUser.find_by(user_id: user.id, ignored_user_id: another_user.id)
expect(ignored_user).to be_present
expect(ignored_user.expiring_at.to_i).to eq(expiring_at.to_i)
expect(MutedUser.count).to eq(0)
end
ignored_user = IgnoredUser.find_by(user_id: user.id, ignored_user_id: another_user.id)
expect(ignored_user).to be_present
expect(ignored_user.expiring_at.to_i).to eq(expiring_at.to_i)
expect(MutedUser.count).to eq(0)
end
end
end

View File

@ -35,10 +35,6 @@ describe UserUpdater do
end
describe '#update_ignored_users' do
before do
SiteSetting.ignore_user_enabled = true
end
it 'updates ignored users' do
u1 = Fabricate(:user, trust_level: 2)
u2 = Fabricate(:user, trust_level: 2)