diff --git a/app/controllers/admin/config/flags_controller.rb b/app/controllers/admin/config/flags_controller.rb index a95afdf5bce..4d0915953cf 100644 --- a/app/controllers/admin/config/flags_controller.rb +++ b/app/controllers/admin/config/flags_controller.rb @@ -29,7 +29,7 @@ class Admin::Config::FlagsController < Admin::AdminController with_service(Flags::CreateFlag) do on_success do Discourse.request_refresh! - render json: result.flag, serializer: FlagSerializer + render json: result.flag, serializer: FlagSerializer, used_flag_ids: Flag.used_flag_ids end on_failure { render(json: failed_json, status: 422) } on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess } @@ -43,7 +43,7 @@ class Admin::Config::FlagsController < Admin::AdminController with_service(Flags::UpdateFlag) do on_success do Discourse.request_refresh! - render json: result.flag, serializer: FlagSerializer + render json: result.flag, serializer: FlagSerializer, used_flag_ids: Flag.used_flag_ids end on_failure { render(json: failed_json, status: 422) } on_model_not_found(:message) { raise Discourse::NotFound } diff --git a/app/jobs/regular/export_user_archive.rb b/app/jobs/regular/export_user_archive.rb index edaf02fd515..56d0315fbaf 100644 --- a/app/jobs/regular/export_user_archive.rb +++ b/app/jobs/regular/export_user_archive.rb @@ -368,20 +368,24 @@ module Jobs end end + def post_action_type_view + @post_action_type_view ||= PostActionTypeView.new + end + def flags_export return enum_for(:flags_export) unless block_given? PostAction .with_deleted .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) .each do |pa| yield( [ pa.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.updated_at, pa.deleted_at, @@ -400,7 +404,7 @@ module Jobs PostAction .with_deleted .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) .each do |pa| post = Post.with_deleted.find_by(id: pa.post_id) @@ -424,7 +428,8 @@ module Jobs PostAction .where(user_id: @current_user.id) .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? end @@ -435,7 +440,8 @@ module Jobs .with_deleted .where(user_id: @current_user.id) .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) .each do |pa| @@ -443,7 +449,7 @@ module Jobs [ pa.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.updated_at, pa.deleted_at, diff --git a/app/models/flag.rb b/app/models/flag.rb index b662dc13e76..e10fffb4aef 100644 --- a/app/models/flag.rb +++ b/app/models/flag.rb @@ -18,7 +18,9 @@ class Flag < ActiveRecord::Base 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? PostAction.exists?(post_action_type_id: self.id) || @@ -30,9 +32,14 @@ class Flag < ActiveRecord::Base end 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 - 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 def system? diff --git a/app/models/post.rb b/app/models/post.rb index 7b2740e0f1b..8f9f3def332 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -555,9 +555,13 @@ class Post < ActiveRecord::Base flags.count != 0 end + def post_action_type_view + @post_action_type_view ||= PostActionTypeView.new + end + def flags 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, ) end @@ -642,7 +646,7 @@ class Post < ActiveRecord::Base edit_delay: SiteSetting.cooldown_minutes_after_hiding_posts, flag_reason: 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, base_path: Discourse.base_path, ), diff --git a/app/models/post_action.rb b/app/models/post_action.rb index d0bd68595bf..1e3b2dd93d9 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -144,17 +144,21 @@ class PostAction < ActiveRecord::Base save end + def post_action_type_view + @post_action_type_view ||= PostActionTypeView.new + end + def is_like? - post_action_type_id == PostActionType.types[:like] + post_action_type_id == post_action_type_view.types[:like] end def is_flag? - !!PostActionType.notify_flag_types[post_action_type_id] + !!post_action_type_view.notify_flag_types[post_action_type_id] end def is_private_message? - post_action_type_id == PostActionType.types[:notify_user] || - post_action_type_id == PostActionType.types[:notify_moderators] + post_action_type_id == post_action_type_view.types[:notify_user] || + post_action_type_id == post_action_type_view.types[:notify_moderators] end # A custom rate limiter for this model @@ -182,7 +186,8 @@ class PostAction < ActiveRecord::Base end 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 = PostAction @@ -198,7 +203,7 @@ class PostAction < ActiveRecord::Base end def post_action_type_key - PostActionType.types[post_action_type_id] + post_action_type_view.types[post_action_type_id] end def update_counters diff --git a/app/models/post_action_type.rb b/app/models/post_action_type.rb index 550e6586864..b85129ce8f4 100644 --- a/app/models/post_action_type.rb +++ b/app/models/post_action_type.rb @@ -13,8 +13,18 @@ class PostActionType < ActiveRecord::Base include AnonCacheInvalidator def expire_cache - ApplicationSerializer.expire_cache_fragment!(/\Apost_action_types_/) - ApplicationSerializer.expire_cache_fragment!(/\Apost_action_flag_types_/) + Discourse.cache.redis.del( + *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 class << self @@ -29,117 +39,35 @@ class PostActionType < ActiveRecord::Base @flag_settings = settings || FlagSettings.new 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 @flag_settings = FlagSettings.new - ReviewableScore.reload_types PostActionType.new.expire_cache + ReviewableScore.reload_types end - def overridden_by_plugin_or_skipped_db? - flag_settings.flag_types.present? || GlobalSetting.skip_db? - end - - def all_flags - Flag.unscoped.order(:position).all - end - - def auto_action_flag_types - return flag_settings.auto_action_types if overridden_by_plugin_or_skipped_db? - flag_enum(all_flags.select(&:auto_action_type)) - 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) + %i[ + expire_cache + all_flags + types + overridden_by_plugin_or_skipped_db? + auto_action_flag_types + public_types + public_type_ids + flag_types_without_additional_message + flags + flag_types + score_types + notify_flag_type_ids + notify_flag_types + topic_flag_types + disabled_flag_types + additional_message_types + names + descriptions + applies_to + is_flag? + ].each do |method_name| + define_method(method_name) { |*args| PostActionTypeView.new.send(method_name, *args) } end end diff --git a/app/models/reviewable_flagged_post.rb b/app/models/reviewable_flagged_post.rb index d35736bc05a..55ecae88441 100644 --- a/app/models/reviewable_flagged_post.rb +++ b/app/models/reviewable_flagged_post.rb @@ -137,12 +137,16 @@ class ReviewableFlaggedPost < Reviewable perform_ignore_and_do_nothing(performed_by, args) end + def post_action_type_view + @post_action_type_view ||= PostActionTypeView.new + end + def perform_ignore_and_do_nothing(performed_by, args) actions = PostAction .active .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| action.deferred_at = Time.zone.now @@ -199,9 +203,9 @@ class ReviewableFlaggedPost < Reviewable # -1 is the automatic system clear action_type_ids = if performed_by.id == Discourse::SYSTEM_USER_ID - PostActionType.auto_action_flag_types.values + post_action_type_view.auto_action_flag_types.values else - PostActionType.notify_flag_type_ids + post_action_type_view.notify_flag_type_ids end actions = @@ -218,7 +222,7 @@ class ReviewableFlaggedPost < Reviewable # reset all cached counters cached = {} 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) end @@ -274,7 +278,7 @@ class ReviewableFlaggedPost < Reviewable PostAction .active .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 actions.each do |action| @@ -285,7 +289,7 @@ class ReviewableFlaggedPost < Reviewable action.save DB.after_commit do 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 diff --git a/app/models/translation_override.rb b/app/models/translation_override.rb index 3ded31ce20d..a9d52c28546 100644 --- a/app/models/translation_override.rb +++ b/app/models/translation_override.rb @@ -107,10 +107,8 @@ class TranslationOverride < ActiveRecord::Base end def self.expire_cache(locale, key) - if key.starts_with?("post_action_types.") - ApplicationSerializer.expire_cache_fragment!("post_action_types_#{locale}") - elsif key.starts_with?("topic_flag_types.") - ApplicationSerializer.expire_cache_fragment!("post_action_flag_types_#{locale}") + if key.starts_with?("post_action_types.") || key.starts_with?("topic_flag_types.") + PostActionType.new.expire_cache else return false end diff --git a/app/models/user.rb b/app/models/user.rb index e8ffd33b44b..cf9dc0009ae 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1229,10 +1229,14 @@ class User < ActiveRecord::Base stat.increment!(:post_edits_count) end + def post_action_type_view + @post_action_type_view ||= PostActionTypeView.new + end + def flags_given_count PostAction.where( 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 end @@ -1245,7 +1249,7 @@ class User < ActiveRecord::Base .includes(:post_actions) .where( "post_actions.post_action_type_id" => - PostActionType.flag_types_without_additional_message.values, + post_action_type_view.flag_types_without_additional_message.values, ) .count end @@ -1459,7 +1463,7 @@ class User < ActiveRecord::Base disagreed_flag_post_ids = 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) .pluck(:post_id) @@ -1587,7 +1591,7 @@ class User < ActiveRecord::Base PostAction .where(user_id: self.id) .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 end diff --git a/app/serializers/flag_serializer.rb b/app/serializers/flag_serializer.rb index eae1b940258..8901710217e 100644 --- a/app/serializers/flag_serializer.rb +++ b/app/serializers/flag_serializer.rb @@ -1,5 +1,45 @@ # frozen_string_literal: true 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 diff --git a/app/serializers/post_action_type_serializer.rb b/app/serializers/post_action_type_serializer.rb index 2ed5bce2d79..4c3afd6f613 100644 --- a/app/serializers/post_action_type_serializer.rb +++ b/app/serializers/post_action_type_serializer.rb @@ -16,12 +16,16 @@ class PostActionTypeSerializer < ApplicationSerializer include ConfigurableUrls + def post_action_type_view + @post_action_type_view ||= PostActionTypeView.new + end + def require_message - !!PostActionType.additional_message_types[object.id] + !!post_action_type_view.additional_message_types[object.id] end def is_flag - !!PostActionType.flag_types[object.id] + !!post_action_type_view.flag_types[object.id] end def name @@ -42,15 +46,16 @@ class PostActionTypeSerializer < ApplicationSerializer end def name_key - PostActionType.types[object.id].to_s + post_action_type_view.types[object.id].to_s end def enabled - !!PostActionType.enabled_flag_types[object.id] + # flags added by API are always enabled + true end def applies_to - Array.wrap(PostActionType.applies_to[object.id]) + Flag.valid_applies_to_types end def is_used diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index 0929031a920..dd444c0b704 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -290,10 +290,12 @@ class PostSerializer < BasicPostSerializer result = [] can_see_post = scope.can_see_post?(object) - public_flag_types = - (@topic_view.present? ? @topic_view.public_flag_types : PostActionType.public_types) + @post_action_type_view = + @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 = object.public_send(count_col) if object.respond_to?(count_col) @@ -304,22 +306,9 @@ class PostSerializer < BasicPostSerializer sym, opts: { taken_actions: actions, - notify_flag_types: - ( - 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 - ), + notify_flag_types: @post_action_type_view.notify_flag_types, + additional_message_types: @post_action_type_view.additional_message_types, + post_action_type_view: @post_action_type_view, }, can_see_post: can_see_post, ) diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index c25a51e794d..5c4515b29f9 100644 --- a/app/serializers/site_serializer.rb +++ b/app/serializers/site_serializer.rb @@ -111,17 +111,44 @@ class SiteSerializer < ApplicationSerializer end def post_action_types - cache_fragment("post_action_types_#{I18n.locale}") do - types = ordered_flags(PostActionType.types.values) - ActiveModel::ArraySerializer.new(types).as_json - end + Discourse + .cache + .fetch("post_action_types_#{I18n.locale}") do + 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 def topic_flag_types - cache_fragment("post_action_flag_types_#{I18n.locale}") do - types = ordered_flags(PostActionType.topic_flag_types.values) - ActiveModel::ArraySerializer.new(types, each_serializer: TopicFlagTypeSerializer).as_json - end + Discourse + .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) + 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 def default_archetype diff --git a/config/initializers/004-message_bus.rb b/config/initializers/004-message_bus.rb index 28ef655dfd5..d9a1959a3be 100644 --- a/config/initializers/004-message_bus.rb +++ b/config/initializers/004-message_bus.rb @@ -132,7 +132,3 @@ if Rails.env == "test" || $0 =~ /rake$/ # disable keepalive in testing MessageBus.keepalive_interval = -1 end - -if !Rails.env.test? - MessageBus.subscribe("/reload_post_action_types") { PostActionType.reload_types } -end diff --git a/db/fixtures/003_flags.rb b/db/fixtures/003_flags.rb index 5518cfcad80..b905f1fc7a1 100644 --- a/db/fixtures/003_flags.rb +++ b/db/fixtures/003_flags.rb @@ -64,3 +64,13 @@ Flag.unscoped.seed do |s| s.applies_to = %w[] s.skip_reset_flag_callback = true 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 diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index 73365e66302..c46a7452c16 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -37,15 +37,18 @@ module PostGuardian end taken = opts[:taken_actions].try(:keys).to_a + post_action_type_view = opts[:post_action_type_view] || PostActionTypeView.new is_flag = if (opts[:notify_flag_types] && opts[:additional_message_types]) opts[:notify_flag_types][action_key] || opts[:additional_message_types][action_key] else - PostActionType.notify_flag_types[action_key] || - PostActionType.additional_message_types[action_key] + post_action_type_view.notify_flag_types[action_key] || + post_action_type_view.additional_message_types[action_key] end - already_taken_this_action = taken.any? && taken.include?(PostActionType.types[action_key]) - already_did_flagging = taken.any? && (taken & PostActionType.notify_flag_types.values).any? + already_taken_this_action = + 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 = if authenticated? && post @@ -61,7 +64,9 @@ module PostGuardian # 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 && 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 && !@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map) @@ -111,12 +116,13 @@ module PostGuardian return true if is_admin? 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 == :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 end diff --git a/lib/post_action_creator.rb b/lib/post_action_creator.rb index 23a5ee75047..5e5e2b86ecf 100644 --- a/lib/post_action_creator.rb +++ b/lib/post_action_creator.rb @@ -57,7 +57,8 @@ class PostActionCreator @post = post @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 @take_action = take_action && guardian.is_staff? @@ -96,7 +97,7 @@ class PostActionCreator if !post_can_act? || (@queue_for_review && !guardian.is_staff?) 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")) else result.add_error(I18n.t("invalid_access")) @@ -115,7 +116,7 @@ class PostActionCreator # create meta topic / post if needed 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, ) creator = create_message_creator @@ -170,11 +171,11 @@ class PostActionCreator private 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 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 = reviewable.reviewable_scores.any? do |rs| rs.reviewable_score_type == @post_action_type_id && !rs.pending? @@ -233,7 +234,8 @@ class PostActionCreator return if @post.hidden? 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 if @queue_for_review @@ -304,14 +306,14 @@ class PostActionCreator if post_action 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) - when PostActionType.types[:like] + when @post_action_type_view.types[:like] DiscourseEvent.trigger(:like_created, post_action, self) 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) end @@ -381,7 +383,7 @@ class PostActionCreator target: @post, topic: @post.topic, 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: { targets_topic: @targets_topic, }, diff --git a/lib/post_action_destroyer.rb b/lib/post_action_destroyer.rb index 55c39469ee8..0de7906cf46 100644 --- a/lib/post_action_destroyer.rb +++ b/lib/post_action_destroyer.rb @@ -17,6 +17,10 @@ class PostActionDestroyer new(destroyed_by, post, PostActionType.types[action_key], opts).perform end + def post_action_type_view + @post_action_type_view ||= PostActionTypeView.new + end + def perform result = DestroyResult.new @@ -50,14 +54,14 @@ class PostActionDestroyer post_action.remove_act!(@destroyed_by) 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) end 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) - when PostActionType.types[:like] + when post_action_type_view.types[:like] DiscourseEvent.trigger(:like_destroyed, post_action, self) end @@ -78,7 +82,7 @@ class PostActionDestroyer end def notify_subscribers - name = PostActionType.types[@post_action_type_id] + name = post_action_type_view.types[@post_action_type_id] if name == :like @post.publish_change_to_clients!( :unliked, diff --git a/lib/post_action_type_view.rb b/lib/post_action_type_view.rb new file mode 100644 index 00000000000..2317b89e6b1 --- /dev/null +++ b/lib/post_action_type_view.rb @@ -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 diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index e3f206be6e5..e302f3228d1 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -348,6 +348,10 @@ class PostDestroyer Jobs.enqueue(:feature_topic_users, topic_id: @post.topic_id) end + def post_action_type_view + @post_action_type_view ||= PostActionTypeView.new + end + def trash_public_post_actions if public_post_actions = PostAction.publics.where(post_id: @post.id) 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.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]) end end @@ -387,7 +391,7 @@ class PostDestroyer # 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 # 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 notify_responders = options[:notify_responders] diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 4123bb48dcf..9b7f0fdd53e 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -598,17 +598,28 @@ class TopicView ReviewableQueuedPost.pending.where(target_created_by: @user, topic: @topic).order(:created_at) end + def post_action_type_view + @post_action_type_view ||= PostActionTypeView.new + end + def actions_summary return @actions_summary unless @actions_summary.nil? @actions_summary = [] 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 << { id: id, count: 0, 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 @@ -623,22 +634,6 @@ class TopicView @pm_params ||= TopicQuery.new(@user).get_pm_params(topic) 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 if @include_suggested @suggested_topics ||= diff --git a/spec/lib/post_action_type_view_spec.rb b/spec/lib/post_action_type_view_spec.rb new file mode 100644 index 00000000000..607839522b7 --- /dev/null +++ b/spec/lib/post_action_type_view_spec.rb @@ -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 diff --git a/spec/models/flag_spec.rb b/spec/models/flag_spec.rb index 884a3b13688..c090b854cd0 100644 --- a/spec/models/flag_spec.rb +++ b/spec/models/flag_spec.rb @@ -70,4 +70,30 @@ RSpec.describe Flag, type: :model do %i[notify_user off_topic inappropriate spam illegal notify_moderators needs_approval], ) 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 diff --git a/spec/models/post_action_type_spec.rb b/spec/models/post_action_type_spec.rb index 77bf6d7947d..7566a1b8f26 100644 --- a/spec/models/post_action_type_spec.rb +++ b/spec/models/post_action_type_spec.rb @@ -4,17 +4,15 @@ RSpec.describe PostActionType do describe "Callbacks" do describe "#expiry_cache" do it "should clear the cache on save" do - cache = ApplicationSerializer.fragment_cache - - cache["post_action_types_#{I18n.locale}"] = "test" - cache["post_action_flag_types_#{I18n.locale}"] = "test2" + Discourse.cache.write("post_action_types_#{I18n.locale}", "test") + Discourse.cache.write("post_action_flag_types_#{I18n.locale}", "test2") PostActionType.new(name_key: "some_key").save! - expect(cache["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_types_#{I18n.locale}")).to eq(nil) + expect(Discourse.cache.read("post_action_flag_types_#{I18n.locale}")).to eq(nil) ensure - ApplicationSerializer.fragment_cache.clear + PostActionType.new.expire_cache end end end @@ -30,7 +28,9 @@ RSpec.describe PostActionType do end 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 let(:overriden) { true } diff --git a/spec/multisite/flags_spec.rb b/spec/multisite/flags_spec.rb new file mode 100644 index 00000000000..4f800e2e385 --- /dev/null +++ b/spec/multisite/flags_spec.rb @@ -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 diff --git a/spec/requests/api/schemas/json/site_response.json b/spec/requests/api/schemas/json/site_response.json index a432991de95..13d7c145cd3 100644 --- a/spec/requests/api/schemas/json/site_response.json +++ b/spec/requests/api/schemas/json/site_response.json @@ -362,6 +362,9 @@ }, "is_used": { "type": "boolean" + }, + "position": { + "type": "integer" } }, "required": [ @@ -414,6 +417,9 @@ }, "is_used": { "type": "boolean" + }, + "position": { + "type": "integer" } }, "required": [ diff --git a/spec/serializers/flag_serializer_spec.rb b/spec/serializers/flag_serializer_spec.rb new file mode 100644 index 00000000000..ad949180a87 --- /dev/null +++ b/spec/serializers/flag_serializer_spec.rb @@ -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