fix: User mute and unmute system messages displayed when hidden - #41751
fix: User mute and unmute system messages displayed when hidden#41751jessicaschelly wants to merge 1 commit into
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: f1aecce The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
apps/meteor/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.spec.ts📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (9)📚 Learning: 2026-02-10T16:32:42.586ZApplied to files:
📚 Learning: 2026-05-11T20:30:35.265ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-05-06T12:21:44.083ZApplied to files:
📚 Learning: 2026-03-16T21:50:37.589ZApplied to files:
📚 Learning: 2026-08-05T22:02:59.828ZApplied to files:
📚 Learning: 2026-02-24T19:22:48.358ZApplied to files:
📚 Learning: 2026-03-06T18:10:15.268ZApplied to files:
🔇 Additional comments (4)
WalkthroughThe change normalizes ChangesSystem message visibility
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change normalizes mute/unmute system-message filtering for hidden-message settings and adds regression coverage; no actionable merge-blocking risk remains beyond normal checks and review. Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #41751 +/- ##
========================================
Coverage 68.69% 68.70%
========================================
Files 4166 4167 +1
Lines 159382 159437 +55
Branches 28219 28258 +39
========================================
+ Hits 109493 109538 +45
- Misses 44730 44734 +4
- Partials 5159 5165 +6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/client/views/room/MessageList/hooks/useMessages.ts">
<violation number="1" location="apps/meteor/client/views/room/MessageList/hooks/useMessages.ts:18">
P3: The mute/unmute grouped-type expansion (`mute_unmute` → `['user-muted', 'user-unmuted']`) is now written twice in this PR (here in `mergeHideSysMessages` and identically in `getHiddenSystemMessages`), even though `server/lib/systemMessage/hideSystemMessage.ts` already centralizes this logic with `isMutedUnmuted`. Extract one shared helper (e.g. `normalizeHiddenSystemMessages`) and reuse it on both client and server so the mapping can't drift when more grouped types are added.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| return Array.from( | ||
| new Set( | ||
| [...sysMesArray1, ...sysMesArray2].flatMap((messageType): MessageTypesValues[] => | ||
| messageType === 'mute_unmute' ? ['user-muted', 'user-unmuted'] : [messageType], |
There was a problem hiding this comment.
P3: The mute/unmute grouped-type expansion (mute_unmute → ['user-muted', 'user-unmuted']) is now written twice in this PR (here in mergeHideSysMessages and identically in getHiddenSystemMessages), even though server/lib/systemMessage/hideSystemMessage.ts already centralizes this logic with isMutedUnmuted. Extract one shared helper (e.g. normalizeHiddenSystemMessages) and reuse it on both client and server so the mapping can't drift when more grouped types are added.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/views/room/MessageList/hooks/useMessages.ts, line 18:
<comment>The mute/unmute grouped-type expansion (`mute_unmute` → `['user-muted', 'user-unmuted']`) is now written twice in this PR (here in `mergeHideSysMessages` and identically in `getHiddenSystemMessages`), even though `server/lib/systemMessage/hideSystemMessage.ts` already centralizes this logic with `isMutedUnmuted`. Extract one shared helper (e.g. `normalizeHiddenSystemMessages`) and reuse it on both client and server so the mapping can't drift when more grouped types are added.</comment>
<file context>
@@ -12,7 +12,13 @@ const mergeHideSysMessages = (
+ return Array.from(
+ new Set(
+ [...sysMesArray1, ...sysMesArray2].flatMap((messageType): MessageTypesValues[] =>
+ messageType === 'mute_unmute' ? ['user-muted', 'user-unmuted'] : [messageType],
+ ),
+ ),
</file context>
Proposed changes (including videos or screenshots)
Fixes user mute and unmute system messages remaining visible in rooms configured to hide them.
The room setting stores the grouped
mute_unmutetype, while mute and unmute events are stored asuser-mutedanduser-unmuted. This change normalizes the grouped type in both client-side filtering and server-loaded message history, and adds regression coverage for both paths.Issue(s)
CORE-2536
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Bug Fixes
Tests