FEATURE: Allow users to DM groups in chat (#25189)

Allows users to create DMs by selecting groups as a target. It also allows adding user groups to an existing chat

- When creating the channel, it expands the user group and adds all its members with chat enabled to the channel.
- After creation, there's no difference between adding a group or adding its members individually.
- Users can add multiple groups and users simultaneously.
- There are UI validations; the member count preview updates according to the member count of added groups, and it does not allow users to add more members than SiteSetting.chat_max_direct_message_users."
This commit is contained in:
Jan Cernik
2024-01-19 11:09:47 -03:00
committed by GitHub
parent bd2ca8d617
commit f4e51e0789
29 changed files with 573 additions and 91 deletions
@@ -0,0 +1,19 @@
# frozen_string_literal: true
module Chat
class UsersFromUsernamesAndGroupsQuery
def self.call(usernames:, groups:, excluded_user_ids: [])
User
.joins(:user_option)
.left_outer_joins(:groups)
.where(user_options: { chat_enabled: true })
.where(
"username IN (?) OR (groups.name IN (?) AND group_users.user_id IS NOT NULL)",
usernames,
groups,
)
.where.not(id: excluded_user_ids)
.distinct
end
end
end
@@ -0,0 +1,20 @@
# frozen_string_literal: true
module Chat
class ChatableGroupSerializer < BasicGroupSerializer
attributes :chat_enabled, :chat_enabled_user_count, :can_chat
def chat_enabled
SiteSetting.chat_enabled
end
def chat_enabled_user_count
object.users.count { |user| user.user_option&.chat_enabled }
end
def can_chat
# + 1 for current user
chat_enabled && chat_enabled_user_count + 1 <= SiteSetting.chat_max_direct_message_users
end
end
end
@@ -3,6 +3,7 @@
module Chat module Chat
class ChatablesSerializer < ::ApplicationSerializer class ChatablesSerializer < ::ApplicationSerializer
attributes :users attributes :users
attributes :groups
attributes :direct_message_channels attributes :direct_message_channels
attributes :category_channels attributes :category_channels
@@ -18,6 +19,18 @@ module Chat
.as_json .as_json
end end
def groups
(object.groups || [])
.map do |group|
{
identifier: "g-#{group.id}",
model: ::Chat::ChatableGroupSerializer.new(group, scope: scope, root: false),
type: "group",
}
end
.as_json
end
def direct_message_channels def direct_message_channels
(object.direct_message_channels || []) (object.direct_message_channels || [])
.map do |channel| .map do |channel|
@@ -20,6 +20,7 @@ module Chat
# @param [Integer] id of the channel # @param [Integer] id of the channel
# @param [Hash] params_to_create # @param [Hash] params_to_create
# @option params_to_create [Array<String>] usernames # @option params_to_create [Array<String>] usernames
# @option params_to_create [Array<String>] groups
# @return [Service::Base::Context] # @return [Service::Base::Context]
contract contract
model :channel model :channel
@@ -27,6 +28,7 @@ module Chat
model :users, optional: true model :users, optional: true
transaction do transaction do
step :validate_user_count
step :upsert_memberships step :upsert_memberships
step :recompute_users_count step :recompute_users_count
step :notice_channel step :notice_channel
@@ -35,20 +37,15 @@ module Chat
# @!visibility private # @!visibility private
class Contract class Contract
attribute :usernames, :array attribute :usernames, :array
validates :usernames, presence: true attribute :groups, :array
attribute :channel_id, :integer attribute :channel_id, :integer
validates :channel_id, presence: true validates :channel_id, presence: true
validate :usernames_length validate :target_presence
def usernames_length def target_presence
if usernames && usernames.length > SiteSetting.chat_max_direct_message_users + 1 # 1 for current user usernames.present? || groups.present?
errors.add(
:usernames,
"should have less than #{SiteSetting.chat_max_direct_message_users} elements",
)
end
end end
end end
@@ -60,17 +57,23 @@ module Chat
end end
def fetch_users(contract:, channel:, **) def fetch_users(contract:, channel:, **)
::User.where( ::Chat::UsersFromUsernamesAndGroupsQuery.call(
"username IN (?) AND id NOT IN (?)", usernames: contract.usernames,
[*contract.usernames], groups: contract.groups,
channel.chatable.direct_message_users.select(:user_id), excluded_user_ids: channel.chatable.direct_message_users.pluck(:user_id),
).to_a )
end end
def fetch_channel(contract:, **) def fetch_channel(contract:, **)
::Chat::Channel.includes(:chatable).find_by(id: contract.channel_id) ::Chat::Channel.includes(:chatable).find_by(id: contract.channel_id)
end end
def validate_user_count(channel:, users:, **)
if channel.user_count + users.length > SiteSetting.chat_max_direct_message_users
fail!("should have less than #{SiteSetting.chat_max_direct_message_users} elements")
end
end
def upsert_memberships(channel:, users:, **) def upsert_memberships(channel:, users:, **)
always_level = ::Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always] always_level = ::Chat::UserChatChannelMembership::NOTIFICATION_LEVELS[:always]
@@ -19,6 +19,7 @@ module Chat
# @param [Guardian] guardian # @param [Guardian] guardian
# @param [Hash] params_to_create # @param [Hash] params_to_create
# @option params_to_create [Array<String>] target_usernames # @option params_to_create [Array<String>] target_usernames
# @option params_to_create [Array<String>] target_groups
# @return [Service::Base::Context] # @return [Service::Base::Context]
policy :can_create_direct_message policy :can_create_direct_message
@@ -32,6 +33,7 @@ module Chat
class_name: Chat::DirectMessageChannel::CanCommunicateAllPartiesPolicy class_name: Chat::DirectMessageChannel::CanCommunicateAllPartiesPolicy
model :direct_message, :fetch_or_create_direct_message model :direct_message, :fetch_or_create_direct_message
model :channel, :fetch_or_create_channel model :channel, :fetch_or_create_channel
step :validate_user_count
step :set_optional_name step :set_optional_name
step :update_memberships step :update_memberships
step :recompute_users_count step :recompute_users_count
@@ -40,7 +42,13 @@ module Chat
class Contract class Contract
attribute :name, :string attribute :name, :string
attribute :target_usernames, :array attribute :target_usernames, :array
validates :target_usernames, presence: true attribute :target_groups, :array
validate :target_presence
def target_presence
target_usernames.present? || target_groups.present?
end
end end
private private
@@ -50,13 +58,22 @@ module Chat
end end
def fetch_target_users(guardian:, contract:, **) def fetch_target_users(guardian:, contract:, **)
User.where(username: [guardian.user.username, *contract.target_usernames]).to_a ::Chat::UsersFromUsernamesAndGroupsQuery.call(
usernames: [*contract.target_usernames, guardian.user.username],
groups: contract.target_groups,
)
end end
def fetch_user_comm_screener(target_users:, guardian:, **) def fetch_user_comm_screener(target_users:, guardian:, **)
UserCommScreener.new(acting_user: guardian.user, target_user_ids: target_users.map(&:id)) UserCommScreener.new(acting_user: guardian.user, target_user_ids: target_users.map(&:id))
end end
def validate_user_count(target_users:, **)
if target_users.length > SiteSetting.chat_max_direct_message_users
fail!("should have less than #{SiteSetting.chat_max_direct_message_users} elements")
end
end
def actor_allows_dms(user_comm_screener:, **) def actor_allows_dms(user_comm_screener:, **)
!user_comm_screener.actor_disallowing_all_pms? !user_comm_screener.actor_disallowing_all_pms?
end end
@@ -1,7 +1,7 @@
# frozen_string_literal: true # frozen_string_literal: true
module Chat module Chat
# Returns a list of chatables (users, category channels, direct message channels) that can be chatted with. # Returns a list of chatables (users, groups ,category channels, direct message channels) that can be chatted with.
# #
# @example # @example
# Chat::SearchChatable.call(term: "@bob", guardian: guardian) # Chat::SearchChatable.call(term: "@bob", guardian: guardian)
@@ -18,6 +18,7 @@ module Chat
step :clean_term step :clean_term
model :memberships, optional: true model :memberships, optional: true
model :users, optional: true model :users, optional: true
model :groups, optional: true
model :category_channels, optional: true model :category_channels, optional: true
model :direct_message_channels, optional: true model :direct_message_channels, optional: true
@@ -25,6 +26,7 @@ module Chat
class Contract class Contract
attribute :term, :string, default: "" attribute :term, :string, default: ""
attribute :include_users, :boolean, default: true attribute :include_users, :boolean, default: true
attribute :include_groups, :boolean, default: true
attribute :include_category_channels, :boolean, default: true attribute :include_category_channels, :boolean, default: true
attribute :include_direct_message_channels, :boolean, default: true attribute :include_direct_message_channels, :boolean, default: true
attribute :excluded_memberships_channel_id, :integer attribute :excluded_memberships_channel_id, :integer
@@ -46,6 +48,12 @@ module Chat
search_users(context, guardian, contract) search_users(context, guardian, contract)
end end
def fetch_groups(guardian:, contract:, **)
return unless contract.include_groups
return unless guardian.can_create_direct_message?
search_groups(context, guardian, contract)
end
def fetch_category_channels(guardian:, contract:, **) def fetch_category_channels(guardian:, contract:, **)
return unless contract.include_category_channels return unless contract.include_category_channels
return if !SiteSetting.enable_public_channels return if !SiteSetting.enable_public_channels
@@ -109,5 +117,15 @@ module Chat
user_search user_search
end end
def search_groups(context, guardian, contract)
Group
.visible_groups(guardian.user)
.includes(users: :user_option)
.where(
"groups.name ILIKE :term_like OR groups.full_name ILIKE :term_like",
term_like: "%#{context.term}%",
)
end
end end
end end
@@ -17,9 +17,14 @@ export default class AddMembers extends Component {
@service loadingSlider; @service loadingSlider;
get membersCount() { get membersCount() {
return ( const userCount = this.args.members?.reduce((acc, member) => {
this.args.members?.length + (this.args.channel?.membershipsCount ?? 0) if (member.type === "group") {
); return acc + member.model.chat_enabled_user_count;
} else {
return acc + 1;
}
}, 0);
return userCount + (this.args.channel?.membershipsCount ?? 0);
} }
@action @action
@@ -27,10 +32,18 @@ export default class AddMembers extends Component {
try { try {
this.loadingSlider.transitionStarted(); this.loadingSlider.transitionStarted();
await this.chatApi.addMembersToChannel( const usernames = this.args.members
this.args.channel.id, .filter((member) => member.type === "user")
this.args.members.mapBy("model.username") .mapBy("model.username");
);
const groups = this.args.members
.filter((member) => member.type === "group")
.mapBy("model.name");
await this.chatApi.addMembersToChannel(this.args.channel.id, {
usernames,
groups,
});
this.toasts.success({ data: { message: I18n.t("saved") } }); this.toasts.success({ data: { message: I18n.t("saved") } });
this.router.transitionTo( this.router.transitionTo(
@@ -58,6 +71,7 @@ export default class AddMembers extends Component {
@onChange={{@onChangeMembers}} @onChange={{@onChangeMembers}}
@close={{@close}} @close={{@close}}
@cancel={{@cancel}} @cancel={{@cancel}}
@membersCount={{this.membersCount}}
@maxReached={{gte @maxReached={{gte
this.membersCount this.membersCount
this.siteSettings.chat_max_direct_message_users this.siteSettings.chat_max_direct_message_users
@@ -2,6 +2,7 @@ import Component from "@glimmer/component";
import { inject as service } from "@ember/service"; import { inject as service } from "@ember/service";
import concatClass from "discourse/helpers/concat-class"; import concatClass from "discourse/helpers/concat-class";
import gt from "truth-helpers/helpers/gt"; import gt from "truth-helpers/helpers/gt";
import not from "truth-helpers/helpers/not";
import ChannelTitle from "discourse/plugins/chat/discourse/components/channel-title"; import ChannelTitle from "discourse/plugins/chat/discourse/components/channel-title";
export default class Channel extends Component { export default class Channel extends Component {
@@ -16,7 +17,10 @@ export default class Channel extends Component {
} }
<template> <template>
<div class="chat-message-creator__chatable -category-channel"> <div
class="chat-message-creator__chatable -category-channel"
data-disabled={{not @item.enabled}}
>
<ChannelTitle @channel={{@item.model}} /> <ChannelTitle @channel={{@item.model}} />
{{#if (gt @item.tracking.unreadCount 0)}} {{#if (gt @item.tracking.unreadCount 0)}}
@@ -0,0 +1,45 @@
import Component from "@glimmer/component";
import { inject as service } from "@ember/service";
import icon from "discourse-common/helpers/d-icon";
import I18n from "discourse-i18n";
export default class ChatableGroup extends Component {
@service currentUser;
@service siteSettings;
group_with_too_many_members = I18n.t(
"chat.new_message_modal.group_with_too_many_members",
{ membersCount: this.args.item.model.chat_enabled_user_count }
);
get isDisabled() {
if (!this.args.membersCount) {
return !this.args.item.enabled;
}
return (
this.args.membersCount + this.args.item.model.chat_enabled_user_count >
this.siteSettings.chat_max_direct_message_users
);
}
<template>
<div
class="chat-message-creator__chatable -group"
data-disabled={{this.isDisabled}}
>
<div class="chat-message-creator__group-icon">
{{icon "user-friends"}}
</div>
<div class="chat-message-creator__group-name">
{{@item.model.name}}
</div>
{{#if this.isDisabled}}
<span class="chat-message-creator__chatable -disabled-chat">
{{this.group_with_too_many_members}}
</span>
{{/if}}
</div>
</template>
}
@@ -20,6 +20,7 @@ export default class ChatablesLoader {
term, term,
options = { options = {
includeUsers: true, includeUsers: true,
includeGroups: true,
includeCategoryChannels: true, includeCategoryChannels: true,
includeDirectMessageChannels: true, includeDirectMessageChannels: true,
excludedUserIds: null, excludedUserIds: null,
@@ -52,6 +53,7 @@ export default class ChatablesLoader {
return [ return [
...results.users, ...results.users,
...results.groups,
...results.direct_message_channels, ...results.direct_message_channels,
...results.category_channels, ...results.category_channels,
] ]
@@ -84,6 +86,10 @@ export default class ChatablesLoader {
} }
#injectTracking(chatable) { #injectTracking(chatable) {
if (!chatable.type === "channel") {
return;
}
return this.chatChannelsManager.allChannels.find( return this.chatChannelsManager.allChannels.find(
(channel) => channel.id === chatable.model.id (channel) => channel.id === chatable.model.id
)?.tracking; )?.tracking;
@@ -5,8 +5,8 @@ import { action } from "@ember/object";
import concatClass from "discourse/helpers/concat-class"; import concatClass from "discourse/helpers/concat-class";
import I18n from "discourse-i18n"; import I18n from "discourse-i18n";
import eq from "truth-helpers/helpers/eq"; import eq from "truth-helpers/helpers/eq";
import not from "truth-helpers/helpers/not";
import Channel from "./channel"; import Channel from "./channel";
import Group from "./group";
import ListAction from "./list-action"; import ListAction from "./list-action";
import User from "./user"; import User from "./user";
@@ -21,6 +21,8 @@ export default class List extends Component {
return ListAction; return ListAction;
case "user": case "user":
return User; return User;
case "group":
return Group;
case "channel": case "channel":
return Channel; return Channel;
} }
@@ -74,9 +76,12 @@ export default class List extends Component {
tabindex="0" tabindex="0"
data-identifier={{item.identifier}} data-identifier={{item.identifier}}
id={{item.id}} id={{item.id}}
data-disabled={{not item.enabled}}
> >
{{component (this.componentForItem item.type) item=item}} {{component
(this.componentForItem item.type)
membersCount=@membersCount
item=item
}}
</li> </li>
{{/each}} {{/each}}
</ul> </ul>
@@ -2,6 +2,7 @@ import { fn } from "@ember/helper";
import DButton from "discourse/components/d-button"; import DButton from "discourse/components/d-button";
import concatClass from "discourse/helpers/concat-class"; import concatClass from "discourse/helpers/concat-class";
import icon from "discourse-common/helpers/d-icon"; import icon from "discourse-common/helpers/d-icon";
import eq from "truth-helpers/helpers/eq";
import ChatUserAvatar from "discourse/plugins/chat/discourse/components/chat-user-avatar"; import ChatUserAvatar from "discourse/plugins/chat/discourse/components/chat-user-avatar";
const Member = <template> const Member = <template>
@@ -12,14 +13,24 @@ const Member = <template>
}} }}
@action={{fn @onSelect @member}} @action={{fn @onSelect @member}}
> >
<ChatUserAvatar {{#if (eq @member.type "user")}}
@user={{@member.model}} <ChatUserAvatar
@interactive={{false}} @user={{@member.model}}
@showPresence={{false}} @interactive={{false}}
/> @showPresence={{false}}
<span class="chat-message-creator__member-username"> />
{{@member.model.username}} <span class="chat-message-creator__member-username">
</span> {{@member.model.username}}
</span>
{{else if (eq @member.type "group")}}
<div class="chat-message-creator__group-icon">
{{icon "user-friends"}}
</div>
<span class="chat-message-creator__member-group">
{{@member.model.name}}
</span>
{{/if}}
{{icon "times"}} {{icon "times"}}
</DButton> </DButton>
</template>; </template>;
@@ -1,7 +1,7 @@
import Component from "@glimmer/component"; import Component from "@glimmer/component";
import concatClass from "discourse/helpers/concat-class"; import concatClass from "discourse/helpers/concat-class";
import I18n from "discourse-i18n"; import I18n from "discourse-i18n";
import eq from "truth-helpers/helpers/eq"; import gte from "truth-helpers/helpers/gte";
export default class MembersCount extends Component { export default class MembersCount extends Component {
get countLabel() { get countLabel() {
@@ -15,7 +15,7 @@ export default class MembersCount extends Component {
<div <div
class={{concatClass class={{concatClass
"chat-message-creator__members-count" "chat-message-creator__members-count"
(if (eq @count @max) "-reached-limit") (if (gte @count @max) "-reached-limit")
}} }}
> >
{{this.countLabel}} {{this.countLabel}}
@@ -47,8 +47,11 @@ export default class MembersSelector extends Component {
return; return;
} }
const chatableMembers =
chatable.type === "group" ? chatable.model.chat_enabled_user_count : 1;
if ( if (
this.args.members.length + (this.args.channel?.membershipsCount ?? 0) >= this.args.membersCount + chatableMembers >
this.siteSettings.chat_max_direct_message_users this.siteSettings.chat_max_direct_message_users
) { ) {
return; return;
@@ -138,6 +141,7 @@ export default class MembersSelector extends Component {
@onSelect={{this.selectChatable}} @onSelect={{this.selectChatable}}
@onHighlight={{this.highlightChatable}} @onHighlight={{this.highlightChatable}}
@maxReached={{@maxReached}} @maxReached={{@maxReached}}
@membersCount={{@membersCount}}
/> />
</ListHandler> </ListHandler>
@@ -20,14 +20,28 @@ export default class NewGroup extends Component {
placeholder = I18n.t("chat.direct_message_creator.group_name"); placeholder = I18n.t("chat.direct_message_creator.group_name");
get membersCount() { get membersCount() {
return this.args.members?.length; return this.args.members?.reduce((acc, member) => {
if (member.type === "group") {
return acc + member.model.chat_enabled_user_count;
} else {
return acc + 1;
}
}, 1);
} }
@action @action
async createGroup() { async createGroup() {
try { try {
const channel = await this.chat.upsertDmChannelForUsernames( const usernames = this.args.members
this.args.members.mapBy("model.username"), .filter((member) => member.type === "user")
.mapBy("model.username");
const groups = this.args.members
.filter((member) => member.type === "group")
.mapBy("model.name");
const channel = await this.chat.upsertDmChannel(
{ usernames, groups },
this.newGroupTitle this.newGroupTitle
); );
@@ -67,6 +81,7 @@ export default class NewGroup extends Component {
@onChange={{@onChangeMembers}} @onChange={{@onChangeMembers}}
@close={{@close}} @close={{@close}}
@cancel={{@cancel}} @cancel={{@cancel}}
@membersCount={{this.membersCount}}
@maxReached={{gte @maxReached={{gte
this.membersCount this.membersCount
this.siteSettings.chat_max_direct_message_users this.siteSettings.chat_max_direct_message_users
@@ -68,6 +68,13 @@ export default class ChatMessageCreatorSearch extends Component {
await this.startOneToOneChannel(item.model.username); await this.startOneToOneChannel(item.model.username);
break; break;
case "group":
if (!item.enabled) {
return;
}
this.args.onChangeMode(MODES.new_group, [item]);
break;
default: default:
this.router.transitionTo("chat.channel", ...item.model.routeModels); this.router.transitionTo("chat.channel", ...item.model.routeModels);
this.args.close(); this.args.close();
@@ -100,7 +107,24 @@ export default class ChatMessageCreatorSearch extends Component {
async startOneToOneChannel(username) { async startOneToOneChannel(username) {
try { try {
const channel = await this.chat.upsertDmChannelForUsernames([username]); const channel = await this.chat.upsertDmChannel({
usernames: [username],
});
if (!channel) {
return;
}
this.args.close?.();
this.router.transitionTo("chat.channel", ...channel.routeModels);
} catch (error) {
popupAjaxError(error);
}
}
async startGroupChannel(group) {
try {
const channel = await this.chat.upsertDmChannel({ groups: [group] });
if (!channel) { if (!channel) {
return; return;
@@ -3,6 +3,7 @@ import { inject as service } from "@ember/service";
import userStatus from "discourse/helpers/user-status"; import userStatus from "discourse/helpers/user-status";
import I18n from "discourse-i18n"; import I18n from "discourse-i18n";
import gt from "truth-helpers/helpers/gt"; import gt from "truth-helpers/helpers/gt";
import not from "truth-helpers/helpers/not";
import ChatUserAvatar from "discourse/plugins/chat/discourse/components/chat-user-avatar"; import ChatUserAvatar from "discourse/plugins/chat/discourse/components/chat-user-avatar";
import ChatUserDisplayName from "discourse/plugins/chat/discourse/components/chat-user-display-name"; import ChatUserDisplayName from "discourse/plugins/chat/discourse/components/chat-user-display-name";
@@ -12,7 +13,10 @@ export default class ChatableUser extends Component {
disabledUserLabel = I18n.t("chat.new_message_modal.disabled_user"); disabledUserLabel = I18n.t("chat.new_message_modal.disabled_user");
<template> <template>
<div class="chat-message-creator__chatable -user"> <div
class="chat-message-creator__chatable -user"
data-disabled={{not @item.enabled}}
>
<ChatUserAvatar @user={{@item.model}} @interactive={{false}} /> <ChatUserAvatar @user={{@item.model}} @interactive={{false}} />
<ChatUserDisplayName @user={{@item.model}} /> <ChatUserDisplayName @user={{@item.model}} />
@@ -15,7 +15,7 @@ export default class ChatUserCardButton extends Component {
@action @action
startChatting() { startChatting() {
return this.chat return this.chat
.upsertDmChannelForUsernames([this.args.user.username]) .upsertDmChannel({ usernames: [this.args.user.username] })
.then((channel) => { .then((channel) => {
this.router.transitionTo("chat.channel", ...channel.routeModels); this.router.transitionTo("chat.channel", ...channel.routeModels);
this.appEvents.trigger("card:close"); this.appEvents.trigger("card:close");
@@ -1,6 +1,7 @@
import { tracked } from "@glimmer/tracking"; import { tracked } from "@glimmer/tracking";
import { inject as service } from "@ember/service"; import { inject as service } from "@ember/service";
import Category from "discourse/models/category"; import Category from "discourse/models/category";
import Group from "discourse/models/group";
import User from "discourse/models/user"; import User from "discourse/models/user";
import ChatChannel from "discourse/plugins/chat/discourse/models/chat-channel"; import ChatChannel from "discourse/plugins/chat/discourse/models/chat-channel";
@@ -17,6 +18,14 @@ export default class ChatChatable {
}); });
} }
static createGroup(model) {
return new ChatChatable({
type: "group",
model,
identifier: `g-${model.id}`,
});
}
static createChannel(model) { static createChannel(model) {
return new ChatChatable({ return new ChatChatable({
type: "channel", type: "channel",
@@ -60,6 +69,16 @@ export default class ChatChatable {
this.model = User.create(args.model); this.model = User.create(args.model);
break; break;
case "group":
this.enabled = args.model.can_chat;
if (args.model instanceof Group) {
this.model = args.model;
break;
}
this.model = Group.create(args.model);
break;
} }
} }
@@ -67,6 +86,10 @@ export default class ChatChatable {
return this.type === "user"; return this.type === "user";
} }
get isGroup() {
return this.type === "group";
}
get isCategory() { get isCategory() {
return this instanceof Category; return this instanceof Category;
} }
@@ -552,11 +552,14 @@ export default class ChatApi extends Service {
* Add members to a channel. * Add members to a channel.
* *
* @param {number} channelId - The ID of the channel. * @param {number} channelId - The ID of the channel.
* @param {Array<string>} usernames - The usernames of the users to add. * @param {object} targets
* @param {Array<string>} targets.usernames - The usernames of the users to add.
* @param {Array<string>} targets.groups - The groups names of the groups to add.
*/ */
addMembersToChannel(channelId, usernames) { addMembersToChannel(channelId, targets) {
return this.#postRequest(`/channels/${channelId}/memberships`, { return this.#postRequest(`/channels/${channelId}/memberships`, {
usernames, usernames: targets.usernames,
groups: targets.groups,
}); });
} }
@@ -390,16 +390,22 @@ export default class Chat extends Service {
.concat(user.username) .concat(user.username)
.uniq(); .uniq();
return this.upsertDmChannelForUsernames(usernames); return this.upsertDmChannel({ usernames });
} }
// @param {array} usernames - The usernames to create or fetch the direct message // @param {object} targets - The targets to create or fetch the direct message
// channel for. The current user will automatically be included in the channel // channel for. The current user will automatically be included in the channel when it is created.
// when it is created. // @param {array} [targets.usernames] - The usernames to include in the direct message channel.
upsertDmChannelForUsernames(usernames, name = null) { // @param {array} [targets.groups] - The groups to include in the direct message channel.
// @param {string|null} [name=null] - Optional name for the direct message channel.
upsertDmChannel(targets, name = null) {
return ajax("/chat/api/direct-message-channels.json", { return ajax("/chat/api/direct-message-channels.json", {
method: "POST", method: "POST",
data: { target_usernames: usernames.uniq(), name }, data: {
target_usernames: targets.usernames?.uniq(),
target_groups: targets.groups?.uniq(),
name,
},
}) })
.then((response) => { .then((response) => {
const channel = this.chatChannelsManager.store(response.channel); const channel = this.chatChannelsManager.store(response.channel);
@@ -38,15 +38,27 @@
color: var(--primary); color: var(--primary);
border-color: var(--tertiary); border-color: var(--tertiary);
.d-icon { .d-icon-times {
color: var(--primary); color: var(--primary);
} }
} }
} }
.d-icon { .d-icon-times {
margin-left: 0.25rem; margin-left: 0.25rem;
} }
.chat-message-creator__group-icon {
display: flex;
justify-content: center;
align-items: center;
height: 24px;
width: 24px;
.d-icon.d-icon-user-friends {
color: var(--primary-high);
}
}
} }
.chat-message-creator__add-members__close-btn { .chat-message-creator__add-members__close-btn {
@@ -144,6 +156,7 @@
.chat-message-creator__members-count { .chat-message-creator__members-count {
white-space: nowrap; white-space: nowrap;
color: var(--primary-medium); color: var(--primary-medium);
&.-reached-limit { &.-reached-limit {
color: var(--danger); color: var(--danger);
} }
@@ -168,6 +181,11 @@
display: flex; display: flex;
flex-direction: column; flex-direction: column;
[data-disabled] {
opacity: 0.5;
cursor: not-allowed;
}
&-item { &-item {
box-sizing: border-box; box-sizing: border-box;
cursor: pointer; cursor: pointer;
@@ -175,11 +193,6 @@
display: flex; display: flex;
align-items: center; align-items: center;
&[data-disabled] {
opacity: 0.5;
cursor: not-allowed;
}
.d-icon-users { .d-icon-users {
padding: 4px 4px; padding: 4px 4px;
box-sizing: border-box; box-sizing: border-box;
@@ -200,28 +213,40 @@
} }
} }
.chat-message-creator__chatable.-user { .chat-message-creator__chatable {
.chat-user-display-name { display: flex;
align-items: center;
width: 100%;
&.-user .chat-user-display-name {
padding-left: 0.5rem; padding-left: 0.5rem;
} }
.chat-message-creator__chatable.-disabled-chat { &.-group {
padding-left: 0.25rem; .chat-message-creator__group-name {
} padding-left: 0.5rem;
} }
.chat-message-creator__chatable.-category-channel { .chat-message-creator__group-icon .d-icon-user-friends {
.chat-channel-title__category-badge { padding: 5px 5px;
box-sizing: border-box;
color: var(--primary-high);
background: var(--primary-low);
border-radius: 100%;
width: 24px;
height: 22px;
}
}
&.-category-channel .chat-channel-title__category-badge {
display: flex; display: flex;
justify-content: center; justify-content: center;
width: 24px; width: 24px;
} }
}
.chat-message-creator__chatable.-user, &.-disabled-chat {
.chat-message-creator__chatable.-category-channel { padding-left: 0.25rem;
display: flex; }
align-items: center;
.unread-indicator { .unread-indicator {
margin-left: 0.5rem; margin-left: 0.5rem;
@@ -275,6 +300,7 @@
transform: scale(0.1); transform: scale(0.1);
opacity: 0; opacity: 0;
} }
100% { 100% {
transform: scale(1); transform: scale(1);
opacity: 1; opacity: 1;
@@ -350,6 +350,7 @@ en:
filter: "Filter" filter: "Filter"
cant_add_more_members: "Maximum number of members reached" cant_add_more_members: "Maximum number of members reached"
create_new_group_chat: "Create group chat" create_new_group_chat: "Create group chat"
group_with_too_many_members: "has too many members (%{membersCount})"
channel_edit_name_slug_modal: channel_edit_name_slug_modal:
title: Edit channel title: Edit channel
@@ -0,0 +1,63 @@
# frozen_string_literal: true
require "rails_helper"
describe Chat::UsersFromUsernamesAndGroupsQuery do
fab!(:user1) { Fabricate(:user) }
fab!(:user2) { Fabricate(:user) }
fab!(:user3) { Fabricate(:user) }
fab!(:user4) { Fabricate(:user) }
fab!(:group1) { Fabricate(:public_group, users: [user1, user2]) }
fab!(:group2) { Fabricate(:public_group, users: [user3]) }
context "when searching by usernames" do
it "returns users matching the usernames" do
result = described_class.call(usernames: [user1.username, user4.username], groups: [])
expect(result).to contain_exactly(user1, user4)
end
end
context "when searching by groups" do
it "returns users belonging to the specified groups" do
result = described_class.call(usernames: [], groups: [group1.name])
expect(result).to contain_exactly(user1, user2)
end
end
context "when searching by both usernames and groups" do
it "returns a unique set of users matching either condition" do
result = described_class.call(usernames: [user2.username], groups: [group2.name])
expect(result).to contain_exactly(user2, user3)
end
end
context "when no usernames or groups are provided" do
it "returns an empty array" do
result = described_class.call(usernames: [], groups: [])
expect(result).to be_empty
end
end
context "when user chat is disabled" do
before { user1.user_option.update!(chat_enabled: false) }
it "does not return users with chat disabled" do
result = described_class.call(usernames: [], groups: [group1.name])
expect(result).not_to include(user1)
expect(result).to include(user2)
end
end
context "when excluding specific user IDs" do
it "does not return users with specified IDs" do
result =
described_class.call(
usernames: [user4.username],
groups: [group1.name, group2.name],
excluded_user_ids: [user1.id, user3.id],
)
expect(result).not_to include(user1, user3)
expect(result).to include(user2, user4)
end
end
end
@@ -5,7 +5,8 @@ RSpec.describe Chat::AddUsersToChannel do
subject(:contract) { described_class.new(usernames: [], channel_id: nil) } subject(:contract) { described_class.new(usernames: [], channel_id: nil) }
it { is_expected.to validate_presence_of :channel_id } it { is_expected.to validate_presence_of :channel_id }
it { is_expected.to validate_presence_of :usernames } it { is_expected.to validate_presence_of :usernames if :groups.blank? }
it { is_expected.to validate_presence_of :groups if :usernames.blank? }
end end
describe ".call" do describe ".call" do
@@ -15,6 +16,9 @@ RSpec.describe Chat::AddUsersToChannel do
fab!(:users) { Fabricate.times(5, :user) } fab!(:users) { Fabricate.times(5, :user) }
fab!(:direct_message) { Fabricate(:direct_message, users: [current_user], group: true) } fab!(:direct_message) { Fabricate(:direct_message, users: [current_user], group: true) }
fab!(:channel) { Fabricate(:direct_message_channel, chatable: direct_message) } fab!(:channel) { Fabricate(:direct_message_channel, chatable: direct_message) }
fab!(:group_user_1) { Fabricate(:user) }
fab!(:group_user_2) { Fabricate(:user) }
fab!(:group) { Fabricate(:public_group, users: [group_user_1, group_user_2]) }
let(:guardian) { Guardian.new(current_user) } let(:guardian) { Guardian.new(current_user) }
let(:params) do let(:params) do
@@ -28,6 +32,30 @@ RSpec.describe Chat::AddUsersToChannel do
expect(result.users.map(&:username)).to contain_exactly(*users.map(&:username)) expect(result.users.map(&:username)).to contain_exactly(*users.map(&:username))
end end
it "includes users from groups" do
params.merge!(groups: [group.name])
expect(result.users.map(&:username)).to include(
group_user_1.username,
group_user_2.username,
)
end
context "with user count validation" do
before { SiteSetting.chat_max_direct_message_users = 8 }
it "succeeds when usernames does not exceed limit" do
expect { result }.to change { Chat::UserChatChannelMembership.count }.by(6)
expect(result).to be_a_success
end
it "succeeds when usernames and groups does not exceed limit" do
params.merge!(groups: [group.name])
expect { result }.to change { Chat::UserChatChannelMembership.count }.by(8)
expect(result).to be_a_success
end
end
it "doesn't include existing direct message users" do it "doesn't include existing direct message users" do
Chat::DirectMessageUser.create!(user: users.first, direct_message: direct_message) Chat::DirectMessageUser.create!(user: users.first, direct_message: direct_message)
@@ -65,10 +93,10 @@ RSpec.describe Chat::AddUsersToChannel do
end end
end end
context "when there are too many usernames" do context "when usernames exceeds chat_max_direct_message_users" do
before { SiteSetting.chat_max_direct_message_users = 2 } before { SiteSetting.chat_max_direct_message_users = 4 }
it { is_expected.to fail_a_contract } it { is_expected.to fail_a_step(:validate_user_count) }
end end
context "when channel is not found" do context "when channel is not found" do
@@ -6,7 +6,8 @@ RSpec.describe Chat::CreateDirectMessageChannel do
let(:params) { { target_usernames: %w[lechuck elaine] } } let(:params) { { target_usernames: %w[lechuck elaine] } }
it { is_expected.to validate_presence_of :target_usernames } it { is_expected.to validate_presence_of :target_usernames if :target_groups.blank? }
it { is_expected.to validate_presence_of :target_groups if :target_usernames.blank? }
context "when the target_usernames argument is a string" do context "when the target_usernames argument is a string" do
let(:params) { { target_usernames: "lechuck,elaine" } } let(:params) { { target_usernames: "lechuck,elaine" } }
@@ -16,6 +17,14 @@ RSpec.describe Chat::CreateDirectMessageChannel do
expect(contract.target_usernames).to eq(%w[lechuck elaine]) expect(contract.target_usernames).to eq(%w[lechuck elaine])
end end
end end
context "when the target_groups argument is a string" do
let(:params) { { target_groups: "admins,moderators" } }
it "splits it into an array" do
contract.validate
expect(contract.target_groups).to eq(%w[admins moderators])
end
end
end end
describe ".call" do describe ".call" do
@@ -24,6 +33,8 @@ RSpec.describe Chat::CreateDirectMessageChannel do
fab!(:current_user) { Fabricate(:user, username: "guybrush") } fab!(:current_user) { Fabricate(:user, username: "guybrush") }
fab!(:user_1) { Fabricate(:user, username: "lechuck") } fab!(:user_1) { Fabricate(:user, username: "lechuck") }
fab!(:user_2) { Fabricate(:user, username: "elaine") } fab!(:user_2) { Fabricate(:user, username: "elaine") }
fab!(:user_3) { Fabricate(:user) }
fab!(:group) { Fabricate(:public_group, users: [user_3]) }
let(:guardian) { Guardian.new(current_user) } let(:guardian) { Guardian.new(current_user) }
let(:target_usernames) { [user_1.username, user_2.username] } let(:target_usernames) { [user_1.username, user_2.username] }
@@ -63,6 +74,40 @@ RSpec.describe Chat::CreateDirectMessageChannel do
end end
end end
it "includes users from target groups" do
params.delete(:target_usernames)
params.merge!(target_groups: [group.name])
expect(result.channel.user_chat_channel_memberships.pluck(:user_id)).to include(user_3.id)
end
it "combines target_usernames and target_groups" do
params.merge!(target_groups: [group.name])
expect(result.channel.user_chat_channel_memberships.pluck(:user_id)).to contain_exactly(
current_user.id,
user_1.id,
user_2.id,
user_3.id,
)
end
context "with user count validation" do
before { SiteSetting.chat_max_direct_message_users = 4 }
it "succeeds when target_usernames does not exceed limit" do
expect { result }.to change { Chat::UserChatChannelMembership.count }.by(3)
expect(result).to be_a_success
end
it "succeeds when target_usernames and target_groups does not exceed limit" do
params.merge!(target_groups: [group.name])
expect { result }.to change { Chat::UserChatChannelMembership.count }.by(4)
expect(result).to be_a_success
end
end
context "when there is an existing direct message channel for the target users" do context "when there is an existing direct message channel for the target users" do
context "when a name has been given" do context "when a name has been given" do
let(:target_usernames) { [user_1.username] } let(:target_usernames) { [user_1.username] }
@@ -121,6 +166,12 @@ RSpec.describe Chat::CreateDirectMessageChannel do
end end
end end
context "when target_usernames exceeds chat_max_direct_message_users" do
before { SiteSetting.chat_max_direct_message_users = 2 }
it { is_expected.to fail_a_step(:validate_user_count) }
end
context "when the current user cannot make direct messages" do context "when the current user cannot make direct messages" do
fab!(:current_user) { Fabricate(:user) } fab!(:current_user) { Fabricate(:user) }
@@ -129,18 +180,6 @@ RSpec.describe Chat::CreateDirectMessageChannel do
it { is_expected.to fail_a_policy(:can_create_direct_message) } it { is_expected.to fail_a_policy(:can_create_direct_message) }
end end
context "when the number of target users exceeds chat_max_direct_message_users" do
before { SiteSetting.chat_max_direct_message_users = 1 }
it { is_expected.to fail_a_policy(:satisfies_dms_max_users_limit) }
context "when the user is staff" do
fab!(:current_user) { Fabricate(:admin) }
it { is_expected.not_to fail_a_policy(:satisfies_dms_max_users_limit) }
end
end
context "when the actor is not allowing anyone to message them" do context "when the actor is not allowing anyone to message them" do
before { current_user.user_option.update!(allow_private_messages: false) } before { current_user.user_option.update!(allow_private_messages: false) }
@@ -8,6 +8,8 @@ RSpec.describe Chat::SearchChatable do
fab!(:sam) { Fabricate(:user, username: "sam-user") } fab!(:sam) { Fabricate(:user, username: "sam-user") }
fab!(:charlie) { Fabricate(:user, username: "charlie-user") } fab!(:charlie) { Fabricate(:user, username: "charlie-user") }
fab!(:alain) { Fabricate(:user, username: "alain-user") } fab!(:alain) { Fabricate(:user, username: "alain-user") }
fab!(:group_1) { Fabricate(:group, name: "awesome-group") }
fab!(:group_2) { Fabricate(:group) }
fab!(:channel_1) { Fabricate(:chat_channel, name: "bob-channel") } fab!(:channel_1) { Fabricate(:chat_channel, name: "bob-channel") }
fab!(:channel_2) { Fabricate(:direct_message_channel, users: [current_user, sam]) } fab!(:channel_2) { Fabricate(:direct_message_channel, users: [current_user, sam]) }
fab!(:channel_3) { Fabricate(:direct_message_channel, users: [current_user, sam, charlie]) } fab!(:channel_3) { Fabricate(:direct_message_channel, users: [current_user, sam, charlie]) }
@@ -17,6 +19,7 @@ RSpec.describe Chat::SearchChatable do
let(:guardian) { Guardian.new(current_user) } let(:guardian) { Guardian.new(current_user) }
let(:term) { "" } let(:term) { "" }
let(:include_users) { false } let(:include_users) { false }
let(:include_groups) { false }
let(:include_category_channels) { false } let(:include_category_channels) { false }
let(:include_direct_message_channels) { false } let(:include_direct_message_channels) { false }
let(:excluded_memberships_channel_id) { nil } let(:excluded_memberships_channel_id) { nil }
@@ -25,6 +28,7 @@ RSpec.describe Chat::SearchChatable do
guardian: guardian, guardian: guardian,
term: term, term: term,
include_users: include_users, include_users: include_users,
include_groups: include_groups,
include_category_channels: include_category_channels, include_category_channels: include_category_channels,
include_direct_message_channels: include_direct_message_channels, include_direct_message_channels: include_direct_message_channels,
excluded_memberships_channel_id: excluded_memberships_channel_id, excluded_memberships_channel_id: excluded_memberships_channel_id,
@@ -88,6 +92,32 @@ RSpec.describe Chat::SearchChatable do
end end
end end
context "when including groups" do
let(:include_groups) { true }
it "fetches groups" do
expect(result.groups).to include(group_1, group_2)
end
it "can filter groups by name" do
params[:term] = "awesome-group"
expect(result.groups).to contain_exactly(group_1)
end
it "excludes groups not matching the search term" do
params[:term] = "nonexistent"
expect(result.groups).to be_empty
end
end
context "when not including groups" do
let(:include_groups) { false }
it "doesnt fetch groups" do
expect(result.groups).to be_nil
end
end
context "when including category channels" do context "when including category channels" do
let(:include_category_channels) { true } let(:include_category_channels) { true }
@@ -6,6 +6,7 @@ RSpec.describe "Flag message", type: :system do
fab!(:current_user) { Fabricate(:user) } fab!(:current_user) { Fabricate(:user) }
before do before do
SiteSetting.chat_max_direct_message_users = 3
chat_system_bootstrap chat_system_bootstrap
sign_in(current_user) sign_in(current_user)
end end
@@ -87,4 +88,42 @@ RSpec.describe "Flag message", type: :system do
expect(page).to have_current_path(%r{/chat/c/cats/\d+}) expect(page).to have_current_path(%r{/chat/c/cats/\d+})
end end
it "can create a new group by clicking on an user group" do
user_1 = Fabricate(:user)
user_2 = Fabricate(:user)
group = Fabricate(:public_group, users: [user_1, user_2])
visit("/")
chat_page.prefers_full_page
chat_page.open_new_message
chat_page.find(".chat-message-creator__search-input__input").fill_in(with: group.name)
chat_page.message_creator.click_row(group)
chat_page.find(".chat-message-creator__new-group-header__input").fill_in(with: "dogs")
chat_page.find(".create-chat-group").click
expect(page).to have_current_path(%r{/chat/c/dogs/\d+})
end
it "doesnt allow adding a user group if it will exceed the member limit" do
user_1 = Fabricate(:user)
user_2 = Fabricate(:user)
user_3 = Fabricate(:user)
group = Fabricate(:public_group, users: [user_1, user_2])
visit("/")
chat_page.prefers_full_page
chat_page.open_new_message
chat_page.find("#new-group-chat").click
chat_page.find(".chat-message-creator__new-group-header__input").fill_in(with: "hamsters")
chat_page.find(".chat-message-creator__members-input").fill_in(with: user_3.username)
chat_page.message_creator.click_row(user_3)
chat_page.find(".chat-message-creator__members-input").fill_in(with: group.name)
chat_page.message_creator.click_row(group)
expect(chat_page.message_creator).to have_css("div[data-disabled]")
expect(chat_page.message_creator).to be_listing(group)
chat_page.message_creator.click_row(group)
expect(chat_page.message_creator).to be_listing(group)
end
end end
@@ -129,6 +129,8 @@ module PageObjects
selector += "[data-identifier='c-#{chatable.id}']" selector += "[data-identifier='c-#{chatable.id}']"
elsif chatable.try(:direct_message_channel?) elsif chatable.try(:direct_message_channel?)
selector += "[data-identifier='c-#{chatable.id}']" selector += "[data-identifier='c-#{chatable.id}']"
elsif chatable.is_a?(Group)
selector += "[data-identifier='g-#{chatable.id}']"
else else
selector += "[data-identifier='u-#{chatable.id}']" selector += "[data-identifier='u-#{chatable.id}']"
end end