Release 7.12.2 #37519
Release 7.12.2 #37519
Conversation
🦋 Changeset detectedLatest commit: 424f3a4 The changes in this PR will be included in the next version bump. 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 |
|
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 |
WalkthroughAdds three changeset files; fixes favicon package to catch and log render/init errors; simplifies extraction of user-agent in device session code; removes a runtime warning from the deprecated /health route; implements per-subscription unread tracking and aggregation in the client unread hook. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App / Service
participant Favicon as packages/favicon/src/index.ts
rect rgba(230,247,255,0.4)
Note right of Favicon: Async initialization (now returns Promise)
App->>Favicon: init()
Favicon->>Favicon: perform init tasks
alt init succeeds
Favicon-->>App: resolved
else init fails
Favicon-->>Favicon: log error
Favicon-->>App: resolved (no throw)
end
end
rect rgba(255,245,230,0.25)
Note right of Favicon: Per-badge update (protected)
App->>Favicon: updateOrCollect(badge)
Favicon->>Favicon: try renderAndUpdate(badge)
alt render succeeds
Favicon-->>App: update applied
else render throws
Favicon-->>Favicon: log error and continue
Favicon-->>App: continue without crash
end
end
sequenceDiagram
participant Client as Client Hook
participant Subs as Subscriptions (list)
rect rgba(230,255,240,0.35)
Client->>Subs: iterate subscriptions
loop per-subscription
Subs-->>Client: unread, alert, unreadAlert
alt values changed vs prevSubsRef
Client->>Client: emit unread-changed-by-subscription
else unchanged
Client-->>Client: no event
end
Client->>Client: accumulate unreadCount and badgeIndicator
end
Client-->>Client: set unread display (number/bullet/empty)
Client-->>Other: trigger favicon update if unread changed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #37519 +/- ##
=======================================
Coverage 66.55% 66.55%
=======================================
Files 3392 3392
Lines 115843 115843
Branches 21393 21394 +1
=======================================
+ Hits 77097 77104 +7
+ Misses 36069 36061 -8
- Partials 2677 2678 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Co-authored-by: Aleksander Nicacio da Silva <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/favicon/src/index.ts (1)
98-102: Good defensive error handling for render failures.The try-catch prevents rendering errors (e.g., cross-origin issues during canvas operations) from crashing the application. Logging to console is appropriate for debugging.
If favicon rendering errors become frequent in production, consider adding telemetry/metrics to track the error rate, which would help identify systemic issues (e.g., misconfigured CORS headers on favicon endpoints).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.changeset/cuddly-toys-pay.md(1 hunks)packages/favicon/src/index.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/cuddly-toys-pay.md
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/keychain.ts:148-156
Timestamp: 2025-10-16T21:09:51.816Z
Learning: In the RocketChat/Rocket.Chat repository, only platforms with native crypto.randomUUID() support are targeted, so fallback implementations for crypto.randomUUID() are not required in E2EE or cryptographic code.
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: 📦 Build Packages
- GitHub Check: update-pr
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
packages/favicon/src/index.ts (1)
111-113: Appropriate error handling for initialization failures.The catch handler prevents unhandled promise rejections while allowing the manager to continue operating in a degraded mode. If initialization fails, the returned function will accumulate badge updates without rendering them—an acceptable trade-off for preventing crashes in a non-critical UI feature.
Co-authored-by: Lucas Pelegrino <[email protected]>
…7518) Co-authored-by: Aaron Ogle <[email protected]>
Co-authored-by: Matheus Cardoso <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/meteor/client/views/root/hooks/loggedIn/useUnread.ts (2)
3-3: Per‑subscription snapshot approach looks solid; consider minor type tighteningThe
UnreadData+prevSubsRefsnapshot pattern is a good way to avoid emitting per‑subscription events when nothing relevant changed, and using aMapkeyed byridkeeps lookups cheap even with many subscriptions.If you want to reduce the risk of future drift, you could consider deriving
UnreadDatafrom the subscription type (e.g.,Pick<ISubscription, 'unread' | 'alert' | 'unreadAlert'>) instead of re‑declaring it withstring | undefined, but this is a nice‑to‑have rather than required.Also applies to: 11-12, 23-28
30-33: Unread aggregation and badge logic seem correct; you might gate some work on value changesThe per‑subscription loop and aggregation look consistent with the intended behavior: you sum
unreadonly for subscriptions that should contribute, and fall back from a numeric total to a single•when there are only “alert without unread” cases (e.g., mentions), which keeps the UI semantics clear.Two small points you may want to consider:
Gating global event / session update (optional):
fireEventUnreadChanged(unreadCount)andsetUnread(...)run on every effect execution, even whenunreadCountand the derived display value are unchanged. If this ends up hot in practice, you could track the previous aggregate in auseRefand only emit/update when the value actually changes, while still keeping the per‑subscription snapshot logic as‑is.Confirm unreadAlert override semantics:
The conditionif (alert === true && subscriptionUnreadAlert !== 'nothing') { if (subscriptionUnreadAlert === 'all' || unreadAlertEnabled !== false) { badgeIndicator = '•'; } }effectively treats any non‑
'nothing'global preference the same (unreadAlertEnabled !== false), and only lets a per‑subscription override of'all'bypass a globalfalse. That’s probably intentional, but it does bake in specific override semantics; worth double‑checking against the previous implementation / product expectations to ensure there’s no behavioral regression for combinations like global “nothing” with per‑room “mentions”.Also applies to: 34-52, 53-53, 55-58, 62-64
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.changeset/quiet-cars-smile.md(1 hunks)apps/meteor/client/views/root/hooks/loggedIn/useUnread.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/quiet-cars-smile.md
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/keychain.ts:148-156
Timestamp: 2025-10-16T21:09:51.816Z
Learning: In the RocketChat/Rocket.Chat repository, only platforms with native crypto.randomUUID() support are targeted, so fallback implementations for crypto.randomUUID() are not required in E2EE or cryptographic code.
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Summary by CodeRabbit
Bug Fixes
Improvements
Chores
You can see below a preview of the release change log:
7.12.2
Engine versions
22.16.01.43.55.0, 6.0, 7.01.57.1Patch Changes
Bump @rocket.chat/meteor version.
(#37540 by @dionisio-bot) Fixes an issue where
user-agentis not properly extracted from the DDP connection headersUpdated dependencies [fadc449]: