diff --git a/app/controllers/admin/config/flags_controller.rb b/app/controllers/admin/config/flags_controller.rb index 4d0915953cf..a95afdf5bce 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, used_flag_ids: Flag.used_flag_ids + render json: result.flag, serializer: FlagSerializer 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, used_flag_ids: Flag.used_flag_ids + render json: result.flag, serializer: FlagSerializer 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 56d0315fbaf..edaf02fd515 100644 --- a/app/jobs/regular/export_user_archive.rb +++ b/app/jobs/regular/export_user_archive.rb @@ -368,24 +368,20 @@ 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: post_action_type_view.flag_types.values) + .where(post_action_type_id: PostActionType.flag_types.values) .order(:created_at) .each do |pa| yield( [ pa.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.updated_at, pa.deleted_at, @@ -404,7 +400,7 @@ module Jobs PostAction .with_deleted .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) .each do |pa| post = Post.with_deleted.find_by(id: pa.post_id) @@ -428,8 +424,7 @@ module Jobs PostAction .where(user_id: @current_user.id) .where.not( - post_action_type_id: - post_action_type_view.flag_types.values + [post_action_type_view.types[:like]], + post_action_type_id: PostActionType.flag_types.values + [PostActionType.types[:like]], ) .exists? end @@ -440,8 +435,7 @@ module Jobs .with_deleted .where(user_id: @current_user.id) .where.not( - post_action_type_id: - post_action_type_view.flag_types.values + [post_action_type_view.types[:like]], + post_action_type_id: PostActionType.flag_types.values + [PostActionType.types[:like]], ) .order(:created_at) .each do |pa| @@ -449,7 +443,7 @@ module Jobs [ pa.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.updated_at, pa.deleted_at, diff --git a/app/models/flag.rb b/app/models/flag.rb index e10fffb4aef..b662dc13e76 100644 --- a/app/models/flag.rb +++ b/app/models/flag.rb @@ -18,9 +18,7 @@ class Flag < ActiveRecord::Base attr_accessor :skip_reset_flag_callback - default_scope do - order(:position).where(score_type: false).where.not(id: PostActionType::LIKE_POST_ACTION_ID) - end + default_scope { order(:position).where(score_type: false) } def used? PostAction.exists?(post_action_type_id: self.id) || @@ -32,14 +30,9 @@ class Flag < ActiveRecord::Base end def self.reset_flag_settings! - # Flags are cached in Redis for better performance. After the update, - # we need to reload them in all processes. + # Flags are memoized for better performance. After the update, we need to reload them in all processes. PostActionType.reload_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) + MessageBus.publish("/reload_post_action_types", {}) end def system? diff --git a/app/models/post.rb b/app/models/post.rb index 8f9f3def332..7b2740e0f1b 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -555,13 +555,9 @@ 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: post_action_type_view.flag_types_without_additional_message.values, + post_action_type_id: PostActionType.flag_types_without_additional_message.values, deleted_at: nil, ) end @@ -646,7 +642,7 @@ class Post < ActiveRecord::Base edit_delay: SiteSetting.cooldown_minutes_after_hiding_posts, flag_reason: 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, base_path: Discourse.base_path, ), diff --git a/app/models/post_action.rb b/app/models/post_action.rb index 1e3b2dd93d9..d0bd68595bf 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -144,21 +144,17 @@ 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 == post_action_type_view.types[:like] + post_action_type_id == PostActionType.types[:like] end def is_flag? - !!post_action_type_view.notify_flag_types[post_action_type_id] + !!PostActionType.notify_flag_types[post_action_type_id] end def is_private_message? - post_action_type_id == post_action_type_view.types[:notify_user] || - post_action_type_id == post_action_type_view.types[:notify_moderators] + post_action_type_id == PostActionType.types[:notify_user] || + post_action_type_id == PostActionType.types[:notify_moderators] end # A custom rate limiter for this model @@ -186,8 +182,7 @@ class PostAction < ActiveRecord::Base end def ensure_unique_actions - post_action_type_ids = - is_flag? ? post_action_type_view.notify_flag_types.values : post_action_type_id + post_action_type_ids = is_flag? ? PostActionType.notify_flag_types.values : post_action_type_id acted = PostAction @@ -203,7 +198,7 @@ class PostAction < ActiveRecord::Base end def post_action_type_key - post_action_type_view.types[post_action_type_id] + PostActionType.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 b0a47dcaa8e..550e6586864 100644 --- a/app/models/post_action_type.rb +++ b/app/models/post_action_type.rb @@ -13,10 +13,8 @@ class PostActionType < ActiveRecord::Base include AnonCacheInvalidator def expire_cache - Discourse.cache.keys("post_action_types_*").each { |key| Discourse.redis.del(key) } - Discourse.cache.keys("post_action_flag_types_*").each { |key| Discourse.redis.del(key) } - Discourse.cache.delete(POST_ACTION_TYPE_ALL_FLAGS_KEY) - Discourse.cache.delete(POST_ACTION_TYPE_PUBLIC_TYPE_IDS_KEY) + ApplicationSerializer.expire_cache_fragment!(/\Apost_action_types_/) + ApplicationSerializer.expire_cache_fragment!(/\Apost_action_flag_types_/) end class << self @@ -31,35 +29,117 @@ class PostActionType < ActiveRecord::Base @flag_settings = settings || FlagSettings.new end - def reload_types - @flag_settings = FlagSettings.new - PostActionType.new.expire_cache - ReviewableScore.reload_types + 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 - %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) } + 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 + 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) end end diff --git a/app/models/reviewable_flagged_post.rb b/app/models/reviewable_flagged_post.rb index 55ecae88441..d35736bc05a 100644 --- a/app/models/reviewable_flagged_post.rb +++ b/app/models/reviewable_flagged_post.rb @@ -137,16 +137,12 @@ 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: post_action_type_view.notify_flag_type_ids) + .where(post_action_type_id: PostActionType.notify_flag_type_ids) actions.each do |action| action.deferred_at = Time.zone.now @@ -203,9 +199,9 @@ class ReviewableFlaggedPost < Reviewable # -1 is the automatic system clear action_type_ids = if performed_by.id == Discourse::SYSTEM_USER_ID - post_action_type_view.auto_action_flag_types.values + PostActionType.auto_action_flag_types.values else - post_action_type_view.notify_flag_type_ids + PostActionType.notify_flag_type_ids end actions = @@ -222,7 +218,7 @@ class ReviewableFlaggedPost < Reviewable # reset all cached counters cached = {} 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) end @@ -278,7 +274,7 @@ class ReviewableFlaggedPost < Reviewable PostAction .active .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 actions.each do |action| @@ -289,7 +285,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 == post_action_type_view.types[:spam] + trigger_spam = true if action.post_action_type_id == PostActionType.types[:spam] end end end diff --git a/app/models/translation_override.rb b/app/models/translation_override.rb index a9d52c28546..3ded31ce20d 100644 --- a/app/models/translation_override.rb +++ b/app/models/translation_override.rb @@ -107,8 +107,10 @@ class TranslationOverride < ActiveRecord::Base end def self.expire_cache(locale, key) - if key.starts_with?("post_action_types.") || key.starts_with?("topic_flag_types.") - PostActionType.new.expire_cache + 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}") else return false end diff --git a/app/models/user.rb b/app/models/user.rb index cf9dc0009ae..e8ffd33b44b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1229,14 +1229,10 @@ 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: post_action_type_view.flag_types_without_additional_message.values, + post_action_type_id: PostActionType.flag_types_without_additional_message.values, ).count end @@ -1249,7 +1245,7 @@ class User < ActiveRecord::Base .includes(:post_actions) .where( "post_actions.post_action_type_id" => - post_action_type_view.flag_types_without_additional_message.values, + PostActionType.flag_types_without_additional_message.values, ) .count end @@ -1463,7 +1459,7 @@ class User < ActiveRecord::Base disagreed_flag_post_ids = 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) .pluck(:post_id) @@ -1591,7 +1587,7 @@ class User < ActiveRecord::Base PostAction .where(user_id: self.id) .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 end diff --git a/app/serializers/flag_serializer.rb b/app/serializers/flag_serializer.rb index 8901710217e..eae1b940258 100644 --- a/app/serializers/flag_serializer.rb +++ b/app/serializers/flag_serializer.rb @@ -1,45 +1,5 @@ # frozen_string_literal: true class FlagSerializer < ApplicationSerializer - 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 + attributes :id, :name, :name_key, :description, :applies_to, :position, :require_message, :enabled end diff --git a/app/serializers/post_action_type_serializer.rb b/app/serializers/post_action_type_serializer.rb index 4c3afd6f613..2ed5bce2d79 100644 --- a/app/serializers/post_action_type_serializer.rb +++ b/app/serializers/post_action_type_serializer.rb @@ -16,16 +16,12 @@ class PostActionTypeSerializer < ApplicationSerializer include ConfigurableUrls - def post_action_type_view - @post_action_type_view ||= PostActionTypeView.new - end - def require_message - !!post_action_type_view.additional_message_types[object.id] + !!PostActionType.additional_message_types[object.id] end def is_flag - !!post_action_type_view.flag_types[object.id] + !!PostActionType.flag_types[object.id] end def name @@ -46,16 +42,15 @@ class PostActionTypeSerializer < ApplicationSerializer end def name_key - post_action_type_view.types[object.id].to_s + PostActionType.types[object.id].to_s end def enabled - # flags added by API are always enabled - true + !!PostActionType.enabled_flag_types[object.id] end def applies_to - Flag.valid_applies_to_types + Array.wrap(PostActionType.applies_to[object.id]) end def is_used diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index dd444c0b704..0929031a920 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -290,12 +290,10 @@ class PostSerializer < BasicPostSerializer result = [] can_see_post = scope.can_see_post?(object) - @post_action_type_view = - @topic_view ? @topic_view.post_action_type_view : PostActionTypeView.new + public_flag_types = + (@topic_view.present? ? @topic_view.public_flag_types : PostActionType.public_types) - public_flag_types = @post_action_type_view.public_types - - @post_action_type_view.types.each do |sym, id| + (@topic_view.present? ? @topic_view.flag_types : PostActionType.types).each do |sym, id| count_col = "#{sym}_count".to_sym count = object.public_send(count_col) if object.respond_to?(count_col) @@ -306,9 +304,22 @@ class PostSerializer < BasicPostSerializer sym, opts: { taken_actions: actions, - 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, + 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 + ), }, can_see_post: can_see_post, ) diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index 5c4515b29f9..c25a51e794d 100644 --- a/app/serializers/site_serializer.rb +++ b/app/serializers/site_serializer.rb @@ -111,44 +111,17 @@ class SiteSerializer < ApplicationSerializer end def post_action_types - 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 + cache_fragment("post_action_types_#{I18n.locale}") do + types = ordered_flags(PostActionType.types.values) + ActiveModel::ArraySerializer.new(types).as_json + end end def topic_flag_types - 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 + 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 end def default_archetype diff --git a/config/initializers/004-message_bus.rb b/config/initializers/004-message_bus.rb index d9a1959a3be..28ef655dfd5 100644 --- a/config/initializers/004-message_bus.rb +++ b/config/initializers/004-message_bus.rb @@ -132,3 +132,7 @@ 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 4a17f6d464c..5518cfcad80 100644 --- a/db/fixtures/003_flags.rb +++ b/db/fixtures/003_flags.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + Flag.seed do |s| s.id = 6 s.name = "notify_user" @@ -63,13 +64,3 @@ 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 c46a7452c16..73365e66302 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -37,18 +37,15 @@ 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 - post_action_type_view.notify_flag_types[action_key] || - post_action_type_view.additional_message_types[action_key] + PostActionType.notify_flag_types[action_key] || + PostActionType.additional_message_types[action_key] end - 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? + already_taken_this_action = taken.any? && taken.include?(PostActionType.types[action_key]) + already_did_flagging = taken.any? && (taken & PostActionType.notify_flag_types.values).any? result = if authenticated? && post @@ -64,9 +61,7 @@ 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? - if is_flag && post_action_type_view.disabled_flag_types.keys.include?(action_key) - return false - end + return false if is_flag && PostActionType.disabled_flag_types.keys.include?(action_key) if action_key == :notify_user && !@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map) @@ -116,13 +111,12 @@ module PostGuardian return true if is_admin? return false unless topic - post_action_type_view = PostActionTypeView.new - type_symbol = post_action_type_view.types[post_action_type_id] + type_symbol = PostActionType.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 post_action_type_view.is_flag?(type_symbol) + return can_see_flags?(topic) if PostActionType.is_flag?(type_symbol) true end diff --git a/lib/post_action_creator.rb b/lib/post_action_creator.rb index 5e5e2b86ecf..23a5ee75047 100644 --- a/lib/post_action_creator.rb +++ b/lib/post_action_creator.rb @@ -57,8 +57,7 @@ class PostActionCreator @post = post @post_action_type_id = post_action_type_id - @post_action_type_view = PostActionTypeView.new - @post_action_name = @post_action_type_view.types[@post_action_type_id] + @post_action_name = PostActionType.types[@post_action_type_id] @is_warning = is_warning @take_action = take_action && guardian.is_staff? @@ -97,7 +96,7 @@ class PostActionCreator if !post_can_act? || (@queue_for_review && !guardian.is_staff?) 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")) else result.add_error(I18n.t("invalid_access")) @@ -116,7 +115,7 @@ class PostActionCreator # create meta topic / post if needed 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, ) creator = create_message_creator @@ -171,11 +170,11 @@ class PostActionCreator private 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 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 = reviewable.reviewable_scores.any? do |rs| rs.reviewable_score_type == @post_action_type_id && !rs.pending? @@ -234,8 +233,7 @@ class PostActionCreator return if @post.hidden? return if !@created_by.staff? && @post.user&.staff? - not_auto_action_flag_type = - !@post_action_type_view.auto_action_flag_types.include?(@post_action_name) + not_auto_action_flag_type = !PostActionType.auto_action_flag_types.include?(@post_action_name) return if not_auto_action_flag_type && !@queue_for_review if @queue_for_review @@ -306,14 +304,14 @@ class PostActionCreator if post_action 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) - when @post_action_type_view.types[:like] + when PostActionType.types[:like] DiscourseEvent.trigger(:like_created, post_action, self) 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) end @@ -383,7 +381,7 @@ class PostActionCreator target: @post, topic: @post.topic, 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: { targets_topic: @targets_topic, }, diff --git a/lib/post_action_destroyer.rb b/lib/post_action_destroyer.rb index 0de7906cf46..55c39469ee8 100644 --- a/lib/post_action_destroyer.rb +++ b/lib/post_action_destroyer.rb @@ -17,10 +17,6 @@ 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 @@ -54,14 +50,14 @@ class PostActionDestroyer post_action.remove_act!(@destroyed_by) 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) end 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) - when post_action_type_view.types[:like] + when PostActionType.types[:like] DiscourseEvent.trigger(:like_destroyed, post_action, self) end @@ -82,7 +78,7 @@ class PostActionDestroyer end def notify_subscribers - name = post_action_type_view.types[@post_action_type_id] + name = PostActionType.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 deleted file mode 100644 index 2317b89e6b1..00000000000 --- a/lib/post_action_type_view.rb +++ /dev/null @@ -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 diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index e302f3228d1..e3f206be6e5 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -348,10 +348,6 @@ 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) } @@ -361,7 +357,7 @@ class PostDestroyer @post.custom_fields["deleted_public_actions"] = public_post_actions.ids @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]) end end @@ -391,7 +387,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 = post_action_type_view.flag_types[rs.reviewable_score_type] + flag_type = PostActionType.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 9b7f0fdd53e..4123bb48dcf 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -598,28 +598,17 @@ 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 - post_action_type_view.topic_flag_types.each do |sym, id| + PostActionType.topic_flag_types.each do |sym, id| @actions_summary << { id: id, count: 0, hidden: false, - can_act: - @guardian.post_can_act?( - post, - sym, - opts: { - post_action_type_view: post_action_type_view, - }, - ), + can_act: @guardian.post_can_act?(post, sym), } end @@ -634,6 +623,22 @@ 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 deleted file mode 100644 index 607839522b7..00000000000 --- a/spec/lib/post_action_type_view_spec.rb +++ /dev/null @@ -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 diff --git a/spec/models/flag_spec.rb b/spec/models/flag_spec.rb index c090b854cd0..884a3b13688 100644 --- a/spec/models/flag_spec.rb +++ b/spec/models/flag_spec.rb @@ -70,30 +70,4 @@ 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 7566a1b8f26..77bf6d7947d 100644 --- a/spec/models/post_action_type_spec.rb +++ b/spec/models/post_action_type_spec.rb @@ -4,15 +4,17 @@ RSpec.describe PostActionType do describe "Callbacks" do describe "#expiry_cache" do it "should clear the cache on save" do - Discourse.cache.write("post_action_types_#{I18n.locale}", "test") - Discourse.cache.write("post_action_flag_types_#{I18n.locale}", "test2") + cache = ApplicationSerializer.fragment_cache + + cache["post_action_types_#{I18n.locale}"] = "test" + cache["post_action_flag_types_#{I18n.locale}"] = "test2" PostActionType.new(name_key: "some_key").save! - 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) + expect(cache["post_action_types_#{I18n.locale}"]).to eq(nil) + expect(cache["post_action_flag_types_#{I18n.locale}"]).to eq(nil) ensure - PostActionType.new.expire_cache + ApplicationSerializer.fragment_cache.clear end end end @@ -28,9 +30,7 @@ RSpec.describe PostActionType do end describe ".additional_message_types" do - before do - PostActionTypeView.any_instance.stubs(:overridden_by_plugin_or_skipped_db?).returns(overriden) - end + before { described_class.stubs(:overridden_by_plugin_or_skipped_db?).returns(overriden) } 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 deleted file mode 100644 index 4f800e2e385..00000000000 --- a/spec/multisite/flags_spec.rb +++ /dev/null @@ -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 diff --git a/spec/requests/api/schemas/json/site_response.json b/spec/requests/api/schemas/json/site_response.json index 13d7c145cd3..a432991de95 100644 --- a/spec/requests/api/schemas/json/site_response.json +++ b/spec/requests/api/schemas/json/site_response.json @@ -362,9 +362,6 @@ }, "is_used": { "type": "boolean" - }, - "position": { - "type": "integer" } }, "required": [ @@ -417,9 +414,6 @@ }, "is_used": { "type": "boolean" - }, - "position": { - "type": "integer" } }, "required": [ diff --git a/spec/serializers/flag_serializer_spec.rb b/spec/serializers/flag_serializer_spec.rb deleted file mode 100644 index ad949180a87..00000000000 --- a/spec/serializers/flag_serializer_spec.rb +++ /dev/null @@ -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