-
Notifications
You must be signed in to change notification settings - Fork 6.4k
feat: Unique Visitors Count on Events Page #3851
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
base: master
Are you sure you want to change the base?
Conversation
|
@Yashh56 is attempting to deploy a commit to the umami-software Team on Vercel. A member of the Team first needs to authorize it. |
Greptile OverviewGreptile SummaryAdded a
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant WebsiteControls
participant UniqueVisitors
participant useWebsiteStatsQuery
participant API
User->>WebsiteControls: Views Events page
WebsiteControls->>UniqueVisitors: Render with websiteId
UniqueVisitors->>useWebsiteStatsQuery: Request stats data
useWebsiteStatsQuery->>API: GET /websites/{websiteId}/stats
alt Loading state
UniqueVisitors->>User: Display Loading dots
end
alt Success
API-->>useWebsiteStatsQuery: Return WebsiteStatsData
useWebsiteStatsQuery-->>UniqueVisitors: data.visitors
UniqueVisitors->>User: Display visitor count with label
end
alt Error
API-->>useWebsiteStatsQuery: Error response
useWebsiteStatsQuery-->>UniqueVisitors: error
UniqueVisitors->>User: Display "Error loading data" in red
end
|
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.
Additional Comments (6)
-
src/components/metrics/UniqueSessions.tsx, line 6 (link)logic: this query only fetches paginated events (default 20 per page), so
uniqueSessionCountwill only count sessions from the first 20 events, not all events in the selected time range. useuseWebsiteStatsQueryinstead which returns avisitorsfield with the correct unique session count. -
src/components/metrics/UniqueSessions.tsx, line 5 (link)syntax: missing TypeScript type for props
-
src/components/metrics/UniqueSessions.tsx, line 11 (link)style: using
anytype defeats the purpose of TypeScript type safety -
src/components/metrics/UniqueSessions.tsx, line 16-27 (link)logic: no loading or error states displayed to user. when data is loading or if query fails, the component shows "Unique Sessions: 0" which is misleading
-
src/components/metrics/UniqueSessions.tsx, line 19-20 (link)style: unnecessary whitespace inside
Textcomponent -
src/components/metrics/UniqueSessions.tsx, line 22-25 (link)style: unnecessary whitespace inside
Textcomponent
2 files reviewed, 6 comments
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.
2 files reviewed, 2 comments
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.
2 files reviewed, 2 comments
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
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.
No files reviewed, no comments
|
Hey @franciscao633, Can You Review this PR? |
This pull request introduces a new
UniqueVisitorscomponent to display the count of unique website sessions, integrates it into the website controls UI and fixes #3830. The main changes are the creation of the new component and its addition to the controls row.Screenshots