Skip to content

feat(application-container-creation-flow): add general step#2451

Merged
rmnbrd merged 10 commits intonew-navigationfrom
feat/new-nav/application-general
Mar 6, 2026
Merged

feat(application-container-creation-flow): add general step#2451
rmnbrd merged 10 commits intonew-navigationfrom
feat/new-nav/application-general

Conversation

@RemiBonnet
Copy link
Member

@RemiBonnet RemiBonnet commented Mar 5, 2026

Summary

Started creation flow for Application and Container with general step

  • Add link between new page to general stop with this format of URL:
    service/create/container/general?template=react
  • Remove general component from shared/console-shared library to domains/services/features to avoid circular dependencies for all general steps (jobs as well)
  • Duplicate container hooks to avoid having circular dependencies between organizations and services
  • I didn't manage dark color for logos (we need to find a solution for it)

Screenshots / Recordings

https://www.loom.com/share/968dd6cadb00494b978c344ccc88e26a

Testing

  • Changes tested locally in the relevant Console's pages and Storybooks
  • yarn test or yarn test -u (if you need to regenerate snapshots)
  • yarn format
  • yarn lint

PR Checklist

  • I followed naming, styling, and TypeScript rules (see .cursor/rules)
  • I performed a self-review (diff inspected, dead code removed)
  • I titled the PR using Conventional Commits with a scope when possible (e.g. feat(service): add new Terraform service) - required for semantic-release
  • I only kept necessary comments, written in English (watch for useless AI comments)
  • I involved a designer to validate UI changes if I am not a designer
  • I covered new business logic with tests (unit)
  • I confirmed CI is green (Codecov red can be accepted)
  • I reviewed and executed locally any AI-assisted code

@RemiBonnet
Copy link
Member Author

Qovery Preview

Qovery can create a Preview Environment for this PR.
To trigger its creation, please post a comment with one of the following command.

Command Blueprint environment
/qovery preview 15d69f24-9bc1-4a8d-80fe-d1bb1b2bcd00 New Navigation
/qovery preview {all|UUID1,UUID2,...} To preview multiple environments

This comment has been generated from Qovery AI 🤖.
Below, a word from its wisdom :

Programs must be written for people to read, and only incidentally for machines to execute

@RemiBonnet RemiBonnet force-pushed the feat/new-nav/application-general branch from 14dada9 to e7c1229 Compare March 5, 2026 15:43
@codecov
Copy link

codecov bot commented Mar 6, 2026

Codecov Report

❌ Patch coverage is 57.82313% with 62 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (new-navigation@44ffd03). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...eation-flow/application-container/step-general.tsx 35.89% 18 Missing and 7 partials ⚠️
...rvices/feature/src/lib/service-new/service-new.tsx 57.89% 4 Missing and 4 partials ⚠️
.../lib/job-general-settings/job-general-settings.tsx 0.00% 6 Missing ⚠️
...-container/application-container-creation-flow.tsx 77.77% 4 Missing and 2 partials ⚠️
...s/use-container-versions/use-container-versions.ts 69.23% 3 Missing and 1 partial ⚠️
...rvice-settings/git-repository-service-settings.tsx 82.35% 2 Missing and 1 partial ⚠️
...-container-settings/general-container-settings.tsx 82.35% 2 Missing and 1 partial ⚠️
...hooks/use-container-images/use-container-images.ts 0.00% 2 Missing ⚠️
...e-container-registries/use-container-registries.ts 0.00% 2 Missing ⚠️
...outer-types/application-container-create-params.ts 0.00% 2 Missing ⚠️
... and 1 more
Additional details and impacted files
@@                Coverage Diff                @@
##             new-navigation    #2451   +/-   ##
=================================================
  Coverage                  ?   44.42%           
=================================================
  Files                     ?      558           
  Lines                     ?    12592           
  Branches                  ?     3632           
=================================================
  Hits                      ?     5594           
  Misses                    ?     6061           
  Partials                  ?      937           
Flag Coverage Δ
unittests 44.42% <57.82%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@RemiBonnet RemiBonnet marked this pull request as ready for review March 6, 2026 08:29
@RemiBonnet RemiBonnet added the V5 label Mar 6, 2026
const watchFieldProvider = watch('source_provider')
const isLifecycleJob = service?.serviceType === 'JOB' && service.job_type === 'LIFECYCLE'

const ContainerSettings = ({ isSetting }: { isSetting?: boolean }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function will be recalculated each time the parent component renders. Could you please cache it?

But more importantly, why do we have this function? It just returns another component. I'm not sure to understand why it's needed.

Copy link
Member Author

@RemiBonnet RemiBonnet Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is in the page folder not necessary for the new-nav we will remove it, I remove it to avoid confusion

},
})
}
renderEditGitSettings={() => <EditGitRepositorySettingsFeature />}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the benefit of having these render props instead of what we had before?

Copy link
Member Author

@RemiBonnet RemiBonnet Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid circular dependencies and removing console-shared lib, but I'll move EditGitRepositorySettingsFeature inside the domains organization because we want to remove console-shared lib. This one is part of the General Settings for Service, I was mainly focus of the step-general in the creation flow for this PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const dataTemplate = serviceTemplates.find((service) => service.slug === slug)
const dataOptionTemplate = option !== 'current' ? findTemplateData(slug, option) : null

const ContainerSettings = ({ organizationId }: { organizationId: string }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. Why do we have this new function?

Copy link
Member Author

@RemiBonnet RemiBonnet Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will remove it when the flow is finish, I remove it to avoid confusion

…ditGitRepositorySettings components with modal support for token management
@RemiBonnet RemiBonnet requested a review from rmnbrd March 6, 2026 10:31
@rmnbrd rmnbrd merged commit 4ee0caf into new-navigation Mar 6, 2026
11 checks passed
@rmnbrd rmnbrd deleted the feat/new-nav/application-general branch March 6, 2026 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants