Release 7.10.2 #37239
Release 7.10.2 #37239
Conversation
🦋 Changeset detectedLatest commit: 73aeb0c The changes in this PR will be included in the next version bump. This PR includes changesets to release 39 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 |
|
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 |
WalkthroughReplaces local chart refs with a new reusable Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Component
participant H as useChartContext (hook)
participant I as init(chart)
participant U as useUpdateChartData
participant CH as Chart.js instance
C->>H: provide { canvas, init, t }
H->>I: call init(...) (async)
I-->>H: returns CH
H-->>C: expose CH (context)
C->>U: call updateChartData({ context: CH, ... })
U->>CH: update datasets / labels
CH-->>U: update complete
Note right of CH: resetChart may be called to clear before updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 #37239 +/- ##
==========================================
- Coverage 66.21% 66.18% -0.03%
==========================================
Files 3289 3289
Lines 110356 110356
Branches 20911 20904 -7
==========================================
- Hits 73070 73040 -30
- Misses 34603 34634 +31
+ Partials 2683 2682 -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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
21-31: Remove fallback initialization in useUpdateChartData to prevent orphaned charts.The
context ?? (await init(...))in useUpdateChartData (useUpdateChartData.ts) can spin up a new Chart.js instance when data arrives beforeuseChartContexthas finished initializing, and that instance is never cleaned up. Replace the fallback with an early return on missing context:- const chartContext = context ?? (await init(canvas, undefined, t)); + if (!context) { + return; + } + const chartContext = context;apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx (1)
17-23: Fix dataset count mismatch in initThree dataset labels are provided, but only two data arrays are passed. Add a third array to match.
- drawLineChart(canvas, context, [t('Open'), t('Closed'), t('On_Hold_Chats')], [], [[], []], { + drawLineChart(canvas, context, [t('Open'), t('Closed'), t('On_Hold_Chats')], [], [[], [], []], {
♻️ Duplicate comments (1)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsChart.tsx (1)
49-50: gcTime usage note already coveredSee earlier comment about ensuring TanStack Query v5 for
gcTime.
🧹 Nitpick comments (4)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ResponseTimesChart.tsx (1)
86-97: Tighten effect guard and depsCombine guards and drop unused
tfrom deps to avoid extra renders.- if (!context) { - return; - } - - if (!isSuccess) { - return; - } + if (!context || !isSuccess) { + return; + } @@ -}, [context, reactionAvg, reactionLongest, responseAvg, responseLongest, isSuccess, t, updateChartData]); +}, [context, reactionAvg, reactionLongest, responseAvg, responseLongest, isSuccess, updateChartData]);apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsChart.tsx (1)
67-80: Reduce unnecessary effect deps and consider batching updates
tisn't used in the effect body; remove from deps.- Optional: if
updateChartDatatriggers a chart update each call, consider a bulk update path to avoid 4 sequential updates.-}, [context, closed, open, queued, onhold, isSuccess, t, updateChartData]); +}, [context, closed, open, queued, onhold, isSuccess, updateChartData]);apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx (1)
55-67: Effect guards/reset look good; trim depsKeep the guards and
resetChart, but drop unusedtfrom deps.-}, [context, data, isSuccess, t, updateChartData]); +}, [context, data, isSuccess, updateChartData]);apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerDepartmentChart.tsx (1)
55-69: Guards and reset look good; trim deps
tisn't used in the effect body; remove from deps.-}, [context, data, isSuccess, t, updateChartData]); +}, [context, data, isSuccess, updateChartData]);
📜 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 (10)
.changeset/rare-schools-laugh.md(1 hunks)apps/meteor/app/livechat/client/lib/chartHandler.ts(1 hunks)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/AgentStatusChart.tsx(2 hunks)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatDurationChart.tsx(3 hunks)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsChart.tsx(2 hunks)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx(2 hunks)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerDepartmentChart.tsx(2 hunks)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ResponseTimesChart.tsx(2 hunks)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx(1 hunks)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatDurationChart.tsx (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(12-44)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
useUpdateChartData(15-32)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/AgentStatusChart.tsx (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(12-44)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
useUpdateChartData(15-32)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ResponseTimesChart.tsx (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(12-44)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
useUpdateChartData(15-32)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
apps/meteor/app/livechat/client/lib/chartHandler.ts (1)
updateChart(187-211)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx (3)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(12-44)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
useUpdateChartData(15-32)apps/meteor/app/livechat/client/lib/chartHandler.ts (1)
resetChart(213-220)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
apps/meteor/app/utils/lib/i18n.ts (1)
t(6-6)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerDepartmentChart.tsx (3)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(12-44)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
useUpdateChartData(15-32)apps/meteor/app/livechat/client/lib/chartHandler.ts (1)
resetChart(213-220)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsChart.tsx (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(12-44)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
useUpdateChartData(15-32)
⏰ 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). (8)
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 📦 Meteor Build - coverage
- GitHub Check: update-pr
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (16)
.changeset/rare-schools-laugh.md (1)
1-5: LGTM!The changeset correctly documents the patch version bump and describes the fix being delivered.
apps/meteor/app/livechat/client/lib/chartHandler.ts (1)
213-220: LGTM!The
resetChartfunction correctly clears all chart data by resetting labels and dataset arrays, then triggers an update. The generic implementation allows it to work with any Chart.js chart type.apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/AgentStatusChart.tsx (3)
51-62: LGTM!The chart context integration is correctly implemented using the new
useChartContexthook, and the context is properly passed touseUpdateChartData.
65-77: LGTM!The effect correctly guards against updates when the context is unavailable and includes
contextin the dependency array, ensuring the effect re-runs once the chart is initialized.
48-48: Confirm intentional use ofgcTime: 0across all real-time monitoring charts
All six charts under apps/meteor/client/views/omnichannel/realTimeMonitoring/charts disable caching by usinggcTime: 0, forcing fresh data on every mount. If you need real-time accuracy but want to reduce network/server load, consider a short nonzero cache (e.g.gcTime: 1000).apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
12-44: LGTM!The hook correctly manages the chart lifecycle with proper async initialization and cleanup. The
unmountedflag at line 17 prevents race conditions by ensuring chart destruction and preventing state updates if the component unmounts during async initialization.apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatDurationChart.tsx (3)
60-71: LGTM!The chart context integration follows the same correct pattern as
AgentStatusChart, properly usinguseChartContextto manage the chart lifecycle and passing the context touseUpdateChartData.
83-93: LGTM!The effect correctly guards against updates when the context is unavailable and includes
contextin the dependency array, ensuring proper synchronization with the chart lifecycle.
57-57: Verify the need for disabling query caching.Similar to
AgentStatusChart, settinggcTime: 0disables query caching. Ensure this is intentional and consistent with real-time monitoring requirements across all chart components.apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ResponseTimesChart.tsx (2)
73-77: Context-based chart lifecycle: LGTMGood migration to
useChartContextand passingcontextintouseUpdateChartData.Also applies to: 79-85
70-70:gcTimeis correct with TanStack Query v5
Project uses@tanstack/[email protected], sogcTimeis a valid option; no update required.apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsChart.tsx (1)
52-56: Hook migration: LGTMCorrect use of
useChartContextand passingcontextintouseUpdateChartData.Also applies to: 58-63
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx (2)
42-46: Hook integration: LGTMGood use of
useChartContextand passingcontextintouseUpdateChartData.Also applies to: 48-53
38-40: Confirmselectsafely strips onlysuccessand yields agent entries
The endpoint returns a plain object mapping agent IDs to their{ open, closed, onhold? }metrics (no extra metadata keys), so usingselect: ({ success: _, ...data }) => Object.entries(data)correctly removes only
success.
(Note: runtime shape omitsonholdwhen zero, despite TS typing.)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerDepartmentChart.tsx (2)
12-15: Reset integration: LGTMImporting and using
resetChartaligns with the new flow.
42-46: Hook integration: LGTMCorrect use of
useChartContextanduseUpdateChartDatawith context.Also applies to: 48-53
Summary by CodeRabbit
Bug Fixes
Improvements
Chores
You can see below a preview of the release change log:
7.10.2
Engine versions
22.16.01.43.55.0, 6.0, 7.01.55.1Patch Changes
Bump @rocket.chat/meteor version.
(#37236 by @dionisio-bot) Fixes real-time monitoring displaying incorrect data
Updated dependencies []: