feature: op insights PR 1#1160
Merged
Merged
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds backend aggregation and endpoints for login trends and facility engagement, integration tests, and a frontend Insights redesign with date-range controls, a 7-day avg login trend chart, metric cards, and a sortable facility comparison table. ChangesOperational Insights Login Trends & Facility Comparison
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 |
CK-7vn
force-pushed
the
CK-7vn/op-insights
branch
from
aef7325 to
0b5ed29
Compare
CK-7vn
force-pushed
the
CK-7vn/op-insights
branch
from
0b5ed29 to
ff1ac9e
Compare
Contributor
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@frontend/src/pages/insights/insightsRange.ts`:
- Around line 42-43: When range === 'Custom' verify that customFrom and customTo
are non-empty and valid dates before returning { start_date: customFrom,
end_date: customTo }; if either is empty/invalid, return a safe fallback (e.g.,
30-day range) or null/undefined to prevent constructing API queries with empty
params. Update the conditional that checks range (the block using range,
customFrom, customTo) to perform this validation and return the fallback/null;
also ensure consumers (OperationalInsights.tsx / OverviewTab) handle the null
case by disabling the API call until valid dates are provided.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 46e3eb26-1abf-46f7-bf5e-1d0bf7da8ad8
📒 Files selected for processing (17)
backend/src/database/DB.gobackend/src/database/users.gobackend/src/handlers/dashboard.gobackend/src/models/logins.gobackend/tests/integration/facility_comparison_test.gobackend/tests/integration/login_trend_test.gofrontend/src/components/charts/LoginTrendChart.tsxfrontend/src/components/charts/OperationalInsightsCharts.tsxfrontend/src/components/navigation/Sidebar.tsxfrontend/src/layouts/AuthenticatedLayout.tsxfrontend/src/pages/insights/KnowledgeCenterTab.tsxfrontend/src/pages/insights/MetricCard.tsxfrontend/src/pages/insights/OperationalInsights.tsxfrontend/src/pages/insights/OverviewTab.tsxfrontend/src/pages/insights/insightsRange.tsfrontend/src/routes/app-routes.tsxfrontend/src/types/insights.ts
💤 Files with no reviewable changes (1)
- frontend/src/components/charts/OperationalInsightsCharts.tsx
Comment on lines
+42
to
+43
| if (range === 'Custom') { | ||
| return { start_date: customFrom, end_date: customTo }; |
There was a problem hiding this comment.
Validate custom date parameters before use.
When range === 'Custom' but customFrom or customTo are empty strings (initial state after selecting Custom), this function returns { start_date: '', end_date: '' }, which will be interpolated into API query strings as start_date=&end_date=. The backend may not handle empty date parameters gracefully, potentially causing errors or unexpected results until the user selects valid dates.
🛡️ Recommended validation approach
Consider one of these solutions:
Option 1: Return a fallback range (e.g., 30D) when custom dates are empty:
export function rangeToParams(
range: InsightsRangeKey,
customFrom: string,
customTo: string
): InsightsDateParams {
if (range === 'Custom') {
+ if (!customFrom || !customTo) {
+ // Fallback to 30D if custom dates not yet selected
+ const end = new Date();
+ const start = daysAgo(end, 29);
+ return { start_date: formatDate(start), end_date: formatDate(end) };
+ }
return { start_date: customFrom, end_date: customTo };
}Option 2: Disable API calls in the consumer until valid custom dates are provided (validate in OperationalInsights.tsx before passing to OverviewTab).
📝 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.
Suggested change
| if (range === 'Custom') { | |
| return { start_date: customFrom, end_date: customTo }; | |
| if (range === 'Custom') { | |
| if (!customFrom || !customTo) { | |
| // Fallback to 30D if custom dates not yet selected | |
| const end = new Date(); | |
| const start = daysAgo(end, 29); | |
| return { start_date: formatDate(start), end_date: formatDate(end) }; | |
| } | |
| return { start_date: customFrom, end_date: customTo }; | |
| } |
🤖 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 `@frontend/src/pages/insights/insightsRange.ts` around lines 42 - 43, When
range === 'Custom' verify that customFrom and customTo are non-empty and valid
dates before returning { start_date: customFrom, end_date: customTo }; if either
is empty/invalid, return a safe fallback (e.g., 30-day range) or null/undefined
to prevent constructing API queries with empty params. Update the conditional
that checks range (the block using range, customFrom, customTo) to perform this
validation and return the fallback/null; also ensure consumers
(OperationalInsights.tsx / OverviewTab) handle the null case by disabling the
API call until valid dates are provided.
…esh button per ticket 701
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
2 participants
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.
Pre-Submission PR Checklist
Description of the change
First PR of the operational insights redesign, redesigned page brings in tabbed Insights and implements the Overview tab end-to-end. Two endpoints added to backend /department-metrics/login-trend - calculates daily login totals and /department-metrics/facility-comparison - - per facility registered/active/logins.
Screenshot(s)
Additional context
This is PR 1 of...multiple....