-
Notifications
You must be signed in to change notification settings - Fork 6k
feat: Unique Sessions 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 SummaryThis PR adds a new Major Issues:
Recommendation: Confidence Score: 1/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant EventsPage
participant WebsiteControls
participant UniqueSessions
participant useWebsiteEventsQuery
participant API
participant Database
User->>EventsPage: Navigate to Events page
EventsPage->>WebsiteControls: Render controls
WebsiteControls->>UniqueSessions: Render with websiteId
UniqueSessions->>useWebsiteEventsQuery: Query events (view: 'all')
useWebsiteEventsQuery->>API: GET /websites/{websiteId}/events
Note over API: Applies pagination (default 20 items)
API->>Database: Query events with filters
Database-->>API: Return paginated events (20 items)
API-->>useWebsiteEventsQuery: PageResult with 20 events
useWebsiteEventsQuery-->>UniqueSessions: Return query result
UniqueSessions->>UniqueSessions: Calculate unique sessions from 20 events
Note over UniqueSessions: BUG: Only counts sessions from first page
UniqueSessions-->>WebsiteControls: Display count (incorrect)
WebsiteControls-->>EventsPage: Render controls with metrics
EventsPage-->>User: Show page with incorrect session count
|
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
|
Hey @mikecao, Can you review this PR? |
This pull request introduces a new
UniqueSessionscomponent 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