Skip to content

Conversation

@abullet33
Copy link
Contributor

ref: #DATATR-2313

Description

Ticket Reference: #...

Additional Information

ref: #DATATR-2313

Signed-off-by: Arthur Bullet <arthur.bullet@ovhcloud.com>
@abullet33 abullet33 requested a review from a team as a code owner October 31, 2025 08:12
onClick: () => navigate('./update-version'),
updateButtonDisplayed: availabilitiesVersionQuery.data?.length > 1,
disable:
service.capabilities.service.update ===
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you could create a const as it's always the same check or maybe move it in a utils cause you have the same check in packages/manager/apps/pci-databases-analytics/src/pages/services/[serviceId]/settings/_components/IpRestrictionsUpdate.component.tsx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand your point but it's only used at 2 places, so for me it's a little bit overkill to create a utils for that, even a const. We are managing all capabilities this way, so I guess it will be easier to understand "global capa management" if we all use it that way

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes use in 2 place but 6 times, and if you forgot to update one it will be painful to find the issue :)

cell: service.plan,
onClick: () => navigate('./update-plan'),
updateButtonDisplayed: availabilitiesPlanQuery.data?.length > 1,
disable:
Copy link
Contributor

Choose a reason for hiding this comment

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

We are using -ed for the updateButtonDisplayed, we should be consistent here:

Suggested change
disable:
disabled:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants