Skip to content

Commit 0cc9293

Browse files
CayoPOliveiragabrieljablonski
authored andcommitted
fix: address PR review feedback for group conversations
- Fix nil safety in group_invites and group_join_requests controllers by replacing group_conversation.inbox.channel with @contact.group_channel - Add before_action guard in group_members_controller to validate contact is a group with identifier before create/update/destroy - Persist metadata locally in group_metadata_controller after provider calls (subject -> name, description -> additional_attributes) - Add server-side allow_group_creation? check in groups_controller - Add word boundary to mention regex to prevent matching inside words - Remove useless catch clauses in groupMembers store (try/finally only) - Default groupType to [] in customViewsHelper to prevent crash - Fix swagger parameter name mismatch (contact_id -> id) across all group endpoint YML files for consistency
1 parent 0ab169f commit 0cc9293

18 files changed

+38
-42
lines changed

app/controllers/api/v1/accounts/contacts/group_invites_controller.rb

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,7 @@ def revoke
1818
private
1919

2020
def channel
21-
@channel ||= group_conversation.inbox.channel
22-
end
23-
24-
def group_conversation
25-
@group_conversation ||= Current.account.conversations
26-
.where(contact_id: @contact.id, group_type: :group, status: %i[open pending])
27-
.first
21+
@channel ||= @contact.group_channel
2822
end
2923

3024
def invite_response(code)

app/controllers/api/v1/accounts/contacts/group_join_requests_controller.rb

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,6 @@ def handle
1818
private
1919

2020
def channel
21-
@channel ||= group_conversation.inbox.channel
22-
end
23-
24-
def group_conversation
25-
@group_conversation ||= Current.account.conversations
26-
.where(contact_id: @contact.id, group_type: :group, status: %i[open pending])
27-
.first
21+
@channel ||= @contact.group_channel
2822
end
2923
end

app/controllers/api/v1/accounts/contacts/group_members_controller.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
class Api::V1::Accounts::Contacts::GroupMembersController < Api::V1::Accounts::Contacts::BaseController
22
DEFAULT_PER_PAGE = 10
33

4+
before_action :ensure_group_contact, only: %i[create update destroy]
5+
46
def index
57
authorize @contact, :show?
68

@@ -59,6 +61,12 @@ def destroy
5961

6062
private
6163

64+
def ensure_group_contact
65+
return if @contact.group_type_group? && @contact.identifier.present?
66+
67+
render json: { error: 'Contact is not a valid group' }, status: :unprocessable_entity
68+
end
69+
6270
def group_members
6371
GroupMember.where(group_contact: @contact)
6472
end

app/controllers/api/v1/accounts/contacts/group_metadata_controller.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,13 @@ def update
1212

1313
def update_subject
1414
channel.update_group_subject(@contact.identifier, params[:subject])
15+
@contact.update!(name: params[:subject])
1516
end
1617

1718
def update_description
1819
channel.update_group_description(@contact.identifier, params[:description])
20+
attrs = @contact.additional_attributes.merge('description' => params[:description])
21+
@contact.update!(additional_attributes: attrs)
1922
end
2023

2124
def channel

app/controllers/api/v1/accounts/groups_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,6 @@ def create
1717
private
1818

1919
def inbox_accessible?(inbox)
20-
inbox.present? && Current.user.assigned_inboxes.exists?(id: inbox.id)
20+
inbox.present? && Current.user.assigned_inboxes.exists?(id: inbox.id) && inbox.channel.try(:allow_group_creation?)
2121
end
2222
end

app/javascript/dashboard/helper/customViewsHelper.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ export const getValuesForFilter = (filter, params) => {
8888
campaigns,
8989
labels,
9090
priority,
91-
group_type: groupType,
91+
group_type: groupType = [],
9292
} = params;
9393
switch (attribute_key) {
9494
case 'status':

app/javascript/dashboard/store/modules/groupMembers.js

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ export const actions = {
4444
try {
4545
const { data } = await GroupMembersAPI.createGroup(params);
4646
return data;
47-
} catch (error) {
48-
throw new Error(error);
4947
} finally {
5048
commit(types.SET_GROUP_MEMBERS_UI_FLAG, { isCreating: false });
5149
}
@@ -68,8 +66,6 @@ export const actions = {
6866
});
6967
}
7068
commit(types.SET_GROUP_MEMBERS_META, { contactId, meta: data.meta });
71-
} catch (error) {
72-
throw new Error(error);
7369
} finally {
7470
commit(
7571
types.SET_GROUP_MEMBERS_UI_FLAG,
@@ -94,8 +90,6 @@ export const actions = {
9490
try {
9591
await GroupMembersAPI.addMembers(contactId, participants);
9692
await dispatch('fetch', { contactId });
97-
} catch (error) {
98-
throw new Error(error);
9993
} finally {
10094
commit(types.SET_GROUP_MEMBERS_UI_FLAG, { isUpdating: false });
10195
}
@@ -115,8 +109,6 @@ export const actions = {
115109
commit(types.SET_GROUP_MEMBERS_UI_FLAG, { isUpdating: true });
116110
try {
117111
await GroupMembersAPI.updateGroupMetadata(contactId, params);
118-
} catch (error) {
119-
throw new Error(error);
120112
} finally {
121113
commit(types.SET_GROUP_MEMBERS_UI_FLAG, { isUpdating: false });
122114
}

app/services/whatsapp/mention_converter_service.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
class Whatsapp::MentionConverterService
22
MENTION_REGEX = %r{\[@([^\]]+)\]\(mention://contact/(\d+)/([^)]+)\)}
33
ALL_MENTION_REGEX = %r{\[@[^\]]*\]\(mention://contact/0/all\)}
4-
INCOMING_ALL_PATTERNS = /@(all|todos|everyone)/i
4+
INCOMING_ALL_PATTERNS = /@(all|todos|everyone)\b/i
55

66
class << self
77
def extract_mentions_for_whatsapp(content, account)

spec/controllers/api/v1/accounts/groups_controller_spec.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@
1212
create(:inbox_member, inbox: inbox, user: agent)
1313
allow(Groups::CreateService).to receive(:new).and_return(create_service)
1414
allow(create_service).to receive(:perform)
15+
Channel::WebWidget.define_method(:allow_group_creation?) { true }
16+
end
17+
18+
after do
19+
Channel::WebWidget.remove_method(:allow_group_creation?) if Channel::WebWidget.method_defined?(:allow_group_creation?)
1520
end
1621

1722
describe 'POST /api/v1/accounts/{account.id}/groups' do

swagger/paths/application/contacts/group_invite.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
parameters:
22
- $ref: '#/components/parameters/account_id'
3-
- name: contact_id
3+
- name: id
44
in: path
55
required: true
66
schema:

0 commit comments

Comments
 (0)