fix(tests): use sql.json in onboarding migration test and refresh metrics snapshot#1420
fix(tests): use sql.json in onboarding migration test and refresh metrics snapshot#1420mantrakp04 wants to merge 3 commits intodevfrom
Conversation
…rics snapshot
- Match the rest of the migration test suite by using `sql.json(...)` instead of
`${JSON.stringify(...)}::jsonb` in the `add_project_onboarding_state` test, so
the helper handles serialization and parameter binding consistently.
- Refresh the `internal-metrics` snapshot so that `active_users_by_country.AQ`
reflects the actual `last_active_at_millis desc` ordering produced by the
test (mailbox-2 became active in AQ after mailbox-1, so it sorts first).
Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates a Prisma migration test to use the Postgres helper ChangesPrisma Migration Test JSON Helper Update
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryTwo test-only maintenance fixes with no production code changes.
Confidence Score: 5/5Both changes are test-only; no production code is touched, and each fix is a straightforward correction that aligns test behavior with actual runtime semantics. The migration test now uses the driver's native No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Test as Migration Test
participant Driver as postgres Driver
participant DB as PostgreSQL
Note over Test,DB: Before (manual cast)
Test->>Driver: "sql`SET onboardingState = ${JSON.stringify(obj)}::jsonb`"
Driver->>DB: literal string + ::jsonb cast
DB-->>Test: result
Note over Test,DB: After (sql.json)
Test->>Driver: "sql`SET onboardingState = ${sql.json(obj)}`"
Driver->>DB: parameterized JSON binding (no manual cast)
DB-->>Test: result
Reviews (1): Last reviewed commit: "fix(tests): use sql.json in onboarding m..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Test-maintenance PR to align JSON parameter binding in a Prisma migration test with existing repo patterns, and to refresh an internal metrics snapshot to match the expected ordering of “active users by country”.
Changes:
- Updated onboarding migration test to use
sql.json(...)for JSON parameter binding instead of manualJSON.stringify(... )::jsonb. - Refreshed the
internal-metricsVitest snapshot to reflect the expected ordering foractive_users_by_country.AQ.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| apps/backend/prisma/migrations/20260420000000_add_project_onboarding_state/tests/default-and-updates.ts | Switches JSONB update to use sql.json(onboardingState) for consistent driver-side serialization/binding. |
| apps/e2e/tests/backend/endpoints/api/v1/snapshots/internal-metrics.test.ts.snap | Updates the expected ordering of AQ active users to match the test’s sign-in activity ordering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Updated the SQL query in the onboarding migration test to explicitly cast the onboardingState to jsonb, ensuring proper data type handling during the update process.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| await sql` | ||
| UPDATE "Project" | ||
| SET "onboardingState" = ${JSON.stringify(onboardingState)}::jsonb | ||
| SET "onboardingState" = ${sql.json(onboardingState)}::jsonb |
There was a problem hiding this comment.
| SET "onboardingState" = ${sql.json(onboardingState)}::jsonb | |
| SET "onboardingState" = ${sql.json(onboardingState)} |
Unnecessary ::jsonb cast applied after sql.json() function in migration test UPDATE statement causes inconsistency with other migration tests
Summary
Two small test-maintenance fixes that came up while running the suite:
apps/backend/prisma/migrations/20260420000000_add_project_onboarding_state/tests/default-and-updates.ts): switch the JSON insert from\${JSON.stringify(onboardingState)}::jsonbto\${sql.json(onboardingState)}. This matches the pattern used by every other migration test in the repo (see20260214000000_fix_trusted_domains_config/tests/*) and lets thepostgresdriver handle serialization and parameter binding consistently rather than relying on a manual::jsonbcast.apps/e2e/tests/backend/endpoints/api/v1/__snapshots__/internal-metrics.test.ts.snap): updateactive_users_by_country.AQto listmailbox-2beforemailbox-1. Theshould return metrics data with userstest signs inmailbox-1(mailboxes[0]) into AQ first, then later signsmailbox-2(mailboxes[1]) into AQ, so sorted bylast_active_at_millis descmailbox-2should come first. The snapshot now matches that ordering.No production code is touched — both changes are limited to test fixtures.
Test plan
pnpm -C apps/backend test run(migration tests)pnpm -C apps/e2e test run internal-metrics(snapshot test)pnpm lintpnpm typecheckMade with Cursor
Summary by CodeRabbit