feat(v5): add "Job configuration" and "Triggers" settings sections#2586
feat(v5): add "Job configuration" and "Triggers" settings sections#2586rmnbrd merged 5 commits intonew-navigationfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## new-navigation #2586 +/- ##
==================================================
- Coverage 45.39% 45.01% -0.39%
==================================================
Files 1120 1068 -52
Lines 22883 21955 -928
Branches 6681 6443 -238
==================================================
- Hits 10388 9882 -506
+ Misses 10630 10292 -338
+ Partials 1865 1781 -84
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:
|
rmnbrd
changed the title
rmnbrd
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8093dae. Configure here.
|
|
||
| function RouteComponent() { | ||
| return <div className="px-10 py-7">Configuration</div> | ||
| useDocumentTitle('Job configuration - Service settings') |
There was a problem hiding this comment.
Document title hardcoded, wrong for lifecycle jobs
Medium Severity
The useDocumentTitle call is hardcoded to 'Job configuration - Service settings', but the JobConfiguration component dynamically renders the heading as either "Job configuration" (for CRON jobs) or "Triggers" (for LIFECYCLE jobs). For lifecycle jobs, the browser tab title will incorrectly say "Job configuration" while the page heading says "Triggers".
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8093dae. Configure here.
There was a problem hiding this comment.
Pull request overview
Adds the new v5 “Job configuration” (cron) / “Triggers” (lifecycle) settings content by moving the job configuration form into the service-settings domain and wiring it into the console-v5 /settings/configure route, while removing the legacy (react-router) application settings “configure job” route/page.
Changes:
- Introduce
JobConfigurationfeature inlibs/domains/service-settings/featureand expose it via the package index. - Update console-v5
/settings/configureroute to render the newJobConfigurationpage and set a document title. - Remove legacy application settings “configure job” page/route/constants and its unit test.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/shared/routes/src/lib/sub-router/application.router.ts | Removes legacy APPLICATION_SETTINGS_CONFIGURE_URL constant. |
| libs/pages/application/src/lib/ui/page-settings-configure-job/page-settings-configure-job.tsx | Deletes legacy job configuration page component (moved to domain feature). |
| libs/pages/application/src/lib/ui/page-settings-configure-job/page-settings-configure-job.spec.tsx | Deletes legacy unit test for the removed page. |
| libs/pages/application/src/lib/router/router.tsx | Removes legacy settings route entry for /configure. |
| libs/pages/application/src/lib/feature/page-settings-feature/page-settings-feature.tsx | Removes legacy sidebar link for job “configure/triggers” settings. |
| libs/domains/service-settings/feature/src/lib/job-configuration/job-configuration.tsx | Adds the new Job configuration/Triggers settings UI + form submission logic, using TanStack router params. |
| libs/domains/service-settings/feature/src/lib/job-configuration/job-configuration.spec.tsx | Updates tests to target JobConfiguration and mocks TanStack router params. |
| libs/domains/service-settings/feature/src/index.ts | Exports JobConfiguration from the feature package. |
| apps/console-v5/src/routes/_authenticated/.../settings/configure.tsx | Uses JobConfiguration in the v5 configure settings route and sets document title. |
Comments suppressed due to low confidence (4)
libs/domains/service-settings/feature/src/lib/job-configuration/job-configuration.tsx:46
JobConfigureSettingsis givenloading={!service}, butserviceis a required prop here and the parent fetch usessuspense: true, so this will always befalse. This makes the loading state misleading; consider wiring it to the query loading state (or remove the prop if not needed with suspense).
libs/domains/service-settings/feature/src/lib/job-configuration/job-configuration.tsx:170if (!service) returnreturnsundefinedfrom a React component. Please returnnull(or a fallback UI) to keep the component return type consistent and avoid lint/TS issues.
libs/domains/service-settings/feature/src/lib/job-configuration/job-configuration.tsx:26<div className="">has an emptyclassName, which is a no-op and adds noise. Consider removing it or adding the intended layout classes (it previously had padding/max-width in the removed page component).
libs/pages/application/src/lib/router/router.tsx:46- Removing the legacy
/settings/configureroute means existing deep links (and any URL-based helpers) to application job configuration will break. There are still references to/application/:applicationId/settings/configureinlibs/shared/assistant/feature/src/lib/hooks/use-contextual-doc-links/use-contextual-doc-links.ts(e.g., mappings around lines ~248 and ~835); please update/remove those mappings or provide a redirect if this removal is intentional.
export const ROUTER_APPLICATION_SETTINGS: Route[] = [
{
path: APPLICATION_SETTINGS_DOCKERFILE_URL,
component: <PageSettingsDockerfileFeature />,
},
{
path: APPLICATION_SETTINGS_ADVANCED_SETTINGS_URL,
component: <PageSettingsAdvancedFeature />,
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Thanks @rmnbrd !
-
Could you update token for
job-configure-settings.tsxand move this component out of the@qovery/shared/console-shared? (this one is soon outdated) -
Could you add an
max-w-content-with-navigation-leftclass for the content (not the header) like what we have for other settings page? -
On lifecycle service (example here) the margin is too huge between title "Events" and description "Select one or more event..."
Summary
PR adding the "Job configuration" and "Triggers" settings sections
Screenshots / Recordings
Note
Medium Risk
Adds a new job settings UI that edits job schedule/trigger payloads and shifts routing/navigation, which could break job settings access or submit incorrect schedules if mismatched with backend expectations.
Overview
Adds a new Console v5
settings/configureroute that renders the newJobConfigurationscreen (and sets the document title) instead of a placeholder.Introduces
JobConfigurationindomains/service-settings, using TanStack Router params and React Hook Form to edit and submit CRON and LIFECYCLE job schedule/trigger settings viauseEditService, and updates unit tests accordingly.Removes the legacy application settings “configure” route/link and the old
PageSettingsConfigureJobUI/tests, and dropsAPPLICATION_SETTINGS_CONFIGURE_URLfrom shared routes.Reviewed by Cursor Bugbot for commit 8093dae. Bugbot is set up for automated code reviews on this repo. Configure here.