From 9bf31add6af562d0ff87463a946ca217bd92ff3f Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Thu, 9 Jan 2025 13:20:59 +1100 Subject: [PATCH] FIX: do not memoize score types (#30657) Score types are dynamic because of custom flags. Therefore we cannot memorize them on class level as it is not multisite safe. --- app/models/reviewable_score.rb | 7 ++++--- spec/multisite/flags_spec.rb | 36 ++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/app/models/reviewable_score.rb b/app/models/reviewable_score.rb index 5101432ce52..17300a52e8d 100644 --- a/app/models/reviewable_score.rb +++ b/app/models/reviewable_score.rb @@ -11,7 +11,7 @@ class ReviewableScore < ActiveRecord::Base # To keep things simple the types correspond to `PostActionType` for backwards # compatibility, but we can add extra reasons for scores. def self.types - @types ||= PostActionType.flag_types.merge(PostActionType.score_types) + PostActionType.flag_types.merge(PostActionType.score_types).merge(@api_types || {}) end def self.type_title(type) @@ -23,14 +23,15 @@ class ReviewableScore < ActiveRecord::Base # When extending post action flags, we need to call this method in order to # get the latests flags. def self.reload_types - @types = nil + @api_types = nil types end def self.add_new_types(type_names) + @api_types ||= {} next_id = types.values.max + 1 - type_names.each_with_index { |name, idx| @types[name] = next_id + idx } + type_names.each_with_index { |name, idx| @api_types[name] = next_id + idx } end def self.score_transitions diff --git a/spec/multisite/flags_spec.rb b/spec/multisite/flags_spec.rb index dbfa0f38465..7c2ac193d13 100644 --- a/spec/multisite/flags_spec.rb +++ b/spec/multisite/flags_spec.rb @@ -4,6 +4,18 @@ RSpec.describe "Custom flags in multisite", type: :multisite do describe "PostACtionType#all_flags" do it "does not share flag definitions between sites" do flag_1 = Flag.create!(name: "test flag 1", position: 99, applies_to: ["Post"]) + expect(ReviewableScore.types).to eq( + { + notify_user: 6, + off_topic: 3, + inappropriate: 4, + spam: 8, + illegal: 10, + notify_moderators: 7, + custom_test_flag_1: flag_1.id, + needs_approval: 9, + }, + ) test_multisite_connection("second") do flag_2 = Flag.create!(name: "test flag 2", position: 99, applies_to: ["Post"]) @@ -11,12 +23,36 @@ RSpec.describe "Custom flags in multisite", type: :multisite do expect(PostActionType.all_flags.last).to eq( flag_2.attributes.except("created_at", "updated_at").transform_keys(&:to_sym), ) + expect(ReviewableScore.types).to eq( + { + notify_user: 6, + off_topic: 3, + inappropriate: 4, + spam: 8, + illegal: 10, + notify_moderators: 7, + custom_test_flag_2: flag_2.id, + needs_approval: 9, + }, + ) end PostActionType.new.expire_cache expect(PostActionType.all_flags.last).to eq( flag_1.attributes.except("created_at", "updated_at").transform_keys(&:to_sym), ) + expect(ReviewableScore.types).to eq( + { + notify_user: 6, + off_topic: 3, + inappropriate: 4, + spam: 8, + illegal: 10, + notify_moderators: 7, + custom_test_flag_1: flag_1.id, + needs_approval: 9, + }, + ) end end end