fix: Notifications for thread messages are sent even after thread has been read

[matheusbsilva137]

Proposed changes (including videos or screenshots)

• Clear notification queue after they thread messages have been read by a user;

Issue(s)

Steps to test or reproduce

Start a thread with two or more users, and keep it open for both of them while no new message is sent in the main room.
Send a thread message with user A, then read it with user B (by keeping the thread open or opening the thread after the message is sent).
Current behavior: a push notification is sent after some minutes;
Expected behavior: no push notifications should be sent since the messages have been read (the queue can be checked in the rocketchat_notification_queue db collection). Notifications should be cleared when using getThreadMessages or readThreads methods.

Further comments

SUP-386

[codecov]

Codecov Report

Merging #31181 (a5f7273) into develop (1bcb57c) will decrease coverage by 28.51%.
The diff coverage is n/a.

Additional details and impacted files

@@             Coverage Diff              @@
##           develop   #31181       +/-   ##
============================================
- Coverage    77.38%   48.87%   -28.51%     
============================================
  Files          276     3270     +2994     
  Lines         8715    80304    +71589     
  Branches      1607    16378    +14771     
============================================
+ Hits          6744    39248    +32504     
- Misses        1565    36405    +34840     
- Partials       406     4651     +4245     

Flag	Coverage Δ
e2e	51.32% <ø> (∅)
e2e-api	40.67% <ø> (?)
unit	77.38% <ø> (ø)

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

[changeset-bot]

🦋 Changeset detected

Latest commit: a5f7273

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages

Name	Type
@rocket.chat/meteor	Patch
@rocket.chat/model-typings	Patch
@rocket.chat/models	Patch
@rocket.chat/account-service	Patch
@rocket.chat/authorization-service	Patch
@rocket.chat/ddp-streamer	Patch
@rocket.chat/omnichannel-transcript	Patch
@rocket.chat/presence-service	Patch
@rocket.chat/queue-worker	Patch
@rocket.chat/stream-hub-service	Patch
@rocket.chat/omnichannel-services	Patch
rocketchat-services	Patch
@rocket.chat/core-services	Patch
@rocket.chat/cron	Patch
@rocket.chat/instance-status	Patch
@rocket.chat/presence	Patch
@rocket.chat/core-typings	Patch
@rocket.chat/rest-typings	Patch
@rocket.chat/gazzodown	Patch
@rocket.chat/livechat	Patch
@rocket.chat/ui-contexts	Patch
@rocket.chat/api-client	Patch
@rocket.chat/license	Patch
@rocket.chat/pdf-worker	Patch
@rocket.chat/ddp-client	Patch
@rocket.chat/fuselage-ui-kit	Patch
@rocket.chat/ui-client	Patch
@rocket.chat/ui-video-conf	Patch
@rocket.chat/uikit-playground	Patch
@rocket.chat/web-ui-registration	Patch

Not sure what this means? Click here to learn what changesets are.

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

[sampaiodiego]

I think the actual solution should be calling await NotificationQueue.clearQueueByUserId(uid); from readThread function:

Rocket.Chat/apps/meteor/app/threads/server/functions.ts

Line 61 in b82de9b

export const readThread = async ({ userId, rid, tmid }: { userId?: string; rid: string; tmid: string }) => {

the way it works we just clear the whole queue if the user is active in the system.. we don't actually care of what is in the queue

[matheusbsilva137]

I think the actual solution should be calling await NotificationQueue.clearQueueByUserId(uid); from readThread function

Ohhh agreed 🤔 that's by the definition of how our notifications should work 😬 not checking "scope"
Thanks!

[sampaiodiego]

I think the actual solution should be calling await NotificationQueue.clearQueueByUserId(uid); from readThread function

Ohhh agreed 🤔 that's by the definition of how our notifications should work 😬 not checking "scope" Thanks!

yes, exactly.. if the user has made any action within the application we just wipe out the notification queue..

the idea is that since the user is active using the application we don't need to bother him by sending push or email notifications.

[MarcosSpessatto]

I think we must do the same here, don't we? https://github.com/RocketChat/Rocket.Chat/blob/develop/apps/meteor/ee/server/local-services/message-reads/service.ts#L11

Ideally we should unify everything in a single place, but we can do that in another PR.

As we discussed, this won't be needed since the MessageReads service is only used in the callbacks (after the notification queue has been cleared). Thanks for the review!