Data-grid overhaul + session-replays / team-payments dashboard surfaces#1424
Open
mantrakp04 wants to merge 14 commits into
Open
Data-grid overhaul + session-replays / team-payments dashboard surfaces#1424mantrakp04 wants to merge 14 commits into
mantrakp04 wants to merge 14 commits into
Conversation
…ents surfaces - Rewrite data-grid with URL-synced state, new sizing logic, tests - Move analytics/replays → session-replays; add per-user session replays card - Add team-analytics and team-payments to team detail page - Add project_user.last_active_at index + permission-definitions pagination - Various editable-input / inline-save-discard / settings polish
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds backend pagination/search and session-replays sorting, extends shared interfaces/SDK and template/server implementations for paginated listings, refactors DataGrid with URL-state and TanStack, converts tables to server-backed infinite sources, adds team analytics/payments and user session-replays UI, updates OpenAPI/docs/registry, adds a DB index, E2E tests, dependency, and AGENTS git-safety guidance. ChangesMain change DAG
UI / DataGrid and Tables
Pages / Features
Estimated code review effort: Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
…sion handling - Introduced pagination for teams in the dashboard, allowing for efficient data retrieval and display. - Updated permission definitions to include cursor-based pagination, improving user experience when navigating through permissions. - Refactored various components to utilize the new paginated API, ensuring consistency across the application. - Added error handling for pagination and improved loading states in user session replays and team member tables. - Enhanced the data grid to support URL-synced state for column widths and visibility, improving user customization options. This update significantly enhances the dashboard's performance and usability, particularly for users managing large teams and permissions.
mantrakp04
force-pushed
the
refactor/data-grid-and-dashboard-surfaces
branch
from
23db822 to
cabcf19
Compare
- Updated the ProjectUser model to simplify indexing on lastActiveAt. - Refactored permission definitions pagination to utilize a unified schema, improving consistency across project and team permissions. - Enhanced error handling in pagination logic for better user feedback. - Improved loading states and data retrieval efficiency in various components, including session replays and team permissions. These changes streamline the permission management experience and optimize data handling in the dashboard.
…election logic - Renamed cursor comparator variable for clarity in session replay queries. - Enhanced search query handling to include escape character for better SQL compatibility. - Introduced error handling for asynchronous queries in the Team Analytics section, providing default values for failed queries. - Refactored data grid selection logic to support flexible row selection modes and improved column width distribution. These changes enhance the robustness and usability of the dashboard components, particularly in data retrieval and user interaction.
Contributor
Greptile SummaryThis PR refactors the dashboard data-grid into a TanStack Table-backed primitive with URL-synced column state, adds per-user session replays and weekly-active metrics to the user detail page, promotes session replays to a top-level route, and lands team analytics/payments tabs. Backend changes introduce cursor+search pagination on the team list and session-replay list endpoints, a shared
Confidence Score: 5/5Safe to merge — the pagination endpoints and data-grid rewrite are structurally sound, with only edge-case cursor misuse footguns that normal dashboard usage will not trigger. The cursor-direction mismatch would only produce wrong pages if a caller reuses a cursor across a sort-direction or filter change; the dashboard resets pagination on such changes. All SQL is parameterized, the index migration is non-blocking, and async handlers follow the runAsynchronouslyWithAlert convention. apps/backend/src/app/api/latest/internal/session-replays/route.tsx and apps/backend/src/app/api/latest/teams/crud.tsx — revisit cursor design before promoting these endpoints to the public API surface. Important Files Changed
Sequence DiagramsequenceDiagram
participant Dashboard
participant TeamsAPI as GET /teams
participant SessionAPI as GET /session-replays
participant PermAPI as GET /permission-definitions
Dashboard->>TeamsAPI: "?limit=10&query=acme&order_by=created_at"
TeamsAPI-->>Dashboard: items + next_cursor
Dashboard->>TeamsAPI: "?limit=10&cursor=prev&query=acme"
Note over TeamsAPI: findUnique validates cursor exists
TeamsAPI-->>Dashboard: items + next_cursor
Dashboard->>SessionAPI: "?limit=50&q=alice&sort_direction=desc"
SessionAPI-->>Dashboard: items + next_cursor
Dashboard->>SessionAPI: "?limit=50&cursor=prev&sort_direction=asc"
Note over SessionAPI: direction mismatch - wrong page boundary
SessionAPI-->>Dashboard: incorrect page (no error)
Dashboard->>PermAPI: "?limit=20&query=read&cursor=perm-id"
Note over PermAPI: in-memory filter+sort+slice
PermAPI-->>Dashboard: items + next_cursor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/backend/src/app/api/latest/internal/session-replays/route.tsx:204-215
**Cursor is not direction-aware — wrong page returned on direction change**
The cursor pivot only stores `lastEventAt` and `id`, with no record of which `sort_direction` it was created under. If a caller issues page 1 with `sort_direction=desc` and then fetches page 2 with the same cursor but `sort_direction=asc`, the `AND (lastEventAt > cursorPivot.lastEventAt …)` condition skips to a completely different position in the ascending order, silently returning an incorrect page rather than surfacing an error. Consider embedding the sort direction in the opaque cursor token, or rejecting cursor requests whose inferred direction doesn't match `sort_direction`.
### Issue 2 of 2
apps/backend/src/app/api/latest/teams/crud.tsx:282-355
**Cursor is not search-filter-aware — Prisma may silently skip matching teams**
The cursor row is validated for existence via `findUnique`, but not for whether it satisfies the active `queryFilter`. Per Prisma's cursor behaviour, when the cursor row does not match the `where` clause, Prisma still positions at that row in the sort order and returns the first `take` rows after it that do match, silently missing every matching team before the cursor position. The OpenAPI description should at minimum warn that cursors are filter- and direction-specific.
Reviews (2): Last reviewed commit: "fix(api): improve pagination error handl..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/backend/src/app/api/latest/internal/session-replays/route.tsx (1)
200-203: ⚡ Quick winLeading-wildcard
ILIKEwon't use a B-tree index — consider a trigram index forpu."displayName"andsr."id"::text.
%${q}%precludes index usage on a regular B-tree, so search latency will scale linearly with the filtered set. For projects with many users/replays this can become a noticeable hot path. If/when this matters in production, apg_trgmGIN index (e.g.,CREATE INDEX ... USING gin (lower("displayName") gin_trgm_ops)) is the standard remedy. Cappingq.lengthserver-side (e.g., reject >128 chars) is also worth adding to bound worst-case work and prevent abuse via huge patterns.🤖 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 `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx` around lines 200 - 203, The leading-wildcard ILIKE pattern built in the Prisma.sql block (searchQuery via escapeLikePattern used against pu."displayName" and sr."id"::text) prevents B-tree index use and will be slow at scale; switch to recommending and documenting creation of pg_trgm trigram GIN/GiST indexes (e.g., on lower("displayName") and on sr.id::text) and change queries to use lower(...) for indexable trigram searches, and also add a server-side cap on searchQuery length (e.g., reject >128 chars) to bound work and avoid abuse; update any relevant query code paths using Prisma.sql and escapeLikePattern to reflect lowercasing and trigram-ready patterns.
🤖 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.
Nitpick comments:
In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx`:
- Around line 200-203: The leading-wildcard ILIKE pattern built in the
Prisma.sql block (searchQuery via escapeLikePattern used against
pu."displayName" and sr."id"::text) prevents B-tree index use and will be slow
at scale; switch to recommending and documenting creation of pg_trgm trigram
GIN/GiST indexes (e.g., on lower("displayName") and on sr.id::text) and change
queries to use lower(...) for indexable trigram searches, and also add a
server-side cap on searchQuery length (e.g., reject >128 chars) to bound work
and avoid abuse; update any relevant query code paths using Prisma.sql and
escapeLikePattern to reflect lowercasing and trigram-ready patterns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35947dcf-fe8d-4a9e-9b2e-afbe9ac9c2dc
📒 Files selected for processing (13)
apps/backend/src/app/api/latest/internal/session-replays/route.tsxapps/backend/src/app/api/latest/teams/crud.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/page-client.tsxapps/dashboard/src/components/data-table/team-member-search-table.tsxapps/dashboard/src/components/data-table/team-member-table.tsxapps/dashboard/src/components/data-table/user-search-picker.tsxapps/dashboard/src/components/data-table/user-table.tsxapps/dashboard/src/components/export-users-dialog.tsxpackages/template/src/lib/stack-app/apps/implementations/server-app-impl.tspackages/template/src/lib/stack-app/apps/interfaces/server-app.tspackages/template/src/lib/stack-app/teams/index.tspackages/template/src/lib/stack-app/users/index.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/dashboard/src/components/data-table/user-search-picker.tsx
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/page-client.tsx
- apps/dashboard/src/components/data-table/team-member-search-table.tsx
- apps/dashboard/src/components/data-table/user-table.tsx
- apps/dashboard/src/components/export-users-dialog.tsx
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx
- apps/dashboard/src/components/data-table/team-member-table.tsx
- Enhanced the `paginatePermissionDefinitions` function to include sorting by ID using a string comparison utility. - Improved error handling for cursor validation, throwing a `StatusError` when the cursor is not found, ensuring better user feedback. - Updated related components to maintain consistency in pagination behavior across the application. These changes optimize the user experience when navigating permission definitions and enhance data retrieval reliability.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@apps/backend/src/app/api/latest/permission-definitions-pagination.ts`:
- Around line 24-25: Change the truthy cursor checks to explicit null/undefined
checks so empty string cursors aren't treated as absent: replace uses of `if
(query.cursor)` (and the other occurrence at lines 41-42) with an explicit
presence check such as `query.cursor != null` (or `query.cursor !== undefined &&
query.cursor !== null`), and if you require non-empty cursors validate
`query.cursor !== ""` as well; keep the existing logic around `query.limit` and
the StatusError thrown when `query.cursor` is present but `query.limit` is
undefined.
In `@docs/content/docs/sdk/types/user.mdx`:
- Line 1948: The anchor link for the React-hook description points to the wrong
anchor `#serveruserlistteamspaginatedoptions`; update the link target to
`#serveruserlistteamspaginated` so it matches the actual heading anchor for
listTeamsPaginated; locate the sentence mentioning listTeamsPaginated (the hook
description) and replace the anchor suffix accordingly to restore the correct
internal link.
- Around line 1716-1718: The TOC anchor IDs and the internal doc link
incorrectly include the "[options]" suffix; update the anchor fragments for
listTeamsPaginated and useTeamsPaginated to follow the established pattern by
removing the "options" suffix: change any occurrences of
"#serveruserlistteamspaginatedoptions" to "#serveruserlistteamspaginated" and
"#serveruseruseteamspaginatedoptions" to "#serveruseruseteamspaginated" (this
affects the TOC entries for the methods listTeamsPaginated and useTeamsPaginated
and the referenced internal documentation link for the same methods).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b62d283-a819-46ef-aee4-e6ed2a88dda0
📒 Files selected for processing (12)
apps/backend/src/app/api/latest/permission-definitions-pagination.tsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/teams/[teamId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsxapps/dashboard/src/components/data-table/team-table.tsxdocs-mintlify/sdk/objects/stack-app.mdxdocs-mintlify/sdk/types/user.mdxdocs/content/docs/sdk/objects/stack-app.mdxdocs/content/docs/sdk/types/user.mdxpackages/dashboard-ui-components/src/components/data-grid/state.tspackages/dashboard-ui-components/src/components/data-grid/use-url-state.tspackages/template/src/dev-tool/index.tspackages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/template/src/dev-tool/index.ts
- apps/dashboard/src/components/data-table/team-table.tsx
- docs-mintlify/sdk/objects/stack-app.mdx
- docs/content/docs/sdk/objects/stack-app.mdx
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx
- packages/dashboard-ui-components/src/components/data-grid/use-url-state.ts
- packages/dashboard-ui-components/src/components/data-grid/state.ts
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/teams/[teamId]/page-client.tsx
- packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts
- docs-mintlify/sdk/types/user.mdx
- Improved cursor-based pagination logic to ensure that the cursor matches the current filter set, preventing issues when filters are swapped between requests. - Updated error handling to throw a `KnownErrors.ItemNotFound` when the cursor is invalid or does not meet duration constraints. - Refactored related components to maintain consistency in pagination behavior across the application. These changes enhance the reliability of data retrieval and improve user experience when navigating session replays and team permissions.
- Updated the user permissions type to allow for null values, indicating permission fetch failures distinctly from empty arrays. - Improved loading state management in the TeamMemberTable and EditPermissionDialog components to provide better user feedback during permission retrieval. - Refactored permission-related logic to ensure consistent handling of permissions across the dashboard. These changes enhance the user experience by clearly indicating permission loading issues and improving the overall reliability of permission management.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/e-commerce/src/app/edit-shop/page.tsx (1)
54-61:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftValidate FormData input before persisting to metadata.
The code extracts
formDatavalues with type casts (as string,Number()) but performs no validation:
- If a product name field is missing,
formData.get(...)returnsnull, which when castas stringbecomes the literal string"null"or causes unexpected behavior.- If a price field is missing or contains non-numeric text,
Number(...)producesNaN, corrupting the product'sdollarPrice.Per coding guidelines, avoid type casts and fail early with explicit errors. Per the retrieved learning, user input (FormData) should be validated before persisting to
serverMetadata.🛡️ Proposed validation fix
const products: Product[] = []; for (let i = 0; i < oldShop.products.length; i++) { + const name = formData.get(`product${i}Name`); + const priceStr = formData.get(`product${i}DollarPrice`); + if (typeof name !== 'string' || !name.trim()) { + throw new Error(`Product ${i} name is required`); + } + const dollarPrice = Number(priceStr); + if (!Number.isFinite(dollarPrice) || dollarPrice < 0) { + throw new Error(`Product ${i} price must be a non-negative number`); + } products.push({ ...oldShop.products[i], - name: formData.get(`product${i}Name`) as string, - dollarPrice: Number(formData.get(`product${i}DollarPrice`)), + name: name.trim(), + dollarPrice, }); }🤖 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 `@examples/e-commerce/src/app/edit-shop/page.tsx` around lines 54 - 61, The loop building products from oldShop.products must validate FormData values before persisting: replace the unchecked casts in the products construction (the block using formData.get(`product${i}Name`) and formData.get(`product${i}DollarPrice`) inside the for loop that fills products) with explicit checks—ensure product name exists and is a non-empty string (reject if formData.get(...) is null or empty) and parse the price with Number/parseFloat and verify !isNaN(value) and value >= 0; if validation fails, throw/return an explicit error rather than writing bad values into dollarPrice or serverMetadata. Keep the rest of the product shape (spread of oldShop.products[i]) but fail early on invalid input so serverMetadata persists only validated product.name and product.dollarPrice.
🧹 Nitpick comments (2)
docs-mintlify/openapi/admin.json (1)
5883-5890: ⚡ Quick winEncode
limitcaps in the schema, not only the description.These parameters document “capped at 200” but the schema omits explicit bounds (
maximum, and ideallyminimum: 1), so generated clients/validators can’t enforce the contract early.Based on learnings: For generated OpenAPI artifacts in
docs-mintlify/openapi/*.json, do not make manual edits; update generator metadata/logic and regenerate schemas.Also applies to: 6454-6461, 8861-8868
🤖 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 `@docs-mintlify/openapi/admin.json` around lines 5883 - 5890, The OpenAPI parameter "limit" currently only documents "capped at 200" in its description but lacks schema bounds; update the generator metadata/logic that produces the docs-mintlify/openapi JSON so the "limit" parameter schema includes "maximum": 200 and "minimum": 1 (and ensure this change is applied wherever the same param is emitted - e.g., the other occurrences referenced) then regenerate the OpenAPI artifacts rather than manually editing docs-mintlify/openapi/*.json so generated clients/validators will enforce the contract.docs-mintlify/openapi/client.json (1)
3277-3283: ⚡ Quick winConstrain AI message roles in the read schema too.
These request schemas now narrow
messages[].roleto"user" | "assistant", but GET/internal/ai-conversations/{conversationId}still exposesmessages[].roleas plainstringat Line 3378. That leaves the regenerated SDK with asymmetric types for the same model and weakens exhaustiveness checks on consumers of the new SDK surface.Based on learnings: “For the generated OpenAPI artifacts in docs-mintlify/openapi/*.json, do not make manual edits. If the schema needs to change, update the schema generator logic (Yup/OpenAPI field metadata) and rerun the ‘Regenerate OpenAPI schemas’ workflow so committed JSON matches the generator output; manual changes will be reverted by regeneration.”
Also applies to: 3513-3519
🤖 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 `@docs-mintlify/openapi/client.json` around lines 3277 - 3283, The read schema for GET /internal/ai-conversations/{conversationId} has messages[].role typed as a plain string while request schemas constrain it to "user" | "assistant"; fix this by updating the schema generator (the Yup/OpenAPI field metadata that defines messages.role) so the generated OpenAPI output sets messages[].role to an enum ["user","assistant"] for the conversation response, then rerun the "Regenerate OpenAPI schemas" workflow to produce matching JSON; do not make a manual edit to the generated client.json since regenerating will overwrite it.
🤖 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 `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx`:
- Around line 183-202: The cursor validation in prisma.sessionReplay.findFirst
is dropping the lower bound because lastEventAtFrom and lastEventAtTo are spread
into separate lastEventAt keys so the second overwrites the first; update the
where clause in route.tsx (the prisma.sessionReplay.findFirst call that uses
cursorId and auth.tenancy.id) to build a single lastEventAt object that
conditionally includes gte: lastEventAtFrom and lte: lastEventAtTo (or omits the
entire lastEventAt filter if neither bound exists), preserving both bounds when
both are provided so cursor validation matches the main query.
In `@docs-mintlify/openapi/server.json`:
- Around line 4921-4924: The OpenAPI change turned the common response field
"success" into a string enum ("maybe, only if user with e-mail exists"); revert
"success" back to a boolean and instead add a separate "message" or "status"
string field (or an enum) to convey account-enumeration-safe text; make this
change in the schema generator (Yup/OpenAPI field metadata) that produces the
JSON (not by hand-editing the generated JSON), then rerun the "Regenerate
OpenAPI schemas" workflow so the committed docs-mintlify/openapi JSON matches
the generator output.
In `@examples/e-commerce/src/app/edit-shop/page.tsx`:
- Line 17: The three server actions that currently spread ...user.serverMetadata
should guard against undefined before spreading; update each place where
user.serverMetadata is spread (the occurrences spreading ...user.serverMetadata)
to use a null-safe pattern that falls back to an empty object when undefined
(e.g., replace spreading user.serverMetadata with spreading (user.serverMetadata
|| {}) or equivalent), ensuring the optional chaining usage
(user.serverMetadata?.eCommerceExample?.shop) remains unchanged and no runtime
errors occur when serverMetadata is undefined.
---
Outside diff comments:
In `@examples/e-commerce/src/app/edit-shop/page.tsx`:
- Around line 54-61: The loop building products from oldShop.products must
validate FormData values before persisting: replace the unchecked casts in the
products construction (the block using formData.get(`product${i}Name`) and
formData.get(`product${i}DollarPrice`) inside the for loop that fills products)
with explicit checks—ensure product name exists and is a non-empty string
(reject if formData.get(...) is null or empty) and parse the price with
Number/parseFloat and verify !isNaN(value) and value >= 0; if validation fails,
throw/return an explicit error rather than writing bad values into dollarPrice
or serverMetadata. Keep the rest of the product shape (spread of
oldShop.products[i]) but fail early on invalid input so serverMetadata persists
only validated product.name and product.dollarPrice.
---
Nitpick comments:
In `@docs-mintlify/openapi/admin.json`:
- Around line 5883-5890: The OpenAPI parameter "limit" currently only documents
"capped at 200" in its description but lacks schema bounds; update the generator
metadata/logic that produces the docs-mintlify/openapi JSON so the "limit"
parameter schema includes "maximum": 200 and "minimum": 1 (and ensure this
change is applied wherever the same param is emitted - e.g., the other
occurrences referenced) then regenerate the OpenAPI artifacts rather than
manually editing docs-mintlify/openapi/*.json so generated clients/validators
will enforce the contract.
In `@docs-mintlify/openapi/client.json`:
- Around line 3277-3283: The read schema for GET
/internal/ai-conversations/{conversationId} has messages[].role typed as a plain
string while request schemas constrain it to "user" | "assistant"; fix this by
updating the schema generator (the Yup/OpenAPI field metadata that defines
messages.role) so the generated OpenAPI output sets messages[].role to an enum
["user","assistant"] for the conversation response, then rerun the "Regenerate
OpenAPI schemas" workflow to produce matching JSON; do not make a manual edit to
the generated client.json since regenerating will overwrite it.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8c6274a1-02aa-4c78-b89f-704af1a806bb
📒 Files selected for processing (14)
apps/backend/src/app/api/latest/internal/session-replays/route.tsxapps/backend/src/app/api/latest/teams/crud.tsxapps/backend/src/lib/openapi.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/session-replays/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/teams/[teamId]/team-analytics.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsxapps/dashboard/src/components/data-table/team-member-table.tsxapps/e2e/tests/backend/endpoints/api/v1/project-permission-definitions.test.tsapps/e2e/tests/backend/endpoints/api/v1/team-permission-definitions.test.tsdocs-mintlify/openapi/admin.jsondocs-mintlify/openapi/client.jsondocs-mintlify/openapi/server.jsondocs-mintlify/openapi/webhooks.jsonexamples/e-commerce/src/app/edit-shop/page.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/teams/[teamId]/team-analytics.tsx
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/session-replays/page-client.tsx
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx
- apps/dashboard/src/components/data-table/team-member-table.tsx
Comment on lines
+183
to
202
| const row = await prisma.sessionReplay.findFirst({ | ||
| where: { | ||
| tenancyId: auth.tenancy.id, | ||
| id: cursorId, | ||
| ...userIdsFilter.length > 0 ? { projectUserId: { in: userIdsFilter } } : {}, | ||
| ...lastEventAtFrom ? { lastEventAt: { gte: lastEventAtFrom } } : {}, | ||
| ...lastEventAtTo ? { lastEventAt: { lte: lastEventAtTo } } : {}, | ||
| ...teamIdsFilter.length > 0 ? { | ||
| projectUser: { | ||
| teamMembers: { | ||
| some: { | ||
| tenancyId: auth.tenancy.id, | ||
| teamId: { in: teamIdsFilter }, | ||
| }, | ||
| }, | ||
| }, | ||
| } : {}, | ||
| }, | ||
| select: { id: true, lastEventAt: true, startedAt: true }, | ||
| }); |
Contributor
There was a problem hiding this comment.
lastEventAtFrom filter is silently dropped from cursor validation when both bounds are set.
Two separate object spreads target the same lastEventAt key:
...lastEventAtFrom ? { lastEventAt: { gte: lastEventAtFrom } } : {},
...lastEventAtTo ? { lastEventAt: { lte: lastEventAtTo } } : {},When both bounds are provided, the second spread overwrites the first, so findFirst only enforces lte and the gte lower bound is lost. This means a cursor whose lastEventAt is before lastEventAtFrom will still pass validation (no ItemNotFound), even though the main query at lines 218–219 correctly excludes such rows. The result is a cursor that anchors paging outside the active filter window — defeating the whole purpose of the validation block called out in the PR commit ("ensure the cursor matches the current filter set").
Note the teamIdsFilter and userIdsFilter are also keyed on tenancyId/teamId but those collisions happen to merge cleanly via Prisma nested types; only the lastEventAt pair self-collides.
🐛 Proposed fix: combine into a single `lastEventAt` clause
const row = await prisma.sessionReplay.findFirst({
where: {
tenancyId: auth.tenancy.id,
id: cursorId,
...userIdsFilter.length > 0 ? { projectUserId: { in: userIdsFilter } } : {},
- ...lastEventAtFrom ? { lastEventAt: { gte: lastEventAtFrom } } : {},
- ...lastEventAtTo ? { lastEventAt: { lte: lastEventAtTo } } : {},
+ ...(lastEventAtFrom || lastEventAtTo) ? {
+ lastEventAt: {
+ ...lastEventAtFrom ? { gte: lastEventAtFrom } : {},
+ ...lastEventAtTo ? { lte: lastEventAtTo } : {},
+ },
+ } : {},
...teamIdsFilter.length > 0 ? {🤖 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 `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx` around
lines 183 - 202, The cursor validation in prisma.sessionReplay.findFirst is
dropping the lower bound because lastEventAtFrom and lastEventAtTo are spread
into separate lastEventAt keys so the second overwrites the first; update the
where clause in route.tsx (the prisma.sessionReplay.findFirst call that uses
cursorId and auth.tenancy.id) to build a single lastEventAt object that
conditionally includes gte: lastEventAtFrom and lte: lastEventAtTo (or omits the
entire lastEventAt filter if neither bound exists), preserving both bounds when
both are provided so cursor validation matches the main query.
Comment on lines
+4921
to
+4924
| "type": "string", | ||
| "enum": [ | ||
| "maybe, only if user with e-mail exists" | ||
| ] |
Contributor
There was a problem hiding this comment.
Don't overload success with a status string.
Changing this endpoint to return success as a literal string makes one of the most common response fields in the spec stop being boolean, and the regenerated SDK will expose that breaking shape. If this endpoint needs an enumeration-safe message for account-enumeration protection, keep success boolean and add a separate message or status field in the generator source.
Based on learnings: For the generated OpenAPI artifacts in docs-mintlify/openapi/*.json, do not make manual edits. If the schema needs to change, update the schema generator logic (Yup/OpenAPI field metadata) and rerun the “Regenerate OpenAPI schemas” workflow so committed JSON matches the generator output; manual changes will be reverted by regeneration.
🤖 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 `@docs-mintlify/openapi/server.json` around lines 4921 - 4924, The OpenAPI
change turned the common response field "success" into a string enum ("maybe,
only if user with e-mail exists"); revert "success" back to a boolean and
instead add a separate "message" or "status" string field (or an enum) to convey
account-enumeration-safe text; make this change in the schema generator
(Yup/OpenAPI field metadata) that produces the JSON (not by hand-editing the
generated JSON), then rerun the "Regenerate OpenAPI schemas" workflow so the
committed docs-mintlify/openapi JSON matches the generator output.
| await (await stackServerApp.listUsers()).find(u => u.id === user.id)!.setServerMetadata({ | ||
| await user.setServerMetadata({ | ||
| // TODO this should be more like a transaction | ||
| ...user.serverMetadata, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if serverMetadata can be undefined in the CurrentServerUser type
# Search for CurrentServerUser or similar type definitions related to getUser
ast-grep --pattern 'interface $_ {
$$$
serverMetadata$$$
$$$
}'
# Search for getUser return type in stackServerApp
rg -nP -A5 'getUser.*\{' --type=ts -g '**/stack*' -g '!**/node_modules/**'Repository: hexclave/stack-auth
Length of output: 3766
🏁 Script executed:
# Search for CurrentServerUser type definition
rg -n "type CurrentServerUser|interface CurrentServerUser" --type=ts --type=tsxRepository: hexclave/stack-auth
Length of output: 91
🏁 Script executed:
# Read the file being reviewed to understand context
cat -n examples/e-commerce/src/app/edit-shop/page.tsx | head -80Repository: hexclave/stack-auth
Length of output: 2786
🏁 Script executed:
# Search for serverMetadata property definition in types
rg -n "serverMetadata" --type=ts --type=tsx -B2 -A2 | head -100Repository: hexclave/stack-auth
Length of output: 91
🏁 Script executed:
# Search for CurrentServerUser type definition
rg -n "type CurrentServerUser|interface CurrentServerUser" -A10 --type tsRepository: hexclave/stack-auth
Length of output: 1032
🏁 Script executed:
# Alternative: search in the stack package since getUser comes from stackServerApp
find . -name "*.ts" -path "*/stack*" -type f | head -20Repository: hexclave/stack-auth
Length of output: 888
🏁 Script executed:
# Search for setServerMetadata to understand the expected type
rg -n "setServerMetadata" -B5 -A5 --type ts | head -80Repository: hexclave/stack-auth
Length of output: 6879
Ensure user.serverMetadata is defined before spreading it.
All three server actions spread ...user.serverMetadata without checking if it exists. Line 11 uses optional chaining (user.serverMetadata?.eCommerceExample?.shop), indicating the developer recognizes it could be undefined. Despite being typed as any, spreading undefined at runtime throws an error. Per coding guidelines, code defensively and explicitly handle undefined cases.
🛡️ Proposed null-safe spread
await user.setServerMetadata({
- ...user.serverMetadata,
+ ...(user.serverMetadata ?? {}),
eCommerceExample: {Also applies to: 39-39, 64-64
🤖 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 `@examples/e-commerce/src/app/edit-shop/page.tsx` at line 17, The three server
actions that currently spread ...user.serverMetadata should guard against
undefined before spreading; update each place where user.serverMetadata is
spread (the occurrences spreading ...user.serverMetadata) to use a null-safe
pattern that falls back to an empty object when undefined (e.g., replace
spreading user.serverMetadata with spreading (user.serverMetadata || {}) or
equivalent), ensuring the optional chaining usage
(user.serverMetadata?.eCommerceExample?.shop) remains unchanged and no runtime
errors occur when serverMetadata is undefined.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/user-analytics.tsx (1)
669-691:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBase the chart empty state on the chart window, not the summary window.
When a day filter is active, the chart is built from the expanded
dailyrange (±15 days), buthasAnyEventstill comes fromsummary.total_events, which only counts the selected day. Filtering to a zero-event day will therefore hide surrounding activity and show the global “No events recorded” empty state instead.🤖 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 `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/users/[userId]/user-analytics.tsx around lines 669 - 691, The empty-state check currently uses data.summary.total_events which reflects only the summary window; change it to derive from the chart window data (the densified daily series) so the empty state matches the displayed chart. Move or recreate hasAnyEvent after the dense computation and set it to something like: hasAnyEvent = dense.some(d => (d.total_events ?? 0) > 0) (or sum dense totals > 0), ensuring the variable depends on dense (and dayFilter) rather than data.summary.total_events so the UI reflects densifyDaily/daily and dayFilter logic.
🤖 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
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/users/[userId]/user-analytics.tsx:
- Around line 433-439: The page currently treats any non-empty dayFilter string
as a “filtered” state even when parseDayFilterRange(dayFilter) returns null; fix
by computing a single validated activeDayFilter/validatedFilterRange (call
parseDayFilterRange once, e.g. const activeDayFilter =
parseDayFilterRange(dayFilter)) and use that value everywhere you currently
check dayFilter or filterRange for rendering and querying (banner,
deltas/sparklines, description rewrite); if parseDayFilterRange returns null
clear/reset the URL param or treat the page as unfiltered so the UI and queries
stay consistent (update all usages of filterRange/dayFilter in
user-analytics.tsx including the sections around the parseDayFilterRange call
and the other affected blocks).
- Around line 753-755: The ActivityChart is being hidden at extra-large
viewports because it's wrapped in a div with the "xl:hidden" utility; to fix,
remove or change that responsive class so the chart remains visible on xl
screens—locate the wrapper around the ActivityChart (the div containing
<ActivityChart daily={dense} hasAnyEvent={hasAnyEvent}
description={chartDescription} />) and either delete the "xl:hidden" class or
replace it with the correct responsive utility (e.g., no hide or "hidden
xl:block" if you need it hidden on small screens but shown on xl).
---
Outside diff comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/users/[userId]/user-analytics.tsx:
- Around line 669-691: The empty-state check currently uses
data.summary.total_events which reflects only the summary window; change it to
derive from the chart window data (the densified daily series) so the empty
state matches the displayed chart. Move or recreate hasAnyEvent after the dense
computation and set it to something like: hasAnyEvent = dense.some(d =>
(d.total_events ?? 0) > 0) (or sum dense totals > 0), ensuring the variable
depends on dense (and dayFilter) rather than data.summary.total_events so the UI
reflects densifyDaily/daily and dayFilter logic.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0dfc9c9f-afbf-417c-9090-dab32256f568
📒 Files selected for processing (6)
apps/backend/src/app/api/latest/permission-definitions-pagination.tsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/user-analytics.tsxapps/dashboard/src/components/data-table/team-member-table.tsxapps/dashboard/src/lib/apps-frontend.tsxdocs/content/docs/sdk/types/user.mdxpackages/template/src/dev-tool/index.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/backend/src/app/api/latest/permission-definitions-pagination.ts
- apps/dashboard/src/lib/apps-frontend.tsx
- packages/template/src/dev-tool/index.ts
- docs/content/docs/sdk/types/user.mdx
- apps/dashboard/src/components/data-table/team-member-table.tsx
Comment on lines
+433
to
+439
| function parseDayFilterRange(dayFilter: string): { since: Date, until: Date } | null { | ||
| const parts = dayFilter.split("-").map(Number); | ||
| if (parts.length !== 3 || parts.some((p) => !Number.isFinite(p))) return null; | ||
| const since = new Date(Date.UTC(parts[0], parts[1] - 1, parts[2])); | ||
| const until = new Date(since); | ||
| until.setUTCDate(until.getUTCDate() + 1); | ||
| return { since, until }; |
Contributor
There was a problem hiding this comment.
Validate dayFilter before treating the page as filtered.
Right now malformed values fall back to the unfiltered queries (filterRange === null), but the render path still shows the filter banner, disables deltas/sparklines, and rewrites descriptions whenever dayFilter is a non-empty string. A bad URL state will therefore show filtered UI around unfiltered analytics. Please derive one validated activeDayFilter and use that for both querying and rendering, or clear invalid values immediately.
Also applies to: 448-451, 586-599
🤖 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
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/users/[userId]/user-analytics.tsx
around lines 433 - 439, The page currently treats any non-empty dayFilter string
as a “filtered” state even when parseDayFilterRange(dayFilter) returns null; fix
by computing a single validated activeDayFilter/validatedFilterRange (call
parseDayFilterRange once, e.g. const activeDayFilter =
parseDayFilterRange(dayFilter)) and use that value everywhere you currently
check dayFilter or filterRange for rendering and querying (banner,
deltas/sparklines, description rewrite); if parseDayFilterRange returns null
clear/reset the URL param or treat the page as unfiltered so the UI and queries
stay consistent (update all usages of filterRange/dayFilter in
user-analytics.tsx including the sections around the parseDayFilterRange call
and the other affected blocks).
Comment on lines
+753
to
+755
| <div className="xl:hidden"> | ||
| <ActivityChart daily={dense} hasAnyEvent={hasAnyEvent} description={chartDescription} /> | ||
| </div> |
Contributor
There was a problem hiding this comment.
The daily activity chart disappears on xl screens.
This is the only ActivityChart render in the component, so wrapping it in xl:hidden removes the chart entirely for extra-large viewports.
♻️ Minimal fix
- <div className="xl:hidden">
- <ActivityChart daily={dense} hasAnyEvent={hasAnyEvent} description={chartDescription} />
- </div>
+ <ActivityChart daily={dense} hasAnyEvent={hasAnyEvent} description={chartDescription} />🤖 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
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/users/[userId]/user-analytics.tsx
around lines 753 - 755, The ActivityChart is being hidden at extra-large
viewports because it's wrapped in a div with the "xl:hidden" utility; to fix,
remove or change that responsive class so the chart remains visible on xl
screens—locate the wrapper around the ActivityChart (the div containing
<ActivityChart daily={dense} hasAnyEvent={hasAnyEvent}
description={chartDescription} />) and either delete the "xl:hidden" class or
replace it with the correct responsive utility (e.g., no hide or "hidden
xl:block" if you need it hidden on small screens but shown on xl).
- Replaced the use of `createDefaultDataGridState` with `useDataGridUrlState` across multiple components to improve state persistence and URL synchronization. - Updated pagination logic in various tables to ensure consistent handling of grid states and improve user experience during data retrieval. - Refactored components to utilize the new user picker table for better user selection functionality. These changes enhance the overall reliability and usability of the dashboard's data grid features.
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
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.
Summary
Refactors the dashboard data-grid into a smaller, URL-state-aware primitive and lands several new dashboard surfaces around it: per-user session replays, team-level analytics and payments, and pagination for permission definitions. Also moves session replays out from under
/analyticsto a top-level surface and adds aproject_user.last_active_atindex that the new weekly-active metrics depend on.Base:
dev→ Head:refactor/data-grid-and-dashboard-surfacesScope: 91 files, +5,644 / −1,858. Assets in this gist.
Screenshots
Captured from a local dev server (dashboard at
:8101, dummy project seeded with 26 users). Standard viewport 1920×1200, widescreen 2560×1440.Users list — data-grid overhaul in context
Widescreen:
User detail — new session-replays card + weekly metrics
Widescreen:
Session replays — moved out of
/analyticsWidescreen:
Project permissions — new pagination
Widescreen:
Other migrated surfaces
Scroll behaviour — new data-grid on the users list
What's new
packages/dashboard-ui-components/src/components/data-grid— rewritten. Trimmeddata-grid.tsxfrom ~1.7k LOC, split sizing logic intodata-grid-sizing.ts, addeduse-url-state.tsfor URL-synced state, and addeddata-grid.test.tsx.…/analytics/replaysto…/session-replays(top-level surface). Newuser-session-replays.tsxcard on the user detail page; new internalroute.tsxto feed it.team-analytics.tsxandteam-payments.tsx.permission-definitions-pagination.tsconsumed by both project and team permission CRUD routes.add_project_user_last_active_at_idx+ alastActiveAtindex that backs the new weekly-active metrics.editable-input,inline-save-discard,settings.tsx, walkthrough steps, and several data-table components touched in line with the data-grid rewrite.Notes for reviewers
apps/dashboard/src/components/data-table/*were updated to match — please scan those for any missed knobs.analytics/replays→session-replaysrename is git-tracked as renames; diffs should be small in those files.packages/template/src/lib/stack-app/session-replays/index.tsand additions inadmin-app-impl.ts/server-app-impl.tsmean OpenAPI specs (docs-mintlify/openapi/{admin,client}.json) regenerate; the diff is mostly mechanical.Test plan
pnpm typecheckcleanpnpm lintcleanpackages/dashboard-ui-components)lastActiveAtindex migration applied (i.e. on a fresh DB) and after applying it/analytics/replays/...URL path is no longer expected to be linked anywhereSummary by CodeRabbit
New Features
UX
Performance
Documentation
Tests