WEB-4654 - Time in Range#1990
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 split into reusable dropdown and adapter components. Query-state handling, clear/reset controls, applied-filter chips, clinic-admin actions, empty states, and related translations are integrated into ChangesClinic patient filtering
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ClinicPatients
participant FilterByTags
participant TagFilterDropdown
participant AppliedFiltersAdapter
participant AppliedFilters
ClinicPatients->>FilterByTags: render active filter control
FilterByTags->>TagFilterDropdown: pass patientTags and onChange
TagFilterDropdown->>FilterByTags: return applied tag selection
FilterByTags->>ClinicPatients: update activeFilters
ClinicPatients->>AppliedFiltersAdapter: pass activeFilters and setActiveFilters
AppliedFiltersAdapter->>AppliedFilters: render removable filter chips
AppliedFilters->>AppliedFiltersAdapter: report chip removal
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
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)
3379-3392: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd the missing
renderPeopleTabledependencies.patientListSearchTextInput,handleClearSearch,handleResetFilters, andhandleClickPatientare all read inside the callback; leaving them out keeps the empty state and actions stale when search/filter state 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 3379 - 3392, Update the dependency array for the renderPeopleTable callback to include patientListSearchTextInput, handleClearSearch, handleResetFilters, and handleClickPatient, preserving the existing dependencies so empty-state content and patient actions stay current.Source: Coding guidelines
🧹 Nitpick comments (4)
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js (1)
1-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply the repository’s required import grouping.
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L1-L22: place third-party imports before Lodash, then theme-ui, then local imports.app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L1-L17: apply the same grouping.app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js#L1-L25: apply the same grouping.app/pages/clinicworkspace/components/AppliedFilters.js#L1-L17: move Lodash before theme-ui and local imports.app/pages/clinicworkspace/AppliedFiltersAdapter.js#L1-L8: order PropTypes before Redux and theme-ui before local imports.As per coding guidelines, imports must be grouped as React, PropTypes, Redux, third-party libraries, Lodash, theme-ui, then local imports.
🤖 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` around lines 1 - 22, Reorder imports in app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js (lines 1-22), SummaryPeriodFilterDropdown.js (lines 1-17), and TimeInRangeFilterDropdown.js (lines 1-25) as React, PropTypes if present, Redux, third-party libraries, Lodash, theme-ui, then local imports. Apply the same grouping in app/pages/clinicworkspace/components/AppliedFilters.js (lines 1-17), moving Lodash before theme-ui and local imports, and in app/pages/clinicworkspace/AppliedFiltersAdapter.js (lines 1-8), placing PropTypes before Redux and theme-ui before local imports.Source: Coding guidelines
app/pages/clinicworkspace/FilterByDataRecency.js (1)
6-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefine PropTypes for the new component contracts.
app/pages/clinicworkspace/FilterByDataRecency.js#L6-L9: typeactiveFiltersandsetActiveFilters.app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L26-L31: typeDropdownContentprops.app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L131-L135: type exported dropdown props.app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L32-L36: typeDropdownContentprops.app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L111-L114: type exported dropdown props.app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js#L87-L91: typeDropdownContentprops.app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js#L262-L265: type exported dropdown props.app/pages/clinicworkspace/components/AppliedFilters.js#L104-L171: typeChip,ChipGroup, andAppliedFilters.app/pages/clinicworkspace/AppliedFiltersAdapter.js#L10-L10: use the existingPropTypesimport to type adapter props.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/FilterByDataRecency.js` around lines 6 - 9, Define PropTypes for every component contract listed: type activeFilters and setActiveFilters on FilterByDataRecency; type DropdownContent and exported dropdown props in DataRecencyFilterDropdown, SummaryPeriodFilterDropdown, and TimeInRangeFilterDropdown; type Chip, ChipGroup, and AppliedFilters in AppliedFilters; and use the existing PropTypes import to type AppliedFiltersAdapter props. Name each component’s actual prop shapes and callback types, preserving existing defaults and behavior across all specified files.Source: Coding guidelines
app/pages/clinicworkspace/FilterBySites.js (2)
1-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply the repository import grouping consistently.
app/pages/clinicworkspace/FilterBySites.js#L1-L9: move local actions after Lodash.app/pages/clinicworkspace/FilterByTags.js#L1-L9: move local actions after Lodash.app/pages/clinicworkspace/components/TagFilterDropdown.js#L1-L39: group React, Redux, third-party, Lodash, theme-ui, then local imports.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L1-L39: apply the same grouping and move all imports above declarations.As per coding guidelines, imports must follow the prescribed grouping order.
🤖 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 1 - 9, Reorder imports to follow the repository grouping convention: in app/pages/clinicworkspace/FilterBySites.js and app/pages/clinicworkspace/FilterByTags.js, place local actions after Lodash imports; in app/pages/clinicworkspace/components/TagFilterDropdown.js and app/pages/clinicworkspace/components/SiteFilterDropdown.js, order React, Redux, third-party, Lodash, theme-ui, then local imports. Move every import in SiteFilterDropdown.js above declarations. No direct changes are required beyond these four files: app/pages/clinicworkspace/FilterBySites.js:1-9, app/pages/clinicworkspace/FilterByTags.js:1-9, app/pages/clinicworkspace/components/TagFilterDropdown.js:1-39, and app/pages/clinicworkspace/components/SiteFilterDropdown.js:1-39.Source: Coding guidelines
13-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd PropTypes for every new component.
app/pages/clinicworkspace/FilterBySites.js#L13-L18: define the wrapper’s four props.app/pages/clinicworkspace/FilterBySummaryPeriod.js#L6-L9: define both summary-period props.app/pages/clinicworkspace/FilterByTimeInRange.js#L6-L9: define both filter-state props.app/pages/clinicworkspace/FilterByTags.js#L13-L18: define the wrapper’s four props.app/pages/clinicworkspace/components/TagFilterDropdown.js#L41-L61: define props forEditTagsActionandDropdownContent.app/pages/clinicworkspace/components/TagFilterDropdown.js#L214-L218: defineTagFilterDropdownprops.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L41-L61: define props forEditSitesActionandDropdownContent.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L218-L222: defineSiteFilterDropdownprops.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/FilterBySites.js` around lines 13 - 18, Define PropTypes for every component prop at the listed sites: FilterBySites.js lines 13-18, FilterBySummaryPeriod.js lines 6-9, FilterByTimeInRange.js lines 6-9, and FilterByTags.js lines 13-18; add prop definitions for EditTagsAction and DropdownContent plus TagFilterDropdown in TagFilterDropdown.js lines 41-61 and 214-218; and add definitions for EditSitesAction and DropdownContent plus SiteFilterDropdown in SiteFilterDropdown.js lines 41-61 and 218-222, matching each component’s existing prop usage and defaults.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: Update handleRemoveFilter to add a timeCGMUsePercent case
that removes the selected value from activeFilters.timeCGMUsePercent using the
same pattern as the other multi-value filters, then updates activeFilters via
setActiveFilters.
In `@app/pages/clinicworkspace/components/AppliedFilters.js`:
- Around line 48-54: Update the timeInTargetPercent label in the AppliedFilters
label mapping to describe patients below the 70% target rather than “Meeting
Targets,” while leaving the other time-in-range labels unchanged.
- Around line 119-133: Add a :focus-within selector alongside the existing hover
selector for .remove-filter-icon in the AppliedFilters styling, applying the
same expansion, spacing, and opacity rules so keyboard-focused remove controls
become visible.
In `@app/pages/clinicworkspace/components/ClearFilterButtons.js`:
- Line 25: Add PropTypes declarations for the ClearFilterButtons component’s
patientQueryState, onClearSearch, and onResetFilters props in
app/pages/clinicworkspace/components/ClearFilterButtons.js at line 25. Also add
PropTypes declarations for ClinicPatients’ patientQueryState and children props
in app/pages/clinicworkspace/ClinicPatients.js at line 200, using appropriate
requiredness and types consistent with their usage.
In `@app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js`:
- Around line 157-164: Translate all new user-facing accessibility labels using
react-i18next: in
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js lines 157-164,
wrap the last-data trigger label with t; in
app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js lines
136-143, wrap the summary-period icon label with t; and in
app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js lines 287-303,
translate both the trigger icon label and pill label, using the existing
component translation hook or pattern.
- Line 24: The placeholder prefixPopHealthMetric helper must return the
clinic-workspace event-name prefix string rather than noop. Update
prefixPopHealthMetric in
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js:24 using the
existing prefix helper pattern, and apply the same correction to
app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js:19 and
app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js:29 so each
trackMetric call receives an event-name string.
In `@app/pages/clinicworkspace/components/TagFilterDropdown.js`:
- Around line 241-268: Update the patient tag filter trigger in
app/pages/clinicworkspace/components/TagFilterDropdown.js lines 241-268 to pass
the iconLabel and Pill label through the existing react-i18next t function.
Apply the corresponding translation updates to the site filter trigger in
app/pages/clinicworkspace/components/SiteFilterDropdown.js lines 245-271,
translating both accessibility labels while preserving their existing meaning.
In `@app/pages/clinicworkspace/FilterBySites.js`:
- Around line 11-32: Restore the shared metrics integration across all affected
sites: in app/pages/clinicworkspace/FilterBySites.js lines 11-32 and
app/pages/clinicworkspace/FilterByTags.js lines 11-32, replace the noop
trackMetric helper with the real helper and place the past-tense metric
immediately after the respective async site/tag fetch dispatch; in
app/pages/clinicworkspace/components/TagFilterDropdown.js lines 33-34 and
app/pages/clinicworkspace/components/SiteFilterDropdown.js lines 33-34, import
the real metric and population-health prefix helpers.
---
Outside diff comments:
In `@app/pages/clinicworkspace/ClinicPatients.js`:
- Around line 3379-3392: Update the dependency array for the renderPeopleTable
callback to include patientListSearchTextInput, handleClearSearch,
handleResetFilters, and handleClickPatient, preserving the existing dependencies
so empty-state content and patient actions stay current.
---
Nitpick comments:
In `@app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js`:
- Around line 1-22: Reorder imports in
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js (lines 1-22),
SummaryPeriodFilterDropdown.js (lines 1-17), and TimeInRangeFilterDropdown.js
(lines 1-25) as React, PropTypes if present, Redux, third-party libraries,
Lodash, theme-ui, then local imports. Apply the same grouping in
app/pages/clinicworkspace/components/AppliedFilters.js (lines 1-17), moving
Lodash before theme-ui and local imports, and in
app/pages/clinicworkspace/AppliedFiltersAdapter.js (lines 1-8), placing
PropTypes before Redux and theme-ui before local imports.
In `@app/pages/clinicworkspace/FilterByDataRecency.js`:
- Around line 6-9: Define PropTypes for every component contract listed: type
activeFilters and setActiveFilters on FilterByDataRecency; type DropdownContent
and exported dropdown props in DataRecencyFilterDropdown,
SummaryPeriodFilterDropdown, and TimeInRangeFilterDropdown; type Chip,
ChipGroup, and AppliedFilters in AppliedFilters; and use the existing PropTypes
import to type AppliedFiltersAdapter props. Name each component’s actual prop
shapes and callback types, preserving existing defaults and behavior across all
specified files.
In `@app/pages/clinicworkspace/FilterBySites.js`:
- Around line 1-9: Reorder imports to follow the repository grouping convention:
in app/pages/clinicworkspace/FilterBySites.js and
app/pages/clinicworkspace/FilterByTags.js, place local actions after Lodash
imports; in app/pages/clinicworkspace/components/TagFilterDropdown.js and
app/pages/clinicworkspace/components/SiteFilterDropdown.js, order React, Redux,
third-party, Lodash, theme-ui, then local imports. Move every import in
SiteFilterDropdown.js above declarations. No direct changes are required beyond
these four files: app/pages/clinicworkspace/FilterBySites.js:1-9,
app/pages/clinicworkspace/FilterByTags.js:1-9,
app/pages/clinicworkspace/components/TagFilterDropdown.js:1-39, and
app/pages/clinicworkspace/components/SiteFilterDropdown.js:1-39.
- Around line 13-18: Define PropTypes for every component prop at the listed
sites: FilterBySites.js lines 13-18, FilterBySummaryPeriod.js lines 6-9,
FilterByTimeInRange.js lines 6-9, and FilterByTags.js lines 13-18; add prop
definitions for EditTagsAction and DropdownContent plus TagFilterDropdown in
TagFilterDropdown.js lines 41-61 and 214-218; and add definitions for
EditSitesAction and DropdownContent plus SiteFilterDropdown in
SiteFilterDropdown.js lines 41-61 and 218-222, matching each component’s
existing prop usage and defaults.
🪄 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: ee678408-c156-460f-976c-5bb7dfe669f6
⛔ Files ignored due to path filters (1)
app/core/icons/tagIcon.svgis excluded by!**/*.svg
📒 Files selected for processing (17)
app/pages/clinicworkspace/AppliedFiltersAdapter.jsapp/pages/clinicworkspace/ClinicPatients.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/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: 8
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)
3379-3392: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd the missing
renderPeopleTabledependencies.patientListSearchTextInput,handleClearSearch,handleResetFilters, andhandleClickPatientare all read inside the callback; leaving them out keeps the empty state and actions stale when search/filter state 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 3379 - 3392, Update the dependency array for the renderPeopleTable callback to include patientListSearchTextInput, handleClearSearch, handleResetFilters, and handleClickPatient, preserving the existing dependencies so empty-state content and patient actions stay current.Source: Coding guidelines
🧹 Nitpick comments (4)
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js (1)
1-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply the repository’s required import grouping.
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L1-L22: place third-party imports before Lodash, then theme-ui, then local imports.app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L1-L17: apply the same grouping.app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js#L1-L25: apply the same grouping.app/pages/clinicworkspace/components/AppliedFilters.js#L1-L17: move Lodash before theme-ui and local imports.app/pages/clinicworkspace/AppliedFiltersAdapter.js#L1-L8: order PropTypes before Redux and theme-ui before local imports.As per coding guidelines, imports must be grouped as React, PropTypes, Redux, third-party libraries, Lodash, theme-ui, then local imports.
🤖 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` around lines 1 - 22, Reorder imports in app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js (lines 1-22), SummaryPeriodFilterDropdown.js (lines 1-17), and TimeInRangeFilterDropdown.js (lines 1-25) as React, PropTypes if present, Redux, third-party libraries, Lodash, theme-ui, then local imports. Apply the same grouping in app/pages/clinicworkspace/components/AppliedFilters.js (lines 1-17), moving Lodash before theme-ui and local imports, and in app/pages/clinicworkspace/AppliedFiltersAdapter.js (lines 1-8), placing PropTypes before Redux and theme-ui before local imports.Source: Coding guidelines
app/pages/clinicworkspace/FilterByDataRecency.js (1)
6-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefine PropTypes for the new component contracts.
app/pages/clinicworkspace/FilterByDataRecency.js#L6-L9: typeactiveFiltersandsetActiveFilters.app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L26-L31: typeDropdownContentprops.app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L131-L135: type exported dropdown props.app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L32-L36: typeDropdownContentprops.app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L111-L114: type exported dropdown props.app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js#L87-L91: typeDropdownContentprops.app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js#L262-L265: type exported dropdown props.app/pages/clinicworkspace/components/AppliedFilters.js#L104-L171: typeChip,ChipGroup, andAppliedFilters.app/pages/clinicworkspace/AppliedFiltersAdapter.js#L10-L10: use the existingPropTypesimport to type adapter props.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/FilterByDataRecency.js` around lines 6 - 9, Define PropTypes for every component contract listed: type activeFilters and setActiveFilters on FilterByDataRecency; type DropdownContent and exported dropdown props in DataRecencyFilterDropdown, SummaryPeriodFilterDropdown, and TimeInRangeFilterDropdown; type Chip, ChipGroup, and AppliedFilters in AppliedFilters; and use the existing PropTypes import to type AppliedFiltersAdapter props. Name each component’s actual prop shapes and callback types, preserving existing defaults and behavior across all specified files.Source: Coding guidelines
app/pages/clinicworkspace/FilterBySites.js (2)
1-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply the repository import grouping consistently.
app/pages/clinicworkspace/FilterBySites.js#L1-L9: move local actions after Lodash.app/pages/clinicworkspace/FilterByTags.js#L1-L9: move local actions after Lodash.app/pages/clinicworkspace/components/TagFilterDropdown.js#L1-L39: group React, Redux, third-party, Lodash, theme-ui, then local imports.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L1-L39: apply the same grouping and move all imports above declarations.As per coding guidelines, imports must follow the prescribed grouping order.
🤖 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 1 - 9, Reorder imports to follow the repository grouping convention: in app/pages/clinicworkspace/FilterBySites.js and app/pages/clinicworkspace/FilterByTags.js, place local actions after Lodash imports; in app/pages/clinicworkspace/components/TagFilterDropdown.js and app/pages/clinicworkspace/components/SiteFilterDropdown.js, order React, Redux, third-party, Lodash, theme-ui, then local imports. Move every import in SiteFilterDropdown.js above declarations. No direct changes are required beyond these four files: app/pages/clinicworkspace/FilterBySites.js:1-9, app/pages/clinicworkspace/FilterByTags.js:1-9, app/pages/clinicworkspace/components/TagFilterDropdown.js:1-39, and app/pages/clinicworkspace/components/SiteFilterDropdown.js:1-39.Source: Coding guidelines
13-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd PropTypes for every new component.
app/pages/clinicworkspace/FilterBySites.js#L13-L18: define the wrapper’s four props.app/pages/clinicworkspace/FilterBySummaryPeriod.js#L6-L9: define both summary-period props.app/pages/clinicworkspace/FilterByTimeInRange.js#L6-L9: define both filter-state props.app/pages/clinicworkspace/FilterByTags.js#L13-L18: define the wrapper’s four props.app/pages/clinicworkspace/components/TagFilterDropdown.js#L41-L61: define props forEditTagsActionandDropdownContent.app/pages/clinicworkspace/components/TagFilterDropdown.js#L214-L218: defineTagFilterDropdownprops.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L41-L61: define props forEditSitesActionandDropdownContent.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L218-L222: defineSiteFilterDropdownprops.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/FilterBySites.js` around lines 13 - 18, Define PropTypes for every component prop at the listed sites: FilterBySites.js lines 13-18, FilterBySummaryPeriod.js lines 6-9, FilterByTimeInRange.js lines 6-9, and FilterByTags.js lines 13-18; add prop definitions for EditTagsAction and DropdownContent plus TagFilterDropdown in TagFilterDropdown.js lines 41-61 and 214-218; and add definitions for EditSitesAction and DropdownContent plus SiteFilterDropdown in SiteFilterDropdown.js lines 41-61 and 218-222, matching each component’s existing prop usage and defaults.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: Update handleRemoveFilter to add a timeCGMUsePercent case
that removes the selected value from activeFilters.timeCGMUsePercent using the
same pattern as the other multi-value filters, then updates activeFilters via
setActiveFilters.
In `@app/pages/clinicworkspace/components/AppliedFilters.js`:
- Around line 48-54: Update the timeInTargetPercent label in the AppliedFilters
label mapping to describe patients below the 70% target rather than “Meeting
Targets,” while leaving the other time-in-range labels unchanged.
- Around line 119-133: Add a :focus-within selector alongside the existing hover
selector for .remove-filter-icon in the AppliedFilters styling, applying the
same expansion, spacing, and opacity rules so keyboard-focused remove controls
become visible.
In `@app/pages/clinicworkspace/components/ClearFilterButtons.js`:
- Line 25: Add PropTypes declarations for the ClearFilterButtons component’s
patientQueryState, onClearSearch, and onResetFilters props in
app/pages/clinicworkspace/components/ClearFilterButtons.js at line 25. Also add
PropTypes declarations for ClinicPatients’ patientQueryState and children props
in app/pages/clinicworkspace/ClinicPatients.js at line 200, using appropriate
requiredness and types consistent with their usage.
In `@app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js`:
- Around line 157-164: Translate all new user-facing accessibility labels using
react-i18next: in
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js lines 157-164,
wrap the last-data trigger label with t; in
app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js lines
136-143, wrap the summary-period icon label with t; and in
app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js lines 287-303,
translate both the trigger icon label and pill label, using the existing
component translation hook or pattern.
- Line 24: The placeholder prefixPopHealthMetric helper must return the
clinic-workspace event-name prefix string rather than noop. Update
prefixPopHealthMetric in
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js:24 using the
existing prefix helper pattern, and apply the same correction to
app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js:19 and
app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js:29 so each
trackMetric call receives an event-name string.
In `@app/pages/clinicworkspace/components/TagFilterDropdown.js`:
- Around line 241-268: Update the patient tag filter trigger in
app/pages/clinicworkspace/components/TagFilterDropdown.js lines 241-268 to pass
the iconLabel and Pill label through the existing react-i18next t function.
Apply the corresponding translation updates to the site filter trigger in
app/pages/clinicworkspace/components/SiteFilterDropdown.js lines 245-271,
translating both accessibility labels while preserving their existing meaning.
In `@app/pages/clinicworkspace/FilterBySites.js`:
- Around line 11-32: Restore the shared metrics integration across all affected
sites: in app/pages/clinicworkspace/FilterBySites.js lines 11-32 and
app/pages/clinicworkspace/FilterByTags.js lines 11-32, replace the noop
trackMetric helper with the real helper and place the past-tense metric
immediately after the respective async site/tag fetch dispatch; in
app/pages/clinicworkspace/components/TagFilterDropdown.js lines 33-34 and
app/pages/clinicworkspace/components/SiteFilterDropdown.js lines 33-34, import
the real metric and population-health prefix helpers.
---
Outside diff comments:
In `@app/pages/clinicworkspace/ClinicPatients.js`:
- Around line 3379-3392: Update the dependency array for the renderPeopleTable
callback to include patientListSearchTextInput, handleClearSearch,
handleResetFilters, and handleClickPatient, preserving the existing dependencies
so empty-state content and patient actions stay current.
---
Nitpick comments:
In `@app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js`:
- Around line 1-22: Reorder imports in
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js (lines 1-22),
SummaryPeriodFilterDropdown.js (lines 1-17), and TimeInRangeFilterDropdown.js
(lines 1-25) as React, PropTypes if present, Redux, third-party libraries,
Lodash, theme-ui, then local imports. Apply the same grouping in
app/pages/clinicworkspace/components/AppliedFilters.js (lines 1-17), moving
Lodash before theme-ui and local imports, and in
app/pages/clinicworkspace/AppliedFiltersAdapter.js (lines 1-8), placing
PropTypes before Redux and theme-ui before local imports.
In `@app/pages/clinicworkspace/FilterByDataRecency.js`:
- Around line 6-9: Define PropTypes for every component contract listed: type
activeFilters and setActiveFilters on FilterByDataRecency; type DropdownContent
and exported dropdown props in DataRecencyFilterDropdown,
SummaryPeriodFilterDropdown, and TimeInRangeFilterDropdown; type Chip,
ChipGroup, and AppliedFilters in AppliedFilters; and use the existing PropTypes
import to type AppliedFiltersAdapter props. Name each component’s actual prop
shapes and callback types, preserving existing defaults and behavior across all
specified files.
In `@app/pages/clinicworkspace/FilterBySites.js`:
- Around line 1-9: Reorder imports to follow the repository grouping convention:
in app/pages/clinicworkspace/FilterBySites.js and
app/pages/clinicworkspace/FilterByTags.js, place local actions after Lodash
imports; in app/pages/clinicworkspace/components/TagFilterDropdown.js and
app/pages/clinicworkspace/components/SiteFilterDropdown.js, order React, Redux,
third-party, Lodash, theme-ui, then local imports. Move every import in
SiteFilterDropdown.js above declarations. No direct changes are required beyond
these four files: app/pages/clinicworkspace/FilterBySites.js:1-9,
app/pages/clinicworkspace/FilterByTags.js:1-9,
app/pages/clinicworkspace/components/TagFilterDropdown.js:1-39, and
app/pages/clinicworkspace/components/SiteFilterDropdown.js:1-39.
- Around line 13-18: Define PropTypes for every component prop at the listed
sites: FilterBySites.js lines 13-18, FilterBySummaryPeriod.js lines 6-9,
FilterByTimeInRange.js lines 6-9, and FilterByTags.js lines 13-18; add prop
definitions for EditTagsAction and DropdownContent plus TagFilterDropdown in
TagFilterDropdown.js lines 41-61 and 214-218; and add definitions for
EditSitesAction and DropdownContent plus SiteFilterDropdown in
SiteFilterDropdown.js lines 41-61 and 218-222, matching each component’s
existing prop usage and defaults.
🪄 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: ee678408-c156-460f-976c-5bb7dfe669f6
⛔ Files ignored due to path filters (1)
app/core/icons/tagIcon.svgis excluded by!**/*.svg
📒 Files selected for processing (17)
app/pages/clinicworkspace/AppliedFiltersAdapter.jsapp/pages/clinicworkspace/ClinicPatients.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/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 (8)
app/pages/clinicworkspace/AppliedFiltersAdapter.js (1)
13-43: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle removal of the CGM-use filter.
AppliedFiltersemitstimeCGMUsePercentchips, but this switch has no matching case, so clicking their remove icon does nothing.Proposed fix
case 'lastData': setActiveFilters({ ...activeFilters, lastData: defaultFilterState.lastData, lastDataType: defaultFilterState.lastDataType, }); break; + case 'timeCGMUsePercent': + setActiveFilters({ + ...activeFilters, + timeCGMUsePercent: defaultFilterState.timeCGMUsePercent, + }); + break; + case 'timeInRange':📝 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 'lastData': setActiveFilters({ ...activeFilters, lastData: defaultFilterState.lastData, lastDataType: defaultFilterState.lastDataType, }); break; case 'timeCGMUsePercent': setActiveFilters({ ...activeFilters, timeCGMUsePercent: defaultFilterState.timeCGMUsePercent, }); 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, Update handleRemoveFilter to add a timeCGMUsePercent case that removes the selected value from activeFilters.timeCGMUsePercent using the same pattern as the other multi-value filters, then updates activeFilters via setActiveFilters.app/pages/clinicworkspace/components/AppliedFilters.js (2)
48-54: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Correct the inverted target-range chip label.
timeInTargetPercentselects patients below the 70% target, but the applied chip says “Meeting Targets.” This misrepresents the active filter.Proposed fix
- timeInTargetPercent: t('%TIR = Meeting Targets'), + timeInTargetPercent: t('%TIR = Not Meeting Targets'),📝 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.label: ({ timeInExtremeHighPercent: t('%TIR = Highest'), timeInVeryHighPercent: t('%TIR = Very High'), timeInAnyHighPercent: t('%TIR = High'), timeInTargetPercent: t('%TIR = Not Meeting Targets'), timeInAnyLowPercent: t('%TIR = Low'), timeInVeryLowPercent: t('%TIR = Very Low'),🤖 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 48 - 54, Update the timeInTargetPercent label in the AppliedFilters label mapping to describe patients below the 70% target rather than “Meeting Targets,” while leaving the other time-in-range labels unchanged.
119-133: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,220p' app/pages/clinicworkspace/components/AppliedFilters.jsRepository: tidepool-org/blip
Length of output: 6353
🏁 Script executed:
cat -n app/pages/clinicworkspace/components/AppliedFilters.js | sed -n '1,220p'Repository: tidepool-org/blip
Length of output: 7893
🏁 Script executed:
ast-grep outline app/components/elements/Icon* --view expandedRepository: tidepool-org/blip
Length of output: 251
🏁 Script executed:
cat -n app/components/elements/Icon.js | sed -n '1,260p'Repository: tidepool-org/blip
Length of output: 2648
Reveal the remove control on keyboard focus. The icon expands only on
:hover, so keyboard users can focus an invisible remove action. Apply the same styles on:focus-within.Proposed fix
- '&:hover .remove-filter-icon': { + '&:hover .remove-filter-icon, &:focus-within .remove-filter-icon': { width: '14px', ml: 1, mr: 1, opacity: 1, },📝 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.'.remove-filter-icon': { color: vizColors.blue80, minWidth: 0, width: 0, mr: 1, opacity: 0, overflow: 'hidden', transition: transitions.easeOut, }, '&:hover .remove-filter-icon, &:focus-within .remove-filter-icon': { width: '14px', ml: 1, mr: 1, opacity: 1, },🤖 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 119 - 133, Add a :focus-within selector alongside the existing hover selector for .remove-filter-icon in the AppliedFilters styling, applying the same expansion, spacing, and opacity rules so keyboard-focused remove controls become visible.app/pages/clinicworkspace/components/ClearFilterButtons.js (1)
25-25: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add PropTypes to the newly introduced component contracts.
app/pages/clinicworkspace/components/ClearFilterButtons.js#L25-L25: validatepatientQueryState,onClearSearch, andonResetFilters.app/pages/clinicworkspace/ClinicPatients.js#L200-L200: validatepatientQueryStateandchildren.As per coding guidelines, “Define PropTypes for all component props.”
📍 Affects 2 files
app/pages/clinicworkspace/components/ClearFilterButtons.js#L25-L25(this comment)app/pages/clinicworkspace/ClinicPatients.js#L200-L200🤖 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/ClearFilterButtons.js` at line 25, Add PropTypes declarations for the ClearFilterButtons component’s patientQueryState, onClearSearch, and onResetFilters props in app/pages/clinicworkspace/components/ClearFilterButtons.js at line 25. Also add PropTypes declarations for ClinicPatients’ patientQueryState and children props in app/pages/clinicworkspace/ClinicPatients.js at line 200, using appropriate requiredness and types consistent with their usage.Source: Coding guidelines
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js (2)
24-24: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -n -C3 '\bprefixPopHealthMetric\b|\btrackMetric\s*\(' app/pages/clinicworkspace app/coreRepository: tidepool-org/blip
Length of output: 50373
Replace the placeholder metric prefix helper.
prefixPopHealthMetriccurrently returnsnoop, so eachtrackMetric(...)call in:
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L24app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L19app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js#L29passes a function instead of the event-name string used elsewhere in clinic workspace. Use the existing prefix helper pattern here.
📍 Affects 3 files
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L24-L24(this comment)app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L19-L19app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js#L29-L29🤖 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 24, The placeholder prefixPopHealthMetric helper must return the clinic-workspace event-name prefix string rather than noop. Update prefixPopHealthMetric in app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js:24 using the existing prefix helper pattern, and apply the same correction to app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js:19 and app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js:29 so each trackMetric call receives an event-name string.Source: Learnings
157-164: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Translate the new accessibility labels.
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L157-L164: useiconLabel={t('Filter by last upload')}.app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L136-L143: translate the summary-period icon label.app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js#L287-L303: translate both the trigger icon label and pill label.As per coding guidelines, user-facing translations must use
react-i18next.📍 Affects 3 files
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L157-L164(this comment)app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L136-L143app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js#L287-L303🤖 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` around lines 157 - 164, Translate all new user-facing accessibility labels using react-i18next: in app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js lines 157-164, wrap the last-data trigger label with t; in app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js lines 136-143, wrap the summary-period icon label with t; and in app/pages/clinicworkspace/components/TimeInRangeFilterDropdown.js lines 287-303, translate both the trigger icon label and pill label, using the existing component translation hook or pattern.Source: Coding guidelines
app/pages/clinicworkspace/components/TagFilterDropdown.js (1)
241-268: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the filter triggers’ accessibility text.
app/pages/clinicworkspace/components/TagFilterDropdown.js#L241-L268: translate the icon and count labels witht.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L245-L271: translate the corresponding site labels.As per coding guidelines, use
react-i18nextfor translations.📍 Affects 2 files
app/pages/clinicworkspace/components/TagFilterDropdown.js#L241-L268(this comment)app/pages/clinicworkspace/components/SiteFilterDropdown.js#L245-L271🤖 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 241 - 268, Update the patient tag filter trigger in app/pages/clinicworkspace/components/TagFilterDropdown.js lines 241-268 to pass the iconLabel and Pill label through the existing react-i18next t function. Apply the corresponding translation updates to the site filter trigger in app/pages/clinicworkspace/components/SiteFilterDropdown.js lines 245-271, translating both accessibility labels while preserving their existing meaning.Source: Coding guidelines
app/pages/clinicworkspace/FilterBySites.js (1)
11-32: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Restore the shared metrics integration. Temporary no-op helpers discard every newly added filter/edit metric.
app/pages/clinicworkspace/FilterBySites.js#L11-L32: restore the helper and move the past-tense event immediately after the site-fetch dispatch.app/pages/clinicworkspace/FilterByTags.js#L11-L32: restore the helper and move the past-tense event immediately after the tag-fetch dispatch.app/pages/clinicworkspace/components/TagFilterDropdown.js#L33-L34: import the real metric and population-health prefix helpers.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L33-L34: import the real metric and population-health prefix helpers.Based on learnings, related metrics use past-tense names and run immediately after dispatching async actions.
📍 Affects 4 files
app/pages/clinicworkspace/FilterBySites.js#L11-L32(this comment)app/pages/clinicworkspace/FilterByTags.js#L11-L32app/pages/clinicworkspace/components/TagFilterDropdown.js#L33-L34app/pages/clinicworkspace/components/SiteFilterDropdown.js#L33-L34🤖 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 - 32, Restore the shared metrics integration across all affected sites: in app/pages/clinicworkspace/FilterBySites.js lines 11-32 and app/pages/clinicworkspace/FilterByTags.js lines 11-32, replace the noop trackMetric helper with the real helper and place the past-tense metric immediately after the respective async site/tag fetch dispatch; in app/pages/clinicworkspace/components/TagFilterDropdown.js lines 33-34 and app/pages/clinicworkspace/components/SiteFilterDropdown.js lines 33-34, import the real metric and population-health prefix helpers.Source: Learnings
WEB-4654