Revert "FIX: Cache all flags multisite-safe (#28204)" (#28236)

This reverts commit 2225c03455.

app/models/flag.rb

@@ -28,8 +28,7 @@ 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
     MessageBus.publish("/reload_post_action_types", {})
   end

app/models/post_action_type.rb

@@ -1,22 +1,6 @@
 # frozen_string_literal: true
 
 class PostActionType < ActiveRecord::Base
-  POST_ACTION_TYPE_ALL_FLAGS_KEY = "post_action_type_all_flags"
-  POST_ACTION_TYPE_PUBLIC_TYPE_IDS_KEY = "post_action_public_type_ids"
-  ATTRIBUTE_NAMES = %i[
-    id
-    name
-    name_key
-    description
-    notify_type
-    auto_action_type
-    require_message
-    applies_to
-    position
-    enabled
-    score_type
-  ]
-
   after_save :expire_cache
   after_destroy :expire_cache
 
@@ -49,15 +33,12 @@ class PostActionType < ActiveRecord::Base
     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) }
-      Discourse.cache.delete(POST_ACTION_TYPE_ALL_FLAGS_KEY)
-      Discourse.cache.delete(POST_ACTION_TYPE_PUBLIC_TYPE_IDS_KEY)
     end
 
     def reload_types
       @flag_settings = FlagSettings.new
-      PostActionType.new.expire_cache
-      PostActionType.expire_cache
       ReviewableScore.reload_types
+      PostActionType.new.expire_cache
     end
 
     def overridden_by_plugin_or_skipped_db?
@@ -65,20 +46,12 @@ class PostActionType < ActiveRecord::Base
     end
 
     def 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
+      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 { |flag| flag[:auto_action_type] })
+      flag_enum(all_flags.select(&:auto_action_type))
     end
 
     def public_types
@@ -86,14 +59,12 @@ class PostActionType < ActiveRecord::Base
     end
 
     def public_type_ids
-      Discourse
-        .cache
-        .fetch(PostActionType::POST_ACTION_TYPE_PUBLIC_TYPE_IDS_KEY) { public_types.values }
+      @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 { |flag| flag[:require_message] })
+      flag_enum(all_flags.reject(&:require_message))
     end
 
     def flag_types
@@ -101,7 +72,7 @@ class PostActionType < ActiveRecord::Base
 
       # 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[:score_type] })
+      flag_enum(all_flags.reject { |flag| flag.respond_to?(:score_type) && flag.score_type })
     end
 
     def score_types
@@ -109,7 +80,7 @@ class PostActionType < ActiveRecord::Base
 
       # 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[:score_type] })
+      flag_enum(all_flags.filter { |flag| flag.respond_to?(:score_type) && flag.score_type })
     end
 
     # flags resulting in mod notifications
@@ -119,49 +90,40 @@ class PostActionType < ActiveRecord::Base
 
     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] })
+      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].include?("Topic") })
+        flag_enum(all_flags.select { |flag| flag.applies_to?("Topic") })
       end
     end
 
     def disabled_flag_types
-      flag_enum(all_flags.reject { |flag| flag[:enabled] })
+      flag_enum(all_flags.reject(&:enabled))
     end
 
     def enabled_flag_types
-      flag_enum(all_flags.filter { |flag| flag[:enabled] })
+      flag_enum(all_flags.filter(&:enabled))
     end
 
     def additional_message_types
       return flag_settings.with_additional_message if overridden_by_plugin_or_skipped_db?
-      flag_enum(all_flags.select { |flag| flag[:require_message] })
+      flag_enum(all_flags.select(&:require_message))
     end
 
     def names
-      all_flags.reduce({}) do |acc, f|
-        acc[f[:id]] = f[:name]
-        acc
-      end
+      all_flags.pluck(:id, :name).to_h
     end
 
     def descriptions
-      all_flags.reduce({}) do |acc, f|
-        acc[f[:id]] = f[:description]
-        acc
-      end
+      all_flags.pluck(:id, :description).to_h
     end
 
     def applies_to
-      all_flags.reduce({}) do |acc, f|
-        acc[f[:id]] = f[:applies_to]
-        acc
-      end
+      all_flags.pluck(:id, :applies_to).to_h
     end
 
     def is_flag?(sym)
@@ -171,12 +133,7 @@ class PostActionType < ActiveRecord::Base
     private
 
     def flag_enum(scope)
-      Enum.new(
-        scope.reduce({}) do |acc, f|
-          acc[f[:name_key].to_sym] = f[:id]
-          acc
-        end,
-      )
+      Enum.new(scope.map { |flag| [flag.name_key.to_sym, flag.id] }.to_h)
     end
   end
 

spec/models/flag_spec.rb

@@ -3,8 +3,6 @@
 RSpec.describe Flag, type: :model do
   after(:each) { Flag.reset_flag_settings! }
 
-  use_redis_snapshotting
-
   it "has id lower than 1000 for system flags" do
     flag = Fabricate(:flag, id: 1)
     expect(flag.system?).to be true

spec/multisite/flags_spec.rb

@@ -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.expire_cache
-        expect(PostActionType.all_flags.last).to eq(
-          flag_2.attributes.except("created_at", "updated_at").transform_keys(&:to_sym),
-        )
-      end
-
-      PostActionType.expire_cache
-      expect(PostActionType.all_flags.last).to eq(
-        flag_1.attributes.except("created_at", "updated_at").transform_keys(&:to_sym),
-      )
-    end
-  end
-end