-
Notifications
You must be signed in to change notification settings - Fork 365
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
upcoming: [M3-7724] - Linode plan table updates for Edge regions #10255
upcoming: [M3-7724] - Linode plan table updates for Edge regions #10255
Conversation
packages/manager/src/features/components/PlansPanel/PlansPanel.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/components/PlansPanel/PlansPanel.tsx
Outdated
Show resolved
Hide resolved
// 256gb and 516gb plans will not be supported for Edge | ||
const plansUpTo128GB = planTypes.dedicated.slice( | ||
0, | ||
planTypes.dedicated.length - 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to double check that this will still work with the 512GB decommissioning work we're doing here: https://github.com/linode/manager/pull/10228/files#diff-eef38141ba2a3f5898e288b786fa5ad972dace25bfd5ad823c481f22ec89f366R65-R67. At first glance we should be fine, but let's keep an eye on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be fine for the immediate future, but I think it'd be safer and more maintainable to remove the 256GB and 512GB plans specifically from the array instead of the last two elements (reason being, if in a few months API turns off the 512GB plan and we don't insert the placeholder, this logic would end up removing the 128GB plan too)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything the other guy said 😉
Coverage Report: ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the Linode Create page:
- (for Edge regions) You should not see the tab control, only the Dedicated Linode Plan Table minus the 256gb and 512gb plans ✅
- (for Edge regions) The summary section should reflect the $0 price ✅
- Core regions should still display the same plans as prod ✅
// 256gb and 516gb plans will not be supported for Edge | ||
const plansUpTo128GB = planTypes.dedicated.slice( | ||
0, | ||
planTypes.dedicated.length - 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be fine for the immediate future, but I think it'd be safer and more maintainable to remove the 256GB and 512GB plans specifically from the array instead of the last two elements (reason being, if in a few months API turns off the 512GB plan and we don't insert the placeholder, this logic would end up removing the 128GB plan too)
Description 📝
API work for Edge plans will not be ready for Beta. So, we need to hardcode the plans ourselves until then.
Changes 🔄
List any change relevant to the reviewer.
Preview 📷
How to test 🧪
Prerequisites
(How to setup test environment)
Verification steps
(How to verify changes)
/linodes/create
and select an Edge regionAs an Author I have considered 🤔
Check all that apply