refactor: move system message calls from sendMessage to saveSystemMessage #32842

ricardogarim · 2024-07-19T02:18:15Z

As per OPI-19, some use cases that send internal live chat messages have been updated. These messages now use the saveSystemMessage method instead of sendMessage. This change maintains the same outcome but bypasses certain validations that increase execution time.

We believe these validations are unnecessary since internal messages are typically predefined (often hardcoded) and do not violate server message rules.

…sage

dionisio-bot · 2024-07-19T02:18:18Z

Looks like this PR is not ready to merge, because of the following issues:

This PR is missing the 'stat: QA assured' label

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

changeset-bot · 2024-07-19T02:18:19Z

⚠️ No Changeset found

Latest commit: 42645fc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

codecov · 2024-07-19T02:35:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.40%. Comparing base (bbdff10) to head (42645fc).
Report is 2 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #32842      +/-   ##
===========================================
- Coverage    59.40%   59.40%   -0.01%     
===========================================
  Files         2541     2541              
  Lines        63176    63171       -5     
  Branches     14220    14218       -2     
===========================================
- Hits         37527    37524       -3     
+ Misses       22934    22933       -1     
+ Partials      2715     2714       -1

Flag	Coverage Δ
unit	`76.02% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

apps/meteor/app/livechat/server/lib/LivechatTyped.ts

sampaiodiego · 2024-08-15T14:34:17Z

apps/meteor/app/livechat/server/lib/Helper.ts

-	// TODO: replace with `Message.saveSystemMessage`
-
-	await sendMessage(guest, { t: 'livechat-started', msg: '', groupable: false, token: guest.token }, room);
+	await Message.saveSystemMessageAndNotifyUser('livechat-started', rid, '', { _id, username }, { groupable: false });


looks like you removed token for some reason.. I'm suggesting removing groupable since saveSystemMessageAndNotifyUser already adds it

Suggested change

await Message.saveSystemMessageAndNotifyUser('livechat-started', rid, '', { _id, username }, { groupable: false });

await Message.saveSystemMessageAndNotifyUser('livechat-started', rid, '', { _id, username }, { token: guest.token });

sampaiodiego · 2024-08-15T14:37:09Z

apps/meteor/app/livechat/server/lib/LivechatTyped.ts

+		await Message.saveSystemMessageAndNotifyUser('livechat-close', rid, comment ?? '', closeData.closedBy, {
+			groupable: false,
+			transcriptRequested,
+		});


Suggested change

await Message.saveSystemMessageAndNotifyUser('livechat-close', rid, comment ?? '', closeData.closedBy, {

groupable: false,

transcriptRequested,

});

await Message.saveSystemMessageAndNotifyUser('livechat-close', rid, comment ?? '', closeData.closedBy, {

transcriptRequested,

...(isRoomClosedByVisitorParams(params) && { token: chatCloser.token }),

});

sampaiodiego · 2024-08-15T14:37:58Z

apps/meteor/app/livechat/server/lib/LivechatTyped.ts

+			t: 'livechat_transfer_history',
+			rid: room._id,
+			ts: new Date(),
+			msg: '',
+			u: {
+				_id,
+				username,
+			},
+			...(transferData.transferredBy.userType === 'visitor' && { token: room.v.token }),
+			groupable: false,


all these properties are already set inside of saveSystemMessageAndNotifyUser so we don't need to pass them

Suggested change

t: 'livechat_transfer_history',

rid: room._id,

ts: new Date(),

msg: '',

u: {

_id,

username,

},

...(transferData.transferredBy.userType === 'visitor' && { token: room.v.token }),

groupable: false,

...(transferData.transferredBy.userType === 'visitor' && { token: room.v.token }),

sampaiodiego · 2024-08-15T14:40:53Z

apps/meteor/server/services/messages/service.ts

+			throw new Error('Failed to find the created message.');
+		}
+
+		void notifyOnMessageChange({ id: createdMessage._id });


since you just fetched the inserted message, we can optimize by passing the data to notifyOnMessageChange otherwise it will fetch it from DB again

Suggested change

void notifyOnMessageChange({ id: createdMessage._id });

void notifyOnMessageChange({ id: createdMessage._id, data: createdMessage });

…sage (#32842) Co-authored-by: Guilherme Gazzo <[email protected]>

refactor: move system message calls from sendMessage to saveSystemMes…

bc19987

…sage

ricardogarim force-pushed the refactor/unify-livechat-system-message-calls branch from 77e3364 to 7d6f676 Compare July 19, 2024 02:27

ricardogarim marked this pull request as ready for review July 22, 2024 11:38

ricardogarim requested review from a team as code owners July 22, 2024 11:38

ricardogarim requested a review from a team as a code owner July 23, 2024 01:46

ricardogarim modified the milestones: 6.11, 6.12 Jul 23, 2024

ricardogarim marked this pull request as draft July 29, 2024 02:36

refactor: move calls from sendMessage to saveSystemMessage

f95aca7

ricardogarim force-pushed the refactor/unify-livechat-system-message-calls branch from 12dabab to f95aca7 Compare July 29, 2024 02:40

ricardogarim added 8 commits July 28, 2024 23:51

chore: remove unneeded logs

8490c5a

merge from develop

92d18ef

chore: linter fix

29f4e9e

chore: typecheck fix

3cf1467

chore: add notifyUsersOnSystemMessage and saveSystemMessageAndNotifyUser

bc13573

Merge from develop

8adebcb

Merge branch 'develop' into refactor/unify-livechat-system-message-calls

f3bd2d6

fix: change notifyUsersOnSystemMessage to use roomUpdater

6dbb2be

sampaiodiego requested changes Aug 14, 2024

View reviewed changes

apps/meteor/app/livechat/server/lib/LivechatTyped.ts Show resolved Hide resolved

ricardogarim added 2 commits August 14, 2024 12:52

fix: livechat_transfer_history input message

c014d84

fix: bring back Rooms.incMsgCountById on saveSystemMessage

c127afe

ricardogarim marked this pull request as ready for review August 15, 2024 00:22

ricardogarim requested a review from a team August 15, 2024 00:23

sampaiodiego requested changes Aug 15, 2024

View reviewed changes

fix: review suggestions

7dd493f

ricardogarim requested a review from sampaiodiego August 15, 2024 23:34

ggazzo approved these changes Aug 19, 2024

View reviewed changes

Merge branch 'develop' into refactor/unify-livechat-system-message-calls

42645fc

ggazzo added the stat: needs testing label Aug 19, 2024

ggazzo merged commit 8162986 into develop Aug 19, 2024
48 checks passed

ggazzo deleted the refactor/unify-livechat-system-message-calls branch August 19, 2024 13:32

abhinavkrin pushed a commit that referenced this pull request Oct 25, 2024

refactor: move system message calls from sendMessage to saveSystemMes…

2b59b53

…sage (#32842) Co-authored-by: Guilherme Gazzo <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: move system message calls from sendMessage to saveSystemMessage #32842

refactor: move system message calls from sendMessage to saveSystemMessage #32842

ricardogarim commented Jul 19, 2024 •

edited by jira bot

Loading

dionisio-bot bot commented Jul 19, 2024 •

edited

Loading

changeset-bot bot commented Jul 19, 2024 •

edited

Loading

codecov bot commented Jul 19, 2024 •

edited

Loading

sampaiodiego Aug 15, 2024

sampaiodiego Aug 15, 2024

sampaiodiego Aug 15, 2024

sampaiodiego Aug 15, 2024

	await Message.saveSystemMessageAndNotifyUser('livechat-started', rid, '', { _id, username }, { groupable: false });
	await Message.saveSystemMessageAndNotifyUser('livechat-started', rid, '', { _id, username }, { token: guest.token });

	void notifyOnMessageChange({ id: createdMessage._id });
	void notifyOnMessageChange({ id: createdMessage._id, data: createdMessage });

refactor: move system message calls from sendMessage to saveSystemMessage #32842

refactor: move system message calls from sendMessage to saveSystemMessage #32842

Conversation

ricardogarim commented Jul 19, 2024 • edited by jira bot Loading

dionisio-bot bot commented Jul 19, 2024 • edited Loading

changeset-bot bot commented Jul 19, 2024 • edited Loading

⚠️ No Changeset found

codecov bot commented Jul 19, 2024 • edited Loading

Codecov Report

sampaiodiego Aug 15, 2024

Choose a reason for hiding this comment

sampaiodiego Aug 15, 2024

Choose a reason for hiding this comment

sampaiodiego Aug 15, 2024

Choose a reason for hiding this comment

sampaiodiego Aug 15, 2024

Choose a reason for hiding this comment

ricardogarim commented Jul 19, 2024 •

edited by jira bot

Loading

dionisio-bot bot commented Jul 19, 2024 •

edited

Loading

changeset-bot bot commented Jul 19, 2024 •

edited

Loading

codecov bot commented Jul 19, 2024 •

edited

Loading