WEB-4654 - CGM Use - #1991
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 is refactored into reusable dropdowns, filter-state adapters, applied-filter chips, and shared query-state controls. ChangesClinic patient filtering
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ClinicPatients
participant FilterByDataRecency
participant DataRecencyFilterDropdown
participant FilterState
ClinicPatients->>FilterByDataRecency: render active filter values
FilterByDataRecency->>DataRecencyFilterDropdown: pass lastData and lastDataType
DataRecencyFilterDropdown->>FilterByDataRecency: apply or clear selection
FilterByDataRecency->>FilterState: setActiveFilters(lastData, lastDataType)
FilterState->>ClinicPatients: update patient query
Possibly related PRs
🚥 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: 11
🧹 Nitpick comments (6)
app/pages/clinicworkspace/useIsClinicAdmin.js (1)
1-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd blank lines between import groups.
As per coding guidelines,
**/*.{js,jsx,ts,tsx}: "Group imports in the required order with blank lines between groups: React, PropTypes, Redux, third-party libraries, Lodash specific imports, theme-ui, then local imports." React, Redux, and Lodash imports here run together with no separation.✏️ Proposed fix
import React from 'react'; + import { useSelector } from 'react-redux'; + import get from 'lodash/get'; import includes from 'lodash/includes';🤖 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, Group the imports in useIsClinicAdmin by adding blank lines according to the project order: keep React first, separate the Redux import, and separate the Lodash imports as their own group.Source: Coding guidelines
app/pages/clinicworkspace/components/SiteFilterDropdown.js (2)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing PropTypes on all new filter components. As per coding guidelines,
**/*.{js,jsx,ts,tsx}: "Define PropTypes for all component props," but none of these new components declare PropTypes for their props.
app/pages/clinicworkspace/components/SiteFilterDropdown.js#L218-222: addpropTypesforSiteFilterDropdown(onChange,clinicSites,onClickEditSites) and forDropdownContent.app/pages/clinicworkspace/FilterByTimeInRange.js#L6-9: addpropTypesforactiveFilters/setActiveFilters.app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L111-114: addpropTypesforSummaryPeriodFilterDropdownandDropdownContent.app/pages/clinicworkspace/components/TagFilterDropdown.js#L214-218: addpropTypesforTagFilterDropdown(onChange,patientTags,onClickEditTags) and forDropdownContent.app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js#L252-255: addpropTypesforTimeInRangeFilterDropdownandDropdownContent.🤖 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/SiteFilterDropdown.js` at line 1, Add PropTypes declarations for every listed new component: SiteFilterDropdown and its DropdownContent, FilterByTimeInRange, SummaryPeriodFilterDropdown and its DropdownContent, TagFilterDropdown and its DropdownContent, and TimeInRangeFilterDropdown and its DropdownContent. Define each component’s declarations for all props it consumes, including the explicitly named callbacks and data props, using the appropriate requiredness and collection/function types.Source: Coding guidelines
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift
SiteFilterDropdown.jsandTagFilterDropdown.jsare near-duplicate implementations. Both files reimplement the same popover/search/checkbox/apply-clear structure differing only by "site" vs "tag" naming; this duplication is likely why thetrackMetricstub diverged between them (see the metrics-disabled comment above).
app/pages/clinicworkspace/components/SiteFilterDropdown.js#L56-209: extract the shared searchable-checkbox-dropdown logic (search box, sorted/filtered options, zero-selection sentinel handling, clear/apply actions) into a reusable hook or generic component parameterized by entity type.app/pages/clinicworkspace/components/TagFilterDropdown.js#L56-205: consume the same shared abstraction instead of duplicating the logic.🤖 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/SiteFilterDropdown.js` at line 1, Extract the duplicated searchable checkbox popover behavior from SiteFilterDropdown and TagFilterDropdown into one reusable hook or generic component, parameterized for site and tag labels, options, selection state, and callbacks. Move shared search, sorting/filtering, zero-selection sentinel handling, and clear/apply actions into that abstraction, then update both dropdowns to consume it while preserving their entity-specific behavior and metric handling.app/pages/clinicworkspace/FilterByTags.js (1)
11-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
trackMetricstub silently drops the "Edit clinic tags open" event; call order also deviates from convention.Same pattern as
FilterBySites.js:trackMetricis a localnoopstub (TODO: FIX), and the call precedes thedispatch(actions.async.fetchClinicPatientTags(...))rather than following it. (See consolidated comment.)🤖 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/FilterByTags.js` around lines 11 - 33, Replace the local noop trackMetric stub in FilterByTags with the established metric-tracking implementation used by FilterBySites, and reorder handleClickEditTags so dispatch(actions.async.fetchClinicPatientTags(...)) runs before recording the “Clinic - Population Health - Edit clinic tags open” event.Source: Learnings
app/pages/clinicworkspace/FilterBySites.js (1)
11-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
trackMetricstub silently drops the "Edit clinic sites open" event; call order also deviates from convention.
trackMetricis a localnoopstub (TODO: FIX), so this metric is never recorded — unlike the sibling dropdown files in this PR that import the realtrackMetricfromcore/metricUtils. Separately, the call precedesdispatch(actions.async.fetchClinicSites(...)), whereas the established convention placestrackMetricimmediately after dispatching the related action. (See consolidated comment; this repeats identically inFilterByTags.js.)🤖 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/FilterBySites.js` around lines 11 - 33, Replace the local trackMetric noop in FilterBySites with the real trackMetric import from core/metricUtils, and move the “Edit clinic sites open” tracking call in handleClickEditSites to immediately after dispatch(actions.async.fetchClinicSites(...)). Apply the same changes to the corresponding FilterByTags implementation.Source: Learnings
app/pages/clinicworkspace/FilterByCGMUse.js (1)
6-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNone of the new filter adapters/dropdowns declare PropTypes. As per coding guidelines, "Define PropTypes for all component props," but every new component in this cohort omits them.
app/pages/clinicworkspace/FilterByCGMUse.js#L6-L9: add PropTypes foractiveFilters,setActiveFilters.app/pages/clinicworkspace/FilterByDataRecency.js#L6-L9: add PropTypes foractiveFilters,setActiveFilters.app/pages/clinicworkspace/FilterBySites.js#L13-L18: add PropTypes forapi,activeFilters,setActiveFilters,setShowClinicSitesDialog.app/pages/clinicworkspace/FilterBySummaryPeriod.js#L6-L9: add PropTypes foractiveSummaryPeriod,setActiveSummaryPeriod.app/pages/clinicworkspace/FilterByTags.js#L13-L18: add PropTypes forapi,activeFilters,setActiveFilters,setShowClinicPatientTagsDialog.app/pages/clinicworkspace/components/CGMUseFilterDropdown.js#L26-L30: add PropTypes forDropdownContent'sonClose,onChange,timeCGMUsePercent.app/pages/clinicworkspace/components/CGMUseFilterDropdown.js#L99-L102: add PropTypes forCGMUseFilterDropdown'sonChange,timeCGMUsePercent.app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L26-L31: add PropTypes forDropdownContent'sonClose,onChange,lastData,lastDataType.app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L131-L135: add PropTypes forDataRecencyFilterDropdown'sonChange,lastData,lastDataType.🤖 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/FilterByCGMUse.js` around lines 6 - 9, Declare PropTypes for every listed component and nested DropdownContent function, matching each component's existing props: activeFilters/setActiveFilters in FilterByCGMUse and FilterByDataRecency; api/activeFilters/setActiveFilters/setShowClinicSitesDialog in FilterBySites; activeSummaryPeriod/setActiveSummaryPeriod in FilterBySummaryPeriod; api/activeFilters/setActiveFilters/setShowClinicPatientTagsDialog in FilterByTags; onClose/onChange/timeCGMUsePercent for CGMUseFilterDropdown's DropdownContent and onChange/timeCGMUsePercent for CGMUseFilterDropdown; and onClose/onChange/lastData/lastDataType for DataRecencyFilterDropdown's DropdownContent and onChange/lastData/lastDataType for DataRecencyFilterDropdown. Use the appropriate PropTypes validators and preserve the existing defaults and 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.
Inline comments:
In `@app/pages/clinicworkspace/AppliedFiltersAdapter.js`:
- Around line 13-44: Add a timeCGMUsePercent case to handleRemoveFilter,
resetting activeFilters.timeCGMUsePercent to
defaultFilterState.timeCGMUsePercent so removing the CGM Use chip clears its
value.
In `@app/pages/clinicworkspace/ClinicPatients.js`:
- Around line 3233-3249: Update the dependency array of renderPeopleTable’s
useCallback to include patientListSearchTextInput, handleClearSearch, and
handleResetFilters, matching the values used by patientQueryState and the
rendered ClearFilterButtons/EmptyContentNode. Preserve the existing dependencies
and ensure both affected JSX paths stay synchronized with current search and
filter state.
In `@app/pages/clinicworkspace/components/AppliedFilters.js`:
- Around line 166-171: Declare PropTypes for all props in AppliedFilters,
including its internal ChipGroup and Chip components; define activeFilters,
setActiveFilters, and rightContent PropTypes in AppliedFiltersAdapter using the
existing PropTypes import; and add patientQueryState, onClearSearch, and
onResetFilters PropTypes to ClearFilterButtons. Use the specified types and
preserve the current component behavior.
In `@app/pages/clinicworkspace/components/CGMUseFilterDropdown.js`:
- Line 19: The prefixPopHealthMetric function must return the provided metric
name as a string rather than the noop function. Update prefixPopHealthMetric in
app/pages/clinicworkspace/components/CGMUseFilterDropdown.js:19 and
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js:24, and apply
the same correction to its usages in TimeInRangeFilterDropdown.js and
SummaryPeriodFilterDropdown.js, preserving the existing trackMetric calls.
In `@app/pages/clinicworkspace/components/SiteFilterDropdown.js`:
- Line 1: Replace the local no-op trackMetric stubs in SiteFilterDropdown and
TagFilterDropdown with imports of the real trackMetric implementation from the
established metricUtils module, preserving all existing metric call sites for
open, close, clear, and apply actions.
- Around line 33-34: Replace the local no-op trackMetric declaration in
SiteFilterDropdown with the real trackMetric import from core/metricUtils,
matching SummaryPeriodFilterDropdown and TimeInRangeFilterDropdown. Preserve
prefixPopHealthMetric as-is unless it is also provided by that module, and leave
the existing open, close, clear, and apply tracking calls unchanged.
In `@app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js`:
- Line 1: Update the prefixPopHealthMetric helper in both
SummaryPeriodFilterDropdown and TimeInRangeFilterDropdown so it accepts a
metric-name argument and returns the correctly prefixed string instead of the
noop function reference. Preserve the existing prefix format expected by
trackMetric calls.
- Line 19: Replace the placeholder implementation of prefixPopHealthMetric so it
accepts the metric-name string and returns the intended prefixed event-name
string. Preserve the existing trackMetric call sites and ensure they pass
strings rather than the noop function reference.
In `@app/pages/clinicworkspace/components/TagFilterDropdown.js`:
- Around line 33-34: Replace the no-op trackMetric definition in
TagFilterDropdown with the real tracking utility import and use it for the
existing tag-filter interaction events, including open, close, clear, and apply.
Keep prefixPopHealthMetric unchanged unless required by the established tracking
integration.
- Around line 139-158: Update the zero-tags option Box in the tag filter
dropdown to use the tag-specific className and key naming convention, replacing
the leftover clinic-site identifiers while preserving the existing styling,
rendering condition, and checkbox behavior.
In `@app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js`:
- Line 29: Fix prefixPopHealthMetric in TimeInRangeFilterDropdown.js so it
returns the provided metric name string rather than the noop function reference.
Match the working implementation in SummaryPeriodFilterDropdown.js and preserve
the existing trackMetric calls.
---
Nitpick comments:
In `@app/pages/clinicworkspace/components/SiteFilterDropdown.js`:
- Line 1: Add PropTypes declarations for every listed new component:
SiteFilterDropdown and its DropdownContent, FilterByTimeInRange,
SummaryPeriodFilterDropdown and its DropdownContent, TagFilterDropdown and its
DropdownContent, and TimeInRangeFilterDropdown and its DropdownContent. Define
each component’s declarations for all props it consumes, including the
explicitly named callbacks and data props, using the appropriate requiredness
and collection/function types.
- Line 1: Extract the duplicated searchable checkbox popover behavior from
SiteFilterDropdown and TagFilterDropdown into one reusable hook or generic
component, parameterized for site and tag labels, options, selection state, and
callbacks. Move shared search, sorting/filtering, zero-selection sentinel
handling, and clear/apply actions into that abstraction, then update both
dropdowns to consume it while preserving their entity-specific behavior and
metric handling.
In `@app/pages/clinicworkspace/FilterByCGMUse.js`:
- Around line 6-9: Declare PropTypes for every listed component and nested
DropdownContent function, matching each component's existing props:
activeFilters/setActiveFilters in FilterByCGMUse and FilterByDataRecency;
api/activeFilters/setActiveFilters/setShowClinicSitesDialog in FilterBySites;
activeSummaryPeriod/setActiveSummaryPeriod in FilterBySummaryPeriod;
api/activeFilters/setActiveFilters/setShowClinicPatientTagsDialog in
FilterByTags; onClose/onChange/timeCGMUsePercent for CGMUseFilterDropdown's
DropdownContent and onChange/timeCGMUsePercent for CGMUseFilterDropdown; and
onClose/onChange/lastData/lastDataType for DataRecencyFilterDropdown's
DropdownContent and onChange/lastData/lastDataType for
DataRecencyFilterDropdown. Use the appropriate PropTypes validators and preserve
the existing defaults and behavior.
In `@app/pages/clinicworkspace/FilterBySites.js`:
- Around line 11-33: Replace the local trackMetric noop in FilterBySites with
the real trackMetric import from core/metricUtils, and move the “Edit clinic
sites open” tracking call in handleClickEditSites to immediately after
dispatch(actions.async.fetchClinicSites(...)). Apply the same changes to the
corresponding FilterByTags implementation.
In `@app/pages/clinicworkspace/FilterByTags.js`:
- Around line 11-33: Replace the local noop trackMetric stub in FilterByTags
with the established metric-tracking implementation used by FilterBySites, and
reorder handleClickEditTags so
dispatch(actions.async.fetchClinicPatientTags(...)) runs before recording the
“Clinic - Population Health - Edit clinic tags open” event.
In `@app/pages/clinicworkspace/useIsClinicAdmin.js`:
- Around line 1-4: Group the imports in useIsClinicAdmin by adding blank lines
according to the project order: keep React first, separate the Redux import, and
separate the Lodash imports as their own group.
🪄 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: 8814d320-6d27-4896-8edd-a866bacdd540
⛔ Files ignored due to path filters (1)
app/core/icons/tagIcon.svgis excluded by!**/*.svg
📒 Files selected for processing (19)
app/pages/clinicworkspace/AppliedFiltersAdapter.jsapp/pages/clinicworkspace/ClinicPatients.jsapp/pages/clinicworkspace/FilterByCGMUse.jsapp/pages/clinicworkspace/FilterByDataRecency.jsapp/pages/clinicworkspace/FilterBySites.jsapp/pages/clinicworkspace/FilterBySummaryPeriod.jsapp/pages/clinicworkspace/FilterByTags.jsapp/pages/clinicworkspace/FilterByTimeInRange.jsapp/pages/clinicworkspace/components/AppliedFilters.jsapp/pages/clinicworkspace/components/CGMUseFilterDropdown.jsapp/pages/clinicworkspace/components/ClearFilterButtons.jsapp/pages/clinicworkspace/components/DataRecencyFilterDropdown.jsapp/pages/clinicworkspace/components/SiteFilterDropdown.jsapp/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.jsapp/pages/clinicworkspace/components/TagFilterDropdown.jsapp/pages/clinicworkspace/components/TimeInRangeFilterDropdown.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: 11
🧹 Nitpick comments (6)
app/pages/clinicworkspace/useIsClinicAdmin.js (1)
1-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd blank lines between import groups.
As per coding guidelines,
**/*.{js,jsx,ts,tsx}: "Group imports in the required order with blank lines between groups: React, PropTypes, Redux, third-party libraries, Lodash specific imports, theme-ui, then local imports." React, Redux, and Lodash imports here run together with no separation.✏️ Proposed fix
import React from 'react'; + import { useSelector } from 'react-redux'; + import get from 'lodash/get'; import includes from 'lodash/includes';🤖 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, Group the imports in useIsClinicAdmin by adding blank lines according to the project order: keep React first, separate the Redux import, and separate the Lodash imports as their own group.Source: Coding guidelines
app/pages/clinicworkspace/components/SiteFilterDropdown.js (2)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing PropTypes on all new filter components. As per coding guidelines,
**/*.{js,jsx,ts,tsx}: "Define PropTypes for all component props," but none of these new components declare PropTypes for their props.
app/pages/clinicworkspace/components/SiteFilterDropdown.js#L218-222: addpropTypesforSiteFilterDropdown(onChange,clinicSites,onClickEditSites) and forDropdownContent.app/pages/clinicworkspace/FilterByTimeInRange.js#L6-9: addpropTypesforactiveFilters/setActiveFilters.app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L111-114: addpropTypesforSummaryPeriodFilterDropdownandDropdownContent.app/pages/clinicworkspace/components/TagFilterDropdown.js#L214-218: addpropTypesforTagFilterDropdown(onChange,patientTags,onClickEditTags) and forDropdownContent.app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js#L252-255: addpropTypesforTimeInRangeFilterDropdownandDropdownContent.🤖 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/SiteFilterDropdown.js` at line 1, Add PropTypes declarations for every listed new component: SiteFilterDropdown and its DropdownContent, FilterByTimeInRange, SummaryPeriodFilterDropdown and its DropdownContent, TagFilterDropdown and its DropdownContent, and TimeInRangeFilterDropdown and its DropdownContent. Define each component’s declarations for all props it consumes, including the explicitly named callbacks and data props, using the appropriate requiredness and collection/function types.Source: Coding guidelines
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift
SiteFilterDropdown.jsandTagFilterDropdown.jsare near-duplicate implementations. Both files reimplement the same popover/search/checkbox/apply-clear structure differing only by "site" vs "tag" naming; this duplication is likely why thetrackMetricstub diverged between them (see the metrics-disabled comment above).
app/pages/clinicworkspace/components/SiteFilterDropdown.js#L56-209: extract the shared searchable-checkbox-dropdown logic (search box, sorted/filtered options, zero-selection sentinel handling, clear/apply actions) into a reusable hook or generic component parameterized by entity type.app/pages/clinicworkspace/components/TagFilterDropdown.js#L56-205: consume the same shared abstraction instead of duplicating the logic.🤖 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/SiteFilterDropdown.js` at line 1, Extract the duplicated searchable checkbox popover behavior from SiteFilterDropdown and TagFilterDropdown into one reusable hook or generic component, parameterized for site and tag labels, options, selection state, and callbacks. Move shared search, sorting/filtering, zero-selection sentinel handling, and clear/apply actions into that abstraction, then update both dropdowns to consume it while preserving their entity-specific behavior and metric handling.app/pages/clinicworkspace/FilterByTags.js (1)
11-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
trackMetricstub silently drops the "Edit clinic tags open" event; call order also deviates from convention.Same pattern as
FilterBySites.js:trackMetricis a localnoopstub (TODO: FIX), and the call precedes thedispatch(actions.async.fetchClinicPatientTags(...))rather than following it. (See consolidated comment.)🤖 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/FilterByTags.js` around lines 11 - 33, Replace the local noop trackMetric stub in FilterByTags with the established metric-tracking implementation used by FilterBySites, and reorder handleClickEditTags so dispatch(actions.async.fetchClinicPatientTags(...)) runs before recording the “Clinic - Population Health - Edit clinic tags open” event.Source: Learnings
app/pages/clinicworkspace/FilterBySites.js (1)
11-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
trackMetricstub silently drops the "Edit clinic sites open" event; call order also deviates from convention.
trackMetricis a localnoopstub (TODO: FIX), so this metric is never recorded — unlike the sibling dropdown files in this PR that import the realtrackMetricfromcore/metricUtils. Separately, the call precedesdispatch(actions.async.fetchClinicSites(...)), whereas the established convention placestrackMetricimmediately after dispatching the related action. (See consolidated comment; this repeats identically inFilterByTags.js.)🤖 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/FilterBySites.js` around lines 11 - 33, Replace the local trackMetric noop in FilterBySites with the real trackMetric import from core/metricUtils, and move the “Edit clinic sites open” tracking call in handleClickEditSites to immediately after dispatch(actions.async.fetchClinicSites(...)). Apply the same changes to the corresponding FilterByTags implementation.Source: Learnings
app/pages/clinicworkspace/FilterByCGMUse.js (1)
6-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNone of the new filter adapters/dropdowns declare PropTypes. As per coding guidelines, "Define PropTypes for all component props," but every new component in this cohort omits them.
app/pages/clinicworkspace/FilterByCGMUse.js#L6-L9: add PropTypes foractiveFilters,setActiveFilters.app/pages/clinicworkspace/FilterByDataRecency.js#L6-L9: add PropTypes foractiveFilters,setActiveFilters.app/pages/clinicworkspace/FilterBySites.js#L13-L18: add PropTypes forapi,activeFilters,setActiveFilters,setShowClinicSitesDialog.app/pages/clinicworkspace/FilterBySummaryPeriod.js#L6-L9: add PropTypes foractiveSummaryPeriod,setActiveSummaryPeriod.app/pages/clinicworkspace/FilterByTags.js#L13-L18: add PropTypes forapi,activeFilters,setActiveFilters,setShowClinicPatientTagsDialog.app/pages/clinicworkspace/components/CGMUseFilterDropdown.js#L26-L30: add PropTypes forDropdownContent'sonClose,onChange,timeCGMUsePercent.app/pages/clinicworkspace/components/CGMUseFilterDropdown.js#L99-L102: add PropTypes forCGMUseFilterDropdown'sonChange,timeCGMUsePercent.app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L26-L31: add PropTypes forDropdownContent'sonClose,onChange,lastData,lastDataType.app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L131-L135: add PropTypes forDataRecencyFilterDropdown'sonChange,lastData,lastDataType.🤖 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/FilterByCGMUse.js` around lines 6 - 9, Declare PropTypes for every listed component and nested DropdownContent function, matching each component's existing props: activeFilters/setActiveFilters in FilterByCGMUse and FilterByDataRecency; api/activeFilters/setActiveFilters/setShowClinicSitesDialog in FilterBySites; activeSummaryPeriod/setActiveSummaryPeriod in FilterBySummaryPeriod; api/activeFilters/setActiveFilters/setShowClinicPatientTagsDialog in FilterByTags; onClose/onChange/timeCGMUsePercent for CGMUseFilterDropdown's DropdownContent and onChange/timeCGMUsePercent for CGMUseFilterDropdown; and onClose/onChange/lastData/lastDataType for DataRecencyFilterDropdown's DropdownContent and onChange/lastData/lastDataType for DataRecencyFilterDropdown. Use the appropriate PropTypes validators and preserve the existing defaults and 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.
Inline comments:
In `@app/pages/clinicworkspace/AppliedFiltersAdapter.js`:
- Around line 13-44: Add a timeCGMUsePercent case to handleRemoveFilter,
resetting activeFilters.timeCGMUsePercent to
defaultFilterState.timeCGMUsePercent so removing the CGM Use chip clears its
value.
In `@app/pages/clinicworkspace/ClinicPatients.js`:
- Around line 3233-3249: Update the dependency array of renderPeopleTable’s
useCallback to include patientListSearchTextInput, handleClearSearch, and
handleResetFilters, matching the values used by patientQueryState and the
rendered ClearFilterButtons/EmptyContentNode. Preserve the existing dependencies
and ensure both affected JSX paths stay synchronized with current search and
filter state.
In `@app/pages/clinicworkspace/components/AppliedFilters.js`:
- Around line 166-171: Declare PropTypes for all props in AppliedFilters,
including its internal ChipGroup and Chip components; define activeFilters,
setActiveFilters, and rightContent PropTypes in AppliedFiltersAdapter using the
existing PropTypes import; and add patientQueryState, onClearSearch, and
onResetFilters PropTypes to ClearFilterButtons. Use the specified types and
preserve the current component behavior.
In `@app/pages/clinicworkspace/components/CGMUseFilterDropdown.js`:
- Line 19: The prefixPopHealthMetric function must return the provided metric
name as a string rather than the noop function. Update prefixPopHealthMetric in
app/pages/clinicworkspace/components/CGMUseFilterDropdown.js:19 and
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js:24, and apply
the same correction to its usages in TimeInRangeFilterDropdown.js and
SummaryPeriodFilterDropdown.js, preserving the existing trackMetric calls.
In `@app/pages/clinicworkspace/components/SiteFilterDropdown.js`:
- Line 1: Replace the local no-op trackMetric stubs in SiteFilterDropdown and
TagFilterDropdown with imports of the real trackMetric implementation from the
established metricUtils module, preserving all existing metric call sites for
open, close, clear, and apply actions.
- Around line 33-34: Replace the local no-op trackMetric declaration in
SiteFilterDropdown with the real trackMetric import from core/metricUtils,
matching SummaryPeriodFilterDropdown and TimeInRangeFilterDropdown. Preserve
prefixPopHealthMetric as-is unless it is also provided by that module, and leave
the existing open, close, clear, and apply tracking calls unchanged.
In `@app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js`:
- Line 1: Update the prefixPopHealthMetric helper in both
SummaryPeriodFilterDropdown and TimeInRangeFilterDropdown so it accepts a
metric-name argument and returns the correctly prefixed string instead of the
noop function reference. Preserve the existing prefix format expected by
trackMetric calls.
- Line 19: Replace the placeholder implementation of prefixPopHealthMetric so it
accepts the metric-name string and returns the intended prefixed event-name
string. Preserve the existing trackMetric call sites and ensure they pass
strings rather than the noop function reference.
In `@app/pages/clinicworkspace/components/TagFilterDropdown.js`:
- Around line 33-34: Replace the no-op trackMetric definition in
TagFilterDropdown with the real tracking utility import and use it for the
existing tag-filter interaction events, including open, close, clear, and apply.
Keep prefixPopHealthMetric unchanged unless required by the established tracking
integration.
- Around line 139-158: Update the zero-tags option Box in the tag filter
dropdown to use the tag-specific className and key naming convention, replacing
the leftover clinic-site identifiers while preserving the existing styling,
rendering condition, and checkbox behavior.
In `@app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js`:
- Line 29: Fix prefixPopHealthMetric in TimeInRangeFilterDropdown.js so it
returns the provided metric name string rather than the noop function reference.
Match the working implementation in SummaryPeriodFilterDropdown.js and preserve
the existing trackMetric calls.
---
Nitpick comments:
In `@app/pages/clinicworkspace/components/SiteFilterDropdown.js`:
- Line 1: Add PropTypes declarations for every listed new component:
SiteFilterDropdown and its DropdownContent, FilterByTimeInRange,
SummaryPeriodFilterDropdown and its DropdownContent, TagFilterDropdown and its
DropdownContent, and TimeInRangeFilterDropdown and its DropdownContent. Define
each component’s declarations for all props it consumes, including the
explicitly named callbacks and data props, using the appropriate requiredness
and collection/function types.
- Line 1: Extract the duplicated searchable checkbox popover behavior from
SiteFilterDropdown and TagFilterDropdown into one reusable hook or generic
component, parameterized for site and tag labels, options, selection state, and
callbacks. Move shared search, sorting/filtering, zero-selection sentinel
handling, and clear/apply actions into that abstraction, then update both
dropdowns to consume it while preserving their entity-specific behavior and
metric handling.
In `@app/pages/clinicworkspace/FilterByCGMUse.js`:
- Around line 6-9: Declare PropTypes for every listed component and nested
DropdownContent function, matching each component's existing props:
activeFilters/setActiveFilters in FilterByCGMUse and FilterByDataRecency;
api/activeFilters/setActiveFilters/setShowClinicSitesDialog in FilterBySites;
activeSummaryPeriod/setActiveSummaryPeriod in FilterBySummaryPeriod;
api/activeFilters/setActiveFilters/setShowClinicPatientTagsDialog in
FilterByTags; onClose/onChange/timeCGMUsePercent for CGMUseFilterDropdown's
DropdownContent and onChange/timeCGMUsePercent for CGMUseFilterDropdown; and
onClose/onChange/lastData/lastDataType for DataRecencyFilterDropdown's
DropdownContent and onChange/lastData/lastDataType for
DataRecencyFilterDropdown. Use the appropriate PropTypes validators and preserve
the existing defaults and behavior.
In `@app/pages/clinicworkspace/FilterBySites.js`:
- Around line 11-33: Replace the local trackMetric noop in FilterBySites with
the real trackMetric import from core/metricUtils, and move the “Edit clinic
sites open” tracking call in handleClickEditSites to immediately after
dispatch(actions.async.fetchClinicSites(...)). Apply the same changes to the
corresponding FilterByTags implementation.
In `@app/pages/clinicworkspace/FilterByTags.js`:
- Around line 11-33: Replace the local noop trackMetric stub in FilterByTags
with the established metric-tracking implementation used by FilterBySites, and
reorder handleClickEditTags so
dispatch(actions.async.fetchClinicPatientTags(...)) runs before recording the
“Clinic - Population Health - Edit clinic tags open” event.
In `@app/pages/clinicworkspace/useIsClinicAdmin.js`:
- Around line 1-4: Group the imports in useIsClinicAdmin by adding blank lines
according to the project order: keep React first, separate the Redux import, and
separate the Lodash imports as their own group.
🪄 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: 8814d320-6d27-4896-8edd-a866bacdd540
⛔ Files ignored due to path filters (1)
app/core/icons/tagIcon.svgis excluded by!**/*.svg
📒 Files selected for processing (19)
app/pages/clinicworkspace/AppliedFiltersAdapter.jsapp/pages/clinicworkspace/ClinicPatients.jsapp/pages/clinicworkspace/FilterByCGMUse.jsapp/pages/clinicworkspace/FilterByDataRecency.jsapp/pages/clinicworkspace/FilterBySites.jsapp/pages/clinicworkspace/FilterBySummaryPeriod.jsapp/pages/clinicworkspace/FilterByTags.jsapp/pages/clinicworkspace/FilterByTimeInRange.jsapp/pages/clinicworkspace/components/AppliedFilters.jsapp/pages/clinicworkspace/components/CGMUseFilterDropdown.jsapp/pages/clinicworkspace/components/ClearFilterButtons.jsapp/pages/clinicworkspace/components/DataRecencyFilterDropdown.jsapp/pages/clinicworkspace/components/SiteFilterDropdown.jsapp/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.jsapp/pages/clinicworkspace/components/TagFilterDropdown.jsapp/pages/clinicworkspace/components/TimeInRangeFilterDropdown.jsapp/pages/clinicworkspace/useClinicPatientsFilters.jsapp/pages/clinicworkspace/useIsClinicAdmin.jslocales/en/translation.json
🛑 Comments failed to post (11)
app/pages/clinicworkspace/AppliedFiltersAdapter.js (1)
13-44: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
git ls-files 'app/pages/clinicworkspace/*' 'app/pages/clinicworkspace/**' 'components/*' 'hooks/*' | sed -n '1,200p'Repository: tidepool-org/blip
Length of output: 1351
🏁 Script executed:
python3 - <<'PY' import os, subprocess, textwrap, json, sys paths = [ 'app/pages/clinicworkspace/AppliedFiltersAdapter.js', 'components/AppliedFilters.js', 'app/pages/clinicworkspace/useClinicPatientsFilters.js', ] for p in paths: print(f'=== {p} ===') if os.path.exists(p): print(subprocess.check_output(['bash','-lc',f'wc -l < {p}']).decode().strip(), 'lines') print(subprocess.check_output(['bash','-lc',f'ast-grep outline {p} --view expanded'], stderr=subprocess.STDOUT).decode()) else: print('missing') PYRepository: tidepool-org/blip
Length of output: 653
🏁 Script executed:
sed -n '1,220p' app/pages/clinicworkspace/AppliedFiltersAdapter.js printf '\n---\n' sed -n '1,240p' components/AppliedFilters.js printf '\n---\n' sed -n '1,260p' app/pages/clinicworkspace/useClinicPatientsFilters.jsRepository: tidepool-org/blip
Length of output: 2787
🏁 Script executed:
rg -n "timeCGMUsePercent|defaultFilterState|handleRemoveFilter|onRemoveFilter|usePrimaryChips|AppliedFilters" app/pages/clinicworkspace components -SRepository: tidepool-org/blip
Length of output: 5386
🏁 Script executed:
python3 - <<'PY' import os, subprocess, textwrap, json, sys checks = [ "grep -RIn --line-number -E 'timeCGMUsePercent|defaultFilterState|handleRemoveFilter|onRemoveFilter|usePrimaryChips|AppliedFilters' app/pages/clinicworkspace components || true", ] for cmd in checks: print(subprocess.check_output(['bash','-lc',cmd]).decode()) PYRepository: tidepool-org/blip
Length of output: 5380
Add
timeCGMUsePercenthandling inapp/pages/clinicworkspace/AppliedFiltersAdapter.js:13-44.The CGM Use chip emits
type: 'timeCGMUsePercent', but this switch has no matching case, so removing that chip is a no-op. Reset it todefaultFilterState.timeCGMUsePercent(null).🤖 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 - 44, Add a timeCGMUsePercent case to handleRemoveFilter, resetting activeFilters.timeCGMUsePercent to defaultFilterState.timeCGMUsePercent so removing the CGM Use chip clears its value.app/pages/clinicworkspace/ClinicPatients.js (1)
3233-3249: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing dependencies in
renderPeopleTable'suseCallback.
patientQueryStatenow depends onpatientListSearchTextInput(line 3233), and the JSX useshandleClearSearch/handleResetFilters(3245-3246, 3266-3267), but the dependency array only gainedactiveFiltersandsetActiveFilters. Until some other listed dependency changes (e.g.patientFetchOptionsafter the search debounce fires), the memoized callback keeps a stalepatientQueryState, soClearFilterButtons/EmptyContentNodecopy can lag behind the actual search/filter state while typing.🐛 Proposed fix
}, [ activeFilters, clinic?.fetchedPatientCount, columns, data, defaultPatientFetchOptions.sort, + handleClearSearch, handlePageChange, + handleResetFilters, handleSortChange, loading, patientFetchOptions, + patientListSearchTextInput, setActiveFilters, showSummaryData, tableStyle, ]);Note
handleClearSearch/handleResetFiltersare plain function declarations (notuseCallback-wrapped), so adding them will make this memoization recompute every render; if that regresses perf, memoizing those handlers is the more complete fix — but that's a pre-existing pattern beyond this diff's scope.Also applies to: 3288-3301
🤖 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 3233 - 3249, Update the dependency array of renderPeopleTable’s useCallback to include patientListSearchTextInput, handleClearSearch, and handleResetFilters, matching the values used by patientQueryState and the rendered ClearFilterButtons/EmptyContentNode. Preserve the existing dependencies and ensure both affected JSX paths stay synchronized with current search and filter state.Source: Coding guidelines
app/pages/clinicworkspace/components/AppliedFilters.js (1)
166-171: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Missing PropTypes across all three new filter-chip/query-state components. None of the newly extracted components declare PropTypes for their props, contrary to the coding guideline "Define PropTypes for all component props."
app/pages/clinicworkspace/components/AppliedFilters.js#L166-L171: add PropTypes forAppliedFilters(filters: PropTypes.object,hasSearchActive: PropTypes.bool,onRemoveFilter: PropTypes.func,rightContent: PropTypes.node) and for internalChipGroup/Chip.app/pages/clinicworkspace/AppliedFiltersAdapter.js#L1-L10: add PropTypes foractiveFilters,setActiveFilters,rightContent(the already-imported but unusedPropTypesmodule suggests this was simply forgotten).app/pages/clinicworkspace/components/ClearFilterButtons.js#L25-L25: add PropTypes forpatientQueryState,onClearSearch,onResetFilters.📍 Affects 3 files
app/pages/clinicworkspace/components/AppliedFilters.js#L166-L171(this comment)app/pages/clinicworkspace/AppliedFiltersAdapter.js#L1-L10app/pages/clinicworkspace/components/ClearFilterButtons.js#L25-L25🤖 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/AppliedFilters.js` around lines 166 - 171, Declare PropTypes for all props in AppliedFilters, including its internal ChipGroup and Chip components; define activeFilters, setActiveFilters, and rightContent PropTypes in AppliedFiltersAdapter using the existing PropTypes import; and add patientQueryState, onClearSearch, and onResetFilters PropTypes to ClearFilterButtons. Use the specified types and preserve the current component behavior.Source: Coding guidelines
app/pages/clinicworkspace/components/CGMUseFilterDropdown.js (1)
19-19: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
git ls-files app/pages/clinicworkspace/components/CGMUseFilterDropdown.js app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js core/metricUtils* core/*metric*Repository: tidepool-org/blip
Length of output: 282
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== CGMUseFilterDropdown outline ==\n' ast-grep outline app/pages/clinicworkspace/components/CGMUseFilterDropdown.js --view expanded || true printf '\n== DataRecencyFilterDropdown outline ==\n' ast-grep outline app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js --view expanded || true printf '\n== Search prefixPopHealthMetric and trackMetric ==\n' rg -n "prefixPopHealthMetric|trackMetric" app/pages/clinicworkspace/components core -S printf '\n== metricUtils files ==\n' git ls-files core | rg 'metricUtils|metric'Repository: tidepool-org/blip
Length of output: 6366
🏁 Script executed:
#!/bin/bash set -euo pipefail for f in app/pages/clinicworkspace/components/CGMUseFilterDropdown.js app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js; do echo "===== $f =====" nl -ba "$f" | sed -n '1,240p' done echo "===== core metric utils candidates =====" git ls-files core | rg 'metricUtils|metric'Repository: tidepool-org/blip
Length of output: 268
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n "function trackMetric|const trackMetric|export .*trackMetric|trackMetric\s*=" core app -SRepository: tidepool-org/blip
Length of output: 8511
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "===== CGMUseFilterDropdown =====" nl -ba app/pages/clinicworkspace/components/CGMUseFilterDropdown.js | sed -n '1,220p' echo "===== DataRecencyFilterDropdown =====" nl -ba app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js | sed -n '1,220p' echo "===== trackMetric definition =====" rg -n "trackMetric" app core -SRepository: tidepool-org/blip
Length of output: 228
🏁 Script executed:
#!/bin/bash set -euo pipefail nl -ba core/metricUtils.js 2>/dev/null | sed -n '1,220p' || true nl -ba core/metricUtils/index.js 2>/dev/null | sed -n '1,220p' || true rg -n "export .*trackMetric|function trackMetric|const trackMetric" core -SRepository: tidepool-org/blip
Length of output: 200
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n -A4 -B4 "function trackMetric|const trackMetric|trackMetric\s*=" core app -SRepository: tidepool-org/blip
Length of output: 50373
🏁 Script executed:
#!/bin/bash set -euo pipefail for f in app/core/metricUtils.js app/bootstrap.js app/pages/clinicworkspace/components/CGMUseFilterDropdown.js app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js; do echo "===== $f =====" sed -n '1,120p' "$f" echo doneRepository: tidepool-org/blip
Length of output: 16372
Fix
prefixPopHealthMetricto return a string
prefixPopHealthMetricignores its argument and returnsnoop, so thetrackMetric(prefixPopHealthMetric('...'), ...)calls inapp/pages/clinicworkspace/components/CGMUseFilterDropdown.js,app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js,app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js, andapp/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.jssend a function as the event name instead of a metric string.📍 Affects 2 files
app/pages/clinicworkspace/components/CGMUseFilterDropdown.js#L19-L19(this comment)app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L24-L24🤖 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/CGMUseFilterDropdown.js` at line 19, The prefixPopHealthMetric function must return the provided metric name as a string rather than the noop function. Update prefixPopHealthMetric in app/pages/clinicworkspace/components/CGMUseFilterDropdown.js:19 and app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js:24, and apply the same correction to its usages in TimeInRangeFilterDropdown.js and SummaryPeriodFilterDropdown.js, preserving the existing trackMetric calls.app/pages/clinicworkspace/components/SiteFilterDropdown.js (2)
1-1: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Metrics silently disabled for clinic-sites and patient-tags filters. Both files locally stub
trackMetricas a no-op instead of importing the real implementation (asSummaryPeriodFilterDropdown.jsandTimeInRangeFilterDropdown.jscorrectly do), so every open/close/clear/apply metric call is dropped.
app/pages/clinicworkspace/components/SiteFilterDropdown.js#L33-34: replace the localtrackMetricstub withimport { trackMetric } from '../../../core/metricUtils';.app/pages/clinicworkspace/components/TagFilterDropdown.js#L33-34: replace the localtrackMetricstub withimport { trackMetric } from '../../../core/metricUtils';.🤖 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/SiteFilterDropdown.js` at line 1, Replace the local no-op trackMetric stubs in SiteFilterDropdown and TagFilterDropdown with imports of the real trackMetric implementation from the established metricUtils module, preserving all existing metric call sites for open, close, clear, and apply actions.
33-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
trackMetricis stubbed as a no-op — clinic sites filter interactions are not tracked.Unlike
SummaryPeriodFilterDropdown.jsandTimeInRangeFilterDropdown.js, which import the realtrackMetricfromcore/metricUtils, this file locally stubs bothtrackMetricandprefixPopHealthMetricas no-ops. EverytrackMetric(...)call below (open/close/clear/apply) silently does nothing.🐛 Proposed fix
-const trackMetric = () => noop; -const prefixPopHealthMetric = () => noop; +import { trackMetric } from '../../../core/metricUtils'; + +const prefixPopHealthMetric = () => noop; // TODO: FIX🤖 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/SiteFilterDropdown.js` around lines 33 - 34, Replace the local no-op trackMetric declaration in SiteFilterDropdown with the real trackMetric import from core/metricUtils, matching SummaryPeriodFilterDropdown and TimeInRangeFilterDropdown. Preserve prefixPopHealthMetric as-is unless it is also provided by that module, and leave the existing open, close, clear, and apply tracking calls unchanged.app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js (2)
1-1: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
prefixPopHealthMetricTODO stub passes a function instead of a string totrackMetric. Both files defineprefixPopHealthMetric = () => noop, discarding the metric-name argument, so every realtrackMetric(prefixPopHealthMetric('...'), {...})call sends thenoopfunction reference as the event name.
app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L19-19: implementprefixPopHealthMetricto actually prefix and return the passed-in metric name string.app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js#L29-29: same fix, implementprefixPopHealthMetricto return the prefixed string.🤖 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/SummaryPeriodFilterDropdown.js` at line 1, Update the prefixPopHealthMetric helper in both SummaryPeriodFilterDropdown and TimeInRangeFilterDropdown so it accepts a metric-name argument and returns the correctly prefixed string instead of the noop function reference. Preserve the existing prefix format expected by trackMetric calls.
19-19: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Address the
prefixPopHealthMetricTODO — it currently breakstrackMetricevent names.
prefixPopHealthMetricignores its string argument and returnsnoop, so everytrackMetric(prefixPopHealthMetric('...'), {...})call passes a function reference instead of the intended prefixed event-name string to the realtrackMetric.🤖 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/SummaryPeriodFilterDropdown.js` at line 19, Replace the placeholder implementation of prefixPopHealthMetric so it accepts the metric-name string and returns the intended prefixed event-name string. Preserve the existing trackMetric call sites and ensure they pass strings rather than the noop function reference.app/pages/clinicworkspace/components/TagFilterDropdown.js (2)
33-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
trackMetricis stubbed as a no-op — patient tag filter interactions are not tracked.Same issue as
SiteFilterDropdown.js: the realtrackMetricis never imported here, so open/close/clear/apply events for tag filtering are silently dropped.🤖 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 33 - 34, Replace the no-op trackMetric definition in TagFilterDropdown with the real tracking utility import and use it for the existing tag-filter interaction events, including open, close, clear, and apply. Keep prefixPopHealthMetric unchanged unless required by the established tracking integration.
139-158: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Leftover "clinic-site" className/key on the tags "zero" option.
This
<Box>renders the "Patients without any tags" option but keepsclassName="clinic-site-filter-option"andkey="clinic-site-filter-option-PWDS_WITH_ZERO_TAGS"copied from the Sites dropdown, while the siblingid/data-testidcorrectly use "tag-filter-option". Any styling or selectors targeting.tag-filter-optionwill miss this element.🐛 Proposed fix
- <Box mt={1} pt={3} px={2} sx={{ borderTop: borders.divider }} className="clinic-site-filter-option" key="clinic-site-filter-option-PWDS_WITH_ZERO_TAGS"> + <Box mt={1} pt={3} px={2} sx={{ borderTop: borders.divider }} className="tag-filter-option" key="tag-filter-option-PWDS_WITH_ZERO_TAGS">📝 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.{ // Display an option to filter for patients with zero tags sortedTagFilterOptions.length > 0 && <Box mt={1} pt={3} px={2} sx={{ borderTop: borders.divider }} className="tag-filter-option" key="tag-filter-option-PWDS_WITH_ZERO_TAGS"> <Checkbox id="tag-filter-option-checkbox-PWDS_WITH_ZERO_TAGS" data-testid="tag-filter-option-checkbox-PWDS_WITH_ZERO_TAGS" label={<Text sx={{ fontSize: 0, fontWeight: 'normal', fontStyle: 'italic' }}> {t('Patients without any tags')} </Text>} checked={isFilteringForZeroTags} onChange={() => { if (isFilteringForZeroTags) { setPendingTags([]); } else { setPendingTags(SPECIAL_FILTER_STATES.ZERO_TAGS); } }} /> </Box> }🤖 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 139 - 158, Update the zero-tags option Box in the tag filter dropdown to use the tag-specific className and key naming convention, replacing the leftover clinic-site identifiers while preserving the existing styling, rendering condition, and checkbox behavior.app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js (1)
29-29: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Same
prefixPopHealthMetricTODO bug asSummaryPeriodFilterDropdown.js.
prefixPopHealthMetricdiscards its argument and returnsnoop, sotrackMetriccalls below receive a function reference instead of the metric name string.🤖 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/TimeInRangeFilterDropdown.js` at line 29, Fix prefixPopHealthMetric in TimeInRangeFilterDropdown.js so it returns the provided metric name string rather than the noop function reference. Match the working implementation in SummaryPeriodFilterDropdown.js and preserve the existing trackMetric calls.
|
/deploy qa2 |
|
henry-tp updated values.yaml file in qa2 |
|
henry-tp updated flux policies file in qa2 |
|
henry-tp deployed blip WEB-4654-cgm-use branch to qa2 namespace |
|
/deploy qa5 |
|
henry-tp updated values.yaml file in qa5 |
|
henry-tp updated flux policies file in qa5 |
|
henry-tp deployed blip WEB-4654-cgm-use branch to qa5 namespace |
2 participants
WEB-4654