feat(new-nav): add "Terraform arguments" settings page#2549
feat(new-nav): add "Terraform arguments" settings page#2549rmnbrd merged 2 commits intonew-navigationfrom
Conversation
|
Qovery can create a Preview Environment for this PR.
This comment has been generated from Qovery AI 🤖.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## new-navigation #2549 +/- ##
=================================================
Coverage ? 43.82%
=================================================
Files ? 25
Lines ? 753
Branches ? 250
=================================================
Hits ? 330
Misses ? 330
Partials ? 93
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a new “Terraform arguments” settings page in the console-v5 service settings area, backed by a reusable settings component in the service-settings domain library.
Changes:
- Introduces
TerraformArgumentsSettingscomponent for configuring per-command Terraform extra arguments. - Wires the new console-v5 route to render the settings component.
- Updates exports and adjusts UI styling/header to match the new settings layout.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| libs/domains/service-settings/feature/src/lib/terraform-arguments-settings/terraform-arguments-settings.tsx | Implements the Terraform arguments settings UI and submit logic. |
| libs/domains/service-settings/feature/src/lib/terraform-arguments-settings/terraform-arguments-settings.spec.tsx | Updates tests to render the renamed component and mocks TanStack Router. |
| libs/domains/service-settings/feature/src/index.ts | Exports the new settings component from the feature library. |
| apps/console-v5/src/routes/_authenticated/organization/$organizationId/project/$projectId/environment/$environmentId/service/$serviceId/settings/terraform-arguments.tsx | Routes the new settings page in console-v5. |
Comments suppressed due to low confidence (3)
libs/domains/service-settings/feature/src/lib/terraform-arguments-settings/terraform-arguments-settings.tsx:99
- Remove the debug
console.logstatements from the submit handler. Leaving these in a settings page will clutter the console in production and can leak potentially sensitive service/form data.
libs/domains/service-settings/feature/src/lib/terraform-arguments-settings/terraform-arguments-settings.tsx:95 useServiceis called asuseService({ serviceId })withoutenvironmentIdor an explicitserviceType. In the currentuseServiceimplementation, service type resolution is disabled unlessenvironmentIdis provided, so this call can leaveresolvedServiceTypeundefined and the details query never enables (service stays undefined). Also, the formdefaultValuesare derived fromservice, but RHF only appliesdefaultValueson first render, so initializing before the service loads can leave the form empty and potentially overwrite existingaction_extra_argumentson save. PreferuseService({ environmentId, serviceId, suspense: true })(oruseService({ serviceId, serviceType: 'TERRAFORM', suspense: true })) and either rely on the existing settings-route<Suspense>boundary or reset the form whenservicebecomes available.
libs/domains/service-settings/feature/src/lib/terraform-arguments-settings/terraform-arguments-settings.spec.tsx:14- The
useParamsmock only returnsorganizationId, but the component readsprojectId,environmentId, andserviceId(and passes them to hooks/props). Keeping these as empty defaults in tests diverges from real route behavior and can hide issues where those params are required. Consider returning all expected params in the mock (similar to other service-settings specs).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...rvice-settings/feature/src/lib/terraform-arguments-settings/terraform-arguments-settings.tsx
Outdated
Show resolved
Hide resolved
...rvice-settings/feature/src/lib/terraform-arguments-settings/terraform-arguments-settings.tsx
Outdated
Show resolved
Hide resolved
...rvice-settings/feature/src/lib/terraform-arguments-settings/terraform-arguments-settings.tsx
Outdated
Show resolved
Hide resolved
| const { data: service } = useService({ serviceId: applicationId }) | ||
| export function TerraformArgumentsSettings() { | ||
| const { organizationId = '', projectId = '', environmentId = '', serviceId = '' } = useParams({ strict: false }) | ||
| const { data: service } = useService({ serviceId }) |
There was a problem hiding this comment.
| const { data: service } = useService({ serviceId }) | |
| const { data: service } = useService({ serviceId, suspense: true }) |
Summary
PR adding the "Terraform arguments" settings section
Screenshots / Recordings
📺 https://www.loom.com/share/920480bfd6b44a1b9cdc8bfe10ae7d81