From e62e93f83a77adfa80b38fbfecf82bbee14e12fe Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Mon, 26 Sep 2022 13:58:40 +1000 Subject: [PATCH] FEATURE: Introduce personal_message_enabled_groups setting (#18042) This will replace `enable_personal_messages` and `min_trust_to_send_messages`, this commit introduces the setting `personal_message_enabled_groups` and uses it in all places that `enable_personal_messages` and `min_trust_to_send_messages` currently apply. A migration is included to set `personal_message_enabled_groups` based on the following rules: * If `enable_personal_messages` was false, then set `personal_message_enabled_groups` to `3`, which is the staff auto group * If `min_trust_to_send_messages` is not default (1) and the above condition is false, then set the `personal_message_enabled_groups` setting to the appropriate auto group based on the trust level * Otherwise just set `personal_message_enabled_groups` to 11 which is the TL1 auto group After follow-up PRs to plugins using these old settings, we will be able to drop the old settings from core, in the meantime I've added DEPRECATED notices to their descriptions and added them to the deprecated site settings list. This commit also introduces a `_map` shortcut method definition for all `group_list` site settings, e.g. `SiteSetting.personal_message_enabled_groups` also has `SiteSetting.personal_message_enabled_groups_map` available, which automatically splits the setting by `|` and converts it into an array of integers. --- .../app/components/sidebar/user/sections.hbs | 2 +- .../app/components/sidebar/user/sections.js | 5 ++ .../app/components/topic-footer-buttons.js | 13 +++- .../app/components/user-menu/menu.js | 4 +- .../discourse/app/controllers/group.js | 8 +- .../controllers/keyboard-shortcuts-help.js | 2 +- .../controllers/preferences/notifications.js | 6 ++ .../app/controllers/preferences/users.js | 5 ++ .../discourse/app/controllers/topic.js | 5 ++ .../discourse/app/controllers/user.js | 10 ++- .../discourse/app/lib/keyboard-shortcuts.js | 3 +- .../javascripts/discourse/app/models/user.js | 24 ++++++ .../templates/preferences/notifications.hbs | 2 +- .../app/templates/preferences/users.hbs | 2 +- .../discourse/app/templates/topic.hbs | 2 +- .../discourse/app/widgets/user-menu.js | 2 +- .../sidebar-user-messages-section-test.js | 8 +- .../tests/fixtures/session-fixtures.js | 16 +++- .../discourse/tests/helpers/component-test.js | 17 ++++ .../discourse/tests/helpers/site-settings.js | 1 + .../components/user-menu/menu-test.js | 10 ++- .../components/widgets/hamburger-menu-test.js | 2 +- .../components/widgets/user-menu-test.js | 5 +- app/controllers/users_controller.rb | 2 +- app/models/site_setting.rb | 1 + app/models/user.rb | 11 +++ app/serializers/current_user_serializer.rb | 2 +- config/locales/server.en.yml | 6 +- config/site_settings.yml | 7 ++ ...led_groups_based_on_deprecated_settings.rb | 35 +++++++++ lib/guardian.rb | 21 +++-- lib/guardian/group_guardian.rb | 6 +- lib/guardian/post_guardian.rb | 6 +- lib/guardian/tag_guardian.rb | 1 + lib/guardian/topic_guardian.rb | 3 +- lib/site_setting_extension.rb | 10 ++- lib/site_settings/deprecated_settings.rb | 2 + lib/topic_query.rb | 13 +++- ...rsonal_message_enabled_groups_validator.rb | 16 ++++ spec/lib/admin_user_index_query_spec.rb | 4 +- spec/lib/guardian_spec.rb | 78 +++++++++++-------- spec/lib/post_action_creator_spec.rb | 4 + spec/lib/topic_creator_spec.rb | 10 +-- spec/lib/topic_query_spec.rb | 14 +++- spec/models/post_action_spec.rb | 1 + spec/models/topic_spec.rb | 7 +- spec/models/trust_level3_requirements_spec.rb | 2 +- spec/requests/groups_controller_spec.rb | 19 +---- spec/requests/list_controller_spec.rb | 4 +- spec/requests/post_actions_controller_spec.rb | 4 + spec/requests/topics_controller_spec.rb | 19 +++++ spec/requests/users_controller_spec.rb | 16 ++-- spec/serializers/post_serializer_spec.rb | 4 + .../serializers/topic_view_serializer_spec.rb | 9 +++ spec/services/user_merger_spec.rb | 4 + 55 files changed, 375 insertions(+), 120 deletions(-) create mode 100644 db/migrate/20220825054405_fill_personal_message_enabled_groups_based_on_deprecated_settings.rb create mode 100644 lib/validators/personal_message_enabled_groups_validator.rb diff --git a/app/assets/javascripts/discourse/app/components/sidebar/user/sections.hbs b/app/assets/javascripts/discourse/app/components/sidebar/user/sections.hbs index 47044fd4e95..a5987ba0d94 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/user/sections.hbs +++ b/app/assets/javascripts/discourse/app/components/sidebar/user/sections.hbs @@ -6,7 +6,7 @@ {{/if}} - {{#if this.siteSettings.enable_personal_messages}} + {{#if this.enableMessagesSection}} {{/if}} diff --git a/app/assets/javascripts/discourse/app/components/sidebar/user/sections.js b/app/assets/javascripts/discourse/app/components/sidebar/user/sections.js index 36428e17a8b..41ed90a1b26 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/user/sections.js +++ b/app/assets/javascripts/discourse/app/components/sidebar/user/sections.js @@ -6,6 +6,7 @@ import { inject as service } from "@ember/service"; export default class SidebarUserSections extends Component { @service siteSettings; @service currentUser; + @service site; customSections; @@ -21,4 +22,8 @@ export default class SidebarUserSections extends Component { return section; }); } + + get enableMessagesSection() { + return this.currentUser?.allowPersonalMessages; + } } diff --git a/app/assets/javascripts/discourse/app/components/topic-footer-buttons.js b/app/assets/javascripts/discourse/app/components/topic-footer-buttons.js index ab1430ae508..e352343170b 100644 --- a/app/assets/javascripts/discourse/app/components/topic-footer-buttons.js +++ b/app/assets/javascripts/discourse/app/components/topic-footer-buttons.js @@ -15,9 +15,9 @@ export default Component.extend({ // Allow us to extend it layoutName: "components/topic-footer-buttons", - @discourseComputed("topic.isPrivateMessage") - canArchive(isPM) { - return this.siteSettings.enable_personal_messages && isPM; + @discourseComputed("canSendPms", "topic.isPrivateMessage") + canArchive(canSendPms, isPM) { + return canSendPms && isPM; }, inlineButtons: getTopicFooterButtons(), @@ -43,7 +43,12 @@ export default Component.extend({ @discourseComputed("topic.isPrivateMessage") showNotificationsButton(isPM) { - return !isPM || this.siteSettings.enable_personal_messages; + return !isPM || this.canSendPms; + }, + + @discourseComputed("currentUser.allowPersonalMessages") + canSendPms() { + return this.currentUser?.allowPersonalMessages; }, canInviteTo: alias("topic.details.can_invite_to"), diff --git a/app/assets/javascripts/discourse/app/components/user-menu/menu.js b/app/assets/javascripts/discourse/app/components/user-menu/menu.js index 501ebaa871e..fc9688a2e1f 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/menu.js +++ b/app/assets/javascripts/discourse/app/components/user-menu/menu.js @@ -163,9 +163,7 @@ const CORE_TOP_TABS = [ } get shouldDisplay() { - return ( - this.siteSettings.enable_personal_messages || this.currentUser.staff - ); + return this.currentUser?.allowPersonalMessages; } get notificationTypes() { diff --git a/app/assets/javascripts/discourse/app/controllers/group.js b/app/assets/javascripts/discourse/app/controllers/group.js index 27f074fd93e..a847b619c20 100644 --- a/app/assets/javascripts/discourse/app/controllers/group.js +++ b/app/assets/javascripts/discourse/app/controllers/group.js @@ -88,9 +88,13 @@ export default Controller.extend({ return defaultTabs; }, - @discourseComputed("model.has_messages", "model.is_group_user") + @discourseComputed( + "model.has_messages", + "model.is_group_user", + "currentUser.allowPersonalMessages" + ) showMessages(hasMessages, isGroupUser) { - if (!this.siteSettings.enable_personal_messages) { + if (!this.currentUser?.allowPersonalMessages) { return false; } diff --git a/app/assets/javascripts/discourse/app/controllers/keyboard-shortcuts-help.js b/app/assets/javascripts/discourse/app/controllers/keyboard-shortcuts-help.js index a1bcbfa432b..efc7ed5e9f4 100644 --- a/app/assets/javascripts/discourse/app/controllers/keyboard-shortcuts-help.js +++ b/app/assets/javascripts/discourse/app/controllers/keyboard-shortcuts-help.js @@ -326,7 +326,7 @@ export default Controller.extend(ModalFunctionality, { bookmarks: buildShortcut("jump_to.bookmarks", { keys1: ["g", "b"] }), profile: buildShortcut("jump_to.profile", { keys1: ["g", "p"] }), }; - if (this.siteSettings.enable_personal_messages) { + if (this.currentUser?.allowPersonalMessages) { shortcuts.messages = buildShortcut("jump_to.messages", { keys1: ["g", "m"], }); diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/notifications.js b/app/assets/javascripts/discourse/app/controllers/preferences/notifications.js index 55b6304d354..897c9fb30a6 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/notifications.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/notifications.js @@ -1,4 +1,5 @@ import Controller from "@ember/controller"; +import discourseComputed from "discourse-common/utils/decorators"; import I18n from "I18n"; import { NotificationLevels } from "discourse/lib/notification-levels"; import { popupAjaxError } from "discourse/lib/ajax-error"; @@ -89,6 +90,11 @@ export default Controller.extend({ ]; }, + @discourseComputed("currentUser.allowPersonalMessages") + showMessageSettings() { + return this.currentUser?.allowPersonalMessages; + }, + actions: { save() { this.set("saved", false); diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/users.js b/app/assets/javascripts/discourse/app/controllers/preferences/users.js index 65f457bbfdc..f20db86e7ba 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/users.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/users.js @@ -72,6 +72,11 @@ export default Controller.extend({ return !allowPrivateMessages; }, + @discourseComputed("currentUser.allowPersonalMessages") + showMessageSettings() { + return this.currentUser?.allowPersonalMessages; + }, + @action save() { this.set("saved", false); diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js index d6604556f0b..aace5530cbe 100644 --- a/app/assets/javascripts/discourse/app/controllers/topic.js +++ b/app/assets/javascripts/discourse/app/controllers/topic.js @@ -212,6 +212,11 @@ export default Controller.extend(bufferedProperty("model"), { ); }, + @discourseComputed("currentUser.allowPersonalMessages") + canSendPms() { + return this.currentUser?.allowPersonalMessages; + }, + @discourseComputed("buffered.category_id") minimumRequiredTags(categoryId) { return Category.findById(categoryId)?.minimumRequiredTags || 0; diff --git a/app/assets/javascripts/discourse/app/controllers/user.js b/app/assets/javascripts/discourse/app/controllers/user.js index 1c00e2fd7c3..4c8ba40cbca 100644 --- a/app/assets/javascripts/discourse/app/controllers/user.js +++ b/app/assets/javascripts/discourse/app/controllers/user.js @@ -116,11 +116,13 @@ export default Controller.extend(CanCheckEmails, { return viewingSelf; }, - @discourseComputed("viewingSelf", "currentUser.admin") + @discourseComputed( + "viewingSelf", + "currentUser.admin", + "currentUser.allowPersonalMessages" + ) showPrivateMessages(viewingSelf, isAdmin) { - return ( - this.siteSettings.enable_personal_messages && (viewingSelf || isAdmin) - ); + return this.currentUser?.allowPersonalMessages && (viewingSelf || isAdmin); }, @discourseComputed("viewingSelf", "currentUser.admin") diff --git a/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js b/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js index 8de8bca0062..f6b8d156eff 100644 --- a/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js +++ b/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js @@ -137,9 +137,10 @@ export default { this.appEvents = this.container.lookup("service:app-events"); this.currentUser = this.container.lookup("service:current-user"); this.siteSettings = this.container.lookup("service:site-settings"); + this.site = this.container.lookup("service:site"); // Disable the shortcut if private messages are disabled - if (!this.siteSettings.enable_personal_messages) { + if (!this.currentUser?.allowPersonalMessages) { delete DEFAULT_BINDINGS["g m"]; } }, diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js index 3e11d6ef094..5b833911933 100644 --- a/app/assets/javascripts/discourse/app/models/user.js +++ b/app/assets/javascripts/discourse/app/models/user.js @@ -626,6 +626,18 @@ const User = RestModel.extend({ return filteredGroups.length > numGroupsToDisplay; }, + isInAnyGroups(groupIds) { + if (!this.groups) { + return; + } + + // auto group ID 0 is "everyone" + return ( + groupIds.includes(0) || + this.groups.mapBy("id").some((groupId) => groupIds.includes(groupId)) + ); + }, + // The user's stat count, excluding PMs. @discourseComputed("statsExcludingPms.@each.count") statsCountNonPM() { @@ -1062,6 +1074,18 @@ const User = RestModel.extend({ trackedTags(trackedTags, watchedTags, watchingFirstPostTags) { return [...trackedTags, ...watchedTags, ...watchingFirstPostTags]; }, + + @discourseComputed("staff", "groups.[]") + allowPersonalMessages() { + return ( + this.staff || + this.isInAnyGroups( + this.siteSettings.personal_message_enabled_groups + .split("|") + .map((groupId) => parseInt(groupId, 10)) + ) + ); + }, }); User.reopenClass(Singleton, { diff --git a/app/assets/javascripts/discourse/app/templates/preferences/notifications.hbs b/app/assets/javascripts/discourse/app/templates/preferences/notifications.hbs index 3d1f882f904..6f142e25a1c 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/notifications.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/notifications.hbs @@ -33,7 +33,7 @@ -{{#if this.siteSettings.enable_personal_messages}} +{{#if this.showMessageSettings}}
diff --git a/app/assets/javascripts/discourse/app/templates/preferences/users.hbs b/app/assets/javascripts/discourse/app/templates/preferences/users.hbs index ebf6b46a744..5589f14d47e 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/users.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/users.hbs @@ -21,7 +21,7 @@
{{i18n "user.muted_users_instructions"}}
-{{#if this.siteSettings.enable_personal_messages}} +{{#if this.showMessageSettings}}
diff --git a/app/assets/javascripts/discourse/app/templates/topic.hbs b/app/assets/javascripts/discourse/app/templates/topic.hbs index 3d49a7357c6..41fa19fd1d5 100644 --- a/app/assets/javascripts/discourse/app/templates/topic.hbs +++ b/app/assets/javascripts/discourse/app/templates/topic.hbs @@ -52,7 +52,7 @@ {{else}}

{{#unless this.model.is_warning}} - {{#if this.siteSettings.enable_personal_messages}} + {{#if this.canSendPms}} {{else}} diff --git a/app/assets/javascripts/discourse/app/widgets/user-menu.js b/app/assets/javascripts/discourse/app/widgets/user-menu.js index 3e4efc53b6e..49e589e7679 100644 --- a/app/assets/javascripts/discourse/app/widgets/user-menu.js +++ b/app/assets/javascripts/discourse/app/widgets/user-menu.js @@ -156,7 +156,7 @@ createWidget("user-menu-links", { glyphs.push(this.bookmarksGlyph()); - if (this.siteSettings.enable_personal_messages || this.currentUser.staff) { + if (this.currentUser?.allowPersonalMessages) { glyphs.push(this.messagesGlyph()); } diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-messages-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-messages-section-test.js index ad18f04e126..c42ba0d83e2 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-messages-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-messages-section-test.js @@ -12,14 +12,14 @@ import { import { NotificationLevels } from "discourse/lib/notification-levels"; acceptance( - "Sidebar - Logged on user - Messages Section - enable_personal_messages disabled", + "Sidebar - Logged on user - Messages Section - user not in personal_message_enabled_groups", function (needs) { - needs.user(); + needs.user({ moderator: false, admin: false }); needs.settings({ enable_experimental_sidebar_hamburger: true, enable_sidebar: true, - enable_personal_messages: false, + personal_message_enabled_groups: "13", // trust_level_3 auto group ID; }); test("clicking on section header button", async function (assert) { @@ -34,7 +34,7 @@ acceptance( ); acceptance( - "Sidebar - Logged on user - Messages Section - enable_personal_messages enabled", + "Sidebar - Logged on user - Messages Section - user in personal_message_enabled_groups", function (needs) { needs.user(); diff --git a/app/assets/javascripts/discourse/tests/fixtures/session-fixtures.js b/app/assets/javascripts/discourse/tests/fixtures/session-fixtures.js index 08a365ecaab..d99b6ab8258 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/session-fixtures.js +++ b/app/assets/javascripts/discourse/tests/fixtures/session-fixtures.js @@ -33,7 +33,21 @@ export default { timezone: "Australia/Brisbane", skip_new_user_tips: false, can_review: true, - ignored_users: [] + ignored_users: [], + groups: [ + { + id: 10, + automatic: true, + name: "trust_level_0", + display_name: "trust_level_0", + }, + { + id: 11, + automatic: true, + name: "trust_level_1", + display_name: "trust_level_1", + } + ] }, }, }; diff --git a/app/assets/javascripts/discourse/tests/helpers/component-test.js b/app/assets/javascripts/discourse/tests/helpers/component-test.js index ed8cfde25d2..4e79dd558a1 100644 --- a/app/assets/javascripts/discourse/tests/helpers/component-test.js +++ b/app/assets/javascripts/discourse/tests/helpers/component-test.js @@ -25,6 +25,23 @@ export function setupRenderingTest(hooks) { const currentUser = User.create({ username: "eviltrout", timezone: "Australia/Brisbane", + name: "Robin Ward", + admin: false, + moderator: false, + groups: [ + { + id: 10, + automatic: true, + name: "trust_level_0", + display_name: "trust_level_0", + }, + { + id: 11, + automatic: true, + name: "trust_level_1", + display_name: "trust_level_1", + }, + ], }); this.currentUser = currentUser; this.owner.unregister("service:current-user"); diff --git a/app/assets/javascripts/discourse/tests/helpers/site-settings.js b/app/assets/javascripts/discourse/tests/helpers/site-settings.js index 95c591654ab..4f02fb7839a 100644 --- a/app/assets/javascripts/discourse/tests/helpers/site-settings.js +++ b/app/assets/javascripts/discourse/tests/helpers/site-settings.js @@ -95,6 +95,7 @@ const ORIGINAL_SETTINGS = { desktop_category_page_style: "categories_and_latest_topics", enable_mentions: true, enable_personal_messages: true, + personal_message_enabled_groups: "11", // TL1 group unicode_usernames: false, secure_media: false, external_emoji_url: "", diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js b/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js index a0de1cada5e..e4e131b88eb 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js @@ -98,10 +98,11 @@ module("Integration | Component | user-menu", function (hooks) { ); }); - test("messages tab isn't shown if current user isn't staff and enable_personal_messages setting is disabled", async function (assert) { + test("messages tab isn't shown if current user isn't staff and user does not belong to personal_message_enabled_groups", async function (assert) { this.currentUser.set("moderator", false); this.currentUser.set("admin", false); - this.siteSettings.enable_personal_messages = false; + this.currentUser.set("groups", []); + this.siteSettings.personal_message_enabled_groups = "13"; // trust_level_3 auto group ID; await render(template); @@ -117,10 +118,11 @@ module("Integration | Component | user-menu", function (hooks) { ); }); - test("messages tab is shown if current user is staff even if enable_personal_messages setting is disabled", async function (assert) { + test("messages tab is shown if current user is staff even if they do not belong to personal_message_enabled_groups", async function (assert) { this.currentUser.set("moderator", true); this.currentUser.set("admin", false); - this.siteSettings.enable_personal_messages = false; + this.currentUser.set("groups", []); + this.siteSettings.personal_message_enabled_groups = "999"; await render(template); diff --git a/app/assets/javascripts/discourse/tests/integration/components/widgets/hamburger-menu-test.js b/app/assets/javascripts/discourse/tests/integration/components/widgets/hamburger-menu-test.js index f0913a66780..23b4a074dbe 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/widgets/hamburger-menu-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/widgets/hamburger-menu-test.js @@ -34,7 +34,7 @@ module("Integration | Component | Widget | hamburger-menu", function (hooks) { }); test("staff menu - not staff", async function (assert) { - this.currentUser.set("staff", false); + this.currentUser.setProperties({ admin: false, moderator: false }); await render(hbs``); diff --git a/app/assets/javascripts/discourse/tests/integration/components/widgets/user-menu-test.js b/app/assets/javascripts/discourse/tests/integration/components/widgets/user-menu-test.js index 3989fd854eb..a3e717edc90 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/widgets/user-menu-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/widgets/user-menu-test.js @@ -92,7 +92,8 @@ module("Integration | Component | Widget | user-menu", function (hooks) { }); test("private messages - disabled", async function (assert) { - this.siteSettings.enable_personal_messages = false; + this.currentUser.setProperties({ admin: false, moderator: false }); + this.siteSettings.personal_message_enabled_groups = "13"; // trust_level_3 auto group ID; await render(hbs``); @@ -100,7 +101,7 @@ module("Integration | Component | Widget | user-menu", function (hooks) { }); test("private messages - enabled", async function (assert) { - this.siteSettings.enable_personal_messages = true; + this.siteSettings.personal_message_enabled_groups = "11"; // trust_level_1 auto group ID; await render(hbs``); diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 662a2c3a6e9..9367dd73ffa 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1799,7 +1799,7 @@ class UsersController < ApplicationController raise Discourse::InvalidAccess.new("username doesn't match current_user's username") end - if !current_user.staff? && !SiteSetting.enable_personal_messages + if !current_user.staff? && !current_user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map) raise Discourse::InvalidAccess.new("personal messages are disabled.") end diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index b074d95af44..fc73aa76dc6 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -193,6 +193,7 @@ class SiteSetting < ActiveRecord::Base def self.whispers_allowed_group_ids if SiteSetting.enable_whispers && SiteSetting.whispers_allowed_groups.present? + # TODO (martin) Change to whispers_allowed_groups_map SiteSetting.whispers_allowed_groups.split("|").map(&:to_i) else [] diff --git a/app/models/user.rb b/app/models/user.rb index 24afd78f3ca..0fbc35849e3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -412,6 +412,14 @@ class User < ActiveRecord::Base find_by(username_lower: normalize_username(username)) end + def in_any_groups?(group_ids) + group_ids.include?(Group::AUTO_GROUPS[:everyone]) || (group_ids & belonging_to_group_ids).any? + end + + def belonging_to_group_ids + @belonging_to_group_ids ||= group_users.pluck(:group_id) + end + def group_granted_trust_level GroupUser .where(user_id: id) @@ -495,6 +503,7 @@ class User < ActiveRecord::Base @user_fields_cache = nil @ignored_user_ids = nil @muted_user_ids = nil + @belonging_to_group_ids = nil super end @@ -1462,6 +1471,8 @@ class User < ActiveRecord::Base GroupActionLogger.new(Discourse.system_user, group).log_add_user_to_group(self) end end + + @belonging_to_group_ids = nil end def email diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index ead635ea801..17d468d0b27 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -347,7 +347,7 @@ class CurrentUserSerializer < BasicUserSerializer def redesigned_user_page_nav_enabled if SiteSetting.enable_new_user_profile_nav_groups.present? - GroupUser.exists?(user_id: object.id, group_id: SiteSetting.enable_new_user_profile_nav_groups.split("|")) + object.in_any_groups?(SiteSetting.enable_new_user_profile_nav_groups_map) else false end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ed274972cbd..50e57853af0 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1566,7 +1566,8 @@ en: summary_max_results: "Maximum posts returned by 'Summarize This Topic'" summary_timeline_button: "Show a 'Summarize' button in the timeline" - enable_personal_messages: "Allow trust level 1 (configurable via min trust to send messages) users to create messages and reply to messages. Note that staff can always send messages no matter what." + enable_personal_messages: "DEPRECATED, use the 'personal message enabled groups' setting instead. Allow trust level 1 (configurable via min trust to send messages) users to create messages and reply to messages. Note that staff can always send messages no matter what." + personal_message_enabled_groups: "Allow users within these groups to create messages and reply to messages. Trust level groups include all trust levels above that number, for example choosing trust_level_1 also allows trust_level_2, 3, 4 users to send PMs. Note that staff can always send messages no matter what." enable_system_message_replies: "Allows users to reply to system messages, even if personal messages are disabled" enable_chunked_encoding: "Enable chunked encoding responses by the server. This feature works on most setups however some proxies may buffer, causing responses to be delayed" long_polling_base_url: "Base URL used for long polling (when a CDN is serving dynamic content, be sure to set this to origin pull) eg: http://origin.site.com" @@ -1889,7 +1890,7 @@ en: min_trust_to_allow_self_wiki: "The minimum trust level required to make user's own post wiki." - min_trust_to_send_messages: "The minimum trust level required to create new personal messages." + min_trust_to_send_messages: "DEPRECATED, use the 'personal message enabled groups' setting instead. The minimum trust level required to create new personal messages." min_trust_to_send_email_messages: "The minimum trust level required to send personal messages via email." min_trust_to_flag_posts: "The minimum trust level required to flag posts" min_trust_to_post_links: "The minimum trust level required to include links in posts" @@ -2388,6 +2389,7 @@ en: reply_by_email_address_is_empty: "You must set a 'reply by email address' before enabling reply by email." email_polling_disabled: "You must enable either manual or POP3 polling before enabling reply by email." user_locale_not_enabled: "You must first enable 'allow user locale' before enabling this setting." + personal_message_enabled_groups_invalid: "You must specify at least one group for this setting. If you do not want anyone except staff to send PMs, choose the staff group." invalid_regex: "Regex is invalid or not allowed." invalid_regex_with_message: "The regex '%{regex}' has an error: %{message}" email_editable_enabled: "You must disable 'email editable' before enabling this setting." diff --git a/config/site_settings.yml b/config/site_settings.yml index 5c83f6af3fc..c447cff5537 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -818,6 +818,13 @@ posting: client: true enable_system_message_replies: default: true + personal_message_enabled_groups: + default: "11" # auto group trust_level_1 + type: group_list + client: true + allow_any: false + refresh: true + validator: "PersonalMessageEnabledGroupsValidator" editing_grace_period: 300 editing_grace_period_max_diff: 100 editing_grace_period_max_diff_high_trust: 400 diff --git a/db/migrate/20220825054405_fill_personal_message_enabled_groups_based_on_deprecated_settings.rb b/db/migrate/20220825054405_fill_personal_message_enabled_groups_based_on_deprecated_settings.rb new file mode 100644 index 00000000000..cc10befd0f7 --- /dev/null +++ b/db/migrate/20220825054405_fill_personal_message_enabled_groups_based_on_deprecated_settings.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class FillPersonalMessageEnabledGroupsBasedOnDeprecatedSettings < ActiveRecord::Migration[7.0] + def up + enable_personal_messages_raw = DB.query_single("SELECT value FROM site_settings WHERE name = 'enable_personal_messages'").first + enable_personal_messages = enable_personal_messages_raw.blank? || enable_personal_messages_raw == 't' + + min_trust_to_send_messages_raw = DB.query_single("SELECT value FROM site_settings WHERE name = 'min_trust_to_send_messages'").first + min_trust_to_send_messages = (min_trust_to_send_messages_raw.blank? ? 1 : min_trust_to_send_messages_raw).to_i + + # default to TL1, Group::AUTO_GROUPS[:trust_level_1] is 11 + personal_message_enabled_groups = "11" + + if min_trust_to_send_messages != 1 + # Group::AUTO_GROUPS[:trust_level_N] range from 10-14 + personal_message_enabled_groups = "1#{min_trust_to_send_messages}" + end + + # only allow staff if the setting was previously disabled, Group::AUTO_GROUPS[:staff] is 3 + if !enable_personal_messages + personal_message_enabled_groups = "3" + end + + # data_type 20 is group_list + DB.exec( + "INSERT INTO site_settings(name, value, data_type, created_at, updated_at) + VALUES('personal_message_enabled_groups', :setting, '20', NOW(), NOW())", + setting: personal_message_enabled_groups + ) + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/guardian.rb b/lib/guardian.rb index a0f98dab147..d4b557ec4ac 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -71,6 +71,9 @@ class Guardian def whisperer? false end + def in_any_groups?(group_ids) + false + end end attr_reader :request @@ -398,7 +401,12 @@ class Guardian if object.is_a?(Topic) if object.private_message? return true if is_admin? - return false unless SiteSetting.enable_personal_messages? + + # TODO (martin) Remove enable_personal_messages here once plugins have been changed. + if !@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map) || + !SiteSetting.enable_personal_messages + return false + end return false if object.reached_recipients_limit? && !is_staff? end @@ -447,12 +455,11 @@ class Guardian (is_group || is_user) && # User is authenticated authenticated? && - # Have to be a basic level at least - (is_group || @user.has_trust_level?(SiteSetting.min_trust_to_send_messages) || notify_moderators) && # User disabled private message (is_staff? || is_group || target.user_option.allow_private_messages) && - # PMs are enabled - (is_staff? || SiteSetting.enable_personal_messages || notify_moderators) && + # User can send PMs, this can be covered by trust levels as well via AUTO_GROUPS + # TODO (martin) Remove enable_personal_messages here once plugins have been changed. + (is_staff? || (@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map) || SiteSetting.enable_personal_messages) || notify_moderators) && # Can't send PMs to suspended users (is_staff? || is_group || !target.suspended?) && # Check group messageable level @@ -467,7 +474,9 @@ class Guardian # User is authenticated return false if !authenticated? # User is trusted enough - SiteSetting.enable_personal_messages && @user.has_trust_level_or_staff?(SiteSetting.min_trust_to_send_email_messages) + # TODO (martin) Remove enable_personal_messages here once plugins have been changed. + (@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map) || SiteSetting.enable_personal_messages) && + @user.has_trust_level_or_staff?(SiteSetting.min_trust_to_send_email_messages) end def can_export_entity?(entity) diff --git a/lib/guardian/group_guardian.rb b/lib/guardian/group_guardian.rb index 1a869e4e7fe..e5df8bfdc66 100644 --- a/lib/guardian/group_guardian.rb +++ b/lib/guardian/group_guardian.rb @@ -33,8 +33,12 @@ module GroupGuardian def can_see_group_messages?(group) return true if is_admin? return true if is_moderator? && group.id == Group::AUTO_GROUPS[:moderators] + return false if user.blank? - SiteSetting.enable_personal_messages? && group.users.include?(user) + # TODO (martin) Remove enable_personal_messages here once plugins have been changed. + (SiteSetting.enable_personal_messages || + user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map)) && + group.users.include?(user) end def can_associate_groups? diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index 15a3c6f3694..a4f9e536ecf 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -50,10 +50,10 @@ module PostGuardian (!SiteSetting.allow_flagging_staff?) && post&.user&.staff? + # TODO (martin) Remove enable_personal_messages here once plugins have been changed. if action_key == :notify_user && - (!SiteSetting.enable_personal_messages? || - !@user.has_trust_level?(SiteSetting.min_trust_to_send_messages)) - + (!@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map) || + !SiteSetting.enable_personal_messages) return false end diff --git a/lib/guardian/tag_guardian.rb b/lib/guardian/tag_guardian.rb index 4f14858f013..8d445fca9c4 100644 --- a/lib/guardian/tag_guardian.rb +++ b/lib/guardian/tag_guardian.rb @@ -15,6 +15,7 @@ module TagGuardian return false if @user.blank? return true if @user == Discourse.system_user + # TODO (martin) Change to pm_tags_allowed_for_groups_map group_ids = SiteSetting.pm_tags_allowed_for_groups.to_s.split("|").map(&:to_i) group_ids.include?(Group::AUTO_GROUPS[:everyone]) || @user.group_users.exists?(group_id: group_ids) end diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index 06141292247..5d974bad280 100644 --- a/lib/guardian/topic_guardian.rb +++ b/lib/guardian/topic_guardian.rb @@ -183,7 +183,8 @@ module TopicGuardian end def can_convert_topic?(topic) - return false unless SiteSetting.enable_personal_messages? + # TODO (martin) Remove enable_personal_messages here once plugins have been changed. + return false unless SiteSetting.enable_personal_messages? || @user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map) return false if topic.blank? return false if topic.trashed? return false if topic.is_category_topic? diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index 8953e4fc6f2..0a5699bbc1d 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -542,6 +542,15 @@ module SiteSettingExtension end end + # Any group_list setting, e.g. personal_message_enabled_groups, will have + # a getter defined with _map on the end, e.g. personal_message_enabled_groups_map, + # to avoid having to manually split and convert to integer for these settings. + if type_supervisor.get_type(name) == :group_list + define_singleton_method("#{clean_name}_map") do + self.public_send(clean_name).to_s.split("|").map(&:to_i) + end + end + define_singleton_method "#{clean_name}?" do self.public_send clean_name end @@ -592,5 +601,4 @@ module SiteSettingExtension def logger Rails.logger end - end diff --git a/lib/site_settings/deprecated_settings.rb b/lib/site_settings/deprecated_settings.rb index cb8f352287c..8690c79ae90 100644 --- a/lib/site_settings/deprecated_settings.rb +++ b/lib/site_settings/deprecated_settings.rb @@ -7,6 +7,8 @@ module SiteSettings::DeprecatedSettings # [, , , ] ['search_tokenize_chinese_japanese_korean', 'search_tokenize_chinese', true, '2.9'], ['default_categories_regular', 'default_categories_normal', true, '3.0'], + ['min_trust_to_send_messages', 'personal_message_enabled_groups', false, '3.0'], + ['enable_personal_messages', 'personal_message_enabled_groups', false, '3.0'], ] def setup_deprecated_methods diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 3f7a5aecb44..4e863226068 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -172,7 +172,11 @@ class TopicQuery def list_related_for(topic, pm_params: nil) return if !topic.private_message? return if @user.blank? - return if !SiteSetting.enable_personal_messages? + + # TODO (martin) Remove enable_personal_messages here once plugins have been changed. + if !SiteSetting.enable_personal_messages || !@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map) + return + end builder = SuggestedTopicsBuilder.new(topic) pm_params = pm_params || get_pm_params(topic) @@ -197,10 +201,13 @@ class TopicQuery # Return a list of suggested topics for a topic def list_suggested_for(topic, pm_params: nil) + # TODO (martin) Remove enable_personal_messages here once plugins have been changed. # Don't suggest messages unless we have a user, and private messages are # enabled. - return if topic.private_message? && - (@user.blank? || !SiteSetting.enable_personal_messages?) + if topic.private_message? && ( + @user.blank? || !@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map || !SiteSetting.enable_personal_messages)) + return + end builder = SuggestedTopicsBuilder.new(topic) diff --git a/lib/validators/personal_message_enabled_groups_validator.rb b/lib/validators/personal_message_enabled_groups_validator.rb new file mode 100644 index 00000000000..8def3bc3e5d --- /dev/null +++ b/lib/validators/personal_message_enabled_groups_validator.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class PersonalMessageEnabledGroupsValidator + + def initialize(opts = {}) + @opts = opts + end + + def valid_value?(val) + val.present? && val != "" + end + + def error_message + I18n.t("site_settings.errors.personal_message_enabled_groups_invalid") + end +end diff --git a/spec/lib/admin_user_index_query_spec.rb b/spec/lib/admin_user_index_query_spec.rb index 4bce40f83e4..71fe1856d8c 100644 --- a/spec/lib/admin_user_index_query_spec.rb +++ b/spec/lib/admin_user_index_query_spec.rb @@ -80,10 +80,12 @@ RSpec.describe AdminUserIndexQuery do TrustLevel.levels.each do |key, value| it "finds user with trust #{key}" do user = Fabricate(:user, trust_level: value) + + next if !TrustLevel.valid?(value + 1) Fabricate(:user, trust_level: value + 1) query = ::AdminUserIndexQuery.new(query: key.to_s) - expect(real_users(query)).to eq([user]) + expect(real_users(query).to_a).to eq([user]) end end diff --git a/spec/lib/guardian_spec.rb b/spec/lib/guardian_spec.rb index cdcab4d22cb..3159e3bfb48 100644 --- a/spec/lib/guardian_spec.rb +++ b/spec/lib/guardian_spec.rb @@ -14,17 +14,21 @@ RSpec.describe Guardian do fab!(:automatic_group) { Fabricate(:group, automatic: true) } fab!(:plain_category) { Fabricate(:category) } - let(:trust_level_0) { build(:user, trust_level: 0) } - let(:trust_level_1) { build(:user, trust_level: 1) } - let(:trust_level_2) { build(:user, trust_level: 2) } - let(:trust_level_3) { build(:user, trust_level: 3) } - let(:trust_level_4) { build(:user, trust_level: 4) } - let(:another_admin) { build(:admin) } - let(:coding_horror) { build(:coding_horror) } + fab!(:trust_level_0) { Fabricate(:user, trust_level: 0) } + fab!(:trust_level_1) { Fabricate(:user, trust_level: 1) } + fab!(:trust_level_2) { Fabricate(:user, trust_level: 2) } + fab!(:trust_level_3) { Fabricate(:user, trust_level: 3) } + fab!(:trust_level_4) { Fabricate(:user, trust_level: 4) } + fab!(:another_admin) { Fabricate(:admin) } + fab!(:coding_horror) { Fabricate(:coding_horror) } let(:topic) { build(:topic, user: user) } let(:post) { build(:post, topic: topic, user: topic.user) } + before do + Group.refresh_automatic_groups! + end + it 'can be created without a user (not logged in)' do expect { Guardian.new }.not_to raise_error end @@ -173,16 +177,11 @@ RSpec.describe Guardian do end end - it "returns false for notify_user if private messages are disabled" do + it "returns false for notify_user if user is not in any group that can send personal messages" do + user = Fabricate(:user) SiteSetting.enable_personal_messages = false - user.trust_level = TrustLevel[2] - expect(Guardian.new(user).post_can_act?(post, :notify_user)).to be_falsey - end - - it "returns false for notify_user if private messages are enabled but threshold not met" do - SiteSetting.enable_personal_messages = true - SiteSetting.min_trust_to_send_messages = 2 - user.trust_level = TrustLevel[1] + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:staff] + user.change_trust_level!(1) expect(Guardian.new(user).post_can_act?(post, :notify_user)).to be_falsey end @@ -268,7 +267,10 @@ RSpec.describe Guardian do end it "returns false when you are untrusted" do - user.trust_level = TrustLevel[0] + SiteSetting.enable_personal_messages = false + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_2] + user.update!(trust_level: TrustLevel[0]) + Group.user_trust_level_change!(user.id, TrustLevel[0]) expect(Guardian.new(user).can_send_private_message?(another_user)).to be_falsey end @@ -277,16 +279,25 @@ RSpec.describe Guardian do end it "disallows pms to other users if trust level is not met" do - SiteSetting.min_trust_to_send_messages = TrustLevel[2] - user.trust_level = TrustLevel[1] + SiteSetting.enable_personal_messages = false + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_2] + user.update!(trust_level: TrustLevel[1]) + Group.user_trust_level_change!(user.id, TrustLevel[1]) expect(Guardian.new(user).can_send_private_message?(another_user)).to be_falsey end - context "when enable_personal_messages is false" do - before { SiteSetting.enable_personal_messages = false } + context "when personal_message_enabled_groups does not contain the user" do + let(:group) { Fabricate(:group) } + before do + SiteSetting.personal_message_enabled_groups = group.id + SiteSetting.enable_personal_messages = false + end it "returns false if user is not staff member" do expect(Guardian.new(trust_level_4).can_send_private_message?(another_user)).to be_falsey + GroupUser.create(user: trust_level_4, group: group) + trust_level_4.reload + expect(Guardian.new(trust_level_4).can_send_private_message?(another_user)).to be_truthy end it "returns true for staff member" do @@ -345,6 +356,8 @@ RSpec.describe Guardian do it "allows TL0 to message group with messageable_level = everyone" do group.update!(messageable_level: Group::ALIAS_LEVELS[:everyone]) + SiteSetting.enable_personal_messages = false + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_0] expect(Guardian.new(trust_level_0).can_send_private_message?(group)).to eq(true) expect(Guardian.new(user).can_send_private_message?(group)).to eq(true) end @@ -356,6 +369,8 @@ RSpec.describe Guardian do group.add(user) expect(Guardian.new(user).can_send_private_message?(group)).to eq(true) + SiteSetting.enable_personal_messages = false + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_0] expect(Guardian.new(trust_level_0).can_send_private_message?(group)).to eq(false) # group membership trumps min_trust_to_send_messages setting @@ -565,11 +580,6 @@ RSpec.describe Guardian do expect(Guardian.new(group_owner).can_invite_to?(group_private_topic)).to be_truthy end - it 'returns true for normal user when inviting to topic and PM disabled' do - SiteSetting.enable_personal_messages = false - expect(Guardian.new(trust_level_2).can_invite_to?(topic)).to be_truthy - end - it 'return true for normal users even if must_approve_users' do SiteSetting.must_approve_users = true expect(Guardian.new(user).can_invite_to?(topic)).to be_truthy @@ -617,10 +627,14 @@ RSpec.describe Guardian do end describe "private messages" do - fab!(:user) { Fabricate(:user, trust_level: TrustLevel[2]) } - fab!(:user) { Fabricate(:user, trust_level: SiteSetting.min_trust_level_to_allow_invite) } + fab!(:user) { Fabricate(:user) } fab!(:pm) { Fabricate(:private_message_topic, user: user) } + before do + user.change_trust_level!(SiteSetting.min_trust_level_to_allow_invite) + moderator.change_trust_level!(SiteSetting.min_trust_level_to_allow_invite) + end + context "when private messages are disabled" do it "allows an admin to invite to the pm" do expect(Guardian.new(admin).can_invite_to?(pm)).to be_truthy @@ -628,9 +642,10 @@ RSpec.describe Guardian do end end - context "when private messages are disabled" do + context "when user does not belong to personal_message_enabled_groups" do before do SiteSetting.enable_personal_messages = false + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:staff] end it "doesn't allow a regular user to invite" do @@ -1307,9 +1322,10 @@ RSpec.describe Guardian do expect(Guardian.new(admin).can_convert_topic?(topic)).to be_truthy end - it 'returns false when personal messages are disabled' do + it 'returns false when user is not in personal_message_enabled_groups' do SiteSetting.enable_personal_messages = false - expect(Guardian.new(admin).can_convert_topic?(topic)).to be_falsey + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_4] + expect(Guardian.new(user).can_convert_topic?(topic)).to be_falsey end end diff --git a/spec/lib/post_action_creator_spec.rb b/spec/lib/post_action_creator_spec.rb index 8343f730964..6c66c50d1c8 100644 --- a/spec/lib/post_action_creator_spec.rb +++ b/spec/lib/post_action_creator_spec.rb @@ -6,6 +6,10 @@ RSpec.describe PostActionCreator do fab!(:post) { Fabricate(:post) } let(:like_type_id) { PostActionType.types[:like] } + before do + Group.refresh_automatic_groups! + end + describe "rate limits" do before do RateLimiter.clear_all! diff --git a/spec/lib/topic_creator_spec.rb b/spec/lib/topic_creator_spec.rb index 0b448a7cc17..6ce006ed851 100644 --- a/spec/lib/topic_creator_spec.rb +++ b/spec/lib/topic_creator_spec.rb @@ -421,9 +421,8 @@ RSpec.describe TopicCreator do expect(TopicCreator.create(user, Guardian.new(user), pm_valid_attrs)).to be_valid end - it "enable_personal_messages setting should not be checked when sending private message to staff via flag" do - SiteSetting.enable_personal_messages = false - SiteSetting.min_trust_to_send_messages = TrustLevel[4] + it "personal_message_enabled_groups setting should not be checked when sending private messages to staff via flag" do + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:staff] expect(TopicCreator.create(user, Guardian.new(user), pm_valid_attrs.merge(subtype: TopicSubtype.notify_moderators))).to be_valid end end @@ -442,8 +441,9 @@ RSpec.describe TopicCreator do end.to raise_error(ActiveRecord::Rollback) end - it "min_trust_to_send_messages setting should be checked when sending private message" do - SiteSetting.min_trust_to_send_messages = TrustLevel[4] + it "personal_message_enabled_groups setting should be checked when sending private message" do + SiteSetting.enable_personal_messages = false + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_4] expect do TopicCreator.create(user, Guardian.new(user), pm_valid_attrs) diff --git a/spec/lib/topic_query_spec.rb b/spec/lib/topic_query_spec.rb index 7ff1fe9bd28..b18f4fd5892 100644 --- a/spec/lib/topic_query_spec.rb +++ b/spec/lib/topic_query_spec.rb @@ -1091,12 +1091,18 @@ RSpec.describe TopicQuery do TopicUser.update_last_read(user, topic, post_number, post_number, 10000) end - it 'returns the correct suggestions' do + before do + user.change_trust_level!(4) + sender.change_trust_level!(4) + end + it 'returns the correct suggestions' do pm_to_group = create_pm(sender, target_group_names: [group_with_user.name]) pm_to_user = create_pm(sender, target_usernames: [user.username]) - old_unrelated_pm = create_pm(target_usernames: [user.username]) + other_user = Fabricate(:user) + other_user.change_trust_level!(1) + old_unrelated_pm = create_pm(other_user, target_usernames: [user.username]) read(user, old_unrelated_pm, 1) related_by_user_pm = create_pm(sender, target_usernames: [user.username]) @@ -1113,7 +1119,7 @@ RSpec.describe TopicQuery do eq([related_by_user_pm.id]) ) - SiteSetting.enable_personal_messages = false + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:staff] expect(TopicQuery.new(user).list_related_for(pm_to_group)).to be_blank expect(TopicQuery.new(user).list_related_for(pm_to_user)).to be_blank end @@ -1236,6 +1242,8 @@ RSpec.describe TopicQuery do before do group.add(group_user) another_group.add(user) + Group.user_trust_level_change!(user.id, user.trust_level) + Group.user_trust_level_change!(group_user.id, group_user.trust_level) end context 'as user not part of group' do diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb index 746f4fc55f0..c24c8d17ac5 100644 --- a/spec/models/post_action_spec.rb +++ b/spec/models/post_action_spec.rb @@ -800,6 +800,7 @@ RSpec.describe PostAction do end it "prevents user to act twice at the same time" do + Group.refresh_automatic_groups! # flags are already being tested all_types_except_flags = PostActionType.types.except(*PostActionType.flag_types_without_custom.keys) all_types_except_flags.values.each do |action| diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 7a8ddeafb52..3dcc50f8782 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -830,7 +830,8 @@ RSpec.describe Topic do context "when PMs are enabled for TL3 or higher only" do before do - SiteSetting.min_trust_to_send_messages = 3 + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_4] + SiteSetting.enable_personal_messages = false end it 'should raise error' do @@ -882,6 +883,10 @@ RSpec.describe Topic do end describe 'by email' do + before do + Group.refresh_automatic_groups! + end + it 'should be able to invite a user' do expect(topic.invite(user, user1.email)).to eq(true) expect(topic.allowed_users).to include(user1) diff --git a/spec/models/trust_level3_requirements_spec.rb b/spec/models/trust_level3_requirements_spec.rb index 9f8dfa3d150..984714f76f3 100644 --- a/spec/models/trust_level3_requirements_spec.rb +++ b/spec/models/trust_level3_requirements_spec.rb @@ -2,7 +2,7 @@ RSpec.describe TrustLevel3Requirements do - let(:user) { Fabricate.build(:user) } + fab!(:user) { Fabricate(:user) } subject(:tl3_requirements) { described_class.new(user) } fab!(:moderator) { Fabricate(:moderator) } diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index 397dd43c9ed..262083c0128 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/spec/requests/groups_controller_spec.rb @@ -375,23 +375,6 @@ RSpec.describe GroupsController do expect(response.headers['X-Robots-Tag']).to eq('noindex') end - it "returns the right response for 'messageable' field" do - sign_in(user) - group.update!(messageable_level: Group::ALIAS_LEVELS[:everyone]) - - get "/groups/#{group.name}.json" - - expect(response.status).to eq(200) - expect(response.parsed_body['group']['messageable']).to eq(true) - - SiteSetting.enable_personal_messages = false - - get "/groups/#{group.name}.json" - - expect(response.status).to eq(200) - expect(response.parsed_body['group']['messageable']).to eq(false) - end - context 'as an admin' do it "returns the right response" do sign_in(admin) @@ -631,6 +614,7 @@ RSpec.describe GroupsController do describe '#messageable' do it "should return the right response" do + user.change_trust_level!(1) sign_in(user) get "/groups/#{group.name}/messageable.json" @@ -650,6 +634,7 @@ RSpec.describe GroupsController do body = response.parsed_body expect(body["messageable"]).to eq(true) + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:staff] SiteSetting.enable_personal_messages = false get "/groups/#{group.name}/messageable.json" diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index 537542eee7d..cc6b941cd84 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -191,12 +191,14 @@ RSpec.describe ListController do describe '#private_messages_group' do fab!(:user) { Fabricate(:user) } - describe 'with personal_messages disabled' do + describe 'when user not in personal_message_enabled_groups group' do let!(:topic) { Fabricate(:private_message_topic, allowed_groups: [group]) } before do group.add(user) + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:staff] SiteSetting.enable_personal_messages = false + Group.refresh_automatic_groups! end it 'should display group private messages for an admin' do diff --git a/spec/requests/post_actions_controller_spec.rb b/spec/requests/post_actions_controller_spec.rb index bfa0544d0d4..8fca3ef9159 100644 --- a/spec/requests/post_actions_controller_spec.rb +++ b/spec/requests/post_actions_controller_spec.rb @@ -4,6 +4,10 @@ RSpec.describe PostActionsController do fab!(:user) { Fabricate(:user) } fab!(:coding_horror) { Fabricate(:coding_horror) } + before do + Group.refresh_automatic_groups! + end + describe '#destroy' do fab!(:post) { Fabricate(:post, user: coding_horror) } diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 0ed754a064a..7f84e4919a4 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -36,6 +36,24 @@ RSpec.describe TopicsController do fab!(:tag) { Fabricate(:tag) } + before do + [ + user, + user_2, + post_author1, + post_author2, + post_author3, + post_author4, + post_author5, + post_author6, + trust_level_0, + trust_level_1, + trust_level_4 + ].each do |u| + Group.user_trust_level_change!(u.id, u.trust_level) + end + end + describe '#wordpress' do before do sign_in(moderator) @@ -4115,6 +4133,7 @@ RSpec.describe TopicsController do before do SiteSetting.max_allowed_message_recipients = 2 + Group.refresh_automatic_groups! end it "doesn't allow normal users to invite" do diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 88ff88ce945..ff74b1f41fa 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -13,6 +13,10 @@ RSpec.describe UsersController do fab!(:moderator) { Fabricate(:moderator) } fab!(:inactive_user) { Fabricate(:inactive_user) } + before do + Group.refresh_automatic_groups! + end + # Unfortunately, there are tests that depend on the user being created too # late for fab! to work. let(:user_deferred) { Fabricate(:user) } @@ -5858,19 +5862,13 @@ RSpec.describe UsersController do expect(response.status).to eq(403) end - it "responds with 403 if private messages are disabled and the user isn't staff" do - SiteSetting.enable_personal_messages = false + it "responds with 403 if personal_message_enabled_groups does not include the user and the user isn't staff" do + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_4] + user.update(trust_level: 1) get "/u/#{user.username}/user-menu-private-messages" expect(response.status).to eq(403) end - it "doesn't respond with 403 if private messages are disabled and the user is staff" do - SiteSetting.enable_personal_messages = false - user.update!(moderator: true) - get "/u/#{user.username}/user-menu-private-messages" - expect(response.status).to eq(200) - end - it "sends an array of unread private_message notifications" do get "/u/#{user.username}/user-menu-private-messages" expect(response.status).to eq(200) diff --git a/spec/serializers/post_serializer_spec.rb b/spec/serializers/post_serializer_spec.rb index 23c3123ad7a..266507082bf 100644 --- a/spec/serializers/post_serializer_spec.rb +++ b/spec/serializers/post_serializer_spec.rb @@ -3,6 +3,10 @@ RSpec.describe PostSerializer do fab!(:post) { Fabricate(:post) } + before do + Group.refresh_automatic_groups! + end + context "with a post with lots of actions" do fab!(:actor) { Fabricate(:user) } fab!(:admin) { Fabricate(:admin) } diff --git a/spec/serializers/topic_view_serializer_spec.rb b/spec/serializers/topic_view_serializer_spec.rb index 75681814d82..d55b848dcb2 100644 --- a/spec/serializers/topic_view_serializer_spec.rb +++ b/spec/serializers/topic_view_serializer_spec.rb @@ -97,6 +97,11 @@ RSpec.describe TopicViewSerializer do end describe '#suggested_topics' do + before do + SiteSetting.enable_personal_messages = false + Group.refresh_automatic_groups! + end + fab!(:topic2) { Fabricate(:topic) } before do @@ -165,6 +170,10 @@ RSpec.describe TopicViewSerializer do fab!(:pm) { Fabricate(:private_message_post).topic } fab!(:group) { Fabricate(:group) } + before do + Group.refresh_automatic_groups! + end + it 'is nil for a regular topic' do json = serialize_topic(topic, user) diff --git a/spec/services/user_merger_spec.rb b/spec/services/user_merger_spec.rb index d1c69c6364d..1a289365640 100644 --- a/spec/services/user_merger_spec.rb +++ b/spec/services/user_merger_spec.rb @@ -13,6 +13,10 @@ RSpec.describe UserMerger do fab!(:p5) { Fabricate(:post) } fab!(:p6) { Fabricate(:post) } + before do + Group.refresh_automatic_groups! + end + def merge_users!(source = nil, target = nil) source ||= source_user target ||= target_user