From fc9648578b81d0cd1ddc128c084bd37e863f49f8 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Mon, 19 Feb 2024 13:25:59 +1100 Subject: [PATCH] DEV: Make more group-based settings client: false (#25735) Affects the following settings: delete_all_posts_and_topics_allowed_groups experimental_new_new_view_groups enable_experimental_admin_ui_groups custom_summarization_allowed_groups pm_tags_allowed_for_groups chat_allowed_groups direct_message_enabled_groups chat_message_flag_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. --- .../javascripts/admin/addon/routes/admin-route-map.js | 2 +- app/assets/javascripts/discourse/app/models/topic.js | 7 +------ app/serializers/current_user_serializer.rb | 10 ++++++++++ config/site_settings.yml | 11 ----------- ...able_experimental_admin_ui_groups_site_settings.rb | 11 +++++++++++ .../javascripts/discourse/initializers/chat-setup.js | 9 +-------- .../assets/javascripts/discourse/services/chat.js | 8 +------- plugins/chat/config/settings.yml | 3 --- plugins/chat/lib/chat/guardian_extensions.rb | 4 ++++ plugins/chat/plugin.rb | 9 +++++++++ 10 files changed, 38 insertions(+), 36 deletions(-) create mode 100644 db/migrate/20240219012001_remove_enable_experimental_admin_ui_groups_site_settings.rb diff --git a/app/assets/javascripts/admin/addon/routes/admin-route-map.js b/app/assets/javascripts/admin/addon/routes/admin-route-map.js index 5a37980aa57..2579e9bccda 100644 --- a/app/assets/javascripts/admin/addon/routes/admin-route-map.js +++ b/app/assets/javascripts/admin/addon/routes/admin-route-map.js @@ -221,7 +221,7 @@ export default function () { ); }); - // EXPERIMENTAL: These admin routes are hidden behind an `enable_experimental_admin_ui_groups` + // EXPERIMENTAL: These admin routes are hidden behind an `admin_sidebar_enabled_groups` // site setting and are subject to constant change. this.route("admin-revamp", { resetNamespace: true }, function () { this.route("lobby", { path: "/" }, function () {}); diff --git a/app/assets/javascripts/discourse/app/models/topic.js b/app/assets/javascripts/discourse/app/models/topic.js index 8c8de83a3aa..212055f222f 100644 --- a/app/assets/javascripts/discourse/app/models/topic.js +++ b/app/assets/javascripts/discourse/app/models/topic.js @@ -735,12 +735,7 @@ export default class Topic extends RestModel { !deleted_by.groups.some( (group) => group.name === this.category?.reviewable_by_group_name ) && - !( - this.siteSettings.delete_all_posts_and_topics_allowed_groups && - deleted_by.isInAnyGroups( - this.siteSettings.delete_all_posts_and_topics_allowed_groups - ) - )) + !deleted_by.can_delete_all_posts_and_topics) ) { DiscourseURL.redirectTo("/"); } diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index adc459419bd..ab6d6a2460f 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -26,6 +26,8 @@ class CurrentUserSerializer < BasicUserSerializer :can_delete_account, :can_post_anonymously, :can_ignore_users, + :can_delete_all_posts_and_topics, + :can_summarize, :custom_fields, :muted_category_ids, :indirectly_muted_category_ids, @@ -142,6 +144,14 @@ class CurrentUserSerializer < BasicUserSerializer !is_anonymous && object.in_any_groups?(SiteSetting.ignore_allowed_groups_map) end + def can_delete_all_posts_and_topics + object.in_any_groups?(SiteSetting.delete_all_posts_and_topics_allowed_groups_map) + end + + def can_summarize + object.in_any_groups?(SiteSetting.custom_summarization_allowed_groups_map) + end + def can_upload_avatar !is_anonymous && object.in_any_groups?(SiteSetting.uploaded_avatars_allowed_groups_map) end diff --git a/config/site_settings.yml b/config/site_settings.yml index 21d288741b4..64bb9209290 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1876,7 +1876,6 @@ trust: type: group_list allow_any: false refresh: true - client: true edit_all_topic_groups: default: "13" type: group_list @@ -2329,14 +2328,6 @@ developer: instrument_gc_stat_per_request: default: false hidden: true - enable_experimental_admin_ui_groups: - type: group_list - list_type: compact - default: "" - allow_any: false - refresh: true - hidden: true - client: true admin_sidebar_enabled_groups: type: group_list list_type: compact @@ -2611,7 +2602,6 @@ uncategorized: enum: "SummarizationStrategy" validator: "SummarizationValidator" custom_summarization_allowed_groups: - client: true type: group_list list_type: compact default: "3|13" # 3: @staff, 13: @trust_level_3 @@ -3091,7 +3081,6 @@ tags: client: true default: false pm_tags_allowed_for_groups: - client: true type: group_list list_type: compact default: "" diff --git a/db/migrate/20240219012001_remove_enable_experimental_admin_ui_groups_site_settings.rb b/db/migrate/20240219012001_remove_enable_experimental_admin_ui_groups_site_settings.rb new file mode 100644 index 00000000000..e8525fe12f0 --- /dev/null +++ b/db/migrate/20240219012001_remove_enable_experimental_admin_ui_groups_site_settings.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class RemoveEnableExperimentalAdminUiGroupsSiteSettings < ActiveRecord::Migration[7.0] + def up + execute "DELETE FROM site_settings WHERE name = 'enable_experimental_admin_ui_groups'" + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js index 5cadafb2238..dde7feb2436 100644 --- a/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js +++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js @@ -96,17 +96,10 @@ export default { }, }); - const summarizationAllowedGroups = - this.siteSettings.custom_summarization_allowed_groups - .split("|") - .map((id) => parseInt(id, 10)); - const canSummarize = this.siteSettings.summarization_strategy && this.currentUser && - this.currentUser.groups.some((g) => - summarizationAllowedGroups.includes(g.id) - ); + this.currentUser.can_summarize; if (canSummarize) { api.registerChatComposerButton({ diff --git a/plugins/chat/assets/javascripts/discourse/services/chat.js b/plugins/chat/assets/javascripts/discourse/services/chat.js index 69e8fd91ccd..291a12f96c8 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat.js @@ -65,13 +65,7 @@ export default class Chat extends Service { return false; } - return ( - this.currentUser.staff || - this.siteSettings.userInAnyGroups( - "direct_message_enabled_groups", - this.currentUser - ) - ); + return this.currentUser.staff || this.currentUser.can_direct_message; } @computed("chatChannelsManager.directMessageChannels") diff --git a/plugins/chat/config/settings.yml b/plugins/chat/config/settings.yml index 07a3b5a2241..c11b03d9b1d 100644 --- a/plugins/chat/config/settings.yml +++ b/plugins/chat/config/settings.yml @@ -6,7 +6,6 @@ chat: default: true client: true chat_allowed_groups: - client: true type: group_list list_type: compact default: "3|11" # 3: @staff, 11: @trust_level_1 @@ -101,14 +100,12 @@ chat: direct_message_enabled_groups: default: "11" # @trust_level_1 type: group_list - client: true allow_any: false refresh: true validator: "Chat::DirectMessageEnabledGroupsValidator" chat_message_flag_allowed_groups: default: "11" # @trust_level_1 type: group_list - client: true allow_any: false refresh: true max_mentions_per_chat_message: diff --git a/plugins/chat/lib/chat/guardian_extensions.rb b/plugins/chat/lib/chat/guardian_extensions.rb index faeefa96739..1158e81d7e4 100644 --- a/plugins/chat/lib/chat/guardian_extensions.rb +++ b/plugins/chat/lib/chat/guardian_extensions.rb @@ -16,6 +16,10 @@ module Chat @user.staff? || @user.in_any_groups?(Chat.allowed_group_ids) end + def can_direct_message? + @user.in_any_groups?(SiteSetting.direct_message_enabled_groups_map) + end + def can_create_chat_message? !SpamRule::AutoSilence.prevent_posting?(@user) end diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index d9ff7bf0365..e3399cf8970 100644 --- a/plugins/chat/plugin.rb +++ b/plugins/chat/plugin.rb @@ -143,6 +143,15 @@ after_initialize do end, ) { true } + add_to_serializer( + :current_user, + :can_direct_message, + include_condition: -> do + return @can_direct_message if defined?(@can_direct_message) + @can_direct_message = SiteSetting.chat_enabled && scope.can_direct_message? + end, + ) { true } + add_to_serializer( :current_user, :has_chat_enabled,