-
Notifications
You must be signed in to change notification settings - Fork 0
Daily Stats #88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Daily Stats #88
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
components/DailyStatsDashboard.tsx
Outdated
| import DailyStatsChart from './DailyStatsChart' | ||
| import FetchMessage from './FetchMessage' | ||
|
|
||
| const DAILY_STATS_URL = `${process.env.API_BASE_URL}/api/secure/dailystats?fromDate=2025-01-01` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const DAILY_STATS_URL = `${process.env.API_BASE_URL}/api/secure/dailystats?fromDate=2025-01-01` | |
| const DAILY_STATS_URL = `${process.env.API_BASE_URL}/api/secure/dailystats?fromDate=${someGeneratedData}` |
Not a formal review but this jumped out to me! Overall this looks exciting thanks for your work
amy-corson-ibigroup
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking really good! Just a few nitpicks and questions
components/CDPUserDashboard.tsx
Outdated
| }, [auth0, currentlyDownloadingFile]) | ||
|
|
||
| let files = Object.keys(swrData).length > 0 ? swrData?.data?.data : [] | ||
| let files = Object.keys(swrData).length > 0 ? swrData?.data?.data || [] : [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this just be let files = swrData?.data?.data || []
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch, updated in fad9c36.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few functions getting flagged for missing return types here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in 40c0caf.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing here, a few missing return types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in 3cc834e
components/DailyStatsChart.tsx
Outdated
|
|
||
| render() { | ||
| const { entityType, records, series } = this.props | ||
| if (records.length === 0) return null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this just be if (!records.length)?
components/Chart.tsx
Outdated
|
|
||
| render() { | ||
| const { data, entityType, title } = this.props | ||
| if (data.length === 0) return null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, could it just be if (!data.length)?
This PR adds a tab page with charts for daily number of trip requests, and OTP users with trip requests, and total number of OTP users. Daily stats are populated only for OTP-middleware deployments that include ibi-group/otp-middleware#349.