-
-
Notifications
You must be signed in to change notification settings - Fork 840
fix(webapp): disable gh-triggered preview deployments if the preview env is disabled #2595
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
Conversation
|
WalkthroughAdds preview environment enablement plumbing end-to-end. Presenter now computes isPreviewEnvironmentEnabled via a Prisma runtimeEnvironment query and restructures flows using ResultAsync.combine, returning gitHubApp { enabled, connectedRepository, installations, isPreviewEnvironmentEnabled } plus parsed buildSettings. Services update createConnectedRepo and updateConnectedRepo signatures to include previewDeploymentsEnabled, add a private isPreviewEnvironmentEnabled(projectId), and adjust connect/update flows to derive previewDeploymentsEnabled from that flag. The route loader exposes isPreviewEnvironmentEnabled to the Page, which passes it to ConnectedGitHubRepoForm. The form conditionally disables the “Create preview deployments” switch and shows an InfoIconTooltip with a billing link when the feature isn’t enabled. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
(8 hunks)apps/webapp/app/services/projectSettings.server.ts
(6 hunks)apps/webapp/app/services/projectSettingsPresenter.server.ts
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}
: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/services/projectSettings.server.ts
apps/webapp/app/services/projectSettingsPresenter.server.ts
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/services/projectSettings.server.ts
apps/webapp/app/services/projectSettingsPresenter.server.ts
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/services/projectSettings.server.ts
apps/webapp/app/services/projectSettingsPresenter.server.ts
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
{apps/webapp/app/**/*.server.{ts,tsx},apps/webapp/app/routes/**/*.ts}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access environment variables only via the env export from app/env.server.ts; do not reference process.env directly
Files:
apps/webapp/app/services/projectSettings.server.ts
apps/webapp/app/services/projectSettingsPresenter.server.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead
Files:
apps/webapp/app/services/projectSettings.server.ts
apps/webapp/app/services/projectSettingsPresenter.server.ts
🧬 Code graph analysis (2)
apps/webapp/app/services/projectSettingsPresenter.server.ts (2)
apps/webapp/app/v3/buildSettings.ts (1)
BuildSettingsSchema
(3-7)apps/webapp/app/services/projectSettings.server.ts (1)
isPreviewEnvironmentEnabled
(312-328)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (5)
apps/webapp/app/services/projectSettings.server.ts (1)
isPreviewEnvironmentEnabled
(312-328)apps/webapp/app/hooks/useOrganizations.ts (1)
useOrganization
(39-43)apps/webapp/app/components/primitives/Tooltip.tsx (1)
InfoIconTooltip
(105-126)apps/webapp/app/components/primitives/TextLink.tsx (1)
TextLink
(29-94)apps/webapp/app/utils/pathBuilder.ts (1)
v3BillingPath
(470-474)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (22)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
The build server already handles this case gracefully, but doesn't hurt showing an early hint when preview deployments are disabled.