FIX: serialize Flags instead of PostActionType (#28362)

### Why?
Before, all flags were static. Therefore, they were stored in class variables and serialized by SiteSerializer. Recently, we added an option for admins to add their own flags or disable existing flags. Therefore, the class variable had to be dropped because it was unsafe for a multisite environment. However, it started causing performance problems. 

### Solution
When a new Flag system is used, instead of using PostActionType, we can serialize Flags and use fragment cache for performance reasons. 

At the same time, we are still supporting deprecated `replace_flags` API call. When it is used, we fall back to the old solution and the admin cannot add custom flags. In a couple of months, we will be able to drop that API function and clean that code properly. However, because it may still be used, redis cache was introduced to improve performance.

To test backward compatibility you can add this code to any plugin
```ruby
  replace_flags do |flag_settings|
    flag_settings.add(
      4,
      :inappropriate,
      topic_type: true,
      notify_type: true,
      auto_action_type: true,
    )
    flag_settings.add(1001, :trolling, topic_type: true, notify_type: true, auto_action_type: true)
  end
```
This commit is contained in:
Krzysztof Kotlarek 2024-08-14 12:13:46 +10:00 committed by GitHub
parent ed11ee9d05
commit e82e255531
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
27 changed files with 595 additions and 227 deletions

View File

@ -29,7 +29,7 @@ class Admin::Config::FlagsController < Admin::AdminController
with_service(Flags::CreateFlag) do with_service(Flags::CreateFlag) do
on_success do on_success do
Discourse.request_refresh! Discourse.request_refresh!
render json: result.flag, serializer: FlagSerializer render json: result.flag, serializer: FlagSerializer, used_flag_ids: Flag.used_flag_ids
end end
on_failure { render(json: failed_json, status: 422) } on_failure { render(json: failed_json, status: 422) }
on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess } on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess }
@ -43,7 +43,7 @@ class Admin::Config::FlagsController < Admin::AdminController
with_service(Flags::UpdateFlag) do with_service(Flags::UpdateFlag) do
on_success do on_success do
Discourse.request_refresh! Discourse.request_refresh!
render json: result.flag, serializer: FlagSerializer render json: result.flag, serializer: FlagSerializer, used_flag_ids: Flag.used_flag_ids
end end
on_failure { render(json: failed_json, status: 422) } on_failure { render(json: failed_json, status: 422) }
on_model_not_found(:message) { raise Discourse::NotFound } on_model_not_found(:message) { raise Discourse::NotFound }

View File

@ -368,20 +368,24 @@ module Jobs
end end
end end
def post_action_type_view
@post_action_type_view ||= PostActionTypeView.new
end
def flags_export def flags_export
return enum_for(:flags_export) unless block_given? return enum_for(:flags_export) unless block_given?
PostAction PostAction
.with_deleted .with_deleted
.where(user_id: @current_user.id) .where(user_id: @current_user.id)
.where(post_action_type_id: PostActionType.flag_types.values) .where(post_action_type_id: post_action_type_view.flag_types.values)
.order(:created_at) .order(:created_at)
.each do |pa| .each do |pa|
yield( yield(
[ [
pa.id, pa.id,
pa.post_id, pa.post_id,
PostActionType.flag_types[pa.post_action_type_id], post_action_type_view.flag_types[pa.post_action_type_id],
pa.created_at, pa.created_at,
pa.updated_at, pa.updated_at,
pa.deleted_at, pa.deleted_at,
@ -400,7 +404,7 @@ module Jobs
PostAction PostAction
.with_deleted .with_deleted
.where(user_id: @current_user.id) .where(user_id: @current_user.id)
.where(post_action_type_id: PostActionType.types[:like]) .where(post_action_type_id: post_action_type_view.types[:like])
.order(:created_at) .order(:created_at)
.each do |pa| .each do |pa|
post = Post.with_deleted.find_by(id: pa.post_id) post = Post.with_deleted.find_by(id: pa.post_id)
@ -424,7 +428,8 @@ module Jobs
PostAction PostAction
.where(user_id: @current_user.id) .where(user_id: @current_user.id)
.where.not( .where.not(
post_action_type_id: PostActionType.flag_types.values + [PostActionType.types[:like]], post_action_type_id:
post_action_type_view.flag_types.values + [post_action_type_view.types[:like]],
) )
.exists? .exists?
end end
@ -435,7 +440,8 @@ module Jobs
.with_deleted .with_deleted
.where(user_id: @current_user.id) .where(user_id: @current_user.id)
.where.not( .where.not(
post_action_type_id: PostActionType.flag_types.values + [PostActionType.types[:like]], post_action_type_id:
post_action_type_view.flag_types.values + [post_action_type_view.types[:like]],
) )
.order(:created_at) .order(:created_at)
.each do |pa| .each do |pa|
@ -443,7 +449,7 @@ module Jobs
[ [
pa.id, pa.id,
pa.post_id, pa.post_id,
PostActionType.types[pa.post_action_type] || pa.post_action_type, post_action_type_view.types[pa.post_action_type] || pa.post_action_type,
pa.created_at, pa.created_at,
pa.updated_at, pa.updated_at,
pa.deleted_at, pa.deleted_at,

View File

@ -18,7 +18,9 @@ class Flag < ActiveRecord::Base
attr_accessor :skip_reset_flag_callback attr_accessor :skip_reset_flag_callback
default_scope { order(:position).where(score_type: false) } default_scope do
order(:position).where(score_type: false).where.not(id: PostActionType::LIKE_POST_ACTION_ID)
end
def used? def used?
PostAction.exists?(post_action_type_id: self.id) || PostAction.exists?(post_action_type_id: self.id) ||
@ -30,9 +32,14 @@ class Flag < ActiveRecord::Base
end end
def self.reset_flag_settings! def self.reset_flag_settings!
# Flags are memoized for better performance. After the update, we need to reload them in all processes. # Flags are cached in Redis for better performance. After the update,
# we need to reload them in all processes.
PostActionType.reload_types PostActionType.reload_types
MessageBus.publish("/reload_post_action_types", {}) end
def self.used_flag_ids
PostAction.distinct(:post_action_type_id).pluck(:post_action_type_id) |
ReviewableScore.distinct(:reviewable_score_type).pluck(:reviewable_score_type)
end end
def system? def system?

View File

@ -555,9 +555,13 @@ class Post < ActiveRecord::Base
flags.count != 0 flags.count != 0
end end
def post_action_type_view
@post_action_type_view ||= PostActionTypeView.new
end
def flags def flags
post_actions.where( post_actions.where(
post_action_type_id: PostActionType.flag_types_without_additional_message.values, post_action_type_id: post_action_type_view.flag_types_without_additional_message.values,
deleted_at: nil, deleted_at: nil,
) )
end end
@ -642,7 +646,7 @@ class Post < ActiveRecord::Base
edit_delay: SiteSetting.cooldown_minutes_after_hiding_posts, edit_delay: SiteSetting.cooldown_minutes_after_hiding_posts,
flag_reason: flag_reason:
I18n.t( I18n.t(
"flag_reasons.#{PostActionType.types[post_action_type_id]}", "flag_reasons.#{post_action_type_view.types[post_action_type_id]}",
locale: SiteSetting.default_locale, locale: SiteSetting.default_locale,
base_path: Discourse.base_path, base_path: Discourse.base_path,
), ),

View File

@ -144,17 +144,21 @@ class PostAction < ActiveRecord::Base
save save
end end
def post_action_type_view
@post_action_type_view ||= PostActionTypeView.new
end
def is_like? def is_like?
post_action_type_id == PostActionType.types[:like] post_action_type_id == post_action_type_view.types[:like]
end end
def is_flag? def is_flag?
!!PostActionType.notify_flag_types[post_action_type_id] !!post_action_type_view.notify_flag_types[post_action_type_id]
end end
def is_private_message? def is_private_message?
post_action_type_id == PostActionType.types[:notify_user] || post_action_type_id == post_action_type_view.types[:notify_user] ||
post_action_type_id == PostActionType.types[:notify_moderators] post_action_type_id == post_action_type_view.types[:notify_moderators]
end end
# A custom rate limiter for this model # A custom rate limiter for this model
@ -182,7 +186,8 @@ class PostAction < ActiveRecord::Base
end end
def ensure_unique_actions def ensure_unique_actions
post_action_type_ids = is_flag? ? PostActionType.notify_flag_types.values : post_action_type_id post_action_type_ids =
is_flag? ? post_action_type_view.notify_flag_types.values : post_action_type_id
acted = acted =
PostAction PostAction
@ -198,7 +203,7 @@ class PostAction < ActiveRecord::Base
end end
def post_action_type_key def post_action_type_key
PostActionType.types[post_action_type_id] post_action_type_view.types[post_action_type_id]
end end
def update_counters def update_counters

View File

@ -13,8 +13,18 @@ class PostActionType < ActiveRecord::Base
include AnonCacheInvalidator include AnonCacheInvalidator
def expire_cache def expire_cache
ApplicationSerializer.expire_cache_fragment!(/\Apost_action_types_/) Discourse.cache.redis.del(
ApplicationSerializer.expire_cache_fragment!(/\Apost_action_flag_types_/) *I18n.available_locales.map do |locale|
Discourse.cache.normalize_key("post_action_types_#{locale}")
end,
)
Discourse.cache.redis.del(
*I18n.available_locales.map do |locale|
Discourse.cache.normalize_key("post_action_flag_types_#{locale}")
end,
)
Discourse.cache.delete(POST_ACTION_TYPE_ALL_FLAGS_KEY)
Discourse.cache.delete(POST_ACTION_TYPE_PUBLIC_TYPE_IDS_KEY)
end end
class << self class << self
@ -29,117 +39,35 @@ class PostActionType < ActiveRecord::Base
@flag_settings = settings || FlagSettings.new @flag_settings = settings || FlagSettings.new
end end
def types
if overridden_by_plugin_or_skipped_db?
return Enum.new(like: 2).merge!(flag_settings.flag_types)
end
Enum.new(like: 2).merge(flag_types)
end
def expire_cache
Discourse.redis.keys("post_action_types_*").each { |key| Discourse.redis.del(key) }
Discourse.redis.keys("post_action_flag_types_*").each { |key| Discourse.redis.del(key) }
end
def reload_types def reload_types
@flag_settings = FlagSettings.new @flag_settings = FlagSettings.new
ReviewableScore.reload_types
PostActionType.new.expire_cache PostActionType.new.expire_cache
ReviewableScore.reload_types
end end
def overridden_by_plugin_or_skipped_db? %i[
flag_settings.flag_types.present? || GlobalSetting.skip_db? expire_cache
end all_flags
types
def all_flags overridden_by_plugin_or_skipped_db?
Flag.unscoped.order(:position).all auto_action_flag_types
end public_types
public_type_ids
def auto_action_flag_types flag_types_without_additional_message
return flag_settings.auto_action_types if overridden_by_plugin_or_skipped_db? flags
flag_enum(all_flags.select(&:auto_action_type)) flag_types
end score_types
notify_flag_type_ids
def public_types notify_flag_types
types.except(*flag_types.keys << :notify_user) topic_flag_types
end disabled_flag_types
additional_message_types
def public_type_ids names
@public_type_ids ||= public_types.values descriptions
end applies_to
is_flag?
def flag_types_without_additional_message ].each do |method_name|
return flag_settings.without_additional_message_types if overridden_by_plugin_or_skipped_db? define_method(method_name) { |*args| PostActionTypeView.new.send(method_name, *args) }
flag_enum(all_flags.reject(&:require_message))
end
def flag_types
return flag_settings.flag_types if overridden_by_plugin_or_skipped_db?
# Once replace_flag API is fully deprecated, then we can drop respond_to. It is needed right now for migration to be evaluated.
# TODO (krisk)
flag_enum(all_flags.reject { |flag| flag.respond_to?(:score_type) && flag.score_type })
end
def score_types
return flag_settings.flag_types if overridden_by_plugin_or_skipped_db?
# Once replace_flag API is fully deprecated, then we can drop respond_to. It is needed right now for migration to be evaluated.
# TODO (krisk)
flag_enum(all_flags.filter { |flag| flag.respond_to?(:score_type) && flag.score_type })
end
# flags resulting in mod notifications
def notify_flag_type_ids
notify_flag_types.values
end
def notify_flag_types
return flag_settings.notify_types if overridden_by_plugin_or_skipped_db?
flag_enum(all_flags.select(&:notify_type))
end
def topic_flag_types
if overridden_by_plugin_or_skipped_db?
flag_settings.topic_flag_types
else
flag_enum(all_flags.select { |flag| flag.applies_to?("Topic") })
end
end
def disabled_flag_types
flag_enum(all_flags.reject(&:enabled))
end
def enabled_flag_types
flag_enum(all_flags.filter(&:enabled))
end
def additional_message_types
return flag_settings.additional_message_types if overridden_by_plugin_or_skipped_db?
flag_enum(all_flags.select(&:require_message))
end
def names
all_flags.pluck(:id, :name).to_h
end
def descriptions
all_flags.pluck(:id, :description).to_h
end
def applies_to
all_flags.pluck(:id, :applies_to).to_h
end
def is_flag?(sym)
flag_types.valid?(sym)
end
private
def flag_enum(scope)
Enum.new(scope.map { |flag| [flag.name_key.to_sym, flag.id] }.to_h)
end end
end end

View File

@ -137,12 +137,16 @@ class ReviewableFlaggedPost < Reviewable
perform_ignore_and_do_nothing(performed_by, args) perform_ignore_and_do_nothing(performed_by, args)
end end
def post_action_type_view
@post_action_type_view ||= PostActionTypeView.new
end
def perform_ignore_and_do_nothing(performed_by, args) def perform_ignore_and_do_nothing(performed_by, args)
actions = actions =
PostAction PostAction
.active .active
.where(post_id: target_id) .where(post_id: target_id)
.where(post_action_type_id: PostActionType.notify_flag_type_ids) .where(post_action_type_id: post_action_type_view.notify_flag_type_ids)
actions.each do |action| actions.each do |action|
action.deferred_at = Time.zone.now action.deferred_at = Time.zone.now
@ -199,9 +203,9 @@ class ReviewableFlaggedPost < Reviewable
# -1 is the automatic system clear # -1 is the automatic system clear
action_type_ids = action_type_ids =
if performed_by.id == Discourse::SYSTEM_USER_ID if performed_by.id == Discourse::SYSTEM_USER_ID
PostActionType.auto_action_flag_types.values post_action_type_view.auto_action_flag_types.values
else else
PostActionType.notify_flag_type_ids post_action_type_view.notify_flag_type_ids
end end
actions = actions =
@ -218,7 +222,7 @@ class ReviewableFlaggedPost < Reviewable
# reset all cached counters # reset all cached counters
cached = {} cached = {}
action_type_ids.each do |atid| action_type_ids.each do |atid|
column = "#{PostActionType.types[atid]}_count" column = "#{post_action_type_view.types[atid]}_count"
cached[column] = 0 if ActiveRecord::Base.connection.column_exists?(:posts, column) cached[column] = 0 if ActiveRecord::Base.connection.column_exists?(:posts, column)
end end
@ -274,7 +278,7 @@ class ReviewableFlaggedPost < Reviewable
PostAction PostAction
.active .active
.where(post_id: target_id) .where(post_id: target_id)
.where(post_action_type_id: PostActionType.notify_flag_types.values) .where(post_action_type_id: post_action_type_view.notify_flag_types.values)
trigger_spam = false trigger_spam = false
actions.each do |action| actions.each do |action|
@ -285,7 +289,7 @@ class ReviewableFlaggedPost < Reviewable
action.save action.save
DB.after_commit do DB.after_commit do
action.add_moderator_post_if_needed(performed_by, :agreed, args[:post_was_deleted]) action.add_moderator_post_if_needed(performed_by, :agreed, args[:post_was_deleted])
trigger_spam = true if action.post_action_type_id == PostActionType.types[:spam] trigger_spam = true if action.post_action_type_id == post_action_type_view.types[:spam]
end end
end end
end end

View File

@ -107,10 +107,8 @@ class TranslationOverride < ActiveRecord::Base
end end
def self.expire_cache(locale, key) def self.expire_cache(locale, key)
if key.starts_with?("post_action_types.") if key.starts_with?("post_action_types.") || key.starts_with?("topic_flag_types.")
ApplicationSerializer.expire_cache_fragment!("post_action_types_#{locale}") PostActionType.new.expire_cache
elsif key.starts_with?("topic_flag_types.")
ApplicationSerializer.expire_cache_fragment!("post_action_flag_types_#{locale}")
else else
return false return false
end end

View File

@ -1229,10 +1229,14 @@ class User < ActiveRecord::Base
stat.increment!(:post_edits_count) stat.increment!(:post_edits_count)
end end
def post_action_type_view
@post_action_type_view ||= PostActionTypeView.new
end
def flags_given_count def flags_given_count
PostAction.where( PostAction.where(
user_id: id, user_id: id,
post_action_type_id: PostActionType.flag_types_without_additional_message.values, post_action_type_id: post_action_type_view.flag_types_without_additional_message.values,
).count ).count
end end
@ -1245,7 +1249,7 @@ class User < ActiveRecord::Base
.includes(:post_actions) .includes(:post_actions)
.where( .where(
"post_actions.post_action_type_id" => "post_actions.post_action_type_id" =>
PostActionType.flag_types_without_additional_message.values, post_action_type_view.flag_types_without_additional_message.values,
) )
.count .count
end end
@ -1459,7 +1463,7 @@ class User < ActiveRecord::Base
disagreed_flag_post_ids = disagreed_flag_post_ids =
PostAction PostAction
.where(post_action_type_id: PostActionType.types[:spam]) .where(post_action_type_id: post_action_type_view.types[:spam])
.where.not(disagreed_at: nil) .where.not(disagreed_at: nil)
.pluck(:post_id) .pluck(:post_id)
@ -1587,7 +1591,7 @@ class User < ActiveRecord::Base
PostAction PostAction
.where(user_id: self.id) .where(user_id: self.id)
.where(disagreed_at: nil) .where(disagreed_at: nil)
.where(post_action_type_id: PostActionType.notify_flag_type_ids) .where(post_action_type_id: post_action_type_view.notify_flag_type_ids)
.count .count
end end

View File

@ -1,5 +1,45 @@
# frozen_string_literal: true # frozen_string_literal: true
class FlagSerializer < ApplicationSerializer class FlagSerializer < ApplicationSerializer
attributes :id, :name, :name_key, :description, :applies_to, :position, :require_message, :enabled attributes :id,
:name,
:name_key,
:description,
:short_description,
:applies_to,
:position,
:require_message,
:enabled,
:is_flag,
:applies_to,
:is_used
def i18n_prefix
"#{@options[:target] || "post_action"}_types.#{object.name_key}"
end
def name
# system flags are using i18n translations when custom flags are using value entered by admin
I18n.t("#{i18n_prefix}.title", default: object.name)
end
def description
I18n.t("#{i18n_prefix}.description", default: object.description)
end
def short_description
I18n.t("#{i18n_prefix}.short_description", base_path: Discourse.base_path, default: "")
end
def is_flag
!object.score_type && object.id != PostActionType::LIKE_POST_ACTION_ID
end
def is_used
@options[:used_flag_ids].include?(object.id)
end
def applies_to
Array.wrap(object.applies_to)
end
end end

View File

@ -16,12 +16,16 @@ class PostActionTypeSerializer < ApplicationSerializer
include ConfigurableUrls include ConfigurableUrls
def post_action_type_view
@post_action_type_view ||= PostActionTypeView.new
end
def require_message def require_message
!!PostActionType.additional_message_types[object.id] !!post_action_type_view.additional_message_types[object.id]
end end
def is_flag def is_flag
!!PostActionType.flag_types[object.id] !!post_action_type_view.flag_types[object.id]
end end
def name def name
@ -42,15 +46,16 @@ class PostActionTypeSerializer < ApplicationSerializer
end end
def name_key def name_key
PostActionType.types[object.id].to_s post_action_type_view.types[object.id].to_s
end end
def enabled def enabled
!!PostActionType.enabled_flag_types[object.id] # flags added by API are always enabled
true
end end
def applies_to def applies_to
Array.wrap(PostActionType.applies_to[object.id]) Flag.valid_applies_to_types
end end
def is_used def is_used

View File

@ -290,10 +290,12 @@ class PostSerializer < BasicPostSerializer
result = [] result = []
can_see_post = scope.can_see_post?(object) can_see_post = scope.can_see_post?(object)
public_flag_types = @post_action_type_view =
(@topic_view.present? ? @topic_view.public_flag_types : PostActionType.public_types) @topic_view ? @topic_view.post_action_type_view : PostActionTypeView.new
(@topic_view.present? ? @topic_view.flag_types : PostActionType.types).each do |sym, id| public_flag_types = @post_action_type_view.public_types
@post_action_type_view.types.each do |sym, id|
count_col = "#{sym}_count".to_sym count_col = "#{sym}_count".to_sym
count = object.public_send(count_col) if object.respond_to?(count_col) count = object.public_send(count_col) if object.respond_to?(count_col)
@ -304,22 +306,9 @@ class PostSerializer < BasicPostSerializer
sym, sym,
opts: { opts: {
taken_actions: actions, taken_actions: actions,
notify_flag_types: notify_flag_types: @post_action_type_view.notify_flag_types,
( additional_message_types: @post_action_type_view.additional_message_types,
if @topic_view.present? post_action_type_view: @post_action_type_view,
@topic_view.notify_flag_types
else
PostActionType.notify_flag_types
end
),
additional_message_types:
(
if @topic_view.present?
@topic_view.additional_message_types
else
PostActionType.additional_message_types
end
),
}, },
can_see_post: can_see_post, can_see_post: can_see_post,
) )

View File

@ -111,17 +111,44 @@ class SiteSerializer < ApplicationSerializer
end end
def post_action_types def post_action_types
cache_fragment("post_action_types_#{I18n.locale}") do Discourse
types = ordered_flags(PostActionType.types.values) .cache
ActiveModel::ArraySerializer.new(types).as_json .fetch("post_action_types_#{I18n.locale}") do
end if PostActionType.overridden_by_plugin_or_skipped_db?
types = ordered_flags(PostActionType.types.values)
ActiveModel::ArraySerializer.new(types).as_json
else
ActiveModel::ArraySerializer.new(
Flag.unscoped.order(:position).where(score_type: false).all,
each_serializer: FlagSerializer,
target: :post_action,
used_flag_ids: Flag.used_flag_ids,
).as_json
end
end
end end
def topic_flag_types def topic_flag_types
cache_fragment("post_action_flag_types_#{I18n.locale}") do Discourse
types = ordered_flags(PostActionType.topic_flag_types.values) .cache
ActiveModel::ArraySerializer.new(types, each_serializer: TopicFlagTypeSerializer).as_json .fetch("post_action_flag_types_#{I18n.locale}") do
end if PostActionType.overridden_by_plugin_or_skipped_db?
types = ordered_flags(PostActionType.topic_flag_types.values)
ActiveModel::ArraySerializer.new(types, each_serializer: TopicFlagTypeSerializer).as_json
else
ActiveModel::ArraySerializer.new(
Flag
.unscoped
.where("'Topic' = ANY(applies_to)")
.where(score_type: false)
.order(:position)
.all,
each_serializer: FlagSerializer,
target: :topic_flag,
used_flag_ids: Flag.used_flag_ids,
).as_json
end
end
end end
def default_archetype def default_archetype

View File

@ -132,7 +132,3 @@ if Rails.env == "test" || $0 =~ /rake$/
# disable keepalive in testing # disable keepalive in testing
MessageBus.keepalive_interval = -1 MessageBus.keepalive_interval = -1
end end
if !Rails.env.test?
MessageBus.subscribe("/reload_post_action_types") { PostActionType.reload_types }
end

View File

@ -64,3 +64,13 @@ Flag.unscoped.seed do |s|
s.applies_to = %w[] s.applies_to = %w[]
s.skip_reset_flag_callback = true s.skip_reset_flag_callback = true
end end
Flag.unscoped.seed do |s|
s.id = 2
s.name = "like"
s.notify_type = false
s.auto_action_type = false
s.require_message = false
s.score_type = false
s.applies_to = %w[Post]
s.skip_reset_flag_callback = true
end

View File

@ -37,15 +37,18 @@ module PostGuardian
end end
taken = opts[:taken_actions].try(:keys).to_a taken = opts[:taken_actions].try(:keys).to_a
post_action_type_view = opts[:post_action_type_view] || PostActionTypeView.new
is_flag = is_flag =
if (opts[:notify_flag_types] && opts[:additional_message_types]) if (opts[:notify_flag_types] && opts[:additional_message_types])
opts[:notify_flag_types][action_key] || opts[:additional_message_types][action_key] opts[:notify_flag_types][action_key] || opts[:additional_message_types][action_key]
else else
PostActionType.notify_flag_types[action_key] || post_action_type_view.notify_flag_types[action_key] ||
PostActionType.additional_message_types[action_key] post_action_type_view.additional_message_types[action_key]
end end
already_taken_this_action = taken.any? && taken.include?(PostActionType.types[action_key]) already_taken_this_action =
already_did_flagging = taken.any? && (taken & PostActionType.notify_flag_types.values).any? taken.any? && taken.include?(post_action_type_view.types[action_key])
already_did_flagging =
taken.any? && (taken & post_action_type_view.notify_flag_types.values).any?
result = result =
if authenticated? && post if authenticated? && post
@ -61,7 +64,9 @@ module PostGuardian
# post made by staff, but we don't allow staff flags # post made by staff, but we don't allow staff flags
return false if is_flag && (!SiteSetting.allow_flagging_staff?) && post&.user&.staff? return false if is_flag && (!SiteSetting.allow_flagging_staff?) && post&.user&.staff?
return false if is_flag && PostActionType.disabled_flag_types.keys.include?(action_key) if is_flag && post_action_type_view.disabled_flag_types.keys.include?(action_key)
return false
end
if action_key == :notify_user && if action_key == :notify_user &&
!@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map) !@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map)
@ -111,12 +116,13 @@ module PostGuardian
return true if is_admin? return true if is_admin?
return false unless topic return false unless topic
type_symbol = PostActionType.types[post_action_type_id] post_action_type_view = PostActionTypeView.new
type_symbol = post_action_type_view.types[post_action_type_id]
return false if type_symbol == :bookmark return false if type_symbol == :bookmark
return false if type_symbol == :notify_user && !is_moderator? return false if type_symbol == :notify_user && !is_moderator?
return can_see_flags?(topic) if PostActionType.is_flag?(type_symbol) return can_see_flags?(topic) if post_action_type_view.is_flag?(type_symbol)
true true
end end

View File

@ -57,7 +57,8 @@ class PostActionCreator
@post = post @post = post
@post_action_type_id = post_action_type_id @post_action_type_id = post_action_type_id
@post_action_name = PostActionType.types[@post_action_type_id] @post_action_type_view = PostActionTypeView.new
@post_action_name = @post_action_type_view.types[@post_action_type_id]
@is_warning = is_warning @is_warning = is_warning
@take_action = take_action && guardian.is_staff? @take_action = take_action && guardian.is_staff?
@ -96,7 +97,7 @@ class PostActionCreator
if !post_can_act? || (@queue_for_review && !guardian.is_staff?) if !post_can_act? || (@queue_for_review && !guardian.is_staff?)
result.forbidden = true result.forbidden = true
if taken_actions&.keys&.include?(PostActionType.types[@post_action_name]) if taken_actions&.keys&.include?(@post_action_type_view.types[@post_action_name])
result.add_error(I18n.t("action_already_performed")) result.add_error(I18n.t("action_already_performed"))
else else
result.add_error(I18n.t("invalid_access")) result.add_error(I18n.t("invalid_access"))
@ -115,7 +116,7 @@ class PostActionCreator
# create meta topic / post if needed # create meta topic / post if needed
if @message.present? && if @message.present? &&
(PostActionType.additional_message_types.keys | %i[spam illegal]).include?( (@post_action_type_view.additional_message_types.keys | %i[spam illegal]).include?(
@post_action_name, @post_action_name,
) )
creator = create_message_creator creator = create_message_creator
@ -170,11 +171,11 @@ class PostActionCreator
private private
def flagging_post? def flagging_post?
PostActionType.notify_flag_type_ids.include?(@post_action_type_id) @post_action_type_view.notify_flag_type_ids.include?(@post_action_type_id)
end end
def cannot_flag_again?(reviewable) def cannot_flag_again?(reviewable)
return false if @post_action_type_id == PostActionType.types[:notify_moderators] return false if @post_action_type_id == @post_action_type_view.types[:notify_moderators]
flag_type_already_used = flag_type_already_used =
reviewable.reviewable_scores.any? do |rs| reviewable.reviewable_scores.any? do |rs|
rs.reviewable_score_type == @post_action_type_id && !rs.pending? rs.reviewable_score_type == @post_action_type_id && !rs.pending?
@ -233,7 +234,8 @@ class PostActionCreator
return if @post.hidden? return if @post.hidden?
return if !@created_by.staff? && @post.user&.staff? return if !@created_by.staff? && @post.user&.staff?
not_auto_action_flag_type = !PostActionType.auto_action_flag_types.include?(@post_action_name) not_auto_action_flag_type =
!@post_action_type_view.auto_action_flag_types.include?(@post_action_name)
return if not_auto_action_flag_type && !@queue_for_review return if not_auto_action_flag_type && !@queue_for_review
if @queue_for_review if @queue_for_review
@ -304,14 +306,14 @@ class PostActionCreator
if post_action if post_action
case @post_action_type_id case @post_action_type_id
when *PostActionType.notify_flag_type_ids when *@post_action_type_view.notify_flag_type_ids
DiscourseEvent.trigger(:flag_created, post_action, self) DiscourseEvent.trigger(:flag_created, post_action, self)
when PostActionType.types[:like] when @post_action_type_view.types[:like]
DiscourseEvent.trigger(:like_created, post_action, self) DiscourseEvent.trigger(:like_created, post_action, self)
end end
end end
if @post_action_type_id == PostActionType.types[:like] if @post_action_type_id == @post_action_type_view.types[:like]
GivenDailyLike.increment_for(@created_by.id) GivenDailyLike.increment_for(@created_by.id)
end end
@ -381,7 +383,7 @@ class PostActionCreator
target: @post, target: @post,
topic: @post.topic, topic: @post.topic,
reviewable_by_moderator: true, reviewable_by_moderator: true,
potential_spam: @post_action_type_id == PostActionType.types[:spam], potential_spam: @post_action_type_id == @post_action_type_view.types[:spam],
payload: { payload: {
targets_topic: @targets_topic, targets_topic: @targets_topic,
}, },

View File

@ -17,6 +17,10 @@ class PostActionDestroyer
new(destroyed_by, post, PostActionType.types[action_key], opts).perform new(destroyed_by, post, PostActionType.types[action_key], opts).perform
end end
def post_action_type_view
@post_action_type_view ||= PostActionTypeView.new
end
def perform def perform
result = DestroyResult.new result = DestroyResult.new
@ -50,14 +54,14 @@ class PostActionDestroyer
post_action.remove_act!(@destroyed_by) post_action.remove_act!(@destroyed_by)
post_action.post.unhide! if post_action.staff_took_action post_action.post.unhide! if post_action.staff_took_action
if @post_action_type_id == PostActionType.types[:like] if @post_action_type_id == post_action_type_view.types[:like]
GivenDailyLike.decrement_for(@destroyed_by.id) GivenDailyLike.decrement_for(@destroyed_by.id)
end end
case @post_action_type_id case @post_action_type_id
when *PostActionType.notify_flag_type_ids when *post_action_type_view.notify_flag_type_ids
DiscourseEvent.trigger(:flag_destroyed, post_action, self) DiscourseEvent.trigger(:flag_destroyed, post_action, self)
when PostActionType.types[:like] when post_action_type_view.types[:like]
DiscourseEvent.trigger(:like_destroyed, post_action, self) DiscourseEvent.trigger(:like_destroyed, post_action, self)
end end
@ -78,7 +82,7 @@ class PostActionDestroyer
end end
def notify_subscribers def notify_subscribers
name = PostActionType.types[@post_action_type_id] name = post_action_type_view.types[@post_action_type_id]
if name == :like if name == :like
@post.publish_change_to_clients!( @post.publish_change_to_clients!(
:unliked, :unliked,

View File

@ -0,0 +1,144 @@
# frozen_string_literal: true
class PostActionTypeView
ATTRIBUTE_NAMES = %i[
id
name
name_key
description
notify_type
auto_action_type
require_message
applies_to
position
enabled
score_type
]
def all_flags
@all_flags ||=
Discourse
.cache
.fetch(PostActionType::POST_ACTION_TYPE_ALL_FLAGS_KEY) do
Flag
.unscoped
.order(:position)
.pluck(ATTRIBUTE_NAMES)
.map { |attributes| ATTRIBUTE_NAMES.zip(attributes).to_h }
end
end
def flag_settings
@flag_settings ||= PostActionType.flag_settings
end
def types
if overridden_by_plugin_or_skipped_db?
return Enum.new(like: PostActionType::LIKE_POST_ACTION_ID).merge!(flag_settings.flag_types)
end
Enum.new(like: PostActionType::LIKE_POST_ACTION_ID).merge(flag_types)
end
def overridden_by_plugin_or_skipped_db?
flag_settings.flag_types.present? || GlobalSetting.skip_db?
end
def auto_action_flag_types
return flag_settings.auto_action_types if overridden_by_plugin_or_skipped_db?
flag_enum(all_flags.select { |flag| flag[:auto_action_type] })
end
def public_types
types.except(*flag_types.keys << :notify_user)
end
def public_type_ids
Discourse
.cache
.fetch(PostActionType::POST_ACTION_TYPE_PUBLIC_TYPE_IDS_KEY) { public_types.values }
end
def flag_types_without_additional_message
return flag_settings.without_additional_message_types if overridden_by_plugin_or_skipped_db?
flag_enum(flags.reject { |flag| flag[:require_message] })
end
def flags
all_flags.reject do |flag|
flag[:score_type] || flag[:id] == PostActionType::LIKE_POST_ACTION_ID
end
end
def flag_types
return flag_settings.flag_types if overridden_by_plugin_or_skipped_db?
flag_enum(flags)
end
def score_types
return flag_settings.flag_types if overridden_by_plugin_or_skipped_db?
flag_enum(all_flags.filter { |flag| flag[:score_type] })
end
# flags resulting in mod notifications
def notify_flag_type_ids
notify_flag_types.values
end
def notify_flag_types
return flag_settings.notify_types if overridden_by_plugin_or_skipped_db?
flag_enum(all_flags.select { |flag| flag[:notify_type] })
end
def topic_flag_types
if overridden_by_plugin_or_skipped_db?
flag_settings.topic_flag_types
else
flag_enum(all_flags.select { |flag| flag[:applies_to].include?("Topic") })
end
end
def disabled_flag_types
flag_enum(all_flags.reject { |flag| flag[:enabled] })
end
def additional_message_types
return flag_settings.additional_message_types if overridden_by_plugin_or_skipped_db?
flag_enum(all_flags.select { |flag| flag[:require_message] })
end
def names
all_flags.reduce({}) do |acc, f|
acc[f[:id]] = f[:name]
acc
end
end
def descriptions
all_flags.reduce({}) do |acc, f|
acc[f[:id]] = f[:description]
acc
end
end
def applies_to
all_flags.reduce({}) do |acc, f|
acc[f[:id]] = f[:applies_to]
acc
end
end
def is_flag?(sym)
flag_types.valid?(sym)
end
private
def flag_enum(scope)
Enum.new(
scope.reduce({}) do |acc, f|
acc[f[:name_key].to_sym] = f[:id]
acc
end,
)
end
end

View File

@ -348,6 +348,10 @@ class PostDestroyer
Jobs.enqueue(:feature_topic_users, topic_id: @post.topic_id) Jobs.enqueue(:feature_topic_users, topic_id: @post.topic_id)
end end
def post_action_type_view
@post_action_type_view ||= PostActionTypeView.new
end
def trash_public_post_actions def trash_public_post_actions
if public_post_actions = PostAction.publics.where(post_id: @post.id) if public_post_actions = PostAction.publics.where(post_id: @post.id)
public_post_actions.each { |pa| permanent? ? pa.destroy! : pa.trash!(@user) } public_post_actions.each { |pa| permanent? ? pa.destroy! : pa.trash!(@user) }
@ -357,7 +361,7 @@ class PostDestroyer
@post.custom_fields["deleted_public_actions"] = public_post_actions.ids @post.custom_fields["deleted_public_actions"] = public_post_actions.ids
@post.save_custom_fields @post.save_custom_fields
f = PostActionType.public_types.map { |k, _| ["#{k}_count", 0] } f = post_action_type_view.public_types.map { |k, _| ["#{k}_count", 0] }
Post.with_deleted.where(id: @post.id).update_all(Hash[*f.flatten]) Post.with_deleted.where(id: @post.id).update_all(Hash[*f.flatten])
end end
end end
@ -387,7 +391,7 @@ class PostDestroyer
# ReviewableScore#types is a superset of PostActionType#flag_types. # ReviewableScore#types is a superset of PostActionType#flag_types.
# If the reviewable score type is not on the latter, it means it's not a flag by a user and # If the reviewable score type is not on the latter, it means it's not a flag by a user and
# must be an automated flag like `needs_approval`. There's no flag reason for these kind of types. # must be an automated flag like `needs_approval`. There's no flag reason for these kind of types.
flag_type = PostActionType.flag_types[rs.reviewable_score_type] flag_type = post_action_type_view.flag_types[rs.reviewable_score_type]
return unless flag_type return unless flag_type
notify_responders = options[:notify_responders] notify_responders = options[:notify_responders]

View File

@ -598,17 +598,28 @@ class TopicView
ReviewableQueuedPost.pending.where(target_created_by: @user, topic: @topic).order(:created_at) ReviewableQueuedPost.pending.where(target_created_by: @user, topic: @topic).order(:created_at)
end end
def post_action_type_view
@post_action_type_view ||= PostActionTypeView.new
end
def actions_summary def actions_summary
return @actions_summary unless @actions_summary.nil? return @actions_summary unless @actions_summary.nil?
@actions_summary = [] @actions_summary = []
return @actions_summary unless post = posts&.first return @actions_summary unless post = posts&.first
PostActionType.topic_flag_types.each do |sym, id| post_action_type_view.topic_flag_types.each do |sym, id|
@actions_summary << { @actions_summary << {
id: id, id: id,
count: 0, count: 0,
hidden: false, hidden: false,
can_act: @guardian.post_can_act?(post, sym), can_act:
@guardian.post_can_act?(
post,
sym,
opts: {
post_action_type_view: post_action_type_view,
},
),
} }
end end
@ -623,22 +634,6 @@ class TopicView
@pm_params ||= TopicQuery.new(@user).get_pm_params(topic) @pm_params ||= TopicQuery.new(@user).get_pm_params(topic)
end end
def flag_types
@flag_types ||= PostActionType.types
end
def public_flag_types
@public_flag_types ||= PostActionType.public_types
end
def notify_flag_types
@notify_flag_types ||= PostActionType.notify_flag_types
end
def additional_message_types
@additional_message_types ||= PostActionType.additional_message_types
end
def suggested_topics def suggested_topics
if @include_suggested if @include_suggested
@suggested_topics ||= @suggested_topics ||=

View File

@ -0,0 +1,77 @@
# frozen_string_literal: true
RSpec.describe PostActionTypeView do
let(:post_action_type_view) { PostActionTypeView.new }
it "returns correct types" do
expect(post_action_type_view.flag_types).to eq(
{
illegal: 10,
inappropriate: 4,
notify_moderators: 7,
notify_user: 6,
off_topic: 3,
spam: 8,
},
)
expect(post_action_type_view.public_types).to eq({ like: 2 })
expect(post_action_type_view.notify_flag_types).to eq(
{ illegal: 10, inappropriate: 4, notify_moderators: 7, off_topic: 3, spam: 8 },
)
expect(post_action_type_view.topic_flag_types).to eq(
{ illegal: 10, inappropriate: 4, notify_moderators: 7, spam: 8 },
)
expect(post_action_type_view.additional_message_types).to eq(
{ illegal: 10, notify_moderators: 7, notify_user: 6 },
)
expect(post_action_type_view.score_types).to eq({ needs_approval: 9 })
flag = Fabricate(:flag, name: "custom", enabled: false)
expect(PostActionTypeView.new.disabled_flag_types).to eq({ custom: flag.id })
flag.destroy!
end
it "defines names of flags" do
expect(post_action_type_view.names).to eq(
{
6 => "notify_user",
3 => "off_topic",
4 => "inappropriate",
8 => "spam",
10 => "illegal",
7 => "notify_moderators",
9 => "needs_approval",
2 => "like",
},
)
end
it "defines descriptions of flags" do
flag = Fabricate(:flag, enabled: false, description: "custom flag description")
expect(post_action_type_view.descriptions[flag.id]).to eq("custom flag description")
flag.destroy!
end
it "defines where flags can be applies to" do
expect(post_action_type_view.applies_to).to eq(
{
6 => %w[Post Chat::Message],
3 => %w[Post Chat::Message],
4 => %w[Post Topic Chat::Message],
8 => %w[Post Topic Chat::Message],
10 => %w[Post Topic Chat::Message],
7 => %w[Post Topic Chat::Message],
9 => [],
2 => ["Post"],
},
)
end
it "defines is post action type is a flag" do
expect(post_action_type_view.is_flag?(:like)).to be false
expect(post_action_type_view.is_flag?(:off_topic)).to be true
end
end

View File

@ -70,4 +70,30 @@ RSpec.describe Flag, type: :model do
%i[notify_user off_topic inappropriate spam illegal notify_moderators needs_approval], %i[notify_user off_topic inappropriate spam illegal notify_moderators needs_approval],
) )
end end
describe ".used_flag_ids" do
fab!(:post_action) { Fabricate(:post_action, post_action_type_id: PostActionType.types[:like]) }
fab!(:post_action_2) do
Fabricate(:post_action, post_action_type_id: PostActionType.types[:like])
end
fab!(:post_action_3) do
Fabricate(:post_action, post_action_type_id: PostActionType.types[:off_topic])
end
fab!(:reviewable_score) do
Fabricate(:reviewable_score, reviewable_score_type: PostActionType.types[:off_topic])
end
fab!(:reviewable_score_2) do
Fabricate(:reviewable_score, reviewable_score_type: PostActionType.types[:illegal])
end
it "returns an array of unique flag ids" do
expect(Flag.used_flag_ids).to eq(
[
PostActionType.types[:like],
PostActionType.types[:off_topic],
PostActionType.types[:illegal],
],
)
end
end
end end

View File

@ -4,17 +4,15 @@ RSpec.describe PostActionType do
describe "Callbacks" do describe "Callbacks" do
describe "#expiry_cache" do describe "#expiry_cache" do
it "should clear the cache on save" do it "should clear the cache on save" do
cache = ApplicationSerializer.fragment_cache Discourse.cache.write("post_action_types_#{I18n.locale}", "test")
Discourse.cache.write("post_action_flag_types_#{I18n.locale}", "test2")
cache["post_action_types_#{I18n.locale}"] = "test"
cache["post_action_flag_types_#{I18n.locale}"] = "test2"
PostActionType.new(name_key: "some_key").save! PostActionType.new(name_key: "some_key").save!
expect(cache["post_action_types_#{I18n.locale}"]).to eq(nil) expect(Discourse.cache.read("post_action_types_#{I18n.locale}")).to eq(nil)
expect(cache["post_action_flag_types_#{I18n.locale}"]).to eq(nil) expect(Discourse.cache.read("post_action_flag_types_#{I18n.locale}")).to eq(nil)
ensure ensure
ApplicationSerializer.fragment_cache.clear PostActionType.new.expire_cache
end end
end end
end end
@ -30,7 +28,9 @@ RSpec.describe PostActionType do
end end
describe ".additional_message_types" do describe ".additional_message_types" do
before { described_class.stubs(:overridden_by_plugin_or_skipped_db?).returns(overriden) } before do
PostActionTypeView.any_instance.stubs(:overridden_by_plugin_or_skipped_db?).returns(overriden)
end
context "when overridden by plugin or skipped DB" do context "when overridden by plugin or skipped DB" do
let(:overriden) { true } let(:overriden) { true }

View File

@ -0,0 +1,24 @@
# frozen_string_literal: true
RSpec.describe "Custom flags in multisite", type: :multisite do
describe "PostACtionType#all_flags" do
use_redis_snapshotting
it "does not share flag definitions between sites" do
flag_1 = Flag.create!(name: "test flag 1", position: 99, applies_to: ["Post"])
test_multisite_connection("second") do
flag_2 = Flag.create!(name: "test flag 2", position: 99, applies_to: ["Post"])
PostActionType.new.expire_cache
expect(PostActionType.all_flags.last).to eq(
flag_2.attributes.except("created_at", "updated_at").transform_keys(&:to_sym),
)
end
PostActionType.new.expire_cache
expect(PostActionType.all_flags.last).to eq(
flag_1.attributes.except("created_at", "updated_at").transform_keys(&:to_sym),
)
end
end
end

View File

@ -362,6 +362,9 @@
}, },
"is_used": { "is_used": {
"type": "boolean" "type": "boolean"
},
"position": {
"type": "integer"
} }
}, },
"required": [ "required": [
@ -414,6 +417,9 @@
}, },
"is_used": { "is_used": {
"type": "boolean" "type": "boolean"
},
"position": {
"type": "integer"
} }
}, },
"required": [ "required": [

View File

@ -0,0 +1,57 @@
# frozen_string_literal: true
RSpec.describe FlagSerializer do
let(:flag) { Flag.find_by(name: "illegal") }
context "when system flag" do
it "returns translated name" do
serialized = described_class.new(flag, used_flag_ids: []).as_json
expect(serialized[:flag][:name]).to eq(I18n.t("post_action_types.illegal.title"))
end
it "returns translated description" do
serialized = described_class.new(flag, used_flag_ids: []).as_json
expect(serialized[:flag][:description]).to eq(I18n.t("post_action_types.illegal.description"))
end
end
context "when custom flag" do
fab!(:flag) { Fabricate(:flag, name: "custom title", description: "custom description") }
it "returns translated name" do
serialized = described_class.new(flag, used_flag_ids: []).as_json
expect(serialized[:flag][:name]).to eq("custom title")
end
it "returns translated description" do
serialized = described_class.new(flag, used_flag_ids: []).as_json
expect(serialized[:flag][:description]).to eq("custom description")
end
end
it "returns is_flag true for flags" do
serialized = described_class.new(flag, used_flag_ids: []).as_json
expect(serialized[:flag][:is_flag]).to be true
end
it "returns is_flag false for like" do
flag = Flag.unscoped.find_by(name: "like")
serialized = described_class.new(flag, used_flag_ids: []).as_json
expect(serialized[:flag][:is_flag]).to be false
end
it "returns is_used false when not used" do
serialized = described_class.new(flag, used_flag_ids: []).as_json
expect(serialized[:flag][:is_used]).to be false
end
it "returns is_used true when used" do
serialized = described_class.new(flag, used_flag_ids: [flag.id]).as_json
expect(serialized[:flag][:is_used]).to be true
end
it "returns applies_to" do
serialized = described_class.new(flag, used_flag_ids: []).as_json
expect(serialized[:flag][:applies_to]).to eq(%w[Post Topic Chat::Message])
end
end