refactor: atoms e2e setup #23900
refactor: atoms e2e setup #23900
Conversation
WalkthroughReworks atoms E2E infra: CI e2e-atoms workflow now runs on pull_request, launches a local API v2 with Postgres and Redis on a larger runner, and manages startup/teardown. Adds examples app E2E support: .env.e2e.example, test Prisma schema and client selection, Playwright config updates (load .env.e2e locally, webServer env, DB reset, globalTeardown), global teardown logic to delete test users/teams and test.db, test script entries, seed addition of an e2e OAuth client, multiple Playwright tests adding page reloads, and a small API test teardown robustness fix. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
keithwillcode
added
core
| areEmailsEnabled Boolean @default(false) | ||
| areDefaultEventTypesEnabled Boolean @default(true) | ||
| areCalendarEventsEnabled Boolean @default(true) | ||
There was a problem hiding this comment.
Added a new line to trigger database re-seeding in CI database because seed.ts was updated to have a new platform oAuth client just for e2e
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
dosubot
bot
added
automated-tests
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
scripts/seed.ts (1)
187-199: Make OAuth client seeding idempotent and avoid hard‑coding secrets.
- Use upsert to prevent unique constraint failures on re‑seeding.
- Read client id/secret from env (NEXT_PUBLIC_X_CAL_ID, X_CAL_SECRET_KEY) to avoid committing long‑lived secrets.
Apply this diff:
- await prisma.platformOAuthClient.create({ - data: { - name: "examples-app", - redirectUris: ["http://localhost:4321"], - permissions: 1023, - areEmailsEnabled: true, - organizationId: team.id, - id: "clxyyy21o0003sbk7yw5z6tzg", - secret: - "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJuYW1lIjoiQWNtZSAiLCJwZXJtaXNzaW9ucyI6MTAyMywicmVkaXJlY3RVcmlzIjpbImh0dHA6Ly9sb2NhbGhvc3Q6NDMyMSJdLCJib29raW5nUmVkaXJlY3RVcmkiOiIiLCJib29raW5nQ2FuY2VsUmVkaXJlY3RVcmkiOiIiLCJib29raW5nUmVzY2hlZHVsZVJlZGlyZWN0VXJpIjoiIiwiYXJlRW1haWxzRW5hYmxlZCI6dHJ1ZSwiaWF0IjoxNzE5NTk1ODA4fQ.L5_jSS14fcKLCD_9_DAOgtGd6lUSZlU5CEpCPaPt41I", - }, - }); + await prisma.platformOAuthClient.upsert({ + where: { id: process.env.NEXT_PUBLIC_X_CAL_ID ?? "clxyyy21o0003sbk7yw5z6tzg" }, + update: { + name: "examples-app", + redirectUris: ["http://localhost:4321"], + permissions: 1023, + areEmailsEnabled: true, + organizationId: team.id, + secret: + process.env.X_CAL_SECRET_KEY ?? + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJuYW1lIjoiQWNtZSAiLCJwZXJtaXNzaW9ucyI6MTAyMywicmVkaXJlY3RVcmlzIjpbImh0dHA6Ly9sb2NhbGhvc3Q6NDMyMSJdLCJib29raW5nUmVkaXJlY3RVcmkiOiIiLCJib29raW5nQ2FuY2VsUmVkaXJlY3RVcmkiOiIiLCJib29raW5nUmVzY2hlZHVsZVJlZGlyZWN0VXJpIjoiIiwiYXJlRW1haWxzRW5hYmxlZCI6dHJ1ZSwiaWF0IjoxNzE5NTk1ODA4fQ.L5_jSS14fcKLCD_9_DAOgtGd6lUSZlU5CEpCPaPt41I", + }, + create: { + id: process.env.NEXT_PUBLIC_X_CAL_ID ?? "clxyyy21o0003sbk7yw5z6tzg", + name: "examples-app", + redirectUris: ["http://localhost:4321"], + permissions: 1023, + areEmailsEnabled: true, + organizationId: team.id, + secret: + process.env.X_CAL_SECRET_KEY ?? + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJuYW1lIjoiQWNtZSAiLCJwZXJtaXNzaW9ucyI6MTAyMywicmVkaXJlY3RVcmlzIjpbImh0dHA6Ly9sb2NhbGhvc3Q6NDMyMSJdLCJib29raW5nUmVkaXJlY3RVcmkiOiIiLCJib29raW5nQ2FuY2VsUmVkaXJlY3RVcmkiOiIiLCJib29raW5nUmVzY2hlZHVsZVJlZGlyZWN0VXJpIjoiIiwiYXJlRW1haWxzRW5hYmxlZCI6dHJ1ZSwiaWF0IjoxNzE5NTk1ODA4fQ.L5_jSS14fcKLCD_9_DAOgtGd6lUSZlU5CEpCPaPt41I", + }, + });
🧹 Nitpick comments (12)
apps/api/v2/package.json (1)
19-19: Avoid manual build ordering; lean on workspace graph/turbo.Hard‑coding the build chain for platform packages is brittle. Prefer driving this via the workspace dependency graph (e.g., turbo run build) or a single repo‑level build to reduce maintenance and ordering drift.
packages/platform/examples/base/.gitignore (1)
39-41: Also ignore prisma/test.db.The E2E setup writes prisma/test.db, but this pattern won’t be ignored by “test.db” at repo root. Add prisma/test.db to avoid accidental commits.
Apply this diff:
dev.db -test.db +test.db +prisma/test.dbpackages/platform/examples/base/.env.e2e.example (1)
9-9: Drop quotes around boolean-ish flag for dotenv linters.Quotes here trigger dotenv‑linter warnings; unquoted 1 is fine.
Apply this diff:
-NEXT_PUBLIC_IS_E2E="1" +NEXT_PUBLIC_IS_E2E=1packages/platform/examples/base/src/lib/prismaClient.ts (1)
1-5: Brittle path to test Prisma client; add fallback and avoid hard failures.Requiring ../../node_modules/.prisma/test-client is fragile in a monorepo. Add a try/catch fallback to @prisma/client so local runs don’t break if the test client isn’t generated.
Apply this diff:
-const isE2E = process.env.NEXT_PUBLIC_IS_E2E === "1"; - -const { PrismaClient } = isE2E - ? require("../../node_modules/.prisma/test-client") - : require("@prisma/client"); +const isE2E = process.env.NEXT_PUBLIC_IS_E2E === "1"; +let PrismaClient: any; +if (isE2E) { + try { + ({ PrismaClient } = require(process.cwd() + "/node_modules/.prisma/test-client")); + } catch { + ({ PrismaClient } = require("@prisma/client")); + } +} else { + ({ PrismaClient } = require("@prisma/client")); +}Ensure your test DB generation step writes the client to node_modules/.prisma/test-client before tests run (e.g., prisma generate --schema prisma/schema.test.prisma).
packages/platform/examples/base/tests/create-event-type-atom/create-event-type.e2e.ts (1)
7-7: Stabilize the reload to reduce flakiness.Wait for network to go idle on reload to avoid race conditions right after the refresh.
- await page.reload(); + await page.reload({ waitUntil: "networkidle" });Optional: consider dropping the initial "/" navigation altogether and navigating directly to "/event-types" with
{ waitUntil: "networkidle" }.packages/platform/examples/base/tests/create-team-event-type-atom/create-team-event-type.e2e.ts (1)
8-8: Make the reload deterministic.Match the intent across tests by waiting for network idle on reload.
- await page.reload(); + await page.reload({ waitUntil: "networkidle" });Since this test is skipped, feel free to apply consistently when unskipping.
packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.ts (1)
8-8: Harden the page reload against races.Wait for network idle to ensure the app is ready before assertions/interactions.
- await page.reload(); + await page.reload({ waitUntil: "networkidle" });apps/api/v2/src/modules/oauth-clients/controllers/oauth-client-users/oauth-client-users.controller.e2e-spec.ts (1)
722-726: Guard optional cleanup to avoid noisy exceptions.Small nit: explicitly check for presence before attempting deletion, instead of relying on try/catch for undefined dereference.
- try { - await userRepositoryFixture.delete(postResponseDataTwo.user.id); - } catch (e) { - // User might have been deleted by the test - } + if (postResponseDataTwo?.user?.id) { + try { + await userRepositoryFixture.delete(postResponseDataTwo.user.id); + } catch { + // User might have been deleted by the test + } + }packages/platform/examples/base/package.json (1)
15-15: Make reset script cross‑platform.Avoid
rm -fto support Windows runners/devs.- "db:reset:test": "rm -f prisma/test.db && yarn db:push:test" + "db:reset:test": "node -e \"require('fs').rmSync('prisma/test.db', { force: true })\" && yarn db:push:test"packages/platform/examples/base/playwright.config.ts (2)
47-50: Quote ORGANIZATION_ID to avoid shell parsing issues.Unquoted values can break if they contain non-alphanumerics (or be empty). Quote it for consistency with other vars.
Apply this diff:
- ? `yarn workspace @calcom/atoms dev-on && yarn workspace @calcom/atoms build && rm -f prisma/test.db && yarn db:generate:test && yarn db:reset:test && NEXT_PUBLIC_IS_E2E=1 NODE_ENV=test NEXT_PUBLIC_X_CAL_ID="${process.env.ATOMS_E2E_OAUTH_CLIENT_ID}" X_CAL_SECRET_KEY="${process.env.ATOMS_E2E_OAUTH_CLIENT_SECRET}" NEXT_PUBLIC_CALCOM_API_URL="${process.env.ATOMS_E2E_API_URL}" VITE_BOOKER_EMBED_OAUTH_CLIENT_ID="${process.env.ATOMS_E2E_OAUTH_CLIENT_ID_BOOKER_EMBED}" VITE_BOOKER_EMBED_API_URL="${process.env.ATOMS_E2E_API_URL}" ORGANIZATION_ID=${process.env.ATOMS_E2E_ORG_ID} yarn dev:e2e` + ? `yarn workspace @calcom/atoms dev-on && yarn workspace @calcom/atoms build && rm -f prisma/test.db && yarn db:generate:test && yarn db:reset:test && NEXT_PUBLIC_IS_E2E=1 NODE_ENV=test NEXT_PUBLIC_X_CAL_ID="${process.env.ATOMS_E2E_OAUTH_CLIENT_ID}" X_CAL_SECRET_KEY="${process.env.ATOMS_E2E_OAUTH_CLIENT_SECRET}" NEXT_PUBLIC_CALCOM_API_URL="${process.env.ATOMS_E2E_API_URL}" VITE_BOOKER_EMBED_OAUTH_CLIENT_ID="${process.env.ATOMS_E2E_OAUTH_CLIENT_ID_BOOKER_EMBED}" VITE_BOOKER_EMBED_API_URL="${process.env.ATOMS_E2E_API_URL}" ORGANIZATION_ID="${process.env.ATOMS_E2E_ORG_ID}" yarn dev:e2e`
5-11: Fail fast in CI if required env vars are missing.Preflight validation avoids opaque failures later in the startup chain.
Insert after dotenv.config:
if (!process.env.CI) { dotenv.config({ path: path.resolve(__dirname, ".env.e2e") }); } + +if (process.env.CI) { + const required = [ + "ATOMS_E2E_OAUTH_CLIENT_ID", + "ATOMS_E2E_OAUTH_CLIENT_SECRET", + "ATOMS_E2E_API_URL", + "ATOMS_E2E_OAUTH_CLIENT_ID_BOOKER_EMBED", + "ATOMS_E2E_ORG_ID", + ]; + const missing = required.filter((k) => !process.env[k]); + if (missing.length) { + throw new Error(`Missing CI env vars for atoms e2e: ${missing.join(", ")}`); + } +}packages/platform/examples/base/prisma/schema.test.prisma (1)
17-18: Use @updatedat for automatic timestamp updates.
@default(now())won’t update on modify;@updatedAtis the canonical choice.Apply this diff:
- updatedAt DateTime @default(now()) + updatedAt DateTime @updatedAt
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (19)
.github/workflows/e2e-atoms.yml(2 hunks)apps/api/v2/package.json(1 hunks)apps/api/v2/src/modules/oauth-clients/controllers/oauth-client-users/oauth-client-users.controller.e2e-spec.ts(1 hunks)packages/platform/examples/base/.env.e2e.example(1 hunks)packages/platform/examples/base/.gitignore(2 hunks)packages/platform/examples/base/global-teardown.ts(1 hunks)packages/platform/examples/base/package.json(1 hunks)packages/platform/examples/base/playwright.config.ts(3 hunks)packages/platform/examples/base/prisma/schema.test.prisma(1 hunks)packages/platform/examples/base/src/lib/prismaClient.ts(1 hunks)packages/platform/examples/base/src/pages/api/managed-user.ts(0 hunks)packages/platform/examples/base/tests/availability-settings-atom/availability-settings-atom.e2e.ts(1 hunks)packages/platform/examples/base/tests/booker-atom/booker-atom.e2e.ts(1 hunks)packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.ts(1 hunks)packages/platform/examples/base/tests/create-event-type-atom/create-event-type.e2e.ts(1 hunks)packages/platform/examples/base/tests/create-team-event-type-atom/create-team-event-type.e2e.ts(1 hunks)packages/prisma/schema.prisma(1 hunks)scripts/seed.ts(2 hunks)turbo.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/platform/examples/base/src/pages/api/managed-user.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/platform/examples/base/tests/availability-settings-atom/availability-settings-atom.e2e.tspackages/platform/examples/base/tests/booker-atom/booker-atom.e2e.tspackages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.tsapps/api/v2/src/modules/oauth-clients/controllers/oauth-client-users/oauth-client-users.controller.e2e-spec.tspackages/platform/examples/base/global-teardown.tsscripts/seed.tspackages/platform/examples/base/src/lib/prismaClient.tspackages/platform/examples/base/tests/create-event-type-atom/create-event-type.e2e.tspackages/platform/examples/base/tests/create-team-event-type-atom/create-team-event-type.e2e.tspackages/platform/examples/base/playwright.config.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/platform/examples/base/tests/availability-settings-atom/availability-settings-atom.e2e.tspackages/platform/examples/base/tests/booker-atom/booker-atom.e2e.tspackages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.tsapps/api/v2/src/modules/oauth-clients/controllers/oauth-client-users/oauth-client-users.controller.e2e-spec.tspackages/platform/examples/base/global-teardown.tsscripts/seed.tspackages/platform/examples/base/src/lib/prismaClient.tspackages/platform/examples/base/tests/create-event-type-atom/create-event-type.e2e.tspackages/platform/examples/base/tests/create-team-event-type-atom/create-team-event-type.e2e.tspackages/platform/examples/base/playwright.config.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/platform/examples/base/tests/availability-settings-atom/availability-settings-atom.e2e.tspackages/platform/examples/base/tests/booker-atom/booker-atom.e2e.tspackages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.tsapps/api/v2/src/modules/oauth-clients/controllers/oauth-client-users/oauth-client-users.controller.e2e-spec.tspackages/platform/examples/base/global-teardown.tsscripts/seed.tspackages/platform/examples/base/src/lib/prismaClient.tspackages/platform/examples/base/tests/create-event-type-atom/create-event-type.e2e.tspackages/platform/examples/base/tests/create-team-event-type-atom/create-team-event-type.e2e.tspackages/platform/examples/base/playwright.config.ts
🧠 Learnings (4)
📚 Learning: 2025-09-09T03:29:43.025Z
Learnt from: emrysal
PR: calcom/cal.com#23692
File: packages/lib/server/service/InsightsBookingBaseService.ts:16-16
Timestamp: 2025-09-09T03:29:43.025Z
Learning: In the Cal.com codebase, readonlyPrisma is still an instance of PrismaClient, making type changes from `typeof readonlyPrisma` to `PrismaClient` less critical since they are fundamentally compatible types.
Applied to files:
packages/platform/examples/base/src/lib/prismaClient.ts
📚 Learning: 2025-09-15T12:58:12.826Z
Learnt from: CR
PR: calcom/cal.com#0
File: AGENTS.md:0-0
Timestamp: 2025-09-15T12:58:12.826Z
Learning: Use `yarn build` to build all packages
Applied to files:
apps/api/v2/package.json
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/platform/examples/base/tests/create-event-type-atom/create-event-type.e2e.tspackages/platform/examples/base/tests/create-team-event-type-atom/create-team-event-type.e2e.ts
📚 Learning: 2025-07-18T08:49:18.779Z
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: .env.example:354-357
Timestamp: 2025-07-18T08:49:18.779Z
Learning: For E2E integration tests that require real third-party service credentials (like Outlook calendar), it's acceptable to temporarily include actual test account credentials in .env.example during feature development and validation, provided there's a clear plan to replace them with placeholder values before final release. Test credentials should be for dedicated test tenants/accounts, not production systems.
Applied to files:
packages/platform/examples/base/.env.e2e.example
🪛 actionlint (1.7.7)
.github/workflows/e2e-atoms.yml
48-48: label "buildjet-16vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🪛 dotenv-linter (3.3.0)
packages/platform/examples/base/.env.e2e.example
[warning] 3-3: [UnorderedKey] The NEXT_PUBLIC_CALCOM_API_URL key should go before the NEXT_PUBLIC_X_CAL_ID key
(UnorderedKey)
[warning] 4-4: [UnorderedKey] The VITE_BOOKER_EMBED_OAUTH_CLIENT_ID key should go before the X_CAL_SECRET_KEY key
(UnorderedKey)
[warning] 5-5: [UnorderedKey] The VITE_BOOKER_EMBED_API_URL key should go before the VITE_BOOKER_EMBED_OAUTH_CLIENT_ID key
(UnorderedKey)
[warning] 6-6: [UnorderedKey] The ORGANIZATION_ID key should go before the VITE_BOOKER_EMBED_API_URL key
(UnorderedKey)
[warning] 7-7: [UnorderedKey] The ATOMS_E2E_APPLE_ID key should go before the NEXT_PUBLIC_CALCOM_API_URL key
(UnorderedKey)
[warning] 8-8: [UnorderedKey] The ATOMS_E2E_APPLE_CONNECT_APP_SPECIFIC_PASSCODE key should go before the ATOMS_E2E_APPLE_ID key
(UnorderedKey)
[warning] 9-9: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 9-9: [UnorderedKey] The NEXT_PUBLIC_IS_E2E key should go before the NEXT_PUBLIC_X_CAL_ID key
(UnorderedKey)
🔇 Additional comments (9)
packages/platform/examples/base/.gitignore (1)
30-31: Good call keeping the e2e example env tracked.packages/prisma/schema.prisma (1)
1889-1893: Whitespace-only change — no functional impact.packages/platform/examples/base/tests/booker-atom/booker-atom.e2e.ts (1)
13-13: LGTM — reload helps stabilize initial state.packages/platform/examples/base/tests/availability-settings-atom/availability-settings-atom.e2e.ts (1)
13-13: LGTM — page reload reduces flakiness before assertions.turbo.json (1)
283-286: Confirm necessity of making the secret global.Adding X_CAL_SECRET_KEY to globalEnv makes it available to all tasks. While not exposed client-side (no NEXT_PUBLIC prefix), prefer scoping secrets to only tasks that need them (e.g., examples app E2E) to reduce blast radius.
If needed only for Playwright/global-teardown, consider removing it from globalEnv and relying on the test’s env loading instead.
.github/workflows/e2e-atoms.yml (1)
30-38: Wire envs used by global teardown (or rely on its fallbacks).CI sets ATOMS_E2E_* while global-teardown reads NEXT_PUBLIC_X_CAL_ID, X_CAL_SECRET_KEY, NEXT_PUBLIC_CALCOM_API_URL. Either keep the teardown fallback (preferred), or export these here to ensure cleanup runs in CI.
Example mapping if you prefer the workflow approach:
## atoms e2e examples app env ATOMS_E2E_OAUTH_CLIENT_ID: ${{ secrets.ATOMS_E2E_OAUTH_CLIENT_ID }} ATOMS_E2E_OAUTH_CLIENT_SECRET: ${{ secrets.ATOMS_E2E_OAUTH_CLIENT_SECRET }} ATOMS_E2E_API_URL: ${{ secrets.ATOMS_E2E_API_URL }} ATOMS_E2E_ORG_ID: ${{ secrets.ATOMS_E2E_ORG_ID }} ATOMS_E2E_OAUTH_CLIENT_ID_BOOKER_EMBED: ${{ secrets.ATOMS_E2E_OAUTH_CLIENT_ID_BOOKER_EMBED }} ATOMS_E2E_APPLE_ID: ${{ secrets.ATOMS_E2E_APPLE_ID }} ATOMS_E2E_APPLE_CONNECT_APP_SPECIFIC_PASSCODE: ${{ secrets.ATOMS_E2E_APPLE_CONNECT_APP_SPECIFIC_PASSCODE }} + + # For global-teardown compatibility + NEXT_PUBLIC_X_CAL_ID: ${{ secrets.ATOMS_E2E_OAUTH_CLIENT_ID }} + X_CAL_SECRET_KEY: ${{ secrets.ATOMS_E2E_OAUTH_CLIENT_SECRET }} + NEXT_PUBLIC_CALCOM_API_URL: ${{ secrets.ATOMS_E2E_API_URL }}packages/platform/examples/base/playwright.config.ts (2)
5-7: Scoped local env loading looks correct.Loading .env.e2e only outside CI aligns with the PR objective to avoid CI relying on local env files.
20-20: Global teardown hook wired properly.This ensures cleanup runs post-tests.
packages/platform/examples/base/prisma/schema.test.prisma (1)
1-8: Test client/output and SQLite datasource look appropriate.Paths resolve to the package-local node_modules and prisma/test.db as intended.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/platform/examples/base/playwright.config.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/platform/examples/base/playwright.config.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/platform/examples/base/playwright.config.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/platform/examples/base/playwright.config.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (2)
packages/platform/examples/base/playwright.config.ts (2)
5-7: LGTM: Local-only .env.e2e loading is correct.Matches the objective to keep CI clean and rely on .env.e2e locally.
20-20: LGTM: Global teardown wiring.
require.resolve("./global-teardown")is a safe way to pass the module path.
E2E results are ready! |
This reverts commit 28b8487.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/features/shell/useBottomNavItems.ts (1)
41-41: LGTM! Improved href values for better accessibility.Changing the
hreffrom empty strings topublicPageUrlimproves accessibility and semantic correctness. TheonClickhandlers still callpreventDefault(), so normal click behavior remains unchanged. However, providing a validhrefbenefits screen readers and keyboard navigation.Note: These changes appear unrelated to the PR's stated objectives (atoms e2e setup refactor). Consider moving unrelated refactors to separate PRs for clearer change tracking.
Also applies to: 58-58
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/shell/useBottomNavItems.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/shell/useBottomNavItems.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/shell/useBottomNavItems.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/shell/useBottomNavItems.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: E2E Atoms
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/shell/useBottomNavItems.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/shell/useBottomNavItems.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/shell/useBottomNavItems.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/shell/useBottomNavItems.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: E2E Atoms
This reverts commit a82b845.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/features/shell/useBottomNavItems.ts (1)
17-27: Consider logging URL construction failures.The
withBasehelper silently returnsundefinedwhen URL construction fails (line 24). For debugging purposes, consider logging these failures during development to help identify configuration issues.Apply this diff to add optional logging:
const withBase = (u?: string): string | undefined => { if (!u) return undefined; const base = process.env.NEXTAUTH_URL || process.env.NEXT_PUBLIC_WEBAPP_URL || "http://localhost:3000"; try { return new URL(u, base).toString(); } catch { + if (process.env.NODE_ENV === 'development') { + console.warn(`Failed to construct URL from base="${base}" and path="${u}"`); + } return undefined; } };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/features/shell/SideBar.tsx(4 hunks)packages/features/shell/useBottomNavItems.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/shell/useBottomNavItems.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/shell/useBottomNavItems.tspackages/features/shell/SideBar.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/shell/useBottomNavItems.tspackages/features/shell/SideBar.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/shell/SideBar.tsx
🧬 Code graph analysis (2)
packages/features/shell/useBottomNavItems.ts (1)
packages/lib/constants.ts (1)
IS_DUB_REFERRALS_ENABLED(222-223)
packages/features/shell/SideBar.tsx (2)
packages/features/auth/lib/next-auth-options.ts (1)
session(746-771)packages/ui/components/dropdown/Dropdown.tsx (1)
ButtonOrLink(144-159)
🔇 Additional comments (6)
packages/features/shell/SideBar.tsx (2)
51-61: LGTM! Addresses previous review concern.The
buildPublicPageUrlhelper now constructs user-specific URLs instead of using a hardcoded constant. The implementation safely handles missing data by returning an empty string, which aligns with the URL normalization inuseBottomNavItems.ts(thewithBasehelper will handle empty/relative values).
197-226: EnsureuseBottomNavItemsfilters out items without validhref
I didn’t find explicit filtering of entries withundefined/nullhrefin the hook—verify that such items are excluded before rendering.packages/features/shell/useBottomNavItems.ts (4)
48-53: LGTM! URL safety improvements are well-implemented.The pre-computed safe href variables ensure that only resolvable absolute URLs are used in navigation items, preventing broken links and SSR issues.
56-67: LGTM!The skip_trial item correctly implements an action-only pattern without an href, appropriate for its button-based interaction.
69-94: LGTM!Both items handle URL safety correctly:
view_public_pageonly renders when a valid absolute URL existscopy_public_page_linkprefers the safe URL and shows an error toast when no valid URL is available (empty string is falsy, so the clipboard operation won't execute)
96-117: LGTM! Filter ensures href safety.The conditional rendering using short-circuit evaluation (e.g.,
safeReferHref && { ... }) combined withfilter(Boolean)ensures that items without valid href values are removed before reaching the rendering code inSideBar.tsx. This addresses the href safety concern raised in the other file's review.
|
This PR is being marked as stale due to inactivity. |
Linear CAL-6422