perf(store): consolidate user stats into single SQL query#149
Conversation
- Replace 3 sequential COUNT queries with one subquery-based query - Remove unused CountActiveTokensByUserID, CountOAuthConnectionsByUserID, and CountUserAuthorizationsByUserID from interface, store, and tests - Reduce admin user detail page from 4 DB round-trips to 2 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR improves performance for the admin user detail page (/admin/users/{uuid}) by consolidating multiple per-user stats queries into a single store call, reducing database round-trips when rendering user stats.
Changes:
- Added
Store.GetUserStatsByUserIDto fetch active token, OAuth connection, and authorization counts via a single SQL statement. - Updated
UserService.GetUserStatsto use the consolidated store method and removed the old per-stat count calls. - Removed the now-unused per-stat count methods from the store interface and regenerated mocks/tests accordingly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/store/user.go | Introduces GetUserStatsByUserID using one raw SQL query with subqueries to return all stats at once. |
| internal/store/user_test.go | Replaces per-count tests with a single test for the consolidated stats method (currently only covers the all-zero case). |
| internal/services/user.go | Switches service stats aggregation to the new consolidated store method and updates error wrapping. |
| internal/mocks/mock_store.go | Regenerates mocks to remove old count methods and add GetUserStatsByUserID. |
| internal/core/store.go | Updates UserReader interface to replace three count methods with GetUserStatsByUserID. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix GORM column mapping: OAuthConnectionCount was mapped to o_auth_connection_count instead of oauth_conn_count by convention - Add explicit gorm:"column:..." tags to prevent naming mismatches - Add non-zero test case with active/revoked tokens, OAuth connection, and user authorizations to verify correct counting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add is_active = true filter to authorization count subquery to match the behavior of ListUserAuthorizations and the admin UI - Add revoked authorization to test to verify inactive grants are excluded Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…stency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ters - Replace 3 bare int64 return values with types.UserStatsCounts struct to prevent positional transposition bugs - Rename authorization_count to active_authorization_count to clarify the is_active filter behavior - Add store/types/user_stats.go for the new struct type Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Clean up unused dependency checksum left after go mod tidy Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
COUNT(*)queries with a single SQL subquery for the admin user detail page (/admin/users/{uuid})CountActiveTokensByUserID,CountOAuthConnectionsByUserID, andCountUserAuthorizationsByUserIDfrom interface, store, mocks, and testsTest plan
TestGetUserStatsByUserID— new store-level test for the combined queryTestGetUserStats— existing service test passes with new implementation/admin/users/{uuid}displays correct token, connection, and authorization counts🤖 Generated with Claude Code