Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent broadcasting of system messages when hide setting is enabled #32522

Conversation

AllanPazRibeiro
Copy link
Contributor

@AllanPazRibeiro AllanPazRibeiro commented May 29, 2024

Proposed changes (including videos or screenshots)

This change ensures that system messages are not broadcasted when the setting to hide system messages is enabled. By checking the Hide_System_Messages configuration, the system will prevent unnecessary message broadcasts, improving message handling efficiency. This fix guarantees adherence to user-defined visibility settings for system messages.

Issue(s)

SUP-559

Steps to test or reproduce

  1. Set the Hide_System_Messages setting to include specific message types.
  2. Send a message of one of these types.
  3. Verify that the message is not broadcasted.

Further comments

This update will enhance the efficiency of message handling by preventing the broadcast of system messages when configured to do so. This helps in maintaining cleaner logs and better performance in scenarios where certain system messages are not needed.

@AllanPazRibeiro AllanPazRibeiro added this to the 6.10 milestone May 29, 2024
@AllanPazRibeiro AllanPazRibeiro self-assigned this May 29, 2024
@AllanPazRibeiro AllanPazRibeiro requested a review from a team as a code owner May 29, 2024 17:31
Copy link
Contributor

dionisio-bot bot commented May 29, 2024

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

Copy link

changeset-bot bot commented May 29, 2024

🦋 Changeset detected

Latest commit: ffc4b55

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

This PR includes changesets to release 32 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/gazzodown Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-contexts 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/api-client Patch
@rocket.chat/license Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/ddp-client Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/models Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/instance-status 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

Copy link

codecov bot commented May 29, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 56.44%. Comparing base (94b12ed) to head (ffc4b55).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #32522      +/-   ##
===========================================
+ Coverage    55.70%   56.44%   +0.73%     
===========================================
  Files         2416     2481      +65     
  Lines        53471    54617    +1146     
  Branches     11002    11283     +281     
===========================================
+ Hits         29787    30826    +1039     
- Misses       21058    21108      +50     
- Partials      2626     2683      +57     
Flag Coverage Δ
e2e 56.09% <ø> (+1.32%) ⬆️
unit 71.99% <87.50%> (-0.01%) ⬇️

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

@AllanPazRibeiro AllanPazRibeiro force-pushed the fix/SUP-559/fix-system-message-stream-when-hide-system-message-is-set branch from 1301b0b to 38800b1 Compare May 29, 2024 19:34
.changeset/grumpy-games-greet.md Outdated Show resolved Hide resolved
apps/meteor/server/modules/watchers/lib/messages.ts Outdated Show resolved Hide resolved
apps/meteor/server/modules/watchers/lib/messages.ts Outdated Show resolved Hide resolved
@AllanPazRibeiro AllanPazRibeiro force-pushed the fix/SUP-559/fix-system-message-stream-when-hide-system-message-is-set branch from 10afc76 to 6d51400 Compare June 7, 2024 20:12
@AllanPazRibeiro AllanPazRibeiro force-pushed the fix/SUP-559/fix-system-message-stream-when-hide-system-message-is-set branch from 6d51400 to 71061cd Compare June 7, 2024 20:14
@AllanPazRibeiro AllanPazRibeiro force-pushed the fix/SUP-559/fix-system-message-stream-when-hide-system-message-is-set branch from 79c05e4 to 0402cab Compare June 11, 2024 17:01
@AllanPazRibeiro AllanPazRibeiro force-pushed the fix/SUP-559/fix-system-message-stream-when-hide-system-message-is-set branch from ca7a74f to a322e44 Compare June 12, 2024 16:42
@scuciatto scuciatto added the stat: QA assured Means it has been tested and approved by a company insider label Jun 17, 2024
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Jun 17, 2024
@dionisio-bot dionisio-bot bot removed the stat: ready to merge PR tested and approved waiting for merge label Jun 17, 2024
@scuciatto scuciatto added the stat: ready to merge PR tested and approved waiting for merge label Jun 18, 2024
@kodiakhq kodiakhq bot merged commit faf386b into develop Jun 18, 2024
45 of 47 checks passed
@kodiakhq kodiakhq bot deleted the fix/SUP-559/fix-system-message-stream-when-hide-system-message-is-set branch June 18, 2024 12:30
This was referenced Jun 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants