REVERT: FIX: serialize Flags instead of PostActionType (#28334)

This commit is contained in:
Krzysztof Kotlarek 2024-08-13 18:32:11 +10:00 committed by GitHub
parent 0954ae70a6
commit 559c9dfe0a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
27 changed files with 231 additions and 590 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, used_flag_ids: Flag.used_flag_ids render json: result.flag, serializer: FlagSerializer
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, used_flag_ids: Flag.used_flag_ids render json: result.flag, serializer: FlagSerializer
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,24 +368,20 @@ 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: post_action_type_view.flag_types.values) .where(post_action_type_id: PostActionType.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,
post_action_type_view.flag_types[pa.post_action_type_id], PostActionType.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,
@ -404,7 +400,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: post_action_type_view.types[:like]) .where(post_action_type_id: PostActionType.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)
@ -428,8 +424,7 @@ module Jobs
PostAction PostAction
.where(user_id: @current_user.id) .where(user_id: @current_user.id)
.where.not( .where.not(
post_action_type_id: post_action_type_id: PostActionType.flag_types.values + [PostActionType.types[:like]],
post_action_type_view.flag_types.values + [post_action_type_view.types[:like]],
) )
.exists? .exists?
end end
@ -440,8 +435,7 @@ 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: post_action_type_id: PostActionType.flag_types.values + [PostActionType.types[:like]],
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|
@ -449,7 +443,7 @@ module Jobs
[ [
pa.id, pa.id,
pa.post_id, pa.post_id,
post_action_type_view.types[pa.post_action_type] || pa.post_action_type, PostActionType.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,9 +18,7 @@ class Flag < ActiveRecord::Base
attr_accessor :skip_reset_flag_callback attr_accessor :skip_reset_flag_callback
default_scope do default_scope { order(:position).where(score_type: false) }
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) ||
@ -32,14 +30,9 @@ class Flag < ActiveRecord::Base
end end
def self.reset_flag_settings! def self.reset_flag_settings!
# Flags are cached in Redis for better performance. After the update, # Flags are memoized for better performance. After the update, we need to reload them in all processes.
# we need to reload them in all processes.
PostActionType.reload_types PostActionType.reload_types
end MessageBus.publish("/reload_post_action_types", {})
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,13 +555,9 @@ 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: post_action_type_view.flag_types_without_additional_message.values, post_action_type_id: PostActionType.flag_types_without_additional_message.values,
deleted_at: nil, deleted_at: nil,
) )
end end
@ -646,7 +642,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.#{post_action_type_view.types[post_action_type_id]}", "flag_reasons.#{PostActionType.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,21 +144,17 @@ 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 == post_action_type_view.types[:like] post_action_type_id == PostActionType.types[:like]
end end
def is_flag? def is_flag?
!!post_action_type_view.notify_flag_types[post_action_type_id] !!PostActionType.notify_flag_types[post_action_type_id]
end end
def is_private_message? def is_private_message?
post_action_type_id == post_action_type_view.types[:notify_user] || post_action_type_id == PostActionType.types[:notify_user] ||
post_action_type_id == post_action_type_view.types[:notify_moderators] post_action_type_id == PostActionType.types[:notify_moderators]
end end
# A custom rate limiter for this model # A custom rate limiter for this model
@ -186,8 +182,7 @@ class PostAction < ActiveRecord::Base
end end
def ensure_unique_actions def ensure_unique_actions
post_action_type_ids = post_action_type_ids = is_flag? ? PostActionType.notify_flag_types.values : post_action_type_id
is_flag? ? post_action_type_view.notify_flag_types.values : post_action_type_id
acted = acted =
PostAction PostAction
@ -203,7 +198,7 @@ class PostAction < ActiveRecord::Base
end end
def post_action_type_key def post_action_type_key
post_action_type_view.types[post_action_type_id] PostActionType.types[post_action_type_id]
end end
def update_counters def update_counters

View File

@ -13,10 +13,8 @@ class PostActionType < ActiveRecord::Base
include AnonCacheInvalidator include AnonCacheInvalidator
def expire_cache def expire_cache
Discourse.cache.keys("post_action_types_*").each { |key| Discourse.redis.del(key) } ApplicationSerializer.expire_cache_fragment!(/\Apost_action_types_/)
Discourse.cache.keys("post_action_flag_types_*").each { |key| Discourse.redis.del(key) } ApplicationSerializer.expire_cache_fragment!(/\Apost_action_flag_types_/)
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
@ -31,35 +29,117 @@ class PostActionType < ActiveRecord::Base
@flag_settings = settings || FlagSettings.new @flag_settings = settings || FlagSettings.new
end end
def reload_types def types
@flag_settings = FlagSettings.new if overridden_by_plugin_or_skipped_db?
PostActionType.new.expire_cache return Enum.new(like: 2).merge!(flag_settings.flag_types)
ReviewableScore.reload_types end
Enum.new(like: 2).merge(flag_types)
end end
%i[ def expire_cache
expire_cache Discourse.redis.keys("post_action_types_*").each { |key| Discourse.redis.del(key) }
all_flags Discourse.redis.keys("post_action_flag_types_*").each { |key| Discourse.redis.del(key) }
types end
overridden_by_plugin_or_skipped_db?
auto_action_flag_types def reload_types
public_types @flag_settings = FlagSettings.new
public_type_ids ReviewableScore.reload_types
flag_types_without_additional_message PostActionType.new.expire_cache
flags end
flag_types
score_types def overridden_by_plugin_or_skipped_db?
notify_flag_type_ids flag_settings.flag_types.present? || GlobalSetting.skip_db?
notify_flag_types end
topic_flag_types
disabled_flag_types def all_flags
additional_message_types Flag.unscoped.order(:position).all
names end
descriptions
applies_to def auto_action_flag_types
is_flag? return flag_settings.auto_action_types if overridden_by_plugin_or_skipped_db?
].each do |method_name| flag_enum(all_flags.select(&:auto_action_type))
define_method(method_name) { |*args| PostActionTypeView.new.send(method_name, *args) } end
def public_types
types.except(*flag_types.keys << :notify_user)
end
def public_type_ids
@public_type_ids ||= 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(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,16 +137,12 @@ 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: post_action_type_view.notify_flag_type_ids) .where(post_action_type_id: PostActionType.notify_flag_type_ids)
actions.each do |action| actions.each do |action|
action.deferred_at = Time.zone.now action.deferred_at = Time.zone.now
@ -203,9 +199,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
post_action_type_view.auto_action_flag_types.values PostActionType.auto_action_flag_types.values
else else
post_action_type_view.notify_flag_type_ids PostActionType.notify_flag_type_ids
end end
actions = actions =
@ -222,7 +218,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 = "#{post_action_type_view.types[atid]}_count" column = "#{PostActionType.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
@ -278,7 +274,7 @@ class ReviewableFlaggedPost < Reviewable
PostAction PostAction
.active .active
.where(post_id: target_id) .where(post_id: target_id)
.where(post_action_type_id: post_action_type_view.notify_flag_types.values) .where(post_action_type_id: PostActionType.notify_flag_types.values)
trigger_spam = false trigger_spam = false
actions.each do |action| actions.each do |action|
@ -289,7 +285,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 == post_action_type_view.types[:spam] trigger_spam = true if action.post_action_type_id == PostActionType.types[:spam]
end end
end end
end end

View File

@ -107,8 +107,10 @@ 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.") || key.starts_with?("topic_flag_types.") if key.starts_with?("post_action_types.")
PostActionType.new.expire_cache ApplicationSerializer.expire_cache_fragment!("post_action_types_#{locale}")
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,14 +1229,10 @@ 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: post_action_type_view.flag_types_without_additional_message.values, post_action_type_id: PostActionType.flag_types_without_additional_message.values,
).count ).count
end end
@ -1249,7 +1245,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" =>
post_action_type_view.flag_types_without_additional_message.values, PostActionType.flag_types_without_additional_message.values,
) )
.count .count
end end
@ -1463,7 +1459,7 @@ class User < ActiveRecord::Base
disagreed_flag_post_ids = disagreed_flag_post_ids =
PostAction PostAction
.where(post_action_type_id: post_action_type_view.types[:spam]) .where(post_action_type_id: PostActionType.types[:spam])
.where.not(disagreed_at: nil) .where.not(disagreed_at: nil)
.pluck(:post_id) .pluck(:post_id)
@ -1591,7 +1587,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: post_action_type_view.notify_flag_type_ids) .where(post_action_type_id: PostActionType.notify_flag_type_ids)
.count .count
end end

View File

@ -1,45 +1,5 @@
# frozen_string_literal: true # frozen_string_literal: true
class FlagSerializer < ApplicationSerializer class FlagSerializer < ApplicationSerializer
attributes :id, attributes :id, :name, :name_key, :description, :applies_to, :position, :require_message, :enabled
: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,16 +16,12 @@ 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
!!post_action_type_view.additional_message_types[object.id] !!PostActionType.additional_message_types[object.id]
end end
def is_flag def is_flag
!!post_action_type_view.flag_types[object.id] !!PostActionType.flag_types[object.id]
end end
def name def name
@ -46,16 +42,15 @@ class PostActionTypeSerializer < ApplicationSerializer
end end
def name_key def name_key
post_action_type_view.types[object.id].to_s PostActionType.types[object.id].to_s
end end
def enabled def enabled
# flags added by API are always enabled !!PostActionType.enabled_flag_types[object.id]
true
end end
def applies_to def applies_to
Flag.valid_applies_to_types Array.wrap(PostActionType.applies_to[object.id])
end end
def is_used def is_used

View File

@ -290,12 +290,10 @@ class PostSerializer < BasicPostSerializer
result = [] result = []
can_see_post = scope.can_see_post?(object) can_see_post = scope.can_see_post?(object)
@post_action_type_view = public_flag_types =
@topic_view ? @topic_view.post_action_type_view : PostActionTypeView.new (@topic_view.present? ? @topic_view.public_flag_types : PostActionType.public_types)
public_flag_types = @post_action_type_view.public_types (@topic_view.present? ? @topic_view.flag_types : PostActionType.types).each do |sym, id|
@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)
@ -306,9 +304,22 @@ class PostSerializer < BasicPostSerializer
sym, sym,
opts: { opts: {
taken_actions: actions, taken_actions: actions,
notify_flag_types: @post_action_type_view.notify_flag_types, notify_flag_types:
additional_message_types: @post_action_type_view.additional_message_types, (
post_action_type_view: @post_action_type_view, if @topic_view.present?
@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,43 +111,16 @@ class SiteSerializer < ApplicationSerializer
end end
def post_action_types def post_action_types
Discourse cache_fragment("post_action_types_#{I18n.locale}") do
.cache
.fetch("post_action_types_#{I18n.locale}") do
if PostActionType.overridden_by_plugin_or_skipped_db?
types = ordered_flags(PostActionType.types.values) types = ordered_flags(PostActionType.types.values)
ActiveModel::ArraySerializer.new(types).as_json 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 end
def topic_flag_types def topic_flag_types
Discourse cache_fragment("post_action_flag_types_#{I18n.locale}") do
.cache
.fetch("post_action_flag_types_#{I18n.locale}") do
if PostActionType.overridden_by_plugin_or_skipped_db?
types = ordered_flags(PostActionType.topic_flag_types.values) types = ordered_flags(PostActionType.topic_flag_types.values)
ActiveModel::ArraySerializer.new(types, each_serializer: TopicFlagTypeSerializer).as_json 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 end

View File

@ -132,3 +132,7 @@ 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

@ -1,4 +1,5 @@
# frozen_string_literal: true # frozen_string_literal: true
Flag.seed do |s| Flag.seed do |s|
s.id = 6 s.id = 6
s.name = "notify_user" s.name = "notify_user"
@ -63,13 +64,3 @@ 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,18 +37,15 @@ 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
post_action_type_view.notify_flag_types[action_key] || PostActionType.notify_flag_types[action_key] ||
post_action_type_view.additional_message_types[action_key] PostActionType.additional_message_types[action_key]
end end
already_taken_this_action = already_taken_this_action = taken.any? && taken.include?(PostActionType.types[action_key])
taken.any? && taken.include?(post_action_type_view.types[action_key]) already_did_flagging = taken.any? && (taken & PostActionType.notify_flag_types.values).any?
already_did_flagging =
taken.any? && (taken & post_action_type_view.notify_flag_types.values).any?
result = result =
if authenticated? && post if authenticated? && post
@ -64,9 +61,7 @@ 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?
if is_flag && post_action_type_view.disabled_flag_types.keys.include?(action_key) return false if is_flag && PostActionType.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)
@ -116,13 +111,12 @@ module PostGuardian
return true if is_admin? return true if is_admin?
return false unless topic return false unless topic
post_action_type_view = PostActionTypeView.new type_symbol = PostActionType.types[post_action_type_id]
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 post_action_type_view.is_flag?(type_symbol) return can_see_flags?(topic) if PostActionType.is_flag?(type_symbol)
true true
end end

View File

@ -57,8 +57,7 @@ class PostActionCreator
@post = post @post = post
@post_action_type_id = post_action_type_id @post_action_type_id = post_action_type_id
@post_action_type_view = PostActionTypeView.new @post_action_name = PostActionType.types[@post_action_type_id]
@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?
@ -97,7 +96,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?(@post_action_type_view.types[@post_action_name]) if taken_actions&.keys&.include?(PostActionType.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"))
@ -116,7 +115,7 @@ class PostActionCreator
# create meta topic / post if needed # create meta topic / post if needed
if @message.present? && if @message.present? &&
(@post_action_type_view.additional_message_types.keys | %i[spam illegal]).include?( (PostActionType.additional_message_types.keys | %i[spam illegal]).include?(
@post_action_name, @post_action_name,
) )
creator = create_message_creator creator = create_message_creator
@ -171,11 +170,11 @@ class PostActionCreator
private private
def flagging_post? def flagging_post?
@post_action_type_view.notify_flag_type_ids.include?(@post_action_type_id) PostActionType.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 == @post_action_type_view.types[:notify_moderators] return false if @post_action_type_id == PostActionType.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?
@ -234,8 +233,7 @@ 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 = not_auto_action_flag_type = !PostActionType.auto_action_flag_types.include?(@post_action_name)
!@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
@ -306,14 +304,14 @@ class PostActionCreator
if post_action if post_action
case @post_action_type_id case @post_action_type_id
when *@post_action_type_view.notify_flag_type_ids when *PostActionType.notify_flag_type_ids
DiscourseEvent.trigger(:flag_created, post_action, self) DiscourseEvent.trigger(:flag_created, post_action, self)
when @post_action_type_view.types[:like] when PostActionType.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 == @post_action_type_view.types[:like] if @post_action_type_id == PostActionType.types[:like]
GivenDailyLike.increment_for(@created_by.id) GivenDailyLike.increment_for(@created_by.id)
end end
@ -383,7 +381,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 == @post_action_type_view.types[:spam], potential_spam: @post_action_type_id == PostActionType.types[:spam],
payload: { payload: {
targets_topic: @targets_topic, targets_topic: @targets_topic,
}, },

View File

@ -17,10 +17,6 @@ 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
@ -54,14 +50,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 == post_action_type_view.types[:like] if @post_action_type_id == PostActionType.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 *post_action_type_view.notify_flag_type_ids when *PostActionType.notify_flag_type_ids
DiscourseEvent.trigger(:flag_destroyed, post_action, self) DiscourseEvent.trigger(:flag_destroyed, post_action, self)
when post_action_type_view.types[:like] when PostActionType.types[:like]
DiscourseEvent.trigger(:like_destroyed, post_action, self) DiscourseEvent.trigger(:like_destroyed, post_action, self)
end end
@ -82,7 +78,7 @@ class PostActionDestroyer
end end
def notify_subscribers def notify_subscribers
name = post_action_type_view.types[@post_action_type_id] name = PostActionType.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

@ -1,144 +0,0 @@
# 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,10 +348,6 @@ 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) }
@ -361,7 +357,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 = post_action_type_view.public_types.map { |k, _| ["#{k}_count", 0] } f = PostActionType.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
@ -391,7 +387,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 = post_action_type_view.flag_types[rs.reviewable_score_type] flag_type = PostActionType.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,28 +598,17 @@ 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
post_action_type_view.topic_flag_types.each do |sym, id| PostActionType.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: can_act: @guardian.post_can_act?(post, sym),
@guardian.post_can_act?(
post,
sym,
opts: {
post_action_type_view: post_action_type_view,
},
),
} }
end end
@ -634,6 +623,22 @@ 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

@ -1,77 +0,0 @@
# 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,30 +70,4 @@ 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,15 +4,17 @@ 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
Discourse.cache.write("post_action_types_#{I18n.locale}", "test") cache = ApplicationSerializer.fragment_cache
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(Discourse.cache.read("post_action_types_#{I18n.locale}")).to eq(nil) expect(cache["post_action_types_#{I18n.locale}"]).to eq(nil)
expect(Discourse.cache.read("post_action_flag_types_#{I18n.locale}")).to eq(nil) expect(cache["post_action_flag_types_#{I18n.locale}"]).to eq(nil)
ensure ensure
PostActionType.new.expire_cache ApplicationSerializer.fragment_cache.clear
end end
end end
end end
@ -28,9 +30,7 @@ RSpec.describe PostActionType do
end end
describe ".additional_message_types" do describe ".additional_message_types" do
before do before { described_class.stubs(:overridden_by_plugin_or_skipped_db?).returns(overriden) }
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

@ -1,24 +0,0 @@
# 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,9 +362,6 @@
}, },
"is_used": { "is_used": {
"type": "boolean" "type": "boolean"
},
"position": {
"type": "integer"
} }
}, },
"required": [ "required": [
@ -417,9 +414,6 @@
}, },
"is_used": { "is_used": {
"type": "boolean" "type": "boolean"
},
"position": {
"type": "integer"
} }
}, },
"required": [ "required": [

View File

@ -1,57 +0,0 @@
# 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