WEB-4654 - Summary Period - #1989
Open
henry-tp wants to merge 24 commits into
Open
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (4)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughClinic patient filtering is reorganized into reusable dropdowns, applied-filter chips, shared query-state controls, clinic-admin actions, and updated ClinicPatients wiring with supporting translations. ChangesClinic patient filtering
Estimated code review effort: 4 (Complex) | ~60 minutes 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: 12
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)
4125-4138: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude the search text in the callback dependencies.
renderPeopleTablederivespatientQueryStatefrompatientListSearchTextInput, but the value is omitted here. During debounced typing, the clear controls and empty-state copy can therefore use stale query state.Proposed fix
}, [ activeFilters, clinic?.fetchedPatientCount, columns, data, defaultPatientFetchOptions.sort, handlePageChange, handleSortChange, loading, patientFetchOptions, + patientListSearchTextInput, setActiveFilters, showSummaryData, tableStyle, ]);As per coding guidelines: respect
react-hooks/exhaustive-deps.🤖 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 4125 - 4138, Update the dependency array for renderPeopleTable to include patientListSearchTextInput, ensuring its derived patientQueryState and related clear controls or empty-state text refresh during debounced typing while preserving the existing dependencies.Source: Coding guidelines
🧹 Nitpick comments (3)
app/pages/clinicworkspace/FilterBySites.js (3)
1-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply the required import grouping consistently.
app/pages/clinicworkspace/FilterBySites.js#L1-L9: move local actions after Lodash and separate React/Redux groups.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L1-L39: move all imports before declarations and reorder the groups.app/pages/clinicworkspace/FilterByTags.js#L1-L9: move local actions after Lodash and separate React/Redux groups.app/pages/clinicworkspace/components/TagFilterDropdown.js#L1-L39: move all imports before declarations and reorder the groups.app/pages/clinicworkspace/useIsClinicAdmin.js#L1-L4: add blank lines between React, Redux, and Lodash.As per coding guidelines, group imports in the prescribed order with blank lines between groups.
🤖 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, Normalize import grouping across app/pages/clinicworkspace/FilterBySites.js (lines 1-9), app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 1-39), app/pages/clinicworkspace/FilterByTags.js (lines 1-9), app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 1-39), and app/pages/clinicworkspace/useIsClinicAdmin.js (lines 1-4): place every import before declarations, separate React, Redux, Lodash, and local imports with blank lines, and order the groups according to the project convention, with local actions after Lodash.Source: Coding guidelines
13-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd PropTypes for every new component contract.
app/pages/clinicworkspace/FilterBySites.js#L13-L18: define the wrapper’s prop types.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L218-L222: define prop types for the exported and nested prop-taking components.app/pages/clinicworkspace/FilterByTags.js#L13-L18: define the wrapper’s prop types.app/pages/clinicworkspace/components/TagFilterDropdown.js#L214-L218: define prop types for the exported and nested prop-taking components.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 prop-taking component in app/pages/clinicworkspace/FilterBySites.js (lines 13-18), app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 218-222), app/pages/clinicworkspace/FilterByTags.js (lines 13-18), and app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 214-218), covering both exported wrappers and nested components and matching each component’s existing prop contract.Source: Coding guidelines
23-33: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMemoize callbacks passed between components.
app/pages/clinicworkspace/FilterBySites.js#L23-L33: memoize both dropdown callbacks.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L233-L235: memoizehandleCloseDropdown.app/pages/clinicworkspace/FilterByTags.js#L23-L33: memoize both dropdown callbacks.app/pages/clinicworkspace/components/TagFilterDropdown.js#L229-L231: memoizehandleCloseDropdown.As per coding guidelines, use
useCallbackfor callback 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 23 - 33, Memoize callback props with useCallback: wrap handleChange and handleClickEditSites in app/pages/clinicworkspace/FilterBySites.js at lines 23-33, handleCloseDropdown in app/pages/clinicworkspace/components/SiteFilterDropdown.js at lines 233-235, handleChange and handleClickEditTags in app/pages/clinicworkspace/FilterByTags.js at lines 23-33, and handleCloseDropdown in app/pages/clinicworkspace/components/TagFilterDropdown.js at lines 229-231; include each callback’s current dependencies.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 10-60: Define PropTypes for every component prop across the
extracted filter components: in
app/pages/clinicworkspace/AppliedFiltersAdapter.js lines 10-60, declare
activeFilters, setActiveFilters, and rightContent; in
app/pages/clinicworkspace/components/ClearFilterButtons.js lines 25-60, declare
the query-state and callback props; in
app/pages/clinicworkspace/components/AppliedFilters.js lines 104-171, declare
props for Chip, ChipGroup, and AppliedFilters; and in
app/pages/clinicworkspace/ClinicPatients.js lines 241-252, declare
patientQueryState and children for EmptyContentNode.
In `@app/pages/clinicworkspace/components/AppliedFilters.js`:
- Around line 119-143: Update the filter container styles around
.remove-filter-icon to apply the same reveal behavior for :focus-within as for
:hover, and configure the Icon component to render as a focusable button so
keyboard users can reach and see the remove action.
In `@app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js`:
- Line 194: Add PropTypes for all props accepted by the
DataRecencyFilterDropdown component, using the component’s existing prop usage
to declare each expected type and requiredness. Keep the default export
unchanged and ensure the declarations are attached to DataRecencyFilterDropdown.
- Around line 1-23: Reorganize imports in DataRecencyFilterDropdown.js into the
required groups—React, PropTypes, Redux, third-party libraries, Lodash,
theme-ui, and local modules—with blank lines between groups. Remove the unused
borders, DialogContent, and DialogActions imports while preserving all imports
referenced by the component.
- Line 24: The prefixPopHealthMetric helper currently returns noop instead of an
event-name string, causing trackMetric calls to receive a function. Update
prefixPopHealthMetric in
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js:24-24 and
app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js:19-19 to use
the shared string prefix behavior, returning “Clinic - Population Health - ”
followed by the metric.
In `@app/pages/clinicworkspace/components/SiteFilterDropdown.js`:
- Around line 101-107: Update SiteFilterDropdown.js lines 101-107 and
TagFilterDropdown.js lines 101-107 so the populated-state CloseRoundedIcon
receives the translated accessible label “Clear search,” while preserving the
SearchIcon behavior. Update the clinic-site trigger in SiteFilterDropdown.js
lines 245-252 and patient-tag trigger in TagFilterDropdown.js lines 241-248 to
use translated labels via the existing react-i18next translation mechanism.
In `@app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js`:
- Line 172: Add PropTypes for every prop consumed by the
SummaryPeriodFilterDropdown component, using the appropriate type and
requiredness for each existing prop; import PropTypes and assign the component’s
propTypes before the existing default export.
- Around line 1-18: Reorder the imports in SummaryPeriodFilterDropdown.js into
the required groups: React, PropTypes, Redux, third-party libraries,
Lodash-specific imports, theme-ui, and local imports, keeping a blank line
between each group. Preserve all existing imports and their usage.
In `@app/pages/clinicworkspace/FilterByDataRecency.js`:
- Around line 1-25: Update FilterByDataRecency to follow the import grouping
order, adding the required PropTypes import and separating import groups with
blank lines. Define PropTypes for activeFilters and setActiveFilters, and wrap
handleChange in useCallback with its relevant dependencies while preserving the
existing filter update behavior.
In `@app/pages/clinicworkspace/FilterBySites.js`:
- Line 11: Replace the placeholder metrics helpers so filter metrics are emitted
through the real implementations: import and use the real trackMetric in
app/pages/clinicworkspace/FilterBySites.js (line 11) and
app/pages/clinicworkspace/FilterByTags.js (line 11), and import and use the real
metric and prefix helpers in
app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 33-34) and
app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 33-34). Remove
the noop/TODO substitutions without changing the existing metric call sites.
- Around line 29-32: Update handleClickEditSites and the corresponding tag
handler to dispatch the async fetch before tracking, then use past-tense event
names. In SiteFilterDropdown.js lines 188-202 and 239-283, and
TagFilterDropdown.js lines 184-198 and 235-279, rename clear/apply and
open/close metrics to their past-tense equivalents while preserving their
existing behavior; apply the dispatch-order change in FilterBySites.js lines
29-32 and FilterByTags.js lines 29-32.
In `@app/pages/clinicworkspace/FilterBySummaryPeriod.js`:
- Around line 1-22: Update FilterBySummaryPeriod imports to follow the project’s
required grouping and spacing, add PropTypes definitions for activeSummaryPeriod
and setActiveSummaryPeriod, and wrap handleChange in useCallback with the
appropriate dependency. Preserve the existing dropdown behavior and default
no-op callback.
---
Outside diff comments:
In `@app/pages/clinicworkspace/ClinicPatients.js`:
- Around line 4125-4138: Update the dependency array for renderPeopleTable to
include patientListSearchTextInput, ensuring its derived patientQueryState and
related clear controls or empty-state text refresh during debounced typing while
preserving the existing dependencies.
---
Nitpick comments:
In `@app/pages/clinicworkspace/FilterBySites.js`:
- Around line 1-9: Normalize import grouping across
app/pages/clinicworkspace/FilterBySites.js (lines 1-9),
app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 1-39),
app/pages/clinicworkspace/FilterByTags.js (lines 1-9),
app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 1-39), and
app/pages/clinicworkspace/useIsClinicAdmin.js (lines 1-4): place every import
before declarations, separate React, Redux, Lodash, and local imports with blank
lines, and order the groups according to the project convention, with local
actions after Lodash.
- Around line 13-18: Define PropTypes for every prop-taking component in
app/pages/clinicworkspace/FilterBySites.js (lines 13-18),
app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 218-222),
app/pages/clinicworkspace/FilterByTags.js (lines 13-18), and
app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 214-218),
covering both exported wrappers and nested components and matching each
component’s existing prop contract.
- Around line 23-33: Memoize callback props with useCallback: wrap handleChange
and handleClickEditSites in app/pages/clinicworkspace/FilterBySites.js at lines
23-33, handleCloseDropdown in
app/pages/clinicworkspace/components/SiteFilterDropdown.js at lines 233-235,
handleChange and handleClickEditTags in
app/pages/clinicworkspace/FilterByTags.js at lines 23-33, and
handleCloseDropdown in app/pages/clinicworkspace/components/TagFilterDropdown.js
at lines 229-231; include each callback’s current dependencies.
🪄 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: 45ef73b3-a35e-4081-ba48-b7cbadd4ee98
⛔ Files ignored due to path filters (1)
app/core/icons/tagIcon.svgis excluded by!**/*.svg
📒 Files selected for processing (15)
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/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/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: 12
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)
4125-4138: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude the search text in the callback dependencies.
renderPeopleTablederivespatientQueryStatefrompatientListSearchTextInput, but the value is omitted here. During debounced typing, the clear controls and empty-state copy can therefore use stale query state.Proposed fix
}, [ activeFilters, clinic?.fetchedPatientCount, columns, data, defaultPatientFetchOptions.sort, handlePageChange, handleSortChange, loading, patientFetchOptions, + patientListSearchTextInput, setActiveFilters, showSummaryData, tableStyle, ]);As per coding guidelines: respect
react-hooks/exhaustive-deps.🤖 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 4125 - 4138, Update the dependency array for renderPeopleTable to include patientListSearchTextInput, ensuring its derived patientQueryState and related clear controls or empty-state text refresh during debounced typing while preserving the existing dependencies.Source: Coding guidelines
🧹 Nitpick comments (3)
app/pages/clinicworkspace/FilterBySites.js (3)
1-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply the required import grouping consistently.
app/pages/clinicworkspace/FilterBySites.js#L1-L9: move local actions after Lodash and separate React/Redux groups.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L1-L39: move all imports before declarations and reorder the groups.app/pages/clinicworkspace/FilterByTags.js#L1-L9: move local actions after Lodash and separate React/Redux groups.app/pages/clinicworkspace/components/TagFilterDropdown.js#L1-L39: move all imports before declarations and reorder the groups.app/pages/clinicworkspace/useIsClinicAdmin.js#L1-L4: add blank lines between React, Redux, and Lodash.As per coding guidelines, group imports in the prescribed order with blank lines between groups.
🤖 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, Normalize import grouping across app/pages/clinicworkspace/FilterBySites.js (lines 1-9), app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 1-39), app/pages/clinicworkspace/FilterByTags.js (lines 1-9), app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 1-39), and app/pages/clinicworkspace/useIsClinicAdmin.js (lines 1-4): place every import before declarations, separate React, Redux, Lodash, and local imports with blank lines, and order the groups according to the project convention, with local actions after Lodash.Source: Coding guidelines
13-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd PropTypes for every new component contract.
app/pages/clinicworkspace/FilterBySites.js#L13-L18: define the wrapper’s prop types.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L218-L222: define prop types for the exported and nested prop-taking components.app/pages/clinicworkspace/FilterByTags.js#L13-L18: define the wrapper’s prop types.app/pages/clinicworkspace/components/TagFilterDropdown.js#L214-L218: define prop types for the exported and nested prop-taking components.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 prop-taking component in app/pages/clinicworkspace/FilterBySites.js (lines 13-18), app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 218-222), app/pages/clinicworkspace/FilterByTags.js (lines 13-18), and app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 214-218), covering both exported wrappers and nested components and matching each component’s existing prop contract.Source: Coding guidelines
23-33: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMemoize callbacks passed between components.
app/pages/clinicworkspace/FilterBySites.js#L23-L33: memoize both dropdown callbacks.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L233-L235: memoizehandleCloseDropdown.app/pages/clinicworkspace/FilterByTags.js#L23-L33: memoize both dropdown callbacks.app/pages/clinicworkspace/components/TagFilterDropdown.js#L229-L231: memoizehandleCloseDropdown.As per coding guidelines, use
useCallbackfor callback 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 23 - 33, Memoize callback props with useCallback: wrap handleChange and handleClickEditSites in app/pages/clinicworkspace/FilterBySites.js at lines 23-33, handleCloseDropdown in app/pages/clinicworkspace/components/SiteFilterDropdown.js at lines 233-235, handleChange and handleClickEditTags in app/pages/clinicworkspace/FilterByTags.js at lines 23-33, and handleCloseDropdown in app/pages/clinicworkspace/components/TagFilterDropdown.js at lines 229-231; include each callback’s current dependencies.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 10-60: Define PropTypes for every component prop across the
extracted filter components: in
app/pages/clinicworkspace/AppliedFiltersAdapter.js lines 10-60, declare
activeFilters, setActiveFilters, and rightContent; in
app/pages/clinicworkspace/components/ClearFilterButtons.js lines 25-60, declare
the query-state and callback props; in
app/pages/clinicworkspace/components/AppliedFilters.js lines 104-171, declare
props for Chip, ChipGroup, and AppliedFilters; and in
app/pages/clinicworkspace/ClinicPatients.js lines 241-252, declare
patientQueryState and children for EmptyContentNode.
In `@app/pages/clinicworkspace/components/AppliedFilters.js`:
- Around line 119-143: Update the filter container styles around
.remove-filter-icon to apply the same reveal behavior for :focus-within as for
:hover, and configure the Icon component to render as a focusable button so
keyboard users can reach and see the remove action.
In `@app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js`:
- Line 194: Add PropTypes for all props accepted by the
DataRecencyFilterDropdown component, using the component’s existing prop usage
to declare each expected type and requiredness. Keep the default export
unchanged and ensure the declarations are attached to DataRecencyFilterDropdown.
- Around line 1-23: Reorganize imports in DataRecencyFilterDropdown.js into the
required groups—React, PropTypes, Redux, third-party libraries, Lodash,
theme-ui, and local modules—with blank lines between groups. Remove the unused
borders, DialogContent, and DialogActions imports while preserving all imports
referenced by the component.
- Line 24: The prefixPopHealthMetric helper currently returns noop instead of an
event-name string, causing trackMetric calls to receive a function. Update
prefixPopHealthMetric in
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js:24-24 and
app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js:19-19 to use
the shared string prefix behavior, returning “Clinic - Population Health - ”
followed by the metric.
In `@app/pages/clinicworkspace/components/SiteFilterDropdown.js`:
- Around line 101-107: Update SiteFilterDropdown.js lines 101-107 and
TagFilterDropdown.js lines 101-107 so the populated-state CloseRoundedIcon
receives the translated accessible label “Clear search,” while preserving the
SearchIcon behavior. Update the clinic-site trigger in SiteFilterDropdown.js
lines 245-252 and patient-tag trigger in TagFilterDropdown.js lines 241-248 to
use translated labels via the existing react-i18next translation mechanism.
In `@app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js`:
- Line 172: Add PropTypes for every prop consumed by the
SummaryPeriodFilterDropdown component, using the appropriate type and
requiredness for each existing prop; import PropTypes and assign the component’s
propTypes before the existing default export.
- Around line 1-18: Reorder the imports in SummaryPeriodFilterDropdown.js into
the required groups: React, PropTypes, Redux, third-party libraries,
Lodash-specific imports, theme-ui, and local imports, keeping a blank line
between each group. Preserve all existing imports and their usage.
In `@app/pages/clinicworkspace/FilterByDataRecency.js`:
- Around line 1-25: Update FilterByDataRecency to follow the import grouping
order, adding the required PropTypes import and separating import groups with
blank lines. Define PropTypes for activeFilters and setActiveFilters, and wrap
handleChange in useCallback with its relevant dependencies while preserving the
existing filter update behavior.
In `@app/pages/clinicworkspace/FilterBySites.js`:
- Line 11: Replace the placeholder metrics helpers so filter metrics are emitted
through the real implementations: import and use the real trackMetric in
app/pages/clinicworkspace/FilterBySites.js (line 11) and
app/pages/clinicworkspace/FilterByTags.js (line 11), and import and use the real
metric and prefix helpers in
app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 33-34) and
app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 33-34). Remove
the noop/TODO substitutions without changing the existing metric call sites.
- Around line 29-32: Update handleClickEditSites and the corresponding tag
handler to dispatch the async fetch before tracking, then use past-tense event
names. In SiteFilterDropdown.js lines 188-202 and 239-283, and
TagFilterDropdown.js lines 184-198 and 235-279, rename clear/apply and
open/close metrics to their past-tense equivalents while preserving their
existing behavior; apply the dispatch-order change in FilterBySites.js lines
29-32 and FilterByTags.js lines 29-32.
In `@app/pages/clinicworkspace/FilterBySummaryPeriod.js`:
- Around line 1-22: Update FilterBySummaryPeriod imports to follow the project’s
required grouping and spacing, add PropTypes definitions for activeSummaryPeriod
and setActiveSummaryPeriod, and wrap handleChange in useCallback with the
appropriate dependency. Preserve the existing dropdown behavior and default
no-op callback.
---
Outside diff comments:
In `@app/pages/clinicworkspace/ClinicPatients.js`:
- Around line 4125-4138: Update the dependency array for renderPeopleTable to
include patientListSearchTextInput, ensuring its derived patientQueryState and
related clear controls or empty-state text refresh during debounced typing while
preserving the existing dependencies.
---
Nitpick comments:
In `@app/pages/clinicworkspace/FilterBySites.js`:
- Around line 1-9: Normalize import grouping across
app/pages/clinicworkspace/FilterBySites.js (lines 1-9),
app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 1-39),
app/pages/clinicworkspace/FilterByTags.js (lines 1-9),
app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 1-39), and
app/pages/clinicworkspace/useIsClinicAdmin.js (lines 1-4): place every import
before declarations, separate React, Redux, Lodash, and local imports with blank
lines, and order the groups according to the project convention, with local
actions after Lodash.
- Around line 13-18: Define PropTypes for every prop-taking component in
app/pages/clinicworkspace/FilterBySites.js (lines 13-18),
app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 218-222),
app/pages/clinicworkspace/FilterByTags.js (lines 13-18), and
app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 214-218),
covering both exported wrappers and nested components and matching each
component’s existing prop contract.
- Around line 23-33: Memoize callback props with useCallback: wrap handleChange
and handleClickEditSites in app/pages/clinicworkspace/FilterBySites.js at lines
23-33, handleCloseDropdown in
app/pages/clinicworkspace/components/SiteFilterDropdown.js at lines 233-235,
handleChange and handleClickEditTags in
app/pages/clinicworkspace/FilterByTags.js at lines 23-33, and
handleCloseDropdown in app/pages/clinicworkspace/components/TagFilterDropdown.js
at lines 229-231; include each callback’s current dependencies.
🪄 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: 45ef73b3-a35e-4081-ba48-b7cbadd4ee98
⛔ Files ignored due to path filters (1)
app/core/icons/tagIcon.svgis excluded by!**/*.svg
📒 Files selected for processing (15)
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/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/useClinicPatientsFilters.jsapp/pages/clinicworkspace/useIsClinicAdmin.jslocales/en/translation.json
🛑 Comments failed to post (12)
app/pages/clinicworkspace/AppliedFiltersAdapter.js (1)
10-60: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add the missing PropTypes contracts across the extracted filter components.
app/pages/clinicworkspace/AppliedFiltersAdapter.js#L10-L60: declareactiveFilters,setActiveFilters, andrightContent.app/pages/clinicworkspace/components/ClearFilterButtons.js#L25-L60: declare the query-state and callback props.app/pages/clinicworkspace/components/AppliedFilters.js#L104-L171: declare props forChip,ChipGroup, andAppliedFilters.app/pages/clinicworkspace/ClinicPatients.js#L241-L252: declarepatientQueryStateandchildrenforEmptyContentNode.As per coding guidelines: “Define PropTypes for all component props.”
📍 Affects 4 files
app/pages/clinicworkspace/AppliedFiltersAdapter.js#L10-L60(this comment)app/pages/clinicworkspace/components/ClearFilterButtons.js#L25-L60app/pages/clinicworkspace/components/AppliedFilters.js#L104-L171app/pages/clinicworkspace/ClinicPatients.js#L241-L252🤖 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 10 - 60, Define PropTypes for every component prop across the extracted filter components: in app/pages/clinicworkspace/AppliedFiltersAdapter.js lines 10-60, declare activeFilters, setActiveFilters, and rightContent; in app/pages/clinicworkspace/components/ClearFilterButtons.js lines 25-60, declare the query-state and callback props; in app/pages/clinicworkspace/components/AppliedFilters.js lines 104-171, declare props for Chip, ChipGroup, and AppliedFilters; and in app/pages/clinicworkspace/ClinicPatients.js lines 241-252, declare patientQueryState and children for EmptyContentNode.Source: Coding guidelines
app/pages/clinicworkspace/components/AppliedFilters.js (1)
119-143: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Expose the remove action on keyboard focus.
The icon expands only on
:hover, leaving keyboard users with an invisible focused control. Apply the same reveal styles for:focus-withinand ensureIconrenders a focusable button.Proposed focus styling
- '&: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, }, }} > <Text sx={{ textDecoration: 'underline', whiteSpace: 'nowrap' }}>{label}</Text> <Icon className="remove-filter-icon" variant="static" icon={CloseRoundedIcon} label={t('Remove {{ label }} filter', { label })} onClick={onRemove} sx={{ fontSize: '14px', color: vizColors.indigo50, flexShrink: 0 }}🤖 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 - 143, Update the filter container styles around .remove-filter-icon to apply the same reveal behavior for :focus-within as for :hover, and configure the Icon component to render as a focusable button so keyboard users can reach and see the remove action.app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js (3)
1-23: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Apply coding guidelines for import grouping and remove unused imports.
As per coding guidelines, imports should be grouped in the required order with blank lines between groups: React, PropTypes, Redux, third-party libraries, Lodash specific imports, theme-ui, then local imports. Additionally,
borders,DialogContent, andDialogActionsare unused and should be removed.♻️ Proposed fix
-import React, { useState } from 'react'; +import React, { useState } from 'react'; + +import PropTypes from 'prop-types'; + import { useSelector } from 'react-redux'; + import { useTranslation } from 'react-i18next'; -import { trackMetric } from '../../../core/metricUtils'; +import { bindPopover, bindTrigger, usePopupState } from 'material-ui-popup-state/hooks'; import { colors as vizColors } from '`@tidepool/viz`'; - -import { Box, Grid, Text } from 'theme-ui'; import KeyboardArrowDownRoundedIcon from '`@material-ui/icons/KeyboardArrowDownRounded`'; import reject from 'lodash/reject'; import noop from 'lodash/noop'; -import { bindPopover, bindTrigger, usePopupState } from 'material-ui-popup-state/hooks'; - +import { Box, Grid, Text } from 'theme-ui'; + +import { trackMetric } from '../../../core/metricUtils'; import Button from '../../../components/elements/Button'; import Popover from '../../../components/elements/Popover'; import RadioGroup from '../../../components/elements/RadioGroup'; import { Body0 } from '../../../components/elements/FontStyles'; - -import { borders } from '../../../themes/baseTheme'; -import { DialogContent, DialogActions } from '../../../components/elements/Dialog'; import { lastDataFilterOptions } from '../../../core/clinicUtils';📝 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.import React, { useState } from 'react'; import PropTypes from 'prop-types'; import { useSelector } from 'react-redux'; import { useTranslation } from 'react-i18next'; import { bindPopover, bindTrigger, usePopupState } from 'material-ui-popup-state/hooks'; import { colors as vizColors } from '`@tidepool/viz`'; import KeyboardArrowDownRoundedIcon from '`@material-ui/icons/KeyboardArrowDownRounded`'; import reject from 'lodash/reject'; import noop from 'lodash/noop'; import { Box, Grid, Text } from 'theme-ui'; import { trackMetric } from '../../../core/metricUtils'; import Button from '../../../components/elements/Button'; import Popover from '../../../components/elements/Popover'; import RadioGroup from '../../../components/elements/RadioGroup'; import { Body0 } from '../../../components/elements/FontStyles'; import { lastDataFilterOptions } from '../../../core/clinicUtils';🤖 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 - 23, Reorganize imports in DataRecencyFilterDropdown.js into the required groups—React, PropTypes, Redux, third-party libraries, Lodash, theme-ui, and local modules—with blank lines between groups. Remove the unused borders, DialogContent, and DialogActions imports while preserving all imports referenced by the component.Source: Coding guidelines
24-24: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg 'prefixPopHealthMetric'Repository: tidepool-org/blip
Length of output: 155
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the two target files around the relevant lines sed -n '1,120p' app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js printf '\n---\n' sed -n '1,120p' app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js printf '\n=== search for metric helper usage ===\n' rg -n "trackMetric|prefixPopHealthMetric|popHealthMetric|noop" app/pages/clinicworkspace app -g '*.js'Repository: tidepool-org/blip
Length of output: 50374
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '=== metricUtils ===\n' sed -n '1,220p' app/core/metricUtils.js printf '\n=== clinicworkspace prefix helper examples ===\n' sed -n '810,840p' app/pages/clinicworkspace/ClinicPatients.js printf '\n=== other dropdown stubs ===\n' sed -n '1,70p' app/pages/clinicworkspace/components/SiteFilterDropdown.js printf '\n---\n' sed -n '1,70p' app/pages/clinicworkspace/components/TagFilterDropdown.js printf '\n=== trackMetric call sites in target files ===\n' sed -n '85,125p' app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js printf '\n---\n' sed -n '75,105p' app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.jsRepository: tidepool-org/blip
Length of output: 9672
Replace the
noopmetric prefix stub
trackMetric()receives the return value ofprefixPopHealthMetric(...); returningnooppasses a function instead of an event name, so these events won’t record correctly.
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L24app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L19Use the same string prefix helper used elsewhere:
metric => \Clinic - Population Health - ${metric}``📍 Affects 2 files
app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js#L24-L24(this comment)app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js#L19-L19🤖 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 prefixPopHealthMetric helper currently returns noop instead of an event-name string, causing trackMetric calls to receive a function. Update prefixPopHealthMetric in app/pages/clinicworkspace/components/DataRecencyFilterDropdown.js:24-24 and app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js:19-19 to use the shared string prefix behavior, returning “Clinic - Population Health - ” followed by the metric.
194-194: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Define PropTypes for components.
As per coding guidelines, define PropTypes for all component props.
♻️ Proposed fix
+DropdownContent.propTypes = { + onClose: PropTypes.func, + onChange: PropTypes.func, + lastData: PropTypes.number, + lastDataType: PropTypes.string, +}; + +DataRecencyFilterDropdown.propTypes = { + onChange: PropTypes.func, + lastData: PropTypes.number, + lastDataType: PropTypes.string, +}; + export default DataRecencyFilterDropdown;📝 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.DropdownContent.propTypes = { onClose: PropTypes.func, onChange: PropTypes.func, lastData: PropTypes.number, lastDataType: PropTypes.string, }; DataRecencyFilterDropdown.propTypes = { onChange: PropTypes.func, lastData: PropTypes.number, lastDataType: PropTypes.string, }; export default DataRecencyFilterDropdown;🤖 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 194, Add PropTypes for all props accepted by the DataRecencyFilterDropdown component, using the component’s existing prop usage to declare each expected type and requiredness. Keep the default export unchanged and ensure the declarations are attached to DataRecencyFilterDropdown.Source: Coding guidelines
app/pages/clinicworkspace/components/SiteFilterDropdown.js (1)
101-107: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Provide accurate, translated accessible labels.
app/pages/clinicworkspace/components/SiteFilterDropdown.js#L101-L107: label the populated-state icon “Clear search.”app/pages/clinicworkspace/components/SiteFilterDropdown.js#L245-L252: translate the clinic-site trigger label.app/pages/clinicworkspace/components/TagFilterDropdown.js#L101-L107: label the populated-state icon “Clear search.”app/pages/clinicworkspace/components/TagFilterDropdown.js#L241-L248: translate the patient-tag trigger label.As per coding guidelines, use
react-i18nextfor translations.📍 Affects 2 files
app/pages/clinicworkspace/components/SiteFilterDropdown.js#L101-L107(this comment)app/pages/clinicworkspace/components/SiteFilterDropdown.js#L245-L252app/pages/clinicworkspace/components/TagFilterDropdown.js#L101-L107app/pages/clinicworkspace/components/TagFilterDropdown.js#L241-L248🤖 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 101 - 107, Update SiteFilterDropdown.js lines 101-107 and TagFilterDropdown.js lines 101-107 so the populated-state CloseRoundedIcon receives the translated accessible label “Clear search,” while preserving the SearchIcon behavior. Update the clinic-site trigger in SiteFilterDropdown.js lines 245-252 and patient-tag trigger in TagFilterDropdown.js lines 241-248 to use translated labels via the existing react-i18next translation mechanism.Source: Coding guidelines
app/pages/clinicworkspace/components/SummaryPeriodFilterDropdown.js (2)
1-18: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Apply coding guidelines for import grouping.
As per coding guidelines, imports should be grouped in the required order with blank lines between groups: React, PropTypes, Redux, third-party libraries, Lodash specific imports, theme-ui, then local imports.
♻️ Proposed fix
-import React, { useState } from 'react'; +import React, { useState } from 'react'; + +import PropTypes from 'prop-types'; + import { useSelector } from 'react-redux'; + import { useTranslation } from 'react-i18next'; -import { trackMetric } from '../../../core/metricUtils'; +import { bindPopover, bindTrigger, usePopupState } from 'material-ui-popup-state/hooks'; import { colors as vizColors } from '`@tidepool/viz`'; - -import { Box, Grid } from 'theme-ui'; import KeyboardArrowDownRoundedIcon from '`@material-ui/icons/KeyboardArrowDownRounded`'; import noop from 'lodash/noop'; -import { bindPopover, bindTrigger, usePopupState } from 'material-ui-popup-state/hooks'; - +import { Box, Grid } from 'theme-ui'; + +import { trackMetric } from '../../../core/metricUtils'; import Button from '../../../components/elements/Button'; import Popover from '../../../components/elements/Popover'; import RadioGroup from '../../../components/elements/RadioGroup'; import { Body0, Body1 } from '../../../components/elements/FontStyles';📝 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.import React, { useState } from 'react'; import PropTypes from 'prop-types'; import { useSelector } from 'react-redux'; import { useTranslation } from 'react-i18next'; import { bindPopover, bindTrigger, usePopupState } from 'material-ui-popup-state/hooks'; import { colors as vizColors } from '`@tidepool/viz`'; import KeyboardArrowDownRoundedIcon from '`@material-ui/icons/KeyboardArrowDownRounded`'; import noop from 'lodash/noop'; import { Box, Grid } from 'theme-ui'; import { trackMetric } from '../../../core/metricUtils'; import Button from '../../../components/elements/Button'; import Popover from '../../../components/elements/Popover'; import RadioGroup from '../../../components/elements/RadioGroup'; import { Body0, Body1 } from '../../../components/elements/FontStyles';🤖 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` around lines 1 - 18, Reorder the imports in SummaryPeriodFilterDropdown.js into the required groups: React, PropTypes, Redux, third-party libraries, Lodash-specific imports, theme-ui, and local imports, keeping a blank line between each group. Preserve all existing imports and their usage.Source: Coding guidelines
172-172: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Define PropTypes for components.
As per coding guidelines, define PropTypes for all component props.
♻️ Proposed fix
+DropdownContent.propTypes = { + onClose: PropTypes.func, + onChange: PropTypes.func, + activeSummaryPeriod: PropTypes.string, +}; + +SummaryPeriodFilterDropdown.propTypes = { + onChange: PropTypes.func, + activeSummaryPeriod: PropTypes.string, +}; + export default SummaryPeriodFilterDropdown;📝 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.DropdownContent.propTypes = { onClose: PropTypes.func, onChange: PropTypes.func, activeSummaryPeriod: PropTypes.string, }; SummaryPeriodFilterDropdown.propTypes = { onChange: PropTypes.func, activeSummaryPeriod: PropTypes.string, }; export default SummaryPeriodFilterDropdown;🤖 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 172, Add PropTypes for every prop consumed by the SummaryPeriodFilterDropdown component, using the appropriate type and requiredness for each existing prop; import PropTypes and assign the component’s propTypes before the existing default export.Source: Coding guidelines
app/pages/clinicworkspace/FilterByDataRecency.js (1)
1-25: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Apply coding guidelines for imports, PropTypes, and hooks.
As per coding guidelines:
- Imports must be grouped in the required order with blank lines between groups (React, PropTypes, Redux, third-party libraries, Lodash, theme-ui, local imports).
- Component props must have PropTypes defined.
- Callback props should be wrapped in
useCallback.♻️ Proposed fixes
-import React from 'react'; -import noop from 'lodash/noop'; +import React, { useCallback } from 'react'; + +import PropTypes from 'prop-types'; + +import noop from 'lodash/noop'; import DataRecencyFilterDropdown from './components/DataRecencyFilterDropdown'; const FilterByDataRecency = ({ activeFilters = {}, setActiveFilters = noop, }) => { - const handleChange = ({ lastData, lastDataType }) => { + const handleChange = useCallback(({ lastData, lastDataType }) => { setActiveFilters({ ...activeFilters, lastData, lastDataType }); - }; + }, [activeFilters, setActiveFilters]); const { lastData, lastDataType } = activeFilters; return ( <DataRecencyFilterDropdown onChange={handleChange} lastData={lastData} lastDataType={lastDataType} /> ); }; + +FilterByDataRecency.propTypes = { + activeFilters: PropTypes.object, + setActiveFilters: PropTypes.func, +}; export default FilterByDataRecency;📝 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.import React, { useCallback } from 'react'; import PropTypes from 'prop-types'; import noop from 'lodash/noop'; import DataRecencyFilterDropdown from './components/DataRecencyFilterDropdown'; const FilterByDataRecency = ({ activeFilters = {}, setActiveFilters = noop, }) => { const handleChange = useCallback(({ lastData, lastDataType }) => { setActiveFilters({ ...activeFilters, lastData, lastDataType }); }, [activeFilters, setActiveFilters]); const { lastData, lastDataType } = activeFilters; return ( <DataRecencyFilterDropdown onChange={handleChange} lastData={lastData} lastDataType={lastDataType} /> ); }; FilterByDataRecency.propTypes = { activeFilters: PropTypes.object, setActiveFilters: PropTypes.func, }; export default FilterByDataRecency;🤖 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 1 - 25, Update FilterByDataRecency to follow the import grouping order, adding the required PropTypes import and separating import groups with blank lines. Define PropTypes for activeFilters and setActiveFilters, and wrap handleChange in useCallback with its relevant dependencies while preserving the existing filter update behavior.Source: Coding guidelines
app/pages/clinicworkspace/FilterBySites.js (2)
11-11: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace all placeholder metrics helpers before merging.
app/pages/clinicworkspace/FilterBySites.js#L11-L11: import the realtrackMetric.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L33-L34: import the real metric and prefix helpers.app/pages/clinicworkspace/FilterByTags.js#L11-L11: import the realtrackMetric.app/pages/clinicworkspace/components/TagFilterDropdown.js#L33-L34: import the real metric and prefix helpers.These placeholders silently discard every newly added filter metric.
📍 Affects 4 files
app/pages/clinicworkspace/FilterBySites.js#L11-L11(this comment)app/pages/clinicworkspace/components/SiteFilterDropdown.js#L33-L34app/pages/clinicworkspace/FilterByTags.js#L11-L11app/pages/clinicworkspace/components/TagFilterDropdown.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` at line 11, Replace the placeholder metrics helpers so filter metrics are emitted through the real implementations: import and use the real trackMetric in app/pages/clinicworkspace/FilterBySites.js (line 11) and app/pages/clinicworkspace/FilterByTags.js (line 11), and import and use the real metric and prefix helpers in app/pages/clinicworkspace/components/SiteFilterDropdown.js (lines 33-34) and app/pages/clinicworkspace/components/TagFilterDropdown.js (lines 33-34). Remove the noop/TODO substitutions without changing the existing metric call sites.
29-32: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Align the new metrics with the established event convention.
app/pages/clinicworkspace/FilterBySites.js#L29-L32: dispatch first, then track with a past-tense event name.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L188-L202: rename clear/apply events to past tense.app/pages/clinicworkspace/components/SiteFilterDropdown.js#L239-L283: rename open/close events to past tense.app/pages/clinicworkspace/FilterByTags.js#L29-L32: dispatch first, then track with a past-tense event name.app/pages/clinicworkspace/components/TagFilterDropdown.js#L184-L198: rename clear/apply events to past tense.app/pages/clinicworkspace/components/TagFilterDropdown.js#L235-L279: rename open/close events to past tense.Based on learnings, metric names should be past tense and async initiation metrics should be emitted immediately after dispatch.
📍 Affects 4 files
app/pages/clinicworkspace/FilterBySites.js#L29-L32(this comment)app/pages/clinicworkspace/components/SiteFilterDropdown.js#L188-L202app/pages/clinicworkspace/components/SiteFilterDropdown.js#L239-L283app/pages/clinicworkspace/FilterByTags.js#L29-L32app/pages/clinicworkspace/components/TagFilterDropdown.js#L184-L198app/pages/clinicworkspace/components/TagFilterDropdown.js#L235-L279🤖 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 29 - 32, Update handleClickEditSites and the corresponding tag handler to dispatch the async fetch before tracking, then use past-tense event names. In SiteFilterDropdown.js lines 188-202 and 239-283, and TagFilterDropdown.js lines 184-198 and 235-279, rename clear/apply and open/close metrics to their past-tense equivalents while preserving their existing behavior; apply the dispatch-order change in FilterBySites.js lines 29-32 and FilterByTags.js lines 29-32.Source: Learnings
app/pages/clinicworkspace/FilterBySummaryPeriod.js (1)
1-22: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Apply coding guidelines for imports, PropTypes, and hooks.
As per coding guidelines:
- Imports must be grouped in the required order with blank lines between groups.
- Component props must have PropTypes defined.
- Callback props should be wrapped in
useCallback.♻️ Proposed fixes
-import React from 'react'; -import noop from 'lodash/noop'; +import React, { useCallback } from 'react'; + +import PropTypes from 'prop-types'; + +import noop from 'lodash/noop'; import SummaryPeriodFilterDropdown from './components/SummaryPeriodFilterDropdown'; const FilterBySummaryPeriod = ({ activeSummaryPeriod, setActiveSummaryPeriod = noop, }) => { - const handleChange = (summaryPeriod) => { + const handleChange = useCallback((summaryPeriod) => { setActiveSummaryPeriod(summaryPeriod); - }; + }, [setActiveSummaryPeriod]); return ( <SummaryPeriodFilterDropdown onChange={handleChange} activeSummaryPeriod={activeSummaryPeriod} /> ); }; + +FilterBySummaryPeriod.propTypes = { + activeSummaryPeriod: PropTypes.string, + setActiveSummaryPeriod: PropTypes.func, +}; export default FilterBySummaryPeriod;📝 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.import React, { useCallback } from 'react'; import PropTypes from 'prop-types'; import noop from 'lodash/noop'; import SummaryPeriodFilterDropdown from './components/SummaryPeriodFilterDropdown'; const FilterBySummaryPeriod = ({ activeSummaryPeriod, setActiveSummaryPeriod = noop, }) => { const handleChange = useCallback((summaryPeriod) => { setActiveSummaryPeriod(summaryPeriod); }, [setActiveSummaryPeriod]); return ( <SummaryPeriodFilterDropdown onChange={handleChange} activeSummaryPeriod={activeSummaryPeriod} /> ); }; FilterBySummaryPeriod.propTypes = { activeSummaryPeriod: PropTypes.string, setActiveSummaryPeriod: PropTypes.func, }; export default FilterBySummaryPeriod;🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 10-10: Avoid using the initial state variable in setState
Context: setActiveSummaryPeriod(summaryPeriod)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(setstate-same-var)
🤖 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/FilterBySummaryPeriod.js` around lines 1 - 22, Update FilterBySummaryPeriod imports to follow the project’s required grouping and spacing, add PropTypes definitions for activeSummaryPeriod and setActiveSummaryPeriod, and wrap handleChange in useCallback with the appropriate dependency. Preserve the existing dropdown behavior and default no-op callback.Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WEB-4654