DEV: Make more group-based settings client: false (#25585)

Affects the following settings: * whispers_allowed_groups * anonymous_posting_allowed_groups * personal_message_enabled_groups * shared_drafts_allowed_groups * here_mention_allowed_groups * uploaded_avatars_allowed_groups * ignore_allowed_groups This turns off `client: true` for these group-based settings, because there is no guarantee that the current user gets all their group memberships serialized to the client. Better to check server-side first.
2025-01-18 14:42:46 +08:00 · 2024-02-08 09:43:34 +10:00 · 2024-02-08 09:43:34 +10:00 · adb4eee153
commit adb4eee153
parent 57c53b0ead
11 changed files with 33 additions and 49 deletions
--- a/app/assets/javascripts/discourse/app/components/user-menu/profile-tab-content.js
+++ b/app/assets/javascripts/discourse/app/components/user-menu/profile-tab-content.js
@ -27,12 +27,7 @@ export default class UserMenuProfileTabContent extends Component {

  get showToggleAnonymousButton() {
    return (
-      (this.siteSettings.allow_anonymous_posting &&
-        this.siteSettings.userInAnyGroups(
-          "anonymous_posting_allowed_groups",
-          this.currentUser
-        )) ||
-      this.currentUser.is_anonymous
+      this.currentUser.can_post_anonymously || this.currentUser.is_anonymous
    );
  }

--- a/app/assets/javascripts/discourse/app/controllers/preferences/users.js
+++ b/app/assets/javascripts/discourse/app/controllers/preferences/users.js
@ -12,7 +12,7 @@ export default Controller.extend({
  userCanIgnore(trustLevel) {
    return (
      trustLevel >= this.siteSettings.min_trust_level_to_allow_ignore ||
-      this.currentUser.isInAnyGroups(this.siteSettings.ignore_allowed_groups)
+      this.currentUser.can_ignore_users
    );
  },

--- a/app/assets/javascripts/discourse/tests/acceptance/preferences-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/preferences-test.js
@ -174,15 +174,15 @@ acceptance(
 );

 acceptance("Ignored users", function (needs) {
-  needs.user();
-  needs.settings({ ignore_allowed_groups: "11" });
+  needs.user({ can_ignore_users: true });

-  test("when trust level < min level to ignore", async function (assert) {
+  test("when user is not allowed to ignore", async function (assert) {
    await visit(`/u/eviltrout/preferences/users`);
    updateCurrentUser({
      trust_level: 0,
      moderator: false,
      admin: false,
+      can_ignore_users: false,
      groups: [
        {
          id: 10,
@ -199,9 +199,9 @@ acceptance("Ignored users", function (needs) {
    );
  });

-  test("when trust level >= min level to ignore", async function (assert) {
+  test("when user is allowed to ignore", async function (assert) {
    await visit(`/u/eviltrout/preferences/users`);
-    updateCurrentUser({ trust_level: 1 });
+    updateCurrentUser({ can_ignore_users: true });
    assert.ok(exists(".user-ignore"), "it shows the list of ignored users");
  });

--- a/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js
@ -24,6 +24,7 @@ acceptance("User menu", function (needs) {
  needs.user({
    unread_high_priority_notifications: 73,
    trust_level: 3,
+    can_post_anonymously: true,
    grouped_unread_notifications: {
      [NOTIFICATION_TYPES.replied]: 2,
    },
@ -31,7 +32,6 @@ acceptance("User menu", function (needs) {

  needs.settings({
    allow_anonymous_posting: true,
-    anonymous_posting_allowed_groups: "3",
  });

  let requestHeaders = {};
@ -609,6 +609,7 @@ acceptance("User menu", function (needs) {
    await click("header.d-header"); // close the menu
    updateCurrentUser({
      is_anonymous: false,
+      can_post_anonymously: false,
      trust_level: 2,
      groups: [
        AUTO_GROUPS.trust_level_0,
@ -632,14 +633,13 @@ acceptance("User menu", function (needs) {
    updateCurrentUser({
      is_anonymous: true,
      trust_level: 2,
+      can_post_anonymously: true,
      groups: [
        AUTO_GROUPS.trust_level_0,
        AUTO_GROUPS.trust_level_1,
        AUTO_GROUPS.trust_level_2,
      ],
    });
-    this.siteSettings.allow_anonymous_posting = false;
-    this.siteSettings.anonymous_posting_allowed_groups = "3";
    await click(".d-header-icons .current-user");
    await click("#user-menu-button-profile");

@ -651,6 +651,7 @@ acceptance("User menu", function (needs) {
    await click("header.d-header"); // close the menu
    updateCurrentUser({
      is_anonymous: true,
+      can_post_anonymously: true,
      trust_level: 4,
      groups: [
        AUTO_GROUPS.trust_level_0,
@ -660,8 +661,6 @@ acceptance("User menu", function (needs) {
        AUTO_GROUPS.trust_level_4,
      ],
    });
-    this.siteSettings.allow_anonymous_posting = false;
-    this.siteSettings.anonymous_posting_allowed_groups = "3";
    await click(".d-header-icons .current-user");
    await click("#user-menu-button-profile");

@ -673,6 +672,7 @@ acceptance("User menu", function (needs) {
    await click("header.d-header"); // close the menu
    updateCurrentUser({
      is_anonymous: false,
+      can_post_anonymously: false,
      trust_level: 2,
      groups: [
        AUTO_GROUPS.trust_level_0,
@ -680,14 +680,12 @@ acceptance("User menu", function (needs) {
        AUTO_GROUPS.trust_level_2,
      ],
    });
-    this.siteSettings.allow_anonymous_posting = true;
-    this.siteSettings.anonymous_posting_allowed_groups = "3";
    await click(".d-header-icons .current-user");
    await click("#user-menu-button-profile");

    assert.notOk(
      exists("#quick-access-profile ul li.enable-anonymous"),
-      "toggle anon button is not shown if the user doesn't have a high enough trust level"
+      "toggle anon button is not shown if the user is not allowed to post anonymously"
    );

    const logoutButton = query("#quick-access-profile ul li.logout .btn");
--- a/app/models/concerns/roleable.rb
+++ b/app/models/concerns/roleable.rb
@ -21,11 +21,10 @@ module Roleable
  def whisperer?
    @whisperer ||=
      begin
-        whispers_allowed_group_ids = SiteSetting.whispers_allowed_group_ids
-        return false if whispers_allowed_group_ids.blank?
+        return false if SiteSetting.whispers_allowed_groups_map.empty?
        return true if admin
-        return true if whispers_allowed_group_ids.include?(primary_group_id)
-        group_users&.exists?(group_id: whispers_allowed_group_ids)
+        return true if SiteSetting.whispers_allowed_groups_map.include?(primary_group_id)
+        group_users&.exists?(group_id: SiteSetting.whispers_allowed_groups_map)
      end
  end

--- a/app/models/group_user.rb
+++ b/app/models/group_user.rb
@ -27,8 +27,6 @@ class GroupUser < ActiveRecord::Base
  end

  def self.update_first_unread_pm(last_seen, limit: 10_000)
-    whisperers_group_ids = SiteSetting.whispers_allowed_group_ids
-
    DB.exec(
      <<~SQL,
    UPDATE group_users gu
@ -57,7 +55,7 @@ class GroupUser < ActiveRecord::Base
          WHERE t.deleted_at IS NULL
          AND t.archetype = :archetype
          AND tu.last_read_post_number < CASE
-                                         WHEN u.admin OR u.moderator #{whisperers_group_ids.present? ? "OR gu2.group_id IN (:whisperers_group_ids)" : ""}
+                                         WHEN u.admin OR u.moderator #{SiteSetting.whispers_allowed_groups_map.any? ? "OR gu2.group_id IN (:whisperers_group_ids)" : ""}
                                         THEN t.highest_staff_post_number
                                         ELSE t.highest_post_number
                                         END
@ -80,7 +78,7 @@ class GroupUser < ActiveRecord::Base
      last_seen: last_seen,
      limit: limit,
      now: 10.minutes.ago,
-      whisperers_group_ids: whisperers_group_ids,
+      whisperers_group_ids: SiteSetting.whispers_allowed_groups_map,
    )
  end

--- a/app/models/site_setting.rb
+++ b/app/models/site_setting.rb
@ -203,14 +203,6 @@ class SiteSetting < ActiveRecord::Base
    SiteSetting::Upload
  end

-  def self.whispers_allowed_group_ids
-    if SiteSetting.whispers_allowed_groups.present?
-      SiteSetting.whispers_allowed_groups_map
-    else
-      []
-    end
-  end
-
  def self.require_invite_code
    invite_code.present?
  end
--- a/app/models/topic_tracking_state.rb
+++ b/app/models/topic_tracking_state.rb
@ -90,7 +90,7 @@ class TopicTrackingState

    group_ids =
      if whisper
-        [Group::AUTO_GROUPS[:staff], *SiteSetting.whispers_allowed_group_ids]
+        [Group::AUTO_GROUPS[:staff], *SiteSetting.whispers_allowed_groups_map].flatten
      else
        secure_category_group_ids(topic)
      end
@ -152,7 +152,7 @@ class TopicTrackingState

    group_ids =
      if post.post_type == Post.types[:whisper]
-        [Group::AUTO_GROUPS[:staff], *SiteSetting.whispers_allowed_group_ids]
+        [Group::AUTO_GROUPS[:staff], *SiteSetting.whispers_allowed_groups_map].flatten
      else
        post.topic.category && post.topic.category.secure_group_ids
      end
--- a/app/models/user_stat.rb
+++ b/app/models/user_stat.rb
@ -18,8 +18,6 @@ class UserStat < ActiveRecord::Base
  UPDATE_UNREAD_USERS_LIMIT = 10_000

  def self.update_first_unread_pm(last_seen, limit: UPDATE_UNREAD_USERS_LIMIT)
-    whisperers_group_ids = SiteSetting.whispers_allowed_group_ids
-
    DB.exec(
      <<~SQL,
    UPDATE user_stats us
@ -37,11 +35,11 @@ class UserStat < ActiveRecord::Base
        INNER JOIN topics t ON t.id = tau.topic_id
        INNER JOIN users u ON u.id = tau.user_id
        LEFT JOIN topic_users tu ON t.id = tu.topic_id AND tu.user_id = tau.user_id
-        #{whisperers_group_ids.present? ? "LEFT JOIN group_users gu ON gu.group_id IN (:whisperers_group_ids) AND gu.user_id = u.id" : ""}
+        #{SiteSetting.whispers_allowed_groups_map.any? ? "LEFT JOIN group_users gu ON gu.group_id IN (:whisperers_group_ids) AND gu.user_id = u.id" : ""}
        WHERE t.deleted_at IS NULL
        AND t.archetype = :archetype
        AND tu.last_read_post_number < CASE
-                                       WHEN u.admin OR u.moderator #{whisperers_group_ids.present? ? "OR gu.id IS NOT NULL" : ""}
+                                       WHEN u.admin OR u.moderator #{SiteSetting.whispers_allowed_groups_map.any? ? "OR gu.id IS NOT NULL" : ""}
                                       THEN t.highest_staff_post_number
                                       ELSE t.highest_post_number
                                       END
@ -71,7 +69,7 @@ class UserStat < ActiveRecord::Base
      now: UPDATE_UNREAD_MINUTES_AGO.minutes.ago,
      last_seen: last_seen,
      limit: limit,
-      whisperers_group_ids: whisperers_group_ids,
+      whisperers_group_ids: SiteSetting.whispers_allowed_groups_map,
    )
  end

--- a/app/serializers/current_user_serializer.rb
+++ b/app/serializers/current_user_serializer.rb
@ -24,6 +24,7 @@ class CurrentUserSerializer < BasicUserSerializer
             :can_invite_to_forum,
             :no_password,
             :can_delete_account,
+             :can_post_anonymously,
             :custom_fields,
             :muted_category_ids,
             :indirectly_muted_category_ids,
@ -121,6 +122,15 @@ class CurrentUserSerializer < BasicUserSerializer
    scope.can_send_private_messages?
  end

+  def can_post_anonymously
+    SiteSetting.allow_anonymous_posting &&
+      (is_anonymous || object.in_any_groups?(SiteSetting.anonymous_posting_allowed_groups_map))
+  end
+
+  def can_ignore_users
+    !is_anonymous && object.in_any_groups?(SiteSetting.ignore_allowed_groups_map)
+  end
+
  def can_upload_avatar
    !is_anonymous && object.in_any_groups?(SiteSetting.uploaded_avatars_allowed_groups_map)
  end
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@ -327,7 +327,6 @@ basic:
    min: 0
    max: 6
  whispers_allowed_groups:
-    client: true
    type: group_list
    list_type: compact
    default: ""
@ -685,7 +684,6 @@ users:
  anonymous_posting_allowed_groups:
    default: "3|11" # auto group staff and trust_level_1
    type: group_list
-    client: true
    allow_any: false
    refresh: true
    validator: "AtLeastOneGroupValidator"
@ -867,7 +865,6 @@ posting:
  personal_message_enabled_groups:
    default: "3|11" # auto group trust_level_1
    type: group_list
-    client: true
    allow_any: false
    refresh: true
    validator: "AtLeastOneGroupValidator"
@ -885,7 +882,6 @@ posting:
  shared_drafts_allowed_groups:
    default: "3" # auto group staff
    type: group_list
-    client: true
    allow_any: false
    refresh: true
    validator: "AtLeastOneGroupValidator"
@ -966,7 +962,6 @@ posting:
  here_mention_allowed_groups:
    default: "3|12" # auto group staff and trust_level_2
    type: group_list
-    client: true
    allow_any: false
    refresh: true
    validator: "AtLeastOneGroupValidator"
@ -1812,7 +1807,6 @@ trust:
  ignore_allowed_groups:
    default: "3|12" # auto group staff and trust_level_2
    type: group_list
-    client: true
    allow_any: false
    refresh: true
    validator: "AtLeastOneGroupValidator"