WEB-4654 - Data Recency - #1986
Open
henry-tp wants to merge 22 commits into
Open
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (4)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughClinic patient filtering was split into dedicated dropdown components, applied-filter chips, clear/reset controls, and combined query-state handling. Clinic-admin actions, special empty-selection states, localized plural labels, and header integration were added. ChangesClinic patient filtering
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ClinicPatients
participant FilterByDataRecency
participant DataRecencyFilterDropdown
participant AppliedFiltersAdapter
participant AppliedFilters
ClinicPatients->>FilterByDataRecency: pass activeFilters
FilterByDataRecency->>DataRecencyFilterDropdown: pass current recency values
DataRecencyFilterDropdown->>FilterByDataRecency: apply recency selection
FilterByDataRecency->>ClinicPatients: update activeFilters
ClinicPatients->>AppliedFiltersAdapter: pass activeFilters and search text
AppliedFiltersAdapter->>AppliedFilters: render applied filter chips
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/pages/clinicworkspace/ClinicPatients.js (1)
3747-3760: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winComplete
renderPeopleTable’s dependency list. It readspatientListSearchTextInputand captureshandleClearSearch,handleResetFilters, andhandleClickPatient, but none are in theuseCallbackdeps. That can leave the query-state controls using stale values after search/filter changes.🤖 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 `@app/pages/clinicworkspace/ClinicPatients.js` around lines 3747 - 3760, Update the dependency array for renderPeopleTable to include patientListSearchTextInput, handleClearSearch, handleResetFilters, and handleClickPatient alongside the existing dependencies, ensuring the callback refreshes when search, filter-reset, or patient-click handlers change.Source: Coding guidelines
🧹 Nitpick comments (3)
app/pages/clinicworkspace/AppliedFiltersAdapter.js (1)
10-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd PropTypes for the newly introduced component interfaces.
app/pages/clinicworkspace/AppliedFiltersAdapter.js#L10-L10: declareactiveFilters,setActiveFilters, andrightContent.app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L25-L30: declareDropdownContentprops.app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L135-L139: declareDataRecencyFilterDropdownprops.app/pages/clinicworkspace/FilterByDataRecency.js#L6-L9: declare the filter adapter props.app/pages/clinicworkspace/components/AppliedFilters.js#L104-L171: declare props forChip,ChipGroup, andAppliedFilters.app/pages/clinicworkspace/components/ClearFilterButtons.js#L25-L25: declare query-state and callback props.app/pages/clinicworkspace/ClinicPatients.js#L243-L243: declareEmptyContentNodeprops.As per coding guidelines, define PropTypes for all component props.
🤖 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 `@app/pages/clinicworkspace/AppliedFiltersAdapter.js` at line 10, The newly introduced clinic workspace components lack PropTypes declarations for their interfaces. Add PropTypes for activeFilters, setActiveFilters, and rightContent on AppliedFiltersAdapter; DropdownContent and DataRecencyFilterDropdown props in DataRecencyFilterDropdown.js; filter adapter props in FilterByDataRecency; Chip, ChipGroup, and AppliedFilters props in AppliedFilters.js; query-state and callback props in ClearFilterButtons.js; and EmptyContentNode props in ClinicPatients.js, matching each component’s actual usage.Source: Coding guidelines
app/pages/clinicworkspace/components/TagFilterDropdown.js (1)
190-194: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefine PropTypes for every component introduced here.
app/pages/clinicworkspace/components/TagFilterDropdown.js#L190-L194: define contracts forTagFilterDropdown,DropdownContent, andEditTagsAction.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L194-L198: define contracts forSiteFilterDropdown,DropdownContent, andEditSitesAction.app/pages/clinicworkspace/FilterByTags.js#L13-L18: define the adapter’s API, filter state, and callbacks.app/pages/clinicworkspace/FilterBySites.js#L13-L18: define the adapter’s API, filter state, and callbacks.As per coding guidelines, “Define PropTypes for all component props.”
🤖 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 `@app/pages/clinicworkspace/components/TagFilterDropdown.js` around lines 190 - 194, Define PropTypes for every prop on TagFilterDropdown, DropdownContent, and EditTagsAction in app/pages/clinicworkspace/components/TagFilterDropdown.js (anchor lines 190-194); apply the same contracts to SiteFilterDropdown, DropdownContent, and EditSitesAction in app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 194-198). Define PropTypes for the adapter API, filter state, and callbacks in FilterByTags.js (lines 13-18) and FilterBySites.js (lines 13-18), using types that match each component’s existing props and defaults.Source: Coding guidelines
app/pages/clinicworkspace/useIsClinicAdmin.js (1)
1-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply the required import grouping across the new modules.
app/pages/clinicworkspace/useIsClinicAdmin.js#L1-L4: separate React, Redux, and Lodash groups with blank lines.app/pages/clinicworkspace/components/TagFilterDropdown.js#L1-L34: order React, Redux, third-party, Lodash, theme-ui, then local imports; keep all imports before declarations.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L1-L34: apply the same required order and move Lines 33-34 above declarations.app/pages/clinicworkspace/FilterByTags.js#L1-L9: place Lodash before local Redux/component imports.app/pages/clinicworkspace/FilterBySites.js#L1-L9: place Lodash before local Redux/component imports.As per coding guidelines, imports must use the prescribed groups with blank lines between them.
🤖 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 `@app/pages/clinicworkspace/useIsClinicAdmin.js` around lines 1 - 4, Reorganize imports in app/pages/clinicworkspace/useIsClinicAdmin.js (lines 1-4) into separate React, Redux, and Lodash groups with blank lines; in app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 1-34) and SiteFilterDropdown.js (lines 1-34), order groups as React, Redux, third-party, Lodash, theme-ui, then local imports, keeping every import before declarations; in app/pages/clinicworkspace/FilterByTags.js (lines 1-9) and FilterBySites.js (lines 1-9), move Lodash imports before local Redux/component imports and separate each prescribed group with blank lines.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.
Inline comments:
In `@app/pages/clinicworkspace/AppliedFiltersAdapter.js`:
- Around line 13-43: Add a timeCGMUsePercent branch to handleRemoveFilter
alongside the existing filter cases, removing the selected value from
activeFilters.timeCGMUsePercent via the same without-based update pattern used
for timeInRange and patientTags.
In `@app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js`:
- Line 23: Update prefixPopHealthMetric and its call sites in
DataRecencyFilterDropdown so trackMetric receives valid past-tense event-name
strings rather than the noop function. Preserve the existing invocation timing,
and apply the same correction to all referenced metric calls.
In `@app/pages/clinicworkspace/components/TagFilterDropdown.js`:
- Around line 30-31: The clinic workspace filter interactions currently discard
metrics by using no-op helpers or default trackers. In
app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 30-31) and
app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 30-31), pass
the real clinic workspace metric helpers instead of no-ops; in
app/pages/clinicworkspace/FilterByTags.js (lines 11-32) and
app/pages/clinicworkspace/FilterBySites.js (lines 11-32), thread the real
trackMetric through or remove the metric calls if these interactions are
intentionally untracked.
- Around line 71-74: Replace the unsupported Array.prototype.toSorted call in
the sortedTagFilterOptions useMemo within
app/pages/clinicworkspace/components/TagFilterDropdown.js lines 71-74 with a
compatible sorting approach. Apply the same change in
app/pages/clinicworkspace/components/SiteFilterDropdown.js lines 71-74,
preserving label-based ordering without relying on toSorted.
---
Outside diff comments:
In `@app/pages/clinicworkspace/ClinicPatients.js`:
- Around line 3747-3760: Update the dependency array for renderPeopleTable to
include patientListSearchTextInput, handleClearSearch, handleResetFilters, and
handleClickPatient alongside the existing dependencies, ensuring the callback
refreshes when search, filter-reset, or patient-click handlers change.
---
Nitpick comments:
In `@app/pages/clinicworkspace/AppliedFiltersAdapter.js`:
- Line 10: The newly introduced clinic workspace components lack PropTypes
declarations for their interfaces. Add PropTypes for activeFilters,
setActiveFilters, and rightContent on AppliedFiltersAdapter; DropdownContent and
DataRecencyFilterDropdown props in DataRecencyFilterDropdown.js; filter adapter
props in FilterByDataRecency; Chip, ChipGroup, and AppliedFilters props in
AppliedFilters.js; query-state and callback props in ClearFilterButtons.js; and
EmptyContentNode props in ClinicPatients.js, matching each component’s actual
usage.
In `@app/pages/clinicworkspace/components/TagFilterDropdown.js`:
- Around line 190-194: Define PropTypes for every prop on TagFilterDropdown,
DropdownContent, and EditTagsAction in
app/pages/clinicworkspace/components/TagFilterDropdown.js (anchor lines
190-194); apply the same contracts to SiteFilterDropdown, DropdownContent, and
EditSitesAction in app/pages/clinicworkspace/components/SiteFilterDropdown.js
(lines 194-198). Define PropTypes for the adapter API, filter state, and
callbacks in FilterByTags.js (lines 13-18) and FilterBySites.js (lines 13-18),
using types that match each component’s existing props and defaults.
In `@app/pages/clinicworkspace/useIsClinicAdmin.js`:
- Around line 1-4: Reorganize imports in
app/pages/clinicworkspace/useIsClinicAdmin.js (lines 1-4) into separate React,
Redux, and Lodash groups with blank lines; in
app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 1-34) and
SiteFilterDropdown.js (lines 1-34), order groups as React, Redux, third-party,
Lodash, theme-ui, then local imports, keeping every import before declarations;
in app/pages/clinicworkspace/FilterByTags.js (lines 1-9) and FilterBySites.js
(lines 1-9), move Lodash imports before local Redux/component imports and
separate each prescribed group with blank lines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cdcaceec-92dd-47f4-8d90-c5a5d64b7834
⛔ Files ignored due to path filters (1)
app/core/icons/tagIcon.svgis excluded by!**/*.svg
📒 Files selected for processing (13)
app/pages/clinicworkspace/AppliedFiltersAdapter.jsapp/pages/clinicworkspace/ClinicPatients.jsapp/pages/clinicworkspace/FilterByDataRecency.jsapp/pages/clinicworkspace/FilterBySites.jsapp/pages/clinicworkspace/FilterByTags.jsapp/pages/clinicworkspace/components/AppliedFilters.jsapp/pages/clinicworkspace/components/ClearFilterButtons.jsapp/pages/clinicworkspace/components/DataRecencyFilterDropdown.jsapp/pages/clinicworkspace/components/SiteFilterDropdown.jsapp/pages/clinicworkspace/components/TagFilterDropdown.jsapp/pages/clinicworkspace/useClinicPatientsFilters.jsapp/pages/clinicworkspace/useIsClinicAdmin.jslocales/en/translation.json
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/pages/clinicworkspace/ClinicPatients.js (1)
3747-3760: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winComplete
renderPeopleTable’s dependency list. It readspatientListSearchTextInputand captureshandleClearSearch,handleResetFilters, andhandleClickPatient, but none are in theuseCallbackdeps. That can leave the query-state controls using stale values after search/filter changes.🤖 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 `@app/pages/clinicworkspace/ClinicPatients.js` around lines 3747 - 3760, Update the dependency array for renderPeopleTable to include patientListSearchTextInput, handleClearSearch, handleResetFilters, and handleClickPatient alongside the existing dependencies, ensuring the callback refreshes when search, filter-reset, or patient-click handlers change.Source: Coding guidelines
🧹 Nitpick comments (3)
app/pages/clinicworkspace/AppliedFiltersAdapter.js (1)
10-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd PropTypes for the newly introduced component interfaces.
app/pages/clinicworkspace/AppliedFiltersAdapter.js#L10-L10: declareactiveFilters,setActiveFilters, andrightContent.app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L25-L30: declareDropdownContentprops.app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L135-L139: declareDataRecencyFilterDropdownprops.app/pages/clinicworkspace/FilterByDataRecency.js#L6-L9: declare the filter adapter props.app/pages/clinicworkspace/components/AppliedFilters.js#L104-L171: declare props forChip,ChipGroup, andAppliedFilters.app/pages/clinicworkspace/components/ClearFilterButtons.js#L25-L25: declare query-state and callback props.app/pages/clinicworkspace/ClinicPatients.js#L243-L243: declareEmptyContentNodeprops.As per coding guidelines, define PropTypes for all component props.
🤖 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 `@app/pages/clinicworkspace/AppliedFiltersAdapter.js` at line 10, The newly introduced clinic workspace components lack PropTypes declarations for their interfaces. Add PropTypes for activeFilters, setActiveFilters, and rightContent on AppliedFiltersAdapter; DropdownContent and DataRecencyFilterDropdown props in DataRecencyFilterDropdown.js; filter adapter props in FilterByDataRecency; Chip, ChipGroup, and AppliedFilters props in AppliedFilters.js; query-state and callback props in ClearFilterButtons.js; and EmptyContentNode props in ClinicPatients.js, matching each component’s actual usage.Source: Coding guidelines
app/pages/clinicworkspace/components/TagFilterDropdown.js (1)
190-194: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefine PropTypes for every component introduced here.
app/pages/clinicworkspace/components/TagFilterDropdown.js#L190-L194: define contracts forTagFilterDropdown,DropdownContent, andEditTagsAction.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L194-L198: define contracts forSiteFilterDropdown,DropdownContent, andEditSitesAction.app/pages/clinicworkspace/FilterByTags.js#L13-L18: define the adapter’s API, filter state, and callbacks.app/pages/clinicworkspace/FilterBySites.js#L13-L18: define the adapter’s API, filter state, and callbacks.As per coding guidelines, “Define PropTypes for all component props.”
🤖 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 `@app/pages/clinicworkspace/components/TagFilterDropdown.js` around lines 190 - 194, Define PropTypes for every prop on TagFilterDropdown, DropdownContent, and EditTagsAction in app/pages/clinicworkspace/components/TagFilterDropdown.js (anchor lines 190-194); apply the same contracts to SiteFilterDropdown, DropdownContent, and EditSitesAction in app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 194-198). Define PropTypes for the adapter API, filter state, and callbacks in FilterByTags.js (lines 13-18) and FilterBySites.js (lines 13-18), using types that match each component’s existing props and defaults.Source: Coding guidelines
app/pages/clinicworkspace/useIsClinicAdmin.js (1)
1-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply the required import grouping across the new modules.
app/pages/clinicworkspace/useIsClinicAdmin.js#L1-L4: separate React, Redux, and Lodash groups with blank lines.app/pages/clinicworkspace/components/TagFilterDropdown.js#L1-L34: order React, Redux, third-party, Lodash, theme-ui, then local imports; keep all imports before declarations.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L1-L34: apply the same required order and move Lines 33-34 above declarations.app/pages/clinicworkspace/FilterByTags.js#L1-L9: place Lodash before local Redux/component imports.app/pages/clinicworkspace/FilterBySites.js#L1-L9: place Lodash before local Redux/component imports.As per coding guidelines, imports must use the prescribed groups with blank lines between them.
🤖 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 `@app/pages/clinicworkspace/useIsClinicAdmin.js` around lines 1 - 4, Reorganize imports in app/pages/clinicworkspace/useIsClinicAdmin.js (lines 1-4) into separate React, Redux, and Lodash groups with blank lines; in app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 1-34) and SiteFilterDropdown.js (lines 1-34), order groups as React, Redux, third-party, Lodash, theme-ui, then local imports, keeping every import before declarations; in app/pages/clinicworkspace/FilterByTags.js (lines 1-9) and FilterBySites.js (lines 1-9), move Lodash imports before local Redux/component imports and separate each prescribed group with blank lines.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.
Inline comments:
In `@app/pages/clinicworkspace/AppliedFiltersAdapter.js`:
- Around line 13-43: Add a timeCGMUsePercent branch to handleRemoveFilter
alongside the existing filter cases, removing the selected value from
activeFilters.timeCGMUsePercent via the same without-based update pattern used
for timeInRange and patientTags.
In `@app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js`:
- Line 23: Update prefixPopHealthMetric and its call sites in
DataRecencyFilterDropdown so trackMetric receives valid past-tense event-name
strings rather than the noop function. Preserve the existing invocation timing,
and apply the same correction to all referenced metric calls.
In `@app/pages/clinicworkspace/components/TagFilterDropdown.js`:
- Around line 30-31: The clinic workspace filter interactions currently discard
metrics by using no-op helpers or default trackers. In
app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 30-31) and
app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 30-31), pass
the real clinic workspace metric helpers instead of no-ops; in
app/pages/clinicworkspace/FilterByTags.js (lines 11-32) and
app/pages/clinicworkspace/FilterBySites.js (lines 11-32), thread the real
trackMetric through or remove the metric calls if these interactions are
intentionally untracked.
- Around line 71-74: Replace the unsupported Array.prototype.toSorted call in
the sortedTagFilterOptions useMemo within
app/pages/clinicworkspace/components/TagFilterDropdown.js lines 71-74 with a
compatible sorting approach. Apply the same change in
app/pages/clinicworkspace/components/SiteFilterDropdown.js lines 71-74,
preserving label-based ordering without relying on toSorted.
---
Outside diff comments:
In `@app/pages/clinicworkspace/ClinicPatients.js`:
- Around line 3747-3760: Update the dependency array for renderPeopleTable to
include patientListSearchTextInput, handleClearSearch, handleResetFilters, and
handleClickPatient alongside the existing dependencies, ensuring the callback
refreshes when search, filter-reset, or patient-click handlers change.
---
Nitpick comments:
In `@app/pages/clinicworkspace/AppliedFiltersAdapter.js`:
- Line 10: The newly introduced clinic workspace components lack PropTypes
declarations for their interfaces. Add PropTypes for activeFilters,
setActiveFilters, and rightContent on AppliedFiltersAdapter; DropdownContent and
DataRecencyFilterDropdown props in DataRecencyFilterDropdown.js; filter adapter
props in FilterByDataRecency; Chip, ChipGroup, and AppliedFilters props in
AppliedFilters.js; query-state and callback props in ClearFilterButtons.js; and
EmptyContentNode props in ClinicPatients.js, matching each component’s actual
usage.
In `@app/pages/clinicworkspace/components/TagFilterDropdown.js`:
- Around line 190-194: Define PropTypes for every prop on TagFilterDropdown,
DropdownContent, and EditTagsAction in
app/pages/clinicworkspace/components/TagFilterDropdown.js (anchor lines
190-194); apply the same contracts to SiteFilterDropdown, DropdownContent, and
EditSitesAction in app/pages/clinicworkspace/components/SiteFilterDropdown.js
(lines 194-198). Define PropTypes for the adapter API, filter state, and
callbacks in FilterByTags.js (lines 13-18) and FilterBySites.js (lines 13-18),
using types that match each component’s existing props and defaults.
In `@app/pages/clinicworkspace/useIsClinicAdmin.js`:
- Around line 1-4: Reorganize imports in
app/pages/clinicworkspace/useIsClinicAdmin.js (lines 1-4) into separate React,
Redux, and Lodash groups with blank lines; in
app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 1-34) and
SiteFilterDropdown.js (lines 1-34), order groups as React, Redux, third-party,
Lodash, theme-ui, then local imports, keeping every import before declarations;
in app/pages/clinicworkspace/FilterByTags.js (lines 1-9) and FilterBySites.js
(lines 1-9), move Lodash imports before local Redux/component imports and
separate each prescribed group with blank lines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cdcaceec-92dd-47f4-8d90-c5a5d64b7834
⛔ Files ignored due to path filters (1)
app/core/icons/tagIcon.svgis excluded by!**/*.svg
📒 Files selected for processing (13)
app/pages/clinicworkspace/AppliedFiltersAdapter.jsapp/pages/clinicworkspace/ClinicPatients.jsapp/pages/clinicworkspace/FilterByDataRecency.jsapp/pages/clinicworkspace/FilterBySites.jsapp/pages/clinicworkspace/FilterByTags.jsapp/pages/clinicworkspace/components/AppliedFilters.jsapp/pages/clinicworkspace/components/ClearFilterButtons.jsapp/pages/clinicworkspace/components/DataRecencyFilterDropdown.jsapp/pages/clinicworkspace/components/SiteFilterDropdown.jsapp/pages/clinicworkspace/components/TagFilterDropdown.jsapp/pages/clinicworkspace/useClinicPatientsFilters.jsapp/pages/clinicworkspace/useIsClinicAdmin.jslocales/en/translation.json
🛑 Comments failed to post (4)
app/pages/clinicworkspace/AppliedFiltersAdapter.js (1)
13-43: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle removal of the CGM-use chip.
AppliedFilters.jsLines 35-42 emitstype: 'timeCGMUsePercent', but this switch ignores that key, so its remove action does nothing.Proposed fix
const handleRemoveFilter = (filterKey, value) => { switch (filterKey) { + case 'timeCGMUsePercent': + setActiveFilters({ + ...activeFilters, + timeCGMUsePercent: defaultFilterState.timeCGMUsePercent, + }); + break; + case 'lastData':📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const handleRemoveFilter = (filterKey, value) => { switch (filterKey) { case 'timeCGMUsePercent': setActiveFilters({ ...activeFilters, timeCGMUsePercent: defaultFilterState.timeCGMUsePercent, }); break; case 'lastData': setActiveFilters({ ...activeFilters, lastData: defaultFilterState.lastData, lastDataType: defaultFilterState.lastDataType, }); break; case 'timeInRange': setActiveFilters({ ...activeFilters, timeInRange: without(activeFilters.timeInRange, value), }); break; case 'patientTags': setActiveFilters({ ...activeFilters, patientTags: without(activeFilters.patientTags, value), }); break; case 'clinicSites': setActiveFilters({ ...activeFilters, clinicSites: without(activeFilters.clinicSites, value), }); break; }🤖 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 `@app/pages/clinicworkspace/AppliedFiltersAdapter.js` around lines 13 - 43, Add a timeCGMUsePercent branch to handleRemoveFilter alongside the existing filter cases, removing the selected value from activeFilters.timeCGMUsePercent via the same without-based update pattern used for timeInRange and patientTags.app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js (1)
23-23: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Pass valid, past-tense event names to
trackMetric.
prefixPopHealthMetriccurrently returns thenoopfunction, so every new call passes a function instead of an event-name string.Proposed fix
-const prefixPopHealthMetric = () => noop; // TODO: FIX +const prefixPopHealthMetric = metric => `Clinic - Population Health - ${metric}`; -trackMetric(prefixPopHealthMetric('Last upload clear filter'), { +trackMetric(prefixPopHealthMetric('Cleared last upload filter'), { -trackMetric(prefixPopHealthMetric('Last upload apply filter'), { +trackMetric(prefixPopHealthMetric('Applied last upload filter'), { -trackMetric(prefixPopHealthMetric('Last data filter open'), { +trackMetric(prefixPopHealthMetric('Opened last data filter'), { -trackMetric(prefixPopHealthMetric('Last upload filter close'), { +trackMetric(prefixPopHealthMetric('Closed last upload filter'), {Based on learnings, metric names should use past tense while retaining the current invocation timing. I can add a focused metric test as well.
Also applies to: 99-99, 118-122, 157-157, 179-179
🤖 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 `@app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js` at line 23, Update prefixPopHealthMetric and its call sites in DataRecencyFilterDropdown so trackMetric receives valid past-tense event-name strings rather than the noop function. Preserve the existing invocation timing, and apply the same correction to all referenced metric calls.Source: Learnings
app/pages/clinicworkspace/components/TagFilterDropdown.js (2)
30-31: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Locate the canonical metrics helpers and nearby established call patterns. rg -n -C3 --type=js \ '\b(trackMetric|prefixPopHealthMetric)\b' \ appRepository: tidepool-org/blip
Length of output: 50374
🏁 Script executed:
#!/bin/bash set -euo pipefail for f in \ app/pages/clinicworkspace/components/TagFilterDropdown.js \ app/pages/clinicworkspace/components/SiteFilterDropdown.js \ app/pages/clinicworkspace/FilterByTags.js \ app/pages/clinicworkspace/FilterBySites.js do echo "### $f" wc -l "$f" echo done echo "### TagFilterDropdown.js" sed -n '1,220p' app/pages/clinicworkspace/components/TagFilterDropdown.js echo "### SiteFilterDropdown.js" sed -n '1,220p' app/pages/clinicworkspace/components/SiteFilterDropdown.js echo "### FilterByTags.js" sed -n '1,220p' app/pages/clinicworkspace/FilterByTags.js echo "### FilterBySites.js" sed -n '1,220p' app/pages/clinicworkspace/FilterBySites.jsRepository: tidepool-org/blip
Length of output: 20534
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n -C2 "dispatch\\(actions\\.async\\.(fetchClinicPatientTags|fetchClinicSites)|trackMetric\\(" \ app/pages/clinicworkspace app/pages/clinicianedit app/pages/clinicinvite app/pages/prescription app/pages/share/PatientInvites.jsRepository: tidepool-org/blip
Length of output: 50373
🏁 Script executed:
#!/bin/bash set -euo pipefail # Find the specific files and print only the relevant sections with line numbers. for f in \ app/pages/clinicworkspace/components/TagFilterDropdown.js \ app/pages/clinicworkspace/components/SiteFilterDropdown.js \ app/pages/clinicworkspace/FilterByTags.js \ app/pages/clinicworkspace/FilterBySites.js do echo "### $f" nl -ba "$f" | sed -n '1,220p' echo doneRepository: tidepool-org/blip
Length of output: 258
🏁 Script executed:
#!/bin/bash set -euo pipefail # Compare the wording used in nearby established clinic metrics calls. rg -n -C1 "Clinic - .* (open|confirmed|back out|close|cancel|refresh|edit|accept|decline|invite|remove|update)" app/pages/clinicworkspace app/pages/share/PatientInvites.js app/pages/clinicianedit/clinicianedit.js app/pages/clinicinvite/clinicinvite.jsRepository: tidepool-org/blip
Length of output: 7705
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "### TagFilterDropdown" nl -ba app/pages/clinicworkspace/components/TagFilterDropdown.js | sed -n '1,120p' echo "### SiteFilterDropdown" nl -ba app/pages/clinicworkspace/components/SiteFilterDropdown.js | sed -n '1,120p' echo "### FilterByTags" nl -ba app/pages/clinicworkspace/FilterByTags.js | sed -n '1,120p' echo "### FilterBySites" nl -ba app/pages/clinicworkspace/FilterBySites.js | sed -n '1,120p'Repository: tidepool-org/blip
Length of output: 217
🏁 Script executed:
#!/bin/bash set -euo pipefail log=/tmp/coderabbit-shell-logs/shell-output-jfWhRa echo "### Log hits for TagFilterDropdown / SiteFilterDropdown / FilterByTags / FilterBySites" rg -n -C3 'TagFilterDropdown|SiteFilterDropdown|FilterByTags|FilterBySites|trackMetric = noop|prefixPopHealthMetric|TODO: FIX' "$log" | sed -n '1,220p'Repository: tidepool-org/blip
Length of output: 19074
Pass the real clinic workspace metric helpers through these filters.
app/pages/clinicworkspace/components/TagFilterDropdown.jsandapp/pages/clinicworkspace/components/SiteFilterDropdown.jsstill definetrackMetric/prefixPopHealthMetricas no-ops, so every open/clear/apply event is dropped.app/pages/clinicworkspace/FilterByTags.jsandapp/pages/clinicworkspace/FilterBySites.jsstill defaulttrackMetrictonoop; thread the real tracker in here or remove the calls if these interactions are intentionally untracked.📍 Affects 4 files
app/pages/clinicworkspace/components/TagFilterDropdown.js#L30-L31(this comment)app/pages/clinicworkspace/components/SiteFilterDropdown.js#L30-L31app/pages/clinicworkspace/FilterByTags.js#L11-L32app/pages/clinicworkspace/FilterBySites.js#L11-L32🤖 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 `@app/pages/clinicworkspace/components/TagFilterDropdown.js` around lines 30 - 31, The clinic workspace filter interactions currently discard metrics by using no-op helpers or default trackers. In app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 30-31) and app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 30-31), pass the real clinic workspace metric helpers instead of no-ops; in app/pages/clinicworkspace/FilterByTags.js (lines 11-32) and app/pages/clinicworkspace/FilterBySites.js (lines 11-32), thread the real trackMetric through or remove the metric calls if these interactions are intentionally untracked.Source: Learnings
71-74: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Inspect declared browser/transpilation targets and existing usage. fd -H -t f \ '^(package\.json|\.browserslistrc|babel\.config\..*|\.babelrc.*)$' \ . -x sh -c 'echo "=== $1 ==="; sed -n "1,240p" "$1"' sh {} rg -n --type=js '\.toSorted\s*\(' appRepository: tidepool-org/blip
Length of output: 9976
🏁 Script executed:
#!/bin/bash set -euo pipefail # Look for explicit browser targets, polyfill setup, and any toSorted-related support code. printf '\n=== browserslist / target config ===\n' fd -H -t f '^(package\.json|\.browserslistrc|browserslist(\..*)?|babel\.config\..*|\.babelrc(\..*)?)$' . \ -x sh -c 'echo "--- $1 ---"; sed -n "1,220p" "$1"' sh {} printf '\n=== polyfill / core-js / react-app-polyfill references ===\n' rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \ '(core-js|react-app-polyfill|polyfill|toSorted)' app config scripts . \ -g '!*.map' -g '!*.min.*' printf '\n=== existing toSorted usage ===\n' rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' '\.toSorted\s*\(' appRepository: tidepool-org/blip
Length of output: 50374
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n=== app entry / bootstrap files ===\n' fd -H -t f '^(index|main|app)\.(js|jsx|ts|tsx)$|^setup.*\.(js|ts)$|^polyfill.*\.(js|ts)$' app . \ | sed 's#^\./##' | sort | head -200 printf '\n=== runtime polyfill imports in app/bootstrap ===\n' rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \ "import ['\\\"]core-js/stable['\\\"]|require\\(['\\\"]core-js/stable['\\\"]|react-app-polyfill|core-js/modules/.*/to-sorted|toSorted" \ app *.js *.jsx *.ts *.tsx config test __tests__ jest.setup.js printf '\n=== files mentioning browserslist ===\n' rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' 'browserslist|targets?:|preset-env' . \ -g '!yarn.lock' -g '!*.map' -g '!*.min.*' | head -200Repository: tidepool-org/blip
Length of output: 5118
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n=== app/main.js ===\n' sed -n '1,220p' app/main.js printf '\n=== app/pages/app/index.js ===\n' sed -n '1,220p' app/pages/app/index.js printf '\n=== imports of core-js/stable or es.array.to-sorted ===\n' rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' -F \ "core-js/stable" app .storybook __tests__ test config rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' -F \ "es.array.to-sorted" app .storybook __tests__ test config printf '\n=== package.json browserslist fields ===\n' python3 - <<'PY' import json from pathlib import Path pkg = json.loads(Path('package.json').read_text()) for key in ('browserslist','babel'): print(key, pkg.get(key)) PYRepository: tidepool-org/blip
Length of output: 2017
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n=== app/main.js ===\n' cat -n app/main.js | sed -n '1,220p' printf '\n=== app/pages/app/index.js ===\n' cat -n app/pages/app/index.js | sed -n '1,220p' printf '\n=== direct references to core-js in app entry paths ===\n' rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' -F 'core-js' app/main.js app/pages app/redux app/componentsRepository: tidepool-org/blip
Length of output: 2360
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n=== app/bootstrap.* ===\n' fd -H -t f '^bootstrap\.(js|jsx|ts|tsx)$' app for f in $(fd -H -t f '^bootstrap\.(js|jsx|ts|tsx)$' app); do echo "--- $f ---" cat -n "$f" | sed -n '1,260p' done printf '\n=== direct polyfill imports from app/bootstrap neighborhood ===\n' rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' -F \ 'core-js' app/bootstrap* app/**/bootstrap* app/config* app/main.js app/pages/app app/redux printf '\n=== toSorted support polyfills ===\n' rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' -F \ 'es.array.to-sorted' . -g '!yarn.lock' -g '!*.map' -g '!*.min.*'Repository: tidepool-org/blip
Length of output: 5928
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n=== config.app files ===\n' fd -H -t f '^config\.app\.(js|jsx|ts|tsx)$' . | sed 's#^\./##' | sort for f in $(fd -H -t f '^config\.app\.(js|jsx|ts|tsx)$' .); do echo "--- $f ---" cat -n "$f" | sed -n '1,240p' done printf '\n=== explicit polyfill entrypoints ===\n' rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' -F \ "`@babel/polyfill`|core-js/stable|regenerator-runtime|react-app-polyfill|es.array.to-sorted" \ . -g '!yarn.lock' -g '!*.map' -g '!*.min.*'Repository: tidepool-org/blip
Length of output: 2901
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n=== webpack-related files ===\n' fd -H -t f 'webpack.*\.(js|cjs|mjs)$|^webpack\.config(\..*)?\.js$|^.*webpack.*config.*\.js$' . | sed 's#^\./##' | sort printf '\n=== target / browsers / preset-env in webpack configs ===\n' rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \ 'target\s*:|browserslist|preset-env|core-js|useBuiltIns|Array\.prototype\.toSorted|toSorted' \ webpack*.js config/**/*.js app/**/*.js .storybook/**/*.jsRepository: tidepool-org/blip
Length of output: 452
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n=== webpack.config.js ===\n' cat -n webpack.config.js | sed -n '1,260p' printf '\n=== config.webpack.js ===\n' cat -n config.webpack.js | sed -n '1,260p' printf '\n=== babel-loader / preset-env references in webpack configs ===\n' rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \ 'babel-loader|preset-env|targets\s*:|core-js|useBuiltIns|browserslist|exclude:|include:' \ webpack.config.js config.webpack.jsRepository: tidepool-org/blip
Length of output: 13264
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n=== `@babel/polyfill` lockfile entry ===\n' rg -n -A12 -B4 '^"`@babel/polyfill`@npm:7\.12\.1":|^ core-js:|^ regenerator-runtime:' yarn.lock printf '\n=== core-js lockfile entries ===\n' rg -n -A8 -B2 '^"core-js@npm:|^"core-js-pure@npm:' yarn.lock printf '\n=== babel polyfill package references in repo ===\n' rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' -F '`@babel/polyfill`' .Repository: tidepool-org/blip
Length of output: 5570
Replace
toSortedhere. The browser bundle only ships@babel/polyfill(core-js 2.x), soArray.prototype.toSortedisn’t covered at runtime.
app/pages/clinicworkspace/components/TagFilterDropdown.js#L71-L74app/pages/clinicworkspace/components/SiteFilterDropdown.js#L71-L74📍 Affects 2 files
app/pages/clinicworkspace/components/TagFilterDropdown.js#L71-L74(this comment)app/pages/clinicworkspace/components/SiteFilterDropdown.js#L71-L74🤖 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 `@app/pages/clinicworkspace/components/TagFilterDropdown.js` around lines 71 - 74, Replace the unsupported Array.prototype.toSorted call in the sortedTagFilterOptions useMemo within app/pages/clinicworkspace/components/TagFilterDropdown.js lines 71-74 with a compatible sorting approach. Apply the same change in app/pages/clinicworkspace/components/SiteFilterDropdown.js lines 71-74, preserving label-based ordering without relying on toSorted.
This was referenced Jul 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WEB-4654