fix: Real-time monitoring displaying incorrect data#37103
fix: Real-time monitoring displaying incorrect data#37103kodiakhq[bot] merged 7 commits intodevelopfrom
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: f143158 The changes in this PR will be included in the next version bump. This PR includes changesets to release 42 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 |
WalkthroughAdds a lifecycle-managed Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Comp as Chart Component
participant UCC as useChartContext
participant Init as init(canvas, prevChart, t)
participant Chart as Chart Instance
participant UUD as useUpdateChartData
participant API as Data Query
Comp->>UCC: request chart ({canvas, init, t})
UCC->>Init: if canvas present, call init(prevChart, t)
Init-->>UCC: Chart instance
UCC-->>Comp: return Chart | undefined
Comp->>API: fetch data (gcTime: 0)
API-->>Comp: { data, isSuccess }
alt context available and isSuccess
Comp->>UUD: updateChartData(chart, data)
UUD->>Chart: mutate datasets/labels & update()
else data empty with context
Comp->>Chart: resetChart(chart)
else
Note over Comp: wait for context and/or successful query
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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 @@
## develop #37103 +/- ##
===========================================
+ Coverage 67.41% 67.45% +0.03%
===========================================
Files 3289 3289
Lines 111887 111887
Branches 20435 20447 +12
===========================================
+ Hits 75429 75469 +40
+ Misses 33771 33734 -37
+ Partials 2687 2684 -3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx (1)
17-23: Dataset init mismatch: 3 series but only 2 data arrays.drawLineChart is given 3 series labels ([Open, Closed, On_Hold_Chats]) but only [[], []] for dataset values. This can break rendering or drop a series.
Apply this fix:
- 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/ResponseTimesChart.tsx (1)
70-71: gcTime: 0 added correctly.Same rationale as other charts; no further action.
🧹 Nitpick comments (8)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatDurationChart.tsx (1)
84-94: Consider combining early return guards.The two separate early returns for
!contextand!isSuccesscould be combined into a single check for clarity.- if (!context) { - return; - } - - if (!isSuccess) { + if (!context || !isSuccess) { return; }apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/AgentStatusChart.tsx (2)
51-56: Migration to useChartContext/useUpdateChartData looks solid.This removes the init race and ensures updates run only after context exists. One improvement: destroy the Chart instance on unmount/canvas change to avoid leaks.
Apply in useChartContext.tsx:
@@ export const useChartContext = <TChartType extends ChartType>({ key, canvas, init, t }: UseChartContextProps<Chart<TChartType>>) => { const contextRef = useRef<Chart<TChartType>>(); + useEffect(() => { + return () => { + contextRef.current?.destroy?.(); + contextRef.current = undefined; + }; + }, [canvas]);Also applies to: 58-63
65-78: Verify label-to-datapoint mapping; optionally batch updates.You update in order Offline, Available, Away, Busy while the init labels are [Available, Away, Busy, Offline]. If updateChart matches by label text, order is fine; if by index, values will misalign. Consider batching to avoid 4 redraws.
Example (if index-based or to batch):
- updateChartData(t('Offline'), [offline]); - updateChartData(t('Available'), [available]); - updateChartData(t('Away'), [away]); - updateChartData(t('Busy'), [busy]); + // If batching is supported by updateChart/updateChartData: + // updateChartData(t('Agents'), [available, away, busy, offline]); + // Otherwise, at least match init order: + updateChartData(t('Available'), [available]); + updateChartData(t('Away'), [away]); + updateChartData(t('Busy'), [busy]); + updateChartData(t('Offline'), [offline]);apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ResponseTimesChart.tsx (1)
87-99: Minor: ensure label exists in X-axis set; consider batching.getMomentCurrentLabel should resolve to one of labels; if not, updates will append a new tick. If supported, batch the 4-series update to one chart.update.
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsChart.tsx (1)
68-81: Verify label mapping; align order or batch.Init labels: [Open, Queued, On_Hold_Chats, Closed]. Updates run Open, Closed, On_Hold_Chats, Queued. If updateChart matches by label, OK; if index-based, this mismatches. Prefer batching to reduce redraws.
- updateChartData(t('Open'), [open]); - updateChartData(t('Closed'), [closed]); - updateChartData(t('On_Hold_Chats'), [onhold]); - updateChartData(t('Queued'), [queued]); + updateChartData(t('Open'), [open]); + updateChartData(t('Queued'), [queued]); + updateChartData(t('On_Hold_Chats'), [onhold]); + updateChartData(t('Closed'), [closed]);apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx (2)
42-47: Context-based init/update path LGTM; consider Chart destroy on unmount.Adoption is correct. Add destroy in useChartContext to prevent leaks, as suggested in related comments.
Also applies to: 49-54
56-72: Reset and ordering strategy for full refresh.If agents change across queries, updating without a reset can leave stale labels. Reset before repopulating and use stable ordering to reduce jitter.
- if (data.length === 0) { - resetChart(context); - return; - } - - data.forEach(([name, value]) => { - updateChartData(name, [value.open, value.closed, value.onhold]); - }); + resetChart(context); + for (const [name, value] of [...data].sort((a, b) => a[0].localeCompare(b[0]))) { + updateChartData(name, [value.open, value.closed, value.onhold]); + }apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerDepartmentChart.tsx (1)
56-73: Optional: Reset chart on query error
DestructureisErrorfrom theuseQueryresult and insert before the!isSuccessguard:- if (!isSuccess) { + if (isError) { + resetChart(context); + return; + } + + if (!isSuccess) { return; }This clears stale data on failure. The existing
data.forEach(([name, value]) => updateChartData(name, [value.open, value.closed]));correctly handles the[departmentName, { open, closed }]tuples returned by yourselectmapping.
📜 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 (7)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/AgentStatusChart.tsx (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(13-35)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
useUpdateChartData(15-32)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerDepartmentChart.tsx (3)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(13-35)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/ResponseTimesChart.tsx (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(13-35)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/ChatDurationChart.tsx (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(13-35)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
useUpdateChartData(15-32)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx (3)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(13-35)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(13-35)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). (6)
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 📦 Meteor Build - coverage
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (18)
.changeset/rare-schools-laugh.md (1)
1-6: LGTM!The changeset correctly documents a patch-level fix for the real-time monitoring data issue.
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (2)
8-13: API simplified from ref to direct context value.The change from
MutableRefObject<TChart | undefined>toTChart | undefinedremoves unnecessary indirection and aligns with the newuseChartContexthook pattern. This is a breaking change but is intentional and correct given the architectural refactor.
21-31: Fallback initialization logic is correct.The logic correctly falls back to
initwhen no context is provided, ensuring the chart is initialized before attempting updates. This maintains compatibility during the transition to context-based initialization.apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (2)
27-28: LGTM on cache configuration.
staleTime: InfinitywithgcTime: 0is the correct combination: data never becomes stale while the query is active, but cache is cleared immediately on unmount, preventing stale data issues identified in the PR objectives.
16-32: Ignore unnecessary refetch warning: canvas is created via useRef and passed as canvas.current, ensuring a stable reference and no extra reinitialization.Likely an incorrect or invalid review comment.
apps/meteor/app/livechat/client/lib/chartHandler.ts (1)
213-220: LGTM!The
resetChartutility correctly clears all labels and dataset data before triggering a chart update. Implementation is clean and follows the same pattern asupdateChart.apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatDurationChart.tsx (2)
57-58: LGTM on gcTime configuration.Setting
gcTime: 0correctly prevents stale cached data when returning to the page, addressing the root cause identified in the PR objectives.
60-72: Chart context integration is correct.The component correctly adopts the new
useChartContextpattern, passing the context directly touseUpdateChartDatainstead of using a ref. This eliminates the race condition described in the PR objectives.apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ResponseTimesChart.tsx (1)
73-78: Context-driven init/update path LGTM.Guarding on context removes the prior race. No issues spotted.
Also applies to: 80-85
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsChart.tsx (2)
49-50: gcTime: 0 to prevent stale cache on return.Matches PR goal; good.
52-58: Adopting useChartContext/useUpdateChartData removes the init/update race.Looks correct; no blocking issues.
Also applies to: 59-64
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/AgentStatusChart.tsx (1)
48-49: Use of gcTime is correct
The project depends on “@tanstack/react-query” v5.65.1, so gcTime is supported and should remain to prevent stale caches.apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx (1)
38-40: Remove unnecessarysuccessdestructuring inselect. The endpoint returns a typedRecord<string, { open: number; closed: number; onhold: number }>, so replaceselect: ({ success: _, ...data }) => Object.entries(data),with
select: metrics => Object.entries(metrics),and, if desired, add generics to
useQueryfor full type safety:useQuery<Record<string, AgentMetrics>, Error, [string, AgentMetrics][]>Likely an incorrect or invalid review comment.
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerDepartmentChart.tsx (5)
12-14: LGTM! Imports align with the new chart context architecture.The addition of
useChartContextandresetChartsupports the PR's goal of eliminating race conditions in chart initialization and providing proper cleanup when data is empty.
73-73: Dependency array is comprehensive.All dependencies are correctly included:
context,data,isSuccessare directly used in the effectupdateChartDatais stable (fromuseEffectEvent) but including it is harmlesstis included transitively throughupdateChartDataand ensures the effect re-runs if translations changeThe exhaustive dependency list prevents stale closures and aligns with React best practices.
42-47: Ignore the canvas ref reactivity concern;useChartContext’senabled: !!canvasguard plus the parent data query-triggered re-render ensure the chart is initialized oncecanvas.currentis available.Likely an incorrect or invalid review comment.
49-54: No race condition detected. AllupdateChartDatainvocations in the real-time-monitoring charts are wrapped inif (!context) return;(andif (!isSuccess) return;) guards, so the fallback initialization path inuseUpdateChartDatawill never run prematurely.
38-39: Verify API response structure for select transformation.The
selectfunction destructuressuccessand converts remaining properties to entries. Ensure the API response from/v1/livechat/analytics/dashboards/charts/chats-per-departmentreturns an object with asuccessfield and department data as top-level properties.The
gcTime: 0configuration correctly prevents stale data after unmount, as per the PR objectives.Run the following script to verify the API response structure:
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx
Outdated
Show resolved
Hide resolved
aleksandernsilva
force-pushed
the
fix/real-time-monitoring
branch
from
15c0b00 to
a99f754
Compare
|
Is it possible to add tests? API ones? |
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx
Outdated
Show resolved
Hide resolved
aleksandernsilva
force-pushed
the
fix/real-time-monitoring
branch
from
a99f754 to
3549835
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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)
🚧 Files skipped from review as they are similar to previous changes (3)
- .changeset/rare-schools-laugh.md
- apps/meteor/app/livechat/client/lib/chartHandler.ts
- apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsChart.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ResponseTimesChart.tsx (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(11-35)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
useUpdateChartData(15-32)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx (3)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(11-35)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/ChatsPerDepartmentChart.tsx (3)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(11-35)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/useUpdateChartData.ts (1)
apps/meteor/app/livechat/client/lib/chartHandler.ts (1)
updateChart(187-211)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/AgentStatusChart.tsx (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(11-35)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
useUpdateChartData(15-32)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatDurationChart.tsx (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(11-35)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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx (1)
38-39: Ignore thedepartmentschange: thechats-per-agentendpoint returns{ success: true, [agentId]: { open, closed, onhold } }, soselect: ({ success: _, ...data }) => Object.entries(data)correctly extracts each agent’s metrics.
Likely an incorrect or invalid review comment.
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerDepartmentChart.tsx (1)
38-39: Keepselectas{ success: _, ...data }
The endpoint returns the metrics map at the top level (e.g.{ success: true, Sales: {…}, Support: {…} }), not nested underdepartments, so the currentselectcorrectly extracts each department’s data.Likely an incorrect or invalid review comment.
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx
Show resolved
Hide resolved
aleksandernsilva
force-pushed
the
fix/real-time-monitoring
branch
from
3549835 to
afacca7
Compare
aleksandernsilva
force-pushed
the
fix/real-time-monitoring
branch
from
afacca7 to
006f4f0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
33-33: Optional: Consider impact of unmemoized init function.The effect includes
initin its dependency array. If callers don't memoize this function (e.g., withuseCallback), the effect will re-run on every parent render, destroying and recreating the chart unnecessarily. Consider documenting this requirement or adding a ref-based approach to avoid dependency oninit.
📜 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 (7)
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)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ResponseTimesChart.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatDurationChart.tsx (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(12-36)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
useUpdateChartData(15-32)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx (3)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(12-36)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-36)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
useUpdateChartData(15-32)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerDepartmentChart.tsx (4)
packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (1)
context(110-120)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(12-36)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/AgentStatusChart.tsx (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
useChartContext(12-36)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useUpdateChartData.ts (1)
useUpdateChartData(15-32)
🔇 Additional comments (2)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatDurationChart.tsx (1)
55-58: ConfirmgcTimeis supported by our React Query version.
gcTimeis only available in TanStack Query v5. If we are still on v4, this option will be ignored (and TS will complain), so please double-check the package version before we rely on this change; otherwise keep usingcacheTimeor bundle the upgrade here.apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx (1)
23-23: Verify: Always passing undefined context prevents chart reuse.The hook always passes
undefinedas the context parameter toinit(line 23), which means the chart is recreated from scratch on every effect run. Chart.js best practice is to reuse instances viachart.update()rather than destroying and recreating. If theinitfunction is designed to handle updates when a chart instance is provided, consider passing the existing context.Based on learnings about Chart.js: "Reuse chart instances (update() instead of re-creating) for performance."
Is the destroy-and-recreate pattern intentional for this use case, or should the hook attempt to update existing charts when dependencies change?
If updates are preferred, apply this diff:
- chart = await init(canvas.current, undefined, t); + chart = await init(canvas.current, context, t);And add
contextto the effect dependencies on line 33.
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx
Show resolved
Hide resolved
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx
Show resolved
Hide resolved
aleksandernsilva
force-pushed
the
fix/real-time-monitoring
branch
from
006f4f0 to
f54227c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (7)
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)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerDepartmentChart.tsx
- apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/AgentStatusChart.tsx
- apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatDurationChart.tsx
- apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/useChartContext.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
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/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/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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (5)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ResponseTimesChart.tsx (5)
14-14: LGTM: Import addition supports the refactoring.The
useChartContextimport is correctly added to support the new centralized chart initialization pattern.
70-70: LGTM: Cache configuration ensures data freshness.Setting
gcTime: 0prevents stale cached data when returning to the real-time monitoring page. This is an appropriate tradeoff for a real-time monitoring dashboard where data freshness is critical, though it will trigger a fresh API call on each mount.
73-77: LGTM: Centralized chart initialization resolves race condition.Using
useChartContextreplaces the previous manual context management and fixes the race condition where charts were initialized in multiple places. The hook properly manages the chart lifecycle and ensures initialization completes before updates are applied.
79-84: LGTM: Chart updates now use initialized context.Passing
contexttouseUpdateChartDataensures that chart updates only occur on the properly initialized chart instance, preventing the race condition where updates might target the wrong chart.
86-97: LGTM: Update logic correctly guards against uninitialized state.The early returns ensure the chart is initialized (
contextis defined) and data is available (isSuccess) before attempting updates. Addingcontextto the dependency array ensures the effect runs when the chart becomes available, properly handling the asynchronous initialization flow.
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx
Show resolved
Hide resolved
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
dougfabris
removed
the
stat: QA assured
dionisio-bot
bot
removed
the
stat: ready to merge
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 (1)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx (1)
17-23: Dataset count mismatch in drawLineChart (3 labels but only 2 datasets).This will misalign series with labels and can break initialization.
Apply this diff:
- 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/ChatsPerAgentChart.tsx (1)
35-40: Fix data selection: unwrap the endpoint payload instead of Object.entries over the whole response.Current selector yields an “agents” meta entry and bogus rows. Explicitly unwrap the agents map.
Apply this diff:
- select: ({ success: _, ...data }) => Object.entries(data), + // API returns { success, agents: Record<string, { open: number; closed: number; onHold?: number; onhold?: number }> } + select: ({ success: _, agents }) => Object.entries(agents ?? {}),
🧹 Nitpick comments (1)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx (1)
62-67: Guard metric fields and casing (onHold vs onhold).Avoid runtime breaks if the server uses different casing or sparse values.
Apply this diff:
- data.forEach(([name, value]) => { - updateChartData(name, [value.open, value.closed, value.onhold]); - }); + data.forEach(([name, value]) => { + const open = value?.open ?? 0; + const closed = value?.closed ?? 0; + const onHold = (value?.onHold ?? value?.onhold) ?? 0; + updateChartData(name, [open, closed, onHold]); + });Please confirm the exact field name returned by
/v1/livechat/analytics/dashboards/charts/chats-per-agentfor “on hold” (onHold vs onhold).
📜 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)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx(2 hunks)apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerDepartmentChart.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
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/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)
⏰ 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). (2)
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (3)
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerAgentChart.tsx (1)
42-53: Lifecycle improvements via useChartContext and resetChart look good.Also applies to: 56-63
apps/meteor/client/views/omnichannel/realTimeMonitoring/charts/ChatsPerDepartmentChart.tsx (2)
42-53: Context-driven init and reset flow is solid.Also applies to: 56-69
35-40: Incorrect suggestion: nodepartmentswrapper
The/chats-per-departmentendpoint returns its metrics as top-level keys (after strippingsuccess), so your selector’s({ success: _, ...data }) => Object.entries(data)already unwraps the payload correctly—there is no nesteddepartmentsproperty.Likely an incorrect or invalid review comment.
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
|
/patch |
|
Pull request #37236 added to Project: "Patch 7.10.2" |
|
/backport 7.9.5 |
|
Pull request #37237 added to Project: "Patch 7.9.5" |
5 participants
Proposed changes (including videos or screenshots)
This PR fixes an issue where real-time monitoring charts were not being initialized correctly and failed to update reactively when filters changed.
Root causes:
useEffectand byupdateChartData, creating a race condition that prevented the correct chart instance from receiving data updates.Solution:
gcTime: 0, ensuring no data is cached after unmount and preventing stale results when returning to the page.useChartContexthook to manage chart initialization. Unlike the previousMutableRefapproach, this context is consumed by the data-update effect, ensuring updates only run after the chart is fully initialized.Issue(s)
SUP-874
Steps to test or reproduce
Further comments
Introduced here: #36468
Summary by CodeRabbit