FIX: don't memoize site setting in guardian (#17788)

* FIX: don't memoize site setting in guardian

Memoizing site settings can make tests more fragile and harder to debug


Co-authored-by: Jarek Radosz <jradosz@gmail.com>

lib/guardian.rb

@@ -110,14 +110,21 @@ class Guardian
   end
 
   def is_category_group_moderator?(category)
-    return false unless category
+    return false if !SiteSetting.enable_category_group_moderation?
-    return false unless authenticated?
+    return false if !category
-    @is_category_group_moderator ||= {}
+    return false if !authenticated?
-    @is_category_group_moderator[category.id] ||= begin
+
-      SiteSetting.enable_category_group_moderation? &&
+    reviewable_by_group_id = category.reviewable_by_group_id
-        category.present? &&
+    return false if reviewable_by_group_id.blank?
-        category.reviewable_by_group_id.present? &&
+
-        GroupUser.where(group_id: category.reviewable_by_group_id, user_id: @user.id).exists?
+    @is_group_member ||= {}
+
+    if @is_group_member.key?(reviewable_by_group_id)
+      @is_group_member[reviewable_by_group_id]
+    else
+      @is_group_member[reviewable_by_group_id] = begin
+        GroupUser.where(group_id: reviewable_by_group_id, user_id: @user.id).exists?
+      end
     end
   end
 

spec/lib/guardian_spec.rb

@@ -3966,8 +3966,15 @@ RSpec.describe Guardian do
       category.reviewable_by_group_id = group.id
       guardian = Guardian.new(user)
 
+      # implementation detail, ensure memoization is good (hence testing twice)
+      expect(guardian.is_category_group_moderator?(category)).to eq(true)
       expect(guardian.is_category_group_moderator?(category)).to eq(true)
       expect(guardian.is_category_group_moderator?(plain_category)).to eq(false)
+      expect(guardian.is_category_group_moderator?(plain_category)).to eq(false)
+
+      # edge case ... site setting disabled while guardian instansiated (can help with test cases)
+      SiteSetting.enable_category_group_moderation = false
+      expect(guardian.is_category_group_moderator?(category)).to eq(false)
     end
   end
 end