Refactor flag types for more customization

This commit is contained in:
Robin Ward 2017-10-17 13:31:45 -04:00
parent e600fb79b3
commit 838568cbc3
26 changed files with 253 additions and 82 deletions

View File

@ -1,10 +0,0 @@
import computed from 'ember-addons/ember-computed-decorators';
export default Ember.Component.extend({
classNames: ['flag-counts'],
@computed('details.flag_type_id')
title(id) {
return I18n.t(`admin.flags.summary.action_type_${id}`, { count: 1 });
}
});

View File

@ -0,0 +1,12 @@
function postActionTitle([id, nameKey]) {
let title = I18n.t(`admin.flags.short_names.${nameKey}`, { defaultValue: null });
// TODO: We can remove this once other translations have been updated
if (!title) {
return I18n.t(`admin.flags.summary.action_type_${id}`, { count: 1 });
}
return title;
}
export default Ember.Helper.helper(postActionTitle);

View File

@ -1,2 +0,0 @@
<span class='type-name'>{{title}}</span>
<span class='type-count'>x{{details.count}}</span>

View File

@ -73,7 +73,7 @@
{{#each flaggedPost.post_actions as |postAction|}}
{{#flag-user user=postAction.user date=postAction.created_at}}
<div class='flagger-flag-type'>
{{i18n (concat "admin.flags.summary.action_type_" postAction.post_action_type_id) count=1}}
{{post-action-title postAction.post_action_type_id postAction.name_key}}
</div>
{{/flag-user}}
{{/each}}

View File

@ -19,7 +19,10 @@
</td>
<td>
{{#each ft.flag_counts as |fc|}}
{{flag-counts details=fc}}
<div class='flag-counts'>
<span class='type-name'>{{post-action-title fc.post_action_type_id fc.name_key}}</span>
<span class='type-count'>x{{fc.count}}</span>
</div>
{{/each}}
</td>
<td class='flagged-topic-users'>

View File

@ -1,9 +1,7 @@
import RestModel from 'discourse/models/rest';
const PostActionType = RestModel.extend({
notCustomFlag: Em.computed.not('is_custom_flag')
});
export const MAX_MESSAGE_LENGTH = 500;
export default PostActionType;
export default RestModel.extend({
notCustomFlag: Em.computed.not('is_custom_flag')
});

View File

@ -411,11 +411,11 @@ class Post < ActiveRecord::Base
end
def is_flagged?
post_actions.where(post_action_type_id: PostActionType.flag_types.values, deleted_at: nil).count != 0
post_actions.where(post_action_type_id: PostActionType.flag_types_without_custom.values, deleted_at: nil).count != 0
end
def has_active_flag?
post_actions.active.where(post_action_type_id: PostActionType.flag_types.values).count != 0
post_actions.active.where(post_action_type_id: PostActionType.flag_types_without_custom.values).count != 0
end
def unhide!

View File

@ -44,7 +44,7 @@ class PostAction < ActiveRecord::Base
def self.flag_count_by_date(start_date, end_date, category_id = nil)
result = where('post_actions.created_at >= ? AND post_actions.created_at <= ?', start_date, end_date)
result = result.where(post_action_type_id: PostActionType.flag_types.values)
result = result.where(post_action_type_id: PostActionType.flag_types_without_custom.values)
result = result.joins(post: :topic).where("topics.category_id = ?", category_id) if category_id
result.group('date(post_actions.created_at)')
.order('date(post_actions.created_at)')
@ -164,7 +164,7 @@ SQL
if moderator.id == Discourse::SYSTEM_USER_ID
PostActionType.auto_action_flag_types.values
else
PostActionType.flag_types.values
PostActionType.flag_types_without_custom.values
end
actions = PostAction.where(post_id: post.id)
@ -179,8 +179,13 @@ SQL
end
# reset all cached counters
f = action_type_ids.map { |t| ["#{PostActionType.types[t]}_count", 0] }
Post.with_deleted.where(id: post.id).update_all(Hash[*f.flatten])
cached = {}
action_type_ids.each do |atid|
column = "#{PostActionType.types[atid]}_count"
cached[column] = 0 if ActiveRecord::Base.connection.column_exists?(:posts, column)
end
Post.with_deleted.where(id: post.id).update_all(cached)
update_flagged_posts_count
end
@ -188,7 +193,7 @@ SQL
def self.defer_flags!(post, moderator, delete_post = false)
actions = PostAction.active
.where(post_id: post.id)
.where(post_action_type_id: PostActionType.flag_types.values)
.where(post_action_type_id: PostActionType.flag_types_without_custom.values)
actions.each do |action|
action.deferred_at = Time.zone.now
@ -355,7 +360,7 @@ SQL
end
def is_flag?
PostActionType.flag_types.values.include?(post_action_type_id)
!!PostActionType.flag_types[post_action_type_id]
end
def is_private_message?
@ -387,7 +392,7 @@ SQL
end
before_create do
post_action_type_ids = is_flag? ? PostActionType.flag_types.values : post_action_type_id
post_action_type_ids = is_flag? ? PostActionType.flag_types_without_custom.values : post_action_type_id
raise AlreadyActed if PostAction.where(user_id: user_id)
.where(post_id: post_id)
.where(post_action_type_id: post_action_type_ids)
@ -445,8 +450,10 @@ SQL
.sum("CASE WHEN users.moderator OR users.admin THEN #{SiteSetting.staff_like_weight} ELSE 1 END")
Post.where(id: post_id).update_all ["like_count = :count, like_score = :score", count: count, score: score]
else
if ActiveRecord::Base.connection.column_exists?(:posts, column)
Post.where(id: post_id).update_all ["#{column} = ?", count]
end
end
topic_id = Post.with_deleted.where(id: post_id).pluck(:topic_id).first
@ -583,7 +590,7 @@ SQL
end
def self.post_action_type_for_post(post_id)
post_action = PostAction.find_by(deferred_at: nil, post_id: post_id, post_action_type_id: PostActionType.flag_types.values, deleted_at: nil)
post_action = PostAction.find_by(deferred_at: nil, post_id: post_id, post_action_type_id: PostActionType.flag_types_without_custom.values, deleted_at: nil)
PostActionType.types[post_action.post_action_type_id]
end

View File

@ -1,5 +1,6 @@
require_dependency 'enum'
require_dependency 'distributed_cache'
require_dependency 'flag_settings'
class PostActionType < ActiveRecord::Base
after_save :expire_cache
@ -14,23 +15,72 @@ class PostActionType < ActiveRecord::Base
class << self
def flag_settings
unless @flag_settings
@flag_settings = FlagSettings.new
@flag_settings.add(
3,
:off_topic,
notify_type: true,
auto_action_type: true
)
@flag_settings.add(
4,
:inappropriate,
topic_type: true,
notify_type: true,
auto_action_type: true
)
@flag_settings.add(
8,
:spam,
topic_type: true,
notify_type: true,
auto_action_type: true
)
@flag_settings.add(
6,
:notify_user,
topic_type: true,
notify_type: true,
custom_type: true
)
@flag_settings.add(
7,
:notify_moderators,
topic_type: true,
notify_type: true,
custom_type: true
)
end
@flag_settings
end
def replace_flag_settings(settings)
@flag_settings = settings
@types = nil
end
def ordered
order('position asc')
end
def types
@types ||= Enum.new(bookmark: 1,
unless @types
@types = Enum.new(
bookmark: 1,
like: 2,
off_topic: 3,
inappropriate: 4,
vote: 5,
notify_user: 6,
notify_moderators: 7,
spam: 8)
vote: 5
)
@types.merge!(flag_settings.flag_types)
end
@types
end
def auto_action_flag_types
@auto_action_flag_types ||= flag_types.except(:notify_user, :notify_moderators)
flag_settings.auto_action_types
end
def public_types
@ -41,17 +91,29 @@ class PostActionType < ActiveRecord::Base
@public_type_ids ||= public_types.values
end
def flag_types_without_custom
flag_settings.without_custom_types
end
def flag_types
@flag_types ||= types.only(:off_topic, :spam, :inappropriate, :notify_moderators)
flag_settings.flag_types
end
# flags resulting in mod notifications
def notify_flag_type_ids
@notify_flag_type_ids ||= types.only(:off_topic, :spam, :inappropriate, :notify_moderators).values
notify_flag_types.values
end
def notify_flag_types
flag_settings.notify_types
end
def topic_flag_types
@topic_flag_types ||= types.only(:spam, :inappropriate, :notify_moderators)
flag_settings.topic_flag_types
end
def custom_types
flag_settings.custom_types
end
def is_flag?(sym)

View File

@ -175,7 +175,7 @@ class Report
# Post action counts:
def self.report_flags(report)
basic_report_about report, PostAction, :flag_count_by_date, report.start_date, report.end_date, report.category_id
countable = PostAction.where(post_action_type_id: PostActionType.flag_types.values)
countable = PostAction.where(post_action_type_id: PostActionType.flag_types_without_custom.values)
countable = countable.joins(post: :topic).where("topics.category_id = ?", report.category_id) if report.category_id
add_counts report, countable, 'post_actions.created_at'
end

View File

@ -615,7 +615,7 @@ class User < ActiveRecord::Base
end
def flags_given_count
PostAction.where(user_id: id, post_action_type_id: PostActionType.flag_types.values).count
PostAction.where(user_id: id, post_action_type_id: PostActionType.flag_types_without_custom.values).count
end
def warnings_received_count
@ -623,7 +623,7 @@ class User < ActiveRecord::Base
end
def flags_received_count
posts.includes(:post_actions).where('post_actions.post_action_type_id' => PostActionType.flag_types.values).count
posts.includes(:post_actions).where('post_actions.post_action_type_id' => PostActionType.flag_types_without_custom.values).count
end
def private_topics_count

View File

@ -15,7 +15,7 @@ class FlaggedTopicSummarySerializer < ActiveModel::Serializer
def flag_counts
object.flag_counts.map do |k, v|
{ flag_type_id: k, count: v }
{ post_action_type_id: k, count: v, name_key: PostActionType.types[k] }
end
end

View File

@ -2,13 +2,25 @@ require_dependency 'configurable_urls'
class PostActionTypeSerializer < ApplicationSerializer
attributes :name_key, :name, :description, :short_description, :long_form, :is_flag, :icon, :id, :is_custom_flag
attributes(
:id,
:name_key,
:name,
:description,
:short_description,
:long_form,
:is_flag,
:is_custom_flag
)
include ConfigurableUrls
def is_custom_flag
object.id == PostActionType.types[:notify_user] ||
object.id == PostActionType.types[:notify_moderators]
!!PostActionType.custom_types[object.id]
end
def is_flag
!!PostActionType.flag_types[object.id]
end
def name
@ -27,10 +39,14 @@ class PostActionTypeSerializer < ApplicationSerializer
i18n('short_description', tos_url: tos_path)
end
def name_key
PostActionType.types[object.id]
end
protected
def i18n(field, vars = nil)
key = "post_action_types.#{object.name_key}.#{field}"
key = "post_action_types.#{name_key}.#{field}"
vars ? I18n.t(key, vars) : I18n.t(key)
end

View File

@ -246,7 +246,7 @@ class PostSerializer < BasicPostSerializer
# The following only applies if you're logged in
if summary[:can_act] && scope.current_user.present?
summary[:can_defer_flags] = true if scope.is_staff? &&
PostActionType.flag_types.values.include?(id) &&
PostActionType.flag_types_without_custom.values.include?(id) &&
active_flags.present? && active_flags.has_key?(id) &&
active_flags[id].count > 0
end

View File

@ -52,13 +52,14 @@ class SiteSerializer < ApplicationSerializer
def post_action_types
cache_fragment("post_action_types_#{I18n.locale}") do
ActiveModel::ArraySerializer.new(PostActionType.ordered).as_json
types = PostActionType.types.values.map { |id| PostActionType.new(id: id) }
ActiveModel::ArraySerializer.new(types).as_json
end
end
def topic_flag_types
cache_fragment("post_action_flag_types_#{I18n.locale}") do
flags = PostActionType.ordered.where(name_key: ['inappropriate', 'spam', 'notify_moderators'])
flags = PostActionType.ordered.where(id: PostActionType.topic_flag_types.values)
ActiveModel::ArraySerializer.new(flags, each_serializer: TopicFlagTypeSerializer).as_json
end

View File

@ -66,7 +66,7 @@ class SpamRule::AutoBlock
def flagged_post_ids
Post.where(user_id: @user.id)
.where('spam_count > ? OR off_topic_count > ? OR inappropriate_count > ?', 0, 0, 0)
.where('spam_count > 0 OR off_topic_count > 0 OR inappropriate_count > 0')
.pluck(:id)
end

View File

@ -2666,22 +2666,12 @@ en:
users: "Users"
last_flagged: "Last Flagged"
summary:
action_type_3:
one: "off-topic"
other: "off-topic x{{count}}"
action_type_4:
one: "inappropriate"
other: "inappropriate x{{count}}"
action_type_6:
one: "custom"
other: "custom x{{count}}"
action_type_7:
one: "custom"
other: "custom x{{count}}"
action_type_8:
one: "spam"
other: "spam x{{count}}"
short_names:
off_topic: "off-topic"
inappropriate: "inappropriate"
spam: "spam"
notify_user: "custom"
notify_moderators: "custom"
groups:
primary: "Primary Group"

View File

@ -70,7 +70,7 @@ SQL
SELECT pa.user_id, min(pa.id) id
FROM post_actions pa
JOIN badge_posts p on p.id = pa.post_id
WHERE post_action_type_id IN (#{PostActionType.flag_types.values.join(",")}) AND
WHERE post_action_type_id IN (#{PostActionType.flag_types_without_custom.values.join(",")}) AND
(:backfill OR pa.post_id IN (:post_ids) )
GROUP BY pa.user_id
) x

42
lib/flag_settings.rb Normal file
View File

@ -0,0 +1,42 @@
class FlagSettings
attr_reader(
:without_custom_types,
:notify_types,
:topic_flag_types,
:auto_action_types,
:custom_types
)
def initialize
@all_flag_types = Enum.new
@topic_flag_types = Enum.new
@notify_types = Enum.new
@auto_action_types = Enum.new
@custom_types = Enum.new
@without_custom_types = Enum.new
end
def add(id, name, details = nil)
details ||= {}
@all_flag_types[name] = id
@topic_flag_types[name] = id if !!details[:topic_type]
@notify_types[name] = id if !!details[:notify_type]
@auto_action_types[name] = id if !!details[:auto_action_type]
if !!details[:custom_type]
@custom_types[name] = id
else
@without_custom_types[name] = id
end
end
def is_flag?(key)
@all_flag_types.valid?(key)
end
def flag_types
@all_flag_types
end
end

View File

@ -10,13 +10,14 @@ module PostGuardian
return false if (action_key == :notify_user && !is_staff? && opts[:is_warning].present? && opts[:is_warning] == 'true')
taken = opts[:taken_actions].try(:keys).to_a
is_flag = PostActionType.is_flag?(action_key)
is_flag = PostActionType.flag_types_without_custom[action_key]
already_taken_this_action = taken.any? && taken.include?(PostActionType.types[action_key])
already_did_flagging = taken.any? && (taken & PostActionType.flag_types.values).any?
already_did_flagging = taken.any? && (taken & PostActionType.flag_types_without_custom.values).any?
result = if authenticated? && post && !@user.anonymous?
return false if action_key == :notify_moderators && !SiteSetting.enable_private_messages
return false if [:notify_user, :notify_moderators].include?(action_key) &&
!SiteSetting.enable_private_messages?
# we allow flagging for trust level 1 and higher
# always allowed for private messages
@ -37,9 +38,6 @@ module PostGuardian
# new users can't notify_user because they are not allowed to send private messages
not(action_key == :notify_user && !@user.has_trust_level?(SiteSetting.min_trust_to_send_messages)) &&
# can't send private messages if they're disabled globally
not(action_key == :notify_user && !SiteSetting.enable_private_messages) &&
# no voting more than once on single vote topics
not(action_key == :vote && opts[:voted_in_topic] && post.topic.has_meta_data_boolean?(:single_vote))
end

View File

@ -90,6 +90,15 @@ class Plugin::Instance
end
end
def replace_flags
settings = ::FlagSettings.new
yield settings
reloadable_patch do |plugin|
::PostActionType.replace_flag_settings(settings) if plugin.enabled?
end
end
def whitelist_staff_user_custom_field(field)
reloadable_patch do |plugin|
::User.register_plugin_staff_custom_field(field, plugin) if plugin.enabled?

View File

@ -316,7 +316,7 @@ class PostRevisor
def remove_flags_and_unhide_post
return unless editing_a_flagged_and_hidden_post?
@post.post_actions.where(post_action_type_id: PostActionType.flag_types.values).each do |action|
@post.post_actions.where(post_action_type_id: PostActionType.flag_types_without_custom.values).each do |action|
action.remove_act!(Discourse.system_user)
end
@post.unhide!

View File

@ -184,7 +184,7 @@ after_initialize do
if self.user.enqueue_narrative_bot_job?
input =
case self.post_action_type_id
when *PostActionType.flag_types.values
when *PostActionType.flag_types_without_custom.values
:flag
when PostActionType.types[:like]
:like

View File

@ -0,0 +1,46 @@
require 'rails_helper'
require 'flag_settings'
RSpec.describe FlagSettings do
let(:settings) { FlagSettings.new }
describe 'add' do
it 'will add a type' do
settings.add(3, :off_topic)
expect(settings.flag_types).to include(:off_topic)
expect(settings.is_flag?(:off_topic)).to eq(true)
expect(settings.is_flag?(:vote)).to eq(false)
expect(settings.topic_flag_types).to be_empty
expect(settings.notify_types).to be_empty
expect(settings.auto_action_types).to be_empty
end
it 'will add a topic type' do
settings.add(4, :inappropriate, topic_type: true)
expect(settings.flag_types).to include(:inappropriate)
expect(settings.topic_flag_types).to include(:inappropriate)
expect(settings.without_custom_types).to include(:inappropriate)
end
it 'will add a notify type' do
settings.add(3, :off_topic, notify_type: true)
expect(settings.flag_types).to include(:off_topic)
expect(settings.notify_types).to include(:off_topic)
end
it 'will add an auto action type' do
settings.add(7, :notify_moderators, auto_action_type: true)
expect(settings.flag_types).to include(:notify_moderators)
expect(settings.auto_action_types).to include(:notify_moderators)
end
it 'will add a custom type' do
settings.add(7, :notify_user, custom_type: true)
expect(settings.flag_types).to include(:notify_user)
expect(settings.custom_types).to include(:notify_user)
expect(settings.without_custom_types).to be_empty
end
end
end

View File

@ -532,7 +532,7 @@ describe PostAction do
it "prevents user to act twice at the same time" do
# flags are already being tested
all_types_except_flags = PostActionType.types.except(PostActionType.flag_types)
all_types_except_flags = PostActionType.types.except(PostActionType.flag_types_without_custom)
all_types_except_flags.values.each do |action|
expect do
PostAction.act(eviltrout, post, action)

View File

@ -9,8 +9,7 @@ describe PostSerializer do
let(:admin) { Fabricate(:admin) }
let(:acted_ids) {
PostActionType.public_types.values
.concat([:notify_user, :spam]
.map { |k| PostActionType.types[k] })
.concat([:notify_user, :spam].map { |k| PostActionType.types[k] })
}
def visible_actions_for(user)