github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-01-18 10:52:45 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

DEV: consolidate chat channel notification settings (#29080)

Browse Source 
On the chat channel settings page, we want to show a single Send push notifications setting instead of the current Desktop notifications and Mobile push notifications settings.

For existing users, use the Mobile push notifications setting value for the new Send push notifications setting.
This commit is contained in:
David Battersby 2024-10-08 13:13:01 +04:00 committed by GitHub
parent 229773e7a8
commit a7a9148b1e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
23 changed files with 97 additions and 132 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
plugins/chat/app/controllers/chat/api/channels_current_user_notifications_settings_controller.rb
View File

@ -1,7 +1,7 @@
# frozen_string_literal: true
class Chat::Api::ChannelsCurrentUserNotificationsSettingsController < Chat::Api::ChannelsController
MEMBERSHIP_EDITABLE_PARAMS = %i[muted desktop_notification_level mobile_notification_level]
MEMBERSHIP_EDITABLE_PARAMS = %i[muted notification_level]
def update
settings_params = params.require(:notifications_settings).permit(MEMBERSHIP_EDITABLE_PARAMS)

5
plugins/chat/app/jobs/regular/chat/notify_mentioned.rb
View File

@ -131,15 +131,12 @@ module Jobs
def send_notifications(membership, mention_type)
payload = build_payload_for(membership, identifier_type: mention_type)
if !membership.desktop_notifications_never? && !membership.muted?
if !membership.notifications_never? && !membership.muted?
::MessageBus.publish(
"/chat/notification-alert/#{membership.user_id}",
payload,
user_ids: [membership.user_id],
)
end
if !membership.mobile_notifications_never? && !membership.muted?
::PostAlerter.push_notification(membership.user, payload)
end
end

6
plugins/chat/app/jobs/regular/chat/notify_watching.rb
View File

@ -24,7 +24,7 @@ module Jobs
.where(chat_channel_id: @chat_channel.id)
.where(following: true)
.where(
"desktop_notification_level = :always OR mobile_notification_level = :always OR users.id IN (SELECT user_id FROM user_chat_thread_memberships WHERE thread_id = :thread_id AND notification_level = :watching)",
"notification_level = :always OR users.id IN (SELECT user_id FROM user_chat_thread_memberships WHERE thread_id = :thread_id AND notification_level = :watching)",
always: always_notification_level,
thread_id: @chat_message.thread_id,
watching: ::Chat::NotificationLevels.all[:watching],
@ -95,7 +95,7 @@ module Jobs
thread_membership && create_watched_thread_notification(thread_membership)
end
if membership.desktop_notifications_always? && !membership.muted?
if membership.notifications_always? && !membership.muted?
send_notification =
DiscoursePluginRegistry.push_notification_filters.all? do |filter|
filter.call(user, payload)
@ -107,9 +107,7 @@ module Jobs
user_ids: [user.id],
)
end
end
if membership.mobile_notifications_always? && !membership.muted?
::PostAlerter.push_notification(user, payload)
end
end

9
plugins/chat/app/models/chat/user_chat_channel_membership.rb
View File

@ -3,6 +3,7 @@
module Chat
class UserChatChannelMembership < ActiveRecord::Base
self.table_name = "user_chat_channel_memberships"
self.ignored_columns = %w[desktop_notification_level mobile_notification_level] # TODO: Remove once 20241003122030_add_notification_level_to_user_chat_channel_memberships has been promoted to pre-deploy
NOTIFICATION_LEVELS = { never: 0, mention: 1, always: 2 }
@ -10,8 +11,7 @@ module Chat
belongs_to :last_read_message, class_name: "Chat::Message", optional: true
belongs_to :chat_channel, class_name: "Chat::Channel", foreign_key: :chat_channel_id
enum :desktop_notification_level, NOTIFICATION_LEVELS, prefix: :desktop_notifications
enum :mobile_notification_level, NOTIFICATION_LEVELS, prefix: :mobile_notifications
enum :notification_level, NOTIFICATION_LEVELS, prefix: :notifications
enum :join_mode, { manual: 0, automatic: 1 }
def mark_read!(new_last_read_id = nil)
@ -30,16 +30,15 @@ end
# last_read_message_id :integer
# following :boolean default(FALSE), not null
# muted :boolean default(FALSE), not null
# desktop_notification_level :integer default("mention"), not null
# mobile_notification_level :integer default("mention"), not null
# created_at :datetime not null
# updated_at :datetime not null
# last_unread_mention_when_emailed_id :integer
# join_mode :integer default("manual"), not null
# last_viewed_at :datetime not null
# notification_level :integer default("mention"), not null
#
# Indexes
#
# user_chat_channel_memberships_index (user_id,chat_channel_id,desktop_notification_level,mobile_notification_level,following)
# user_chat_channel_memberships_index (user_id,chat_channel_id,notification_level,following)
# user_chat_channel_unique_memberships (user_id,chat_channel_id) UNIQUE
#

3
plugins/chat/app/serializers/chat/base_channel_membership_serializer.rb
View File

@ -4,8 +4,7 @@ module Chat
class BaseChannelMembershipSerializer < ApplicationSerializer
attributes :following,
:muted,
:desktop_notification_level,
:mobile_notification_level,
:notification_level,
:chat_channel_id,
:last_read_message_id,
:last_viewed_at

3
plugins/chat/app/services/chat/add_users_to_channel.rb
View File

@ -74,8 +74,7 @@ module Chat
chat_channel_id: channel.id,
muted: false,
following: true,
desktop_notification_level: always_level,
mobile_notification_level: always_level,
notification_level: always_level,
created_at: Time.zone.now,
updated_at: Time.zone.now,
}

3
plugins/chat/app/services/chat/create_direct_message_channel.rb
View File

@ -105,8 +105,7 @@ module Chat
chat_channel_id: channel.id,
muted: false,
following: false,
desktop_notification_level: always_level,
mobile_notification_level: always_level,
notification_level: always_level,
created_at: Time.zone.now,
updated_at: Time.zone.now,
}

43
plugins/chat/assets/javascripts/discourse/components/chat/routes/channel-info-settings.gjs
View File

@ -62,12 +62,7 @@ export default class ChatRouteChannelInfoSettings extends Component {
"chat.settings.channel_wide_mentions_label"
);
autoJoinLabel = I18n.t("chat.settings.auto_join_users_label");
desktopNotificationsLevelLabel = I18n.t(
"chat.settings.desktop_notification_level"
);
mobileNotificationsLevelLabel = I18n.t(
"chat.settings.mobile_notification_level"
);
notificationsLevelLabel = I18n.t("chat.settings.notification_level");
get canEditChannel() {
if (
@ -121,11 +116,7 @@ export default class ChatRouteChannelInfoSettings extends Component {
return this.args.channel.isCategoryChannel;
}
get shouldRenderDesktopNotificationsLevelSection() {
return !this.isChannelMuted;
}
get shouldRenderMobileNotificationsLevelSection() {
get shouldRenderNotificationsLevelSection() {
return !this.isChannelMuted;
}
@ -412,37 +403,19 @@ export default class ChatRouteChannelInfoSettings extends Component {
</:action>
</section.row>
{{#if this.shouldRenderDesktopNotificationsLevelSection}}
<section.row @label={{this.desktopNotificationsLevelLabel}}>
{{#if this.shouldRenderNotificationsLevelSection}}
<section.row @label={{this.notificationsLevelLabel}}>
<:action>
<ComboBox
@content={{this.notificationLevels}}
@value={{@channel.currentUserMembership.desktopNotificationLevel}}
@value={{@channel.currentUserMembership.notificationLevel}}
@valueProperty="value"
@onChange={{fn
this.saveNotificationSettings
"desktopNotificationLevel"
"desktop_notification_level"
"notificationLevel"
"notification_level"
}}
class="c-channel-settings__selector c-channel-settings__desktop-notifications-selector"
/>
</:action>
</section.row>
{{/if}}
{{#if this.shouldRenderMobileNotificationsLevelSection}}
<section.row @label={{this.mobileNotificationsLevelLabel}}>
<:action>
<ComboBox
@content={{this.notificationLevels}}
@value={{@channel.currentUserMembership.mobileNotificationLevel}}
@valueProperty="value"
@onChange={{fn
this.saveNotificationSettings
"mobileNotificationLevel"
"mobile_notification_level"
}}
class="c-channel-settings__selector c-channel-settings__mobile-notifications-selector"
class="c-channel-settings__selector c-channel-settings__notifications-selector"
/>
</:action>
</section.row>

6
plugins/chat/assets/javascripts/discourse/models/user-chat-channel-membership.js
View File

@ -8,8 +8,7 @@ export default class UserChatChannelMembership {
@tracked following;
@tracked muted;
@tracked desktopNotificationLevel;
@tracked mobileNotificationLevel;
@tracked notificationLevel;
@tracked lastReadMessageId;
@tracked lastViewedAt;
@tracked user;
@ -17,8 +16,7 @@ export default class UserChatChannelMembership {
constructor(args = {}) {
this.following = args.following;
this.muted = args.muted;
this.desktopNotificationLevel = args.desktop_notification_level;
this.mobileNotificationLevel = args.mobile_notification_level;
this.notificationLevel = args.notification_level;
this.lastReadMessageId = args.last_read_message_id;
this.lastViewedAt = new Date(args.last_viewed_at);
this.user = this.#initUserModel(args.user);

3
plugins/chat/assets/javascripts/discourse/services/chat-api.js
View File

@ -358,8 +358,7 @@ export default class ChatApi extends Service {
* @param {number} channelId - The ID of the channel.
* @param {object} data - The settings to modify.
* @param {boolean} [data.muted] - Mutes the channel.
* @param {string} [data.desktop_notification_level] - Notifications level on desktop: never, mention or always.
* @param {string} [data.mobile_notification_level] - Notifications level on mobile: never, mention or always.
* @param {string} [data.notification_level] - Notifications level: never, mention or always.
* @returns {Promise}
*/
updateCurrentUserChannelNotificationsSettings(channelId, data = {}) {

3
plugins/chat/config/locales/client.en.yml
View File

@ -479,10 +479,9 @@ en:
auto_join_users_info: "Check hourly which users have been active in the last 3 months. Add them to this channel if they have access to the %{category} category."
auto_join_users_info_no_category: "Check hourly which users have been active in the last 3 months. Add them to this channel if they have access to the selected category."
auto_join_users_warning: "Every user who isn't a member of this channel and has access to the %{category} category will join. Are you sure?"
desktop_notification_level: "Desktop notifications"
notification_level: "Send push notifications"
follow: "Join"
followed: "Joined"
mobile_notification_level: "Mobile push notifications"
mute: "Mute channel"
threading_enabled: "Enabled"
threading_disabled: "Disabled"

32
plugins/chat/db/migrate/20241003122030_add_notification_level_to_user_chat_channel_memberships.rb Normal file
View File

@ -0,0 +1,32 @@
# frozen_string_literal: true
class AddNotificationLevelToUserChatChannelMemberships < ActiveRecord::Migration[7.1]
disable_ddl_transaction!
def up
execute <<~SQL
DROP INDEX CONCURRENTLY IF EXISTS user_chat_channel_memberships_index
SQL
add_column :user_chat_channel_memberships, :notification_level, :integer, null: true
batch_size = 10_000
min_id, max_id = execute("SELECT MIN(id), MAX(id) FROM user_chat_channel_memberships")[0].values
(min_id..max_id).step(batch_size) { |start_id| execute <<~SQL.squish } if min_id && max_id
UPDATE user_chat_channel_memberships
SET notification_level = mobile_notification_level
WHERE id >= #{start_id} AND id < #{start_id + batch_size}
SQL
change_column_default :user_chat_channel_memberships, :notification_level, 1
change_column_null :user_chat_channel_memberships, :notification_level, false
execute <<~SQL
CREATE INDEX CONCURRENTLY user_chat_channel_memberships_index ON user_chat_channel_memberships using btree (user_id, chat_channel_id, notification_level, following)
SQL
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

3
plugins/chat/spec/fabricators/chat_fabricator.rb
View File

@ -193,8 +193,7 @@ Fabricator(:user_chat_channel_membership_for_dm, from: :user_chat_channel_member
user
chat_channel
following true
desktop_notification_level 2
mobile_notification_level 2
notification_level 2
end
Fabricator(:chat_draft, class_name: "Chat::Draft") do

8
plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb
View File

@ -164,7 +164,7 @@ describe Jobs::Chat::NotifyMentioned do
it "skips desktop notifications based on user preferences" do
message = create_chat_message
Chat::UserChatChannelMembership.find_by(chat_channel: public_channel, user: user_2).update!(
desktop_notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:never],
notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:never],
)
desktop_notification =
@ -176,7 +176,7 @@ describe Jobs::Chat::NotifyMentioned do
it "skips push notifications based on user preferences" do
message = create_chat_message
Chat::UserChatChannelMembership.find_by(chat_channel: public_channel, user: user_2).update!(
mobile_notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:never],
notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:never],
)
PostAlerter.expects(:push_notification).never
@ -191,7 +191,7 @@ describe Jobs::Chat::NotifyMentioned do
it "skips desktop notifications based on user muting preferences" do
message = create_chat_message
Chat::UserChatChannelMembership.find_by(chat_channel: public_channel, user: user_2).update!(
desktop_notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
muted: true,
)
@ -204,7 +204,7 @@ describe Jobs::Chat::NotifyMentioned do
it "skips push notifications based on user muting preferences" do
message = create_chat_message
Chat::UserChatChannelMembership.find_by(chat_channel: public_channel, user: user_2).update!(
mobile_notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
muted: true,
)

37
plugins/chat/spec/jobs/regular/chat/notify_watching_spec.rb
View File

@ -44,7 +44,7 @@ RSpec.describe Jobs::Chat::NotifyWatching do
before do
membership2.update!(
desktop_notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
)
end
@ -76,10 +76,11 @@ RSpec.describe Jobs::Chat::NotifyWatching do
before { channel.update!(threading_enabled: true) }
context "with channel notification_level is always" do
context "when channel notification_level is always" do
before do
always = Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always]
membership1.update!(desktop_notification_level: always, mobile_notification_level: always)
membership1.update!(
notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
)
end
it "creates a core notification when watching the thread" do
@ -110,8 +111,14 @@ RSpec.describe Jobs::Chat::NotifyWatching do
end
end
context "without channel notifications" do
context "when channel notification_level is never" do
before do
membership1.update!(
notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:never],
)
membership2.update!(
notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:never],
)
thread.membership_for(user1).update!(
notification_level: Chat::NotificationLevels.all[:watching],
)
@ -181,15 +188,14 @@ RSpec.describe Jobs::Chat::NotifyWatching do
end
end
context "when mobile_notification_level is always and desktop_notification_level is none" do
context "when notification_level is always" do
before do
membership2.update!(
desktop_notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:never],
mobile_notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
)
end
it "sends a mobile notification" do
it "sends push notifications and message bus notifications" do
PostAlerter.expects(:push_notification).with(
user2,
has_entries(
@ -208,7 +214,7 @@ RSpec.describe Jobs::Chat::NotifyWatching do
),
)
messages = notification_messages_for(user2)
expect(messages.length).to be_zero
expect(messages.length).to eq(1)
end
context "when the channel is muted via membership preferences" do
@ -280,7 +286,7 @@ RSpec.describe Jobs::Chat::NotifyWatching do
before do
membership2.update!(
desktop_notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
)
end
@ -313,15 +319,14 @@ RSpec.describe Jobs::Chat::NotifyWatching do
end
end
context "when mobile_notification_level is always and desktop_notification_level is none" do
context "when notification_level is always" do
before do
membership2.update!(
desktop_notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:never],
mobile_notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
)
end
it "sends a mobile notification" do
it "sends both push notifications and message bus notifications" do
PostAlerter.expects(:push_notification).with(
user2,
has_entries(
@ -340,7 +345,7 @@ RSpec.describe Jobs::Chat::NotifyWatching do
),
)
messages = notification_messages_for(user2)
expect(messages.length).to be_zero
expect(messages.length).to eq(1)
end
context "when the channel is muted via membership preferences" do

3
plugins/chat/spec/lib/chat/channel_fetcher_spec.rb
View File

@ -121,8 +121,7 @@ describe Chat::ChannelFetcher do
user: user1,
chat_channel: direct_message_channel1,
following: true,
desktop_notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
mobile_notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
notification_level: Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
)
end

12
plugins/chat/spec/requests/chat/api/channels_current_user_notifications_settings_controller_spec.rb
View File

@ -54,8 +54,7 @@ RSpec.describe Chat::Api::ChannelsCurrentUserNotificationsSettingsController do
params: {
notifications_settings: {
muted: true,
desktop_notification_level: "always",
mobile_notification_level: "never",
notification_level: "always",
},
}
@ -67,8 +66,7 @@ RSpec.describe Chat::Api::ChannelsCurrentUserNotificationsSettingsController do
membership = channel_1.membership_for(current_user)
expect(membership.muted).to eq(true)
expect(membership.desktop_notification_level).to eq("always")
expect(membership.mobile_notification_level).to eq("never")
expect(membership.notification_level).to eq("always")
end
end
@ -123,8 +121,7 @@ RSpec.describe Chat::Api::ChannelsCurrentUserNotificationsSettingsController do
params: {
notifications_settings: {
muted: true,
desktop_notification_level: "always",
mobile_notification_level: "never",
notification_level: "always",
},
}
@ -136,8 +133,7 @@ RSpec.describe Chat::Api::ChannelsCurrentUserNotificationsSettingsController do
membership = dm_channel_1.membership_for(current_user)
expect(membership.muted).to eq(true)
expect(membership.desktop_notification_level).to eq("always")
expect(membership.mobile_notification_level).to eq("never")
expect(membership.notification_level).to eq("always")
end
end
end

6
plugins/chat/spec/serializer/chat/structured_channel_serializer_spec.rb
View File

@ -46,10 +46,9 @@ RSpec.describe Chat::StructuredChannelSerializer do
.as_json,
).to include(
"chat_channel_id" => channel1.id,
"desktop_notification_level" => "mention",
"notification_level" => "mention",
"following" => true,
"last_read_message_id" => nil,
"mobile_notification_level" => "mention",
"muted" => false,
)
end
@ -64,10 +63,9 @@ RSpec.describe Chat::StructuredChannelSerializer do
.as_json,
).to include(
"chat_channel_id" => channel3.id,
"desktop_notification_level" => "always",
"notification_level" => "always",
"following" => true,
"last_read_message_id" => nil,
"mobile_notification_level" => "always",
"muted" => false,
)
end

3
plugins/chat/spec/services/chat/create_direct_message_channel_spec.rb
View File

@ -69,8 +69,7 @@ RSpec.describe Chat::CreateDirectMessageChannel do
expect(membership).to have_attributes(
following: false,
muted: false,
desktop_notification_level: "always",
mobile_notification_level: "always",
notification_level: "always",
)
end
end

6
plugins/chat/spec/services/chat/message_destroyer_spec.rb
View File

@ -12,8 +12,7 @@ RSpec.describe Chat::MessageDestroyer do
chat_channel: message_1.chat_channel,
last_read_message: message_1,
following: true,
desktop_notification_level: 2,
mobile_notification_level: 2,
notification_level: 2,
)
described_class.new.destroy_in_batches(Chat::Message.where(id: message_1.id))
@ -33,8 +32,7 @@ RSpec.describe Chat::MessageDestroyer do
chat_channel: message_1.chat_channel,
last_read_message: message_4,
following: true,
desktop_notification_level: 2,
mobile_notification_level: 2,
notification_level: 2,
)
described_class.new.destroy_in_batches(Chat::Message.where(id: message_4.id))

3
plugins/chat/spec/support/api/schemas/category_chat_channel.json
View File

@ -9,8 +9,7 @@
"muted": { "type": "boolean" },
"unread_count": { "type": "number" },
"unread_mentions": { "type": "number" },
"desktop_notification_level": { "type": "string" },
"mobile_notification_level": { "type": "string" },
"notification_level": { "type": "string" },
"following": { "type": "boolean" }
}
}

6
plugins/chat/spec/support/api/schemas/user_chat_channel_membership.json
View File

@ -4,8 +4,7 @@
"chat_channel_id",
"last_read_message_id",
"muted",
"desktop_notification_level",
"mobile_notification_level",
"notification_level",
"following",
"last_viewed_at"
],
@ -13,8 +12,7 @@
"chat_channel_id": { "type": "number" },
"last_read_message_id": { "type": ["number", "null"] },
"muted": { "type": "boolean" },
"desktop_notification_level": { "type": "string" },
"mobile_notification_level": { "type": "string" },
"notification_level": { "type": "string" },
"following": { "type": "boolean" },
"user": {
"type": ["object", "null"],

24
plugins/chat/spec/system/channel_settings_page_spec.rb
View File

@ -122,36 +122,18 @@ RSpec.describe "Channel - Info - Settings page", type: :system do
}.to change { membership.reload.muted }.from(false).to(true)
end
it "can change desktop notification level" do
it "can change notification level" do
chat_page.visit_channel_settings(channel_1)
membership = channel_1.membership_for(current_user)
expect {
select_kit =
PageObjects::Components::SelectKit.new(
".c-channel-settings__desktop-notifications-selector",
)
PageObjects::Components::SelectKit.new(".c-channel-settings__notifications-selector")
select_kit.expand
select_kit.select_row_by_name("Never")
expect(toasts).to have_success(I18n.t("js.saved"))
}.to change { membership.reload.desktop_notification_level }.from("mention").to("never")
end
it "can change mobile notification level" do
chat_page.visit_channel_settings(channel_1)
membership = channel_1.membership_for(current_user)
expect {
select_kit =
PageObjects::Components::SelectKit.new(
".c-channel-settings__mobile-notifications-selector",
)
select_kit.expand
select_kit.select_row_by_name("Never")
expect(toasts).to have_success(I18n.t("js.saved"))
}.to change { membership.reload.mobile_notification_level }.from("mention").to("never")
}.to change { membership.reload.notification_level }.from("mention").to("never")
end
it "can unfollow channel" do