DEV: Remove enable_whispers site setting (#19196)

* DEV: Remove enable_whispers site setting

Whispers are enabled as long as there is at least one group allowed to
whisper, see whispers_allowed_groups site setting.

* DEV: Always enable whispers for admins if at least one group is allowed.

app/assets/javascripts/discourse/app/controllers/composer.js

@@ -296,11 +296,7 @@ export default Controller.extend({
 
   @discourseComputed("whisperer", "model.action")
   canWhisper(whisperer, modelAction) {
-    return (
+    return whisperer && modelAction === Composer.REPLY;
-      this.siteSettings.enable_whispers &&
-      Composer.REPLY === modelAction &&
-      whisperer
-    );
   },
 
   _setupPopupMenuOption(callback) {

app/assets/javascripts/discourse/tests/acceptance/composer-actions-test.js

@@ -25,7 +25,6 @@ acceptance("Composer Actions", function (needs) {
   needs.settings({
     prioritize_username_in_ux: true,
     display_name_on_posts: false,
-    enable_whispers: true,
   });
   needs.site({ can_tag_topics: true });
   needs.pretender((server, helper) => {
@@ -412,10 +411,7 @@ function stubDraftResponse() {
 }
 
 acceptance("Composer Actions With New Topic Draft", function (needs) {
-  needs.user();
+  needs.user({ whisperer: true });
-  needs.settings({
-    enable_whispers: true,
-  });
   needs.site({
     can_tag_topics: true,
   });

app/assets/javascripts/discourse/tests/acceptance/composer-image-preview-test.js

@@ -10,8 +10,8 @@ import {
 import { test } from "qunit";
 
 acceptance("Composer - Image Preview", function (needs) {
-  needs.user();
+  needs.user({});
-  needs.settings({ enable_whispers: true, allow_uncategorized_topics: true });
+  needs.settings({ allow_uncategorized_topics: true });
   needs.site({ can_tag_topics: true });
   needs.pretender((server, helper) => {
     server.post("/uploads/lookup-urls", () => {

app/assets/javascripts/discourse/tests/acceptance/composer-test.js

@@ -39,7 +39,6 @@ acceptance("Composer", function (needs) {
     whisperer: true,
   });
   needs.settings({
-    enable_whispers: true,
     general_category_id: 1,
     default_composer_category: 1,
   });

app/assets/javascripts/discourse/tests/acceptance/composer-uncategorized-test.js

@@ -6,11 +6,8 @@ import { test } from "qunit";
 acceptance(
   "Composer disabled, uncategorized not allowed when any topic_template present",
   function (needs) {
-    needs.user();
+    needs.user({ whisperer: true });
-    needs.settings({
+    needs.settings({ allow_uncategorized_topics: false });
-      enable_whispers: true,
-      allow_uncategorized_topics: false,
-    });
 
     test("Disable body until category is selected", async function (assert) {
       await visit("/");

app/models/concerns/roleable.rb

@@ -20,10 +20,9 @@ module Roleable
 
   def whisperer?
     @whisperer ||= begin
-      return false if !SiteSetting.enable_whispers?
-      return true if staff?
       whispers_allowed_group_ids = SiteSetting.whispers_allowed_group_ids
       return false if whispers_allowed_group_ids.blank?
+      return true if admin
       return true if whispers_allowed_group_ids.include?(primary_group_id)
       group_users&.exists?(group_id: whispers_allowed_group_ids)
     end

app/models/site_setting.rb

@@ -184,7 +184,7 @@ class SiteSetting < ActiveRecord::Base
   end
 
   def self.whispers_allowed_group_ids
-    if SiteSetting.enable_whispers && SiteSetting.whispers_allowed_groups.present?
+    if SiteSetting.whispers_allowed_groups.present?
       SiteSetting.whispers_allowed_groups_map
     else
       []

config/locales/server.en.yml

@@ -1683,7 +1683,6 @@ en:
 
     enable_badges: "Enable the badge system"
     max_favorite_badges: "Maximum number of badges that user can select"
-    enable_whispers: "Allow staff private communication within topics."
     whispers_allowed_groups: "Allow private communication within topics for members of specified groups."
 
     allow_index_in_robots_txt: "Specify in robots.txt that this site is allowed to be indexed by web search engines. In exceptional cases you can permanently <a href='%{base_path}/admin/customize/robots'>override robots.txt</a>."

config/site_settings.yml

@@ -328,9 +328,6 @@ basic:
     default: 2
     min: 0
     max: 6
-  enable_whispers:
-    client: true
-    default: false
   whispers_allowed_groups:
     client: true
     type: group_list

db/migrate/20221125173217_remove_enable_whispers_site_setting.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+class RemoveEnableWhispersSiteSetting < ActiveRecord::Migration[7.0]
+  def up
+    # If enable_whispers was true, insert whispers_allowed_groups or add
+    # staff group to whispers_allowed_groups. This is necessary to keep
+    # the current behavior which has a bypass for staff members.
+    execute <<~SQL
+      INSERT INTO site_settings(name, data_type, value, created_at, updated_at)
+      SELECT 'whispers_allowed_groups', '20', '3', created_at, NOW()
+      FROM site_settings
+      WHERE name = 'enable_whispers' AND value = 't'
+      ON CONFLICT DO NOTHING
+    SQL
+
+    execute <<~SQL
+      UPDATE site_settings
+      SET value = array_to_string(array_append(string_to_array(value, '|'), '3'), '|')
+      WHERE name = 'whispers_allowed_groups' AND
+            EXISTS(SELECT 1 FROM site_settings WHERE name = 'enable_whispers' AND value = 't') AND
+            NOT '3' = ANY(string_to_array(value, '|'))
+    SQL
+
+    # If enable_whispers was false, reset whispers_allowed_groups
+    execute <<~SQL
+      UPDATE site_settings
+      SET value = ''
+      WHERE name = 'whispers_allowed_groups' AND
+            NOT EXISTS(SELECT 1 FROM site_settings WHERE name = 'enable_whispers' AND value = 't')
+    SQL
+
+    # Delete enable_whispers site setting
+    execute <<~SQL
+      DELETE FROM site_settings WHERE name = 'enable_whispers'
+    SQL
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/email/receiver.rb

@@ -286,7 +286,7 @@ module Email
         Email::Receiver.update_bounce_score(@from_email, SiteSetting.hard_bounce_score)
       end
 
-      if SiteSetting.enable_whispers? && @from_user&.staged?
+      if SiteSetting.whispers_allowed_groups.present? && @from_user&.staged?
         return if email_log.blank?
 
         if post.present? && topic.present? && topic.archetype == Archetype.private_message

plugins/discourse-presence/test/javascripts/acceptance/discourse-presence-test.js

@@ -16,7 +16,6 @@ import selectKit from "discourse/tests/helpers/select-kit-helper";
 
 acceptance("Discourse Presence Plugin", function (needs) {
   needs.user({ whisperer: true });
-  needs.settings({ enable_whispers: true });
 
   test("Doesn't break topic creation", async function (assert) {
     await visit("/");

spec/fabricators/user_fabricator.rb

@@ -54,6 +54,8 @@ Fabricator(:moderator, from: :user) do
 
   after_create do |user|
     user.group_users << Fabricate(:group_user, user: user, group: Group[:moderators])
+    # HACK: Some plugins add the user to staff group already which breaks because of duplication
+    user.group_users << Fabricate(:group_user, user: user, group: Group[:staff]) if group_users.none? { |gu| gu.group == Group[:staff] }
   end
 end
 
@@ -65,6 +67,8 @@ Fabricator(:admin, from: :user) do
 
   after_create do |user|
     user.group_users << Fabricate(:group_user, user: user, group: Group[:admins])
+    # HACK: Some plugins add the user to staff group already which breaks because of duplication
+    user.group_users << Fabricate(:group_user, user: user, group: Group[:staff]) if group_users.none? { |gu| gu.group == Group[:staff] }
   end
 end
 

spec/lib/bookmark_query_spec.rb

@@ -119,19 +119,19 @@ RSpec.describe BookmarkQuery do
     context "for a whispered post" do
       before do
         post_bookmark.bookmarkable.update(post_type: Post.types[:whisper])
-        SiteSetting.enable_whispers = true
+        SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
       end
       fab!(:whisperers_group) { Fabricate(:group) }
 
       context "when the user is moderator" do
         it "does return the whispered post" do
-          user.update!(moderator: true)
+          user.grant_moderation!
           expect(bookmark_query.list_all.count).to eq(3)
         end
       end
       context "when the user is admin" do
         it "does return the whispered post" do
-          user.update!(admin: true)
+          user.grant_admin!
           expect(bookmark_query.list_all.count).to eq(3)
         end
       end

spec/lib/email/receiver_spec.rb

@@ -126,7 +126,7 @@ RSpec.describe Email::Receiver do
 
       before do
         SiteSetting.enable_staged_users = true
-        SiteSetting.enable_whispers = true
+        SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
       end
 
       def create_post_reply_key(value)

spec/lib/guardian_spec.rb

@@ -997,8 +997,8 @@ RSpec.describe Guardian do
       end
 
       it 'respects whispers' do
-        SiteSetting.enable_whispers = true
+        SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}|#{group.id}"
-        SiteSetting.whispers_allowed_groups = "#{group.id}"
+
         regular_post = post
         whisper_post = Fabricate(:post, post_type: Post.types[:whisper])
 

spec/lib/post_creator_spec.rb

@@ -1046,7 +1046,7 @@ RSpec.describe PostCreator do
     end
 
     it 'does not add whisperers to allowed users of the topic' do
-      SiteSetting.enable_whispers = true
+      SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
       unrelated_user.update!(admin: true)
 
       PostCreator.create!(

spec/lib/post_destroyer_spec.rb

@@ -647,7 +647,7 @@ RSpec.describe PostDestroyer do
 
   describe "deleting a post directly after a whisper" do
     before do
-      SiteSetting.enable_whispers = true
+      SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
     end
 
     it 'should not set Topic#last_post_user_id to a whisperer' do

spec/lib/search_spec.rb

@@ -945,8 +945,7 @@ RSpec.describe Search do
     it 'allows staff and members of whisperers group to search for whispers' do
       whisperers_group = Fabricate(:group)
       user = Fabricate(:user)
-      SiteSetting.enable_whispers = true
+      SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}|#{whisperers_group.id}"
-      SiteSetting.whispers_allowed_groups = "#{whisperers_group.id}"
 
       post.update!(post_type: Post.types[:whisper], raw: 'this is a tiger')
 

spec/lib/topic_query_spec.rb

@@ -829,7 +829,7 @@ RSpec.describe TopicQuery do
 
     context 'with whispers' do
       before do
-        SiteSetting.enable_whispers = true
+        SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
       end
 
       it 'correctly shows up in unread for staff' do
@@ -1079,13 +1079,8 @@ RSpec.describe TopicQuery do
   end
 
   describe '#list_related_for' do
-    let(:user) do
+    let(:user) { Fabricate(:user) }
-      Fabricate(:admin)
+    let(:sender) { Fabricate(:user) }
-    end
-
-    let(:sender) do
-      Fabricate(:admin)
-    end
 
     let(:group_with_user) do
       group = Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone])

spec/lib/topic_view_spec.rb

@@ -342,7 +342,7 @@ RSpec.describe TopicView do
 
     describe '.post_counts_by_user' do
       it 'returns the two posters with their appropriate counts' do
-        SiteSetting.enable_whispers = true
+        SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
         Fabricate(:post, topic: topic, user: evil_trout, post_type: Post.types[:whisper])
         # Should not be counted
         Fabricate(:post, topic: topic, user: evil_trout, post_type: Post.types[:whisper], action_code: 'assign')
@@ -480,7 +480,7 @@ RSpec.describe TopicView do
 
   describe 'whispers' do
     it "handles their visibility properly" do
-      SiteSetting.enable_whispers = true
+      SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
       p1 = Fabricate(:post, topic: topic, user: evil_trout)
       p2 = Fabricate(:post, topic: topic, user: evil_trout, post_type: Post.types[:whisper])
       p3 = Fabricate(:post, topic: topic, user: evil_trout)

spec/lib/topics_bulk_action_spec.rb

@@ -95,7 +95,7 @@ RSpec.describe TopicsBulkAction do
 
       context "when the highest_staff_post_number is > highest_post_number for a topic (e.g. whisper is last post)" do
         it "dismisses posts" do
-          SiteSetting.enable_whispers = true
+          SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
           post1 = create_post(user: user)
           p = create_post(topic_id: post1.topic_id)
           create_post(topic_id: post1.topic_id)

spec/lib/unread_spec.rb

@@ -27,8 +27,8 @@ RSpec.describe Unread do
 
   describe 'staff counts' do
     it 'should correctly return based on staff post number' do
-      SiteSetting.enable_whispers = true
+      SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
-      user.admin = true
+      user.grant_admin!
 
       topic_user.last_read_post_number = 13
 
@@ -48,15 +48,13 @@ RSpec.describe Unread do
     end
 
     it 'returns the right unread posts for a staff user' do
-      SiteSetting.enable_whispers = true
+      SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
-      SiteSetting.whispers_allowed_groups = ""
+      user.grant_admin!
-      user.admin = true
       topic_user.last_read_post_number = 10
       expect(unread.unread_posts).to eq(5)
     end
 
     it 'returns the right unread posts for a whisperer user' do
-      SiteSetting.enable_whispers = true
       SiteSetting.whispers_allowed_groups = "#{whisperers_group.id}"
       topic_user.last_read_post_number = 10
       expect(unread.unread_posts).to eq(5)

spec/models/post_action_spec.rb

@@ -427,7 +427,7 @@ RSpec.describe PostAction do
     end
 
     it 'should not increase topic like count when liking a whisper' do
-      SiteSetting.set(:enable_whispers, true)
+      SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
       post.revise(admin, post_type: Post.types[:whisper])
 
       PostActionCreator.like(admin, post)

spec/models/post_spec.rb

@@ -1616,7 +1616,7 @@ RSpec.describe Post do
     end
 
     it "will update topic updated_at for all topic related events" do
-      SiteSetting.enable_whispers = true
+      SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
 
       post = updates_topic_updated_at do
         create_post(topic_id: topic.id, post_type: Post.types[:whisper])

spec/models/private_message_topic_tracking_state_spec.rb

@@ -69,7 +69,7 @@ RSpec.describe PrivateMessageTopicTrackingState do
     end
 
     it 'returns the right tracking state when topics contain whispers' do
-      SiteSetting.enable_whispers = true
+      SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
       TopicUser.find_by(user: user_2, topic: private_message).update!(
         last_read_post_number: 1
       )

spec/models/topic_spec.rb

@@ -168,8 +168,7 @@ RSpec.describe Topic do
     let(:types) { Post.types }
 
     before do
-      SiteSetting.enable_whispers = true
+      SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}|#{whisperers_group.id}"
-      SiteSetting.whispers_allowed_groups = "#{whisperers_group.id}"
     end
 
     it "returns the appropriate types for anonymous users" do

spec/models/topic_tracking_state_spec.rb

@@ -29,7 +29,6 @@ RSpec.describe TopicTrackingState do
       whisperers_group = Fabricate(:group)
       Fabricate(:user, groups: [whisperers_group])
       Fabricate(:topic_user_watching, topic: topic, user: user)
-      SiteSetting.enable_whispers = true
       SiteSetting.whispers_allowed_groups = "#{whisperers_group.id}"
       post.update!(post_type: Post.types[:whisper])
 
@@ -69,7 +68,7 @@ RSpec.describe TopicTrackingState do
     end
 
     it 'correctly publish read for staff' do
-      SiteSetting.enable_whispers = true
+      SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
       create_post(
         raw: "this is a test post",
         topic: post.topic,
@@ -137,7 +136,6 @@ RSpec.describe TopicTrackingState do
     it "publishes whisper post to staff users and members of whisperers group" do
       whisperers_group = Fabricate(:group)
       Fabricate(:topic_user_watching, topic: topic, user: user)
-      SiteSetting.enable_whispers = true
       SiteSetting.whispers_allowed_groups = "#{whisperers_group.id}"
       post.update!(post_type: Post.types[:whisper])
 
@@ -159,7 +157,7 @@ RSpec.describe TopicTrackingState do
     end
 
     it "does not publish whisper post to non-staff users" do
-      SiteSetting.enable_whispers = true
+      SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
       post.update!(post_type: Post.types[:whisper])
 
       messages = MessageBus.track_publish("/unread") do
@@ -676,7 +674,7 @@ RSpec.describe TopicTrackingState do
 
   describe ".report" do
     it "correctly reports topics with staff posts" do
-      SiteSetting.enable_whispers = true
+      SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
       create_post(
         raw: "this is a test post",
         topic: topic,

spec/models/user_spec.rb

@@ -2920,24 +2920,21 @@ RSpec.describe User do
   end
 
   describe "#whisperer?" do
-    before do
+    fab!(:group) { Fabricate(:group) }
-      SiteSetting.enable_whispers = true
-    end
 
     it 'returns true for an admin user' do
+      SiteSetting.whispers_allowed_groups = "#{group.id}"
+
       admin = Fabricate.create(:admin)
       expect(admin.whisperer?).to eq(true)
     end
 
     it 'returns false for an admin user when whispers are not enabled' do
-      SiteSetting.enable_whispers = false
-
       admin = Fabricate.create(:admin)
       expect(admin.whisperer?).to eq(false)
     end
 
     it 'returns true for user belonging to whisperers groups' do
-      group = Fabricate(:group)
       whisperer = Fabricate(:user)
       user = Fabricate(:user)
       SiteSetting.whispers_allowed_groups = "#{group.id}"
@@ -2950,6 +2947,10 @@ RSpec.describe User do
       expect(whisperer.whisperer?).to eq(true)
       expect(user.whisperer?).to eq(false)
     end
+
+    it 'returns false if no whispers groups exist' do
+      expect(subject.whisperer?).to eq(false)
+    end
   end
 
   describe "#grouped_unread_notifications" do

spec/requests/list_controller_spec.rb

@@ -209,8 +209,6 @@ RSpec.describe ListController do
   end
 
   describe '#private_messages_group' do
-    fab!(:user) { Fabricate(:user) }
-
     describe 'when user not in personal_message_enabled_groups group' do
       let!(:topic) { Fabricate(:private_message_topic, allowed_groups: [group]) }
 
@@ -245,7 +243,6 @@ RSpec.describe ListController do
       it "should not display group private messages for a moderator's group" do
         moderator = Fabricate(:moderator)
         sign_in(moderator)
-        group.add(moderator)
 
         get "/topics/private-messages-group/#{user.username}/#{group.name}.json"
 

spec/requests/posts_controller_spec.rb

@@ -806,7 +806,7 @@ RSpec.describe PostsController do
 
     before do
       SiteSetting.min_first_post_typing_time = 0
-      SiteSetting.enable_whispers = true
+      SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
     end
 
     context 'with api' do

spec/requests/users_controller_spec.rb

@@ -4202,7 +4202,7 @@ RSpec.describe UsersController do
         end
 
         it "includes all post types for staff members" do
-          SiteSetting.enable_whispers = true
+          SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
           sign_in(admin)
 
           get "/u/#{admin.username}.json", params: { include_post_count_for: topic.id }

spec/serializers/user_post_bookmark_serializer_spec.rb

@@ -10,7 +10,6 @@ RSpec.describe UserPostBookmarkSerializer do
     let(:whisperers_group) { Fabricate(:group) }
 
     before do
-      SiteSetting.enable_whispers = true
       SiteSetting.whispers_allowed_groups = "#{whisperers_group.id}"
     end