Skip to content
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.

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

feat(pci-gateway): Gateway 3AZ beta #14706

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

Tsiorifamonjena
Copy link
Contributor

@Tsiorifamonjena Tsiorifamonjena commented Dec 23, 2024

Question Answer
Branch? develop
Bug fix? no
New feature? yes
Breaking change? yes/no
Tickets Fix #TAPC-2413, #TAPC-2485, #TAPC-2604
License BSD 3-Clause
  • Try to keep pull requests small so they can be easily reviewed.
  • Commits are signed-off
  • Only FR translations have been updated
  • Branch is up-to-date with target branch
  • Lint has passed locally
  • Standalone app was ran and tested locally
  • Ticket reference is mentioned in linked commits (internal only)
  • Breaking change is mentioned in relevant commits

Description

  • TAPC-2413
  • TAPC-2485
  • TAPC-2604

Related

sachinrameshn
sachinrameshn previously approved these changes Dec 24, 2024
oalkabouss
oalkabouss previously approved these changes Dec 26, 2024
lionel95200x
lionel95200x previously approved these changes Jan 3, 2025
@Tsiorifamonjena Tsiorifamonjena marked this pull request as draft January 6, 2025 11:05
MaximeBajeux
MaximeBajeux previously approved these changes Jan 7, 2025
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Jan 15, 2025
Copy link
Contributor

yarn.lock changes

Click to toggle table visibility
Name Status Previous Current
@ovh-ux/manager-pci-common UPDATED 0.14.2 0.15.0

@Tsiorifamonjena Tsiorifamonjena marked this pull request as ready for review January 15, 2025 11:46
if (cloudCatalog) {
return gatewayRefPlans.map((plan) => {
const addonHourly = cloudCatalog.addons.find(
($addon) => $addon.planCode === plan.code,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why adding $ on params ?

ref: TAPC-2413
Signed-off-by: tsiorifamonjena <[email protected]>
ref: TAPC-2413
Signed-off-by: tsiorifamonjena <[email protected]>
ref: TAPC-2485
Signed-off-by: tsiorifamonjena <[email protected]>
ref-2413

Signed-off-by: tsiorifamonjena <[email protected]>
ref: TAPC-2306

Signed-off-by: tsiorifamonjena <[email protected]>
ref: TAPC-2306

Signed-off-by: tsiorifamonjena <[email protected]>
ref: TAPC-2306

Signed-off-by: tsiorifamonjena <[email protected]>
ref: TAPC-2306

Signed-off-by: tsiorifamonjena <[email protected]>
ref: TAPC-2306

Signed-off-by: tsiorifamonjena <[email protected]>
ref: TAPC-2306

Signed-off-by: tsiorifamonjena <[email protected]>
@@ -98,21 +99,25 @@ export const LocationStep = () => {
}
}, [searchParams, state.regions]);

const has3AZ = useMemo(() => isRegionWith3AZ(state.regions), [state.regions]);

const pciCommonProperties = usePCICommonContextFactory({ has3AZ });
Copy link
Contributor

Choose a reason for hiding this comment

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

Naming should be agnostic from the library :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Since it is the pci common context, how would you name it ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could be directly const meta = usePCICommonContextFactory({ has3AZ }); or something like that ;)

const { data: cloudCatalog } = useCloudCatalog();
const { data: availableGatewayPlans } = useAvailableGatewayPlans(projectId);

const { data: inactiveRegions } = useInactiveRegions(projectId);
const getBandWidthLabel = (bandwidth: number) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be memoized

availableGatewayPlans?.plans.reduce(
(result: TAvailablePlansGrouped, currentValue: TPlan) => {
const code = currentValue.code.replace(/\.3AZ$/, '');
const newResult = { ...result };
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not directly returning accumulator?

const getPrice = (addon: TAddon): number | undefined =>
addon?.pricings.find((price) => price.capacities.includes('consumption'))
?.price;

export const useData = (projectId: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

As a general improvement, will be great to move some memoized functions out of the hook (SRP)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file feature New feature translation required universe-public-cloud
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants