Skip to content

Feat/manager 20320 #20577

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: feat/pci-project
Choose a base branch
from
Open

Feat/manager 20320 #20577

wants to merge 1 commit into from

Conversation

@FabienHenon
Copy link
Contributor

@FabienHenon FabienHenon commented Nov 24, 2025

Description

Ticket Reference: #...

Additional Information

@github-actions github-actions bot added has conflicts Has conflicts to resolve before merging universe-public-cloud feature New feature labels Nov 24, 2025
@github-actions github-actions bot removed the has conflicts Has conflicts to resolve before merging label Nov 25, 2025
@FabienHenon FabienHenon marked this pull request as ready for review November 25, 2025 15:35
@FabienHenon FabienHenon requested review from a team as code owners November 25, 2025 15:35
@github-actions github-actions bot added the has conflicts Has conflicts to resolve before merging label Nov 26, 2025
@FabienHenon FabienHenon force-pushed the feat/MANAGER-20320 branch 2 times, most recently from 6f5bf13 to 0c4974f Compare November 27, 2025 09:01
@github-actions github-actions bot removed the has conflicts Has conflicts to resolve before merging label Nov 27, 2025
@github-actions github-actions bot added has conflicts Has conflicts to resolve before merging dependencies Pull requests that update a dependency file labels Nov 27, 2025
@helabenkhalfallah helabenkhalfallah requested a review from a team as a code owner November 27, 2025 12:26
@github-actions github-actions bot added the continuous-integration Changes to our CI configuration files and scripts label Nov 27, 2025
@github-actions github-actions bot removed the continuous-integration Changes to our CI configuration files and scripts label Nov 27, 2025
@github-actions github-actions bot removed the has conflicts Has conflicts to resolve before merging label Nov 28, 2025
@github-actions github-actions bot added the has conflicts Has conflicts to resolve before merging label Nov 28, 2025
@FabienHenon
Copy link
Contributor Author

FabienHenon commented Nov 28, 2025

@copilot Could you find any breaking change (like different behavior in useEffect hooks with different dependencies, etc...) in the changes in this PR (Which should only impact the linter)

Copilot AI mentioned this pull request Nov 28, 2025
Closed
Copy link

Copilot AI commented Nov 28, 2025

@FabienHenon I've opened a new pull request, #20700, to work on those changes. Once the pull request is ready, I'll request review from you.

@github-actions github-actions bot removed the has conflicts Has conflicts to resolve before merging label Nov 28, 2025
seven-amid
seven-amid reviewed Nov 28, 2025
View reviewed changes
packages/manager/apps/pci-project/src/data/hooks/useServices.tsx
export const useCartServiceOption = (projectId?: string) =>
useQuery({
queryKey: getCartServiceOptionQueryKey(projectId as string),
queryKey: ['/order/cartServiceOption/cloud', projectId],
Copy link
Contributor

@seven-amid seven-amid Nov 28, 2025

Choose a reason for hiding this comment

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

you hard coded the query key, so we don't use the getCartServiceOptionQueryKey function anymore ?

Copy link
Contributor Author

@FabienHenon FabienHenon Nov 28, 2025

Choose a reason for hiding this comment

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

no, indeed

packages/manager/apps/pci-project/src/pages/creation/hooks/useConfigForm.tsx Outdated
...prev,
isHdsChecked: !!hdsItem,
}));
if (hdsItem && form.isHdsChecked !== !!hdsItem) {
Copy link
Contributor

@seven-amid seven-amid Nov 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

a new instruction on condition ! we should pay attention to that

Copy link
Contributor Author

@FabienHenon FabienHenon Nov 28, 2025

Choose a reason for hiding this comment

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

I reverted

packages/manager/apps/pci-project/src/pages/detail/activate/hooks/useActivateProject.tsx Outdated
const path = promotionVoucher
? `../../updating/${orderId}/${DISCOVERY_PROMOTION_VOUCHER}`
: `../../updating/${orderId}`;
? `../updating/${orderId}/${DISCOVERY_PROMOTION_VOUCHER}`
Copy link
Contributor

@seven-amid seven-amid Nov 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Is this safe ?

Copy link
Contributor Author

@FabienHenon FabienHenon Nov 28, 2025

Choose a reason for hiding this comment

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

no, it's a regression. I reverted

packages/manager/apps/pci-project/src/pages/detail/activate/Activate.page.tsx Outdated
handleActivateProject();
void handleActivateProject();
} else if (isCreditPayment && creditPaymentAmount) {
void handleCreditPayment();
Copy link
Contributor

@seven-amid seven-amid Nov 28, 2025

Choose a reason for hiding this comment

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

There is a regression here, please check to fix bellow
f80ca90

Copy link
Contributor Author

@FabienHenon FabienHenon Nov 28, 2025

Choose a reason for hiding this comment

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

I fixed it as you suggested

@FabienHenon
refactor(pci-project): static analysis kit
0c243d5 
ref: #MANAGER-20320

Signed-off-by: Fabien Henon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seven-amid seven-amid seven-amid left review comments

@ppprevost ppprevost Awaiting requested review from ppprevost ppprevost is a code owner automatically assigned from ovh/dev-manager-pci-all

@Tsiorifamonjena Tsiorifamonjena Awaiting requested review from Tsiorifamonjena Tsiorifamonjena is a code owner automatically assigned from ovh/dev-manager-pci-all

@aTurmo aTurmo Awaiting requested review from aTurmo aTurmo is a code owner automatically assigned from ovh/dev-manager-pci-all

@Taelar Taelar Awaiting requested review from Taelar Taelar is a code owner automatically assigned from ovh/dev-manager-pci-all

@lolottetheclash lolottetheclash Awaiting requested review from lolottetheclash lolottetheclash is a code owner automatically assigned from ovh/dev-manager-pci-all

@ZyaCon ZyaCon Awaiting requested review from ZyaCon ZyaCon is a code owner automatically assigned from ovh/dev-manager-pci-all

@fredericvilcot fredericvilcot Awaiting requested review from fredericvilcot fredericvilcot is a code owner automatically assigned from ovh/dev-manager-pci-all

@helabenkhalfallah helabenkhalfallah Awaiting requested review from helabenkhalfallah helabenkhalfallah is a code owner automatically assigned from ovh/dev-manager-control-tower

@anooparveti anooparveti Awaiting requested review from anooparveti anooparveti is a code owner automatically assigned from ovh/dev-manager-control-tower

@aboungnaseng-ovhcloud aboungnaseng-ovhcloud Awaiting requested review from aboungnaseng-ovhcloud aboungnaseng-ovhcloud is a code owner automatically assigned from ovh/dev-manager-control-tower

Assignees

No one assigned

Labels

dependencies Pull requests that update a dependency file feature New feature universe-public-cloud

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@FabienHenon @seven-amid