[WEB-55] Daily view site change events - #651
Conversation
- EventTooltip: add a site-change hover case rendering "Site Change: <label>" with a days-since duration, per Figma. - EventsInfoTooltip: list site changes in the subtitle. - DailyPrintView: render site-change icons in the printed Daily view, resolving the image per subtype/manufacturer and deduplicating events within a 5-minute window; export getSiteChangeSource from the package index. - Tests: migrate EventTooltip tests to RTL queries, add an EventsInfoTooltip test, and cover print-view site-change rendering.
- getLegendItems: add a "Site Change" entry after Notes, shown only when site changes actually render in the report (selected source has a resolvable icon and matching deviceEvents exist). - Render it with the same subtype/manufacturer icon used in the report, at the event-icon legend size, with a 2-line "Site"/"Change" label. - Test the item's presence when site changes exist and absence when they don't.
There was a problem hiding this comment.
Pull request overview
Adds support for “site change” events in the Daily view, ensuring they appear consistently across the PDF daily print output and UI tooltips, with accompanying test coverage.
Changes:
- Add site-change legend entry and icon rendering (with subtype selection + 5-minute dedupe) to the DailyPrintView PDF output.
- Extend daily event tooltips to render site-change titles and “days since” duration, plus add/update related tests.
- Expose
getSiteChangeSourcevia the publicutils.aggregationexport and bump package version.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/modules/print/DailyPrintView.test.js | Adds tests for site-change legend inclusion and PDF icon rendering/deduping. |
| test/components/daily/EventTooltip.test.js | Refactors assertions to Testing Library screen and adds site-change tooltip test cases. |
| test/components/common/tooltip/EventsInfoTooltip.test.js | Adds coverage for updated “Events Shown” tooltip copy including site changes. |
| src/modules/print/DailyPrintView.js | Implements site-change subtype detection, image selection, legend item, and rendering in PDF. |
| src/index.js | Re-exports getSiteChangeSource under utils.aggregation. |
| src/components/daily/eventtooltip/EventTooltip.js | Adds tooltip content handling for site-change events (title + duration). |
| src/components/common/tooltips/EventsInfoTooltip.js | Updates tooltip subtitle copy to include site changes. |
| package.json | Bumps package version for the WEB-55 change set. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| title: t('Events Shown'), | ||
| subtitle: t('Tidepool displays certain pump alarms, device time changes, and notes.'), | ||
| subtitle: t('Tidepool displays certain pump alarms, device time changes, site changes and notes.'), | ||
| sections: [ |
✅ Action performedReview finished.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds site change event handling to daily tooltips and print views, including localized duration content, subtype- and manufacturer-specific icons, conditional legends, five-minute deduplication, public source-helper exposure, and related tests. ChangesSite change events
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant DailyPrintView
participant getSiteChangeSource
participant DeviceEventRecords
participant renderDeviceEvents
DailyPrintView->>getSiteChangeSource: resolve patient and manufacturer source
DailyPrintView->>DeviceEventRecords: find matching site change subtype
DailyPrintView->>renderDeviceEvents: render deduplicated matching events
renderDeviceEvents->>DailyPrintView: draw timestamp markers and selected icons
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/components/common/tooltip/EventsInfoTooltip.test.js (1)
1-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMirror the source directory structure.
Move this test to
test/components/common/tooltips/EventsInfoTooltip.test.jsso its path matchessrc/components/common/tooltips/EventsInfoTooltip.js.As per coding guidelines, “Tests must mirror the
src/structure and use the.test.jssuffix.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/components/common/tooltip/EventsInfoTooltip.test.js` around lines 1 - 4, Move EventsInfoTooltip.test.js from the common/tooltip directory into common/tooltips so its location mirrors the EventsInfoTooltip source module under src/components/common/tooltips, preserving the .test.js suffix and existing test contents.Source: Coding guidelines
src/modules/print/DailyPrintView.js (1)
116-143: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse arrow functions for the new helper utilities.
Convert the three new helper declarations to
constarrow functions to follow the project’s ES6 convention.Proposed refactor
-function getSiteChangeSubType(d) { +const getSiteChangeSubType = (d) => { // ... -} +}; -function getSiteChangeImage(subType, manufacturer) { +const getSiteChangeImage = (subType, manufacturer) => { // ... -} +}; -function dedupeSiteChangesWithinWindow(siteChanges) { +const dedupeSiteChangesWithinWindow = (siteChanges) => { // ... -} +};As per coding guidelines, “Use ES6 constructs such as
const/let, arrow functions, and destructuring.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modules/print/DailyPrintView.js` around lines 116 - 143, Convert the new helper declarations getSiteChangeSubType, getSiteChangeImage, and dedupeSiteChangesWithinWindow from function declarations to const-assigned arrow functions, preserving their existing parameters, logic, and return behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/modules/print/DailyPrintView.js`:
- Around line 116-143: Convert the new helper declarations getSiteChangeSubType,
getSiteChangeImage, and dedupeSiteChangesWithinWindow from function declarations
to const-assigned arrow functions, preserving their existing parameters, logic,
and return behavior.
In `@test/components/common/tooltip/EventsInfoTooltip.test.js`:
- Around line 1-4: Move EventsInfoTooltip.test.js from the common/tooltip
directory into common/tooltips so its location mirrors the EventsInfoTooltip
source module under src/components/common/tooltips, preserving the .test.js
suffix and existing test contents.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b27b7c9-5db3-47b2-9152-07bd51681aeb
📒 Files selected for processing (8)
package.jsonsrc/components/common/tooltips/EventsInfoTooltip.jssrc/components/daily/eventtooltip/EventTooltip.jssrc/index.jssrc/modules/print/DailyPrintView.jstest/components/common/tooltip/EventsInfoTooltip.test.jstest/components/daily/EventTooltip.test.jstest/modules/print/DailyPrintView.test.js
WEB-55
Related PRs:
tidepool-org/blip#1965
tidepool-org/tideline#553