feat: [M3-7373] - Sold Out Chips #10013

abailly-akamai · 2023-12-20T18:37:13Z

Description 📝

This PR implements sold out chips to our PlanSelection components (both Linode and Kubernetes). It helps our users with picking a GPU or Premium CPU plan by showing them plans that are actually available to them instead of trying to select unavailable plans that would lead to an API error upon submission.

It utilizes the new regions/availability endpoint which only returns availability for GPU or Premium CPU plans (it may be expanded later on). In the background, the endpoint gets refreshed at 15m via a cron job checking available resources in our DCs.

⚠️: While the endpoint is already live in production, we're waiting for an API update that will convert the plan value to our regular plan Ids. This is why this PR is to be tested with mock data.

⚠️: The feature flag is currently turned on for testing, but will be off in production till the fix above is completed. The flag controls the enabling of the API query so when off and no data is returned no plans will be shown as sold out.

Changes 🔄

Implement logic to add the sold out chip to GPU or Premium CPU plans
Add test coverage for new feature
Modify the useRegionsAvailabilitiesQuery to be abled to be disabled
Add ability to pass JSX element to subheadings mobile selection cards
Get rid of obsolete soldOutSingapore & soldOutTokyo flags
Replace instances of selectedRegionID with selectedRegionId

Preview 📷

Linode Creation Flow

Mobile	Desktop

Kubernetes Creation Flow

Mobile	Desktop

How to test 🧪

Prerequisites

pull and run code locally
turn on MSW

Verification steps

for both the Linode and Kubernetes creation flows:

select either Newark, NJ (us-east) or Dallas, TX (us-central)
confirm sold out chips in either the Dedicated CPU or Shared CPU tabs (those plans are not representative of what is returned by the API but what was mocked for testing purposes)
Confirm mobile and desktop styling
Confirm light and dark mode (ignore known visual defect for disabled radios which will be released at the same time)
Confirm chip doesn't show if panel is disabled (you can do that by making this prop true)

As an Author I have considered 🤔

Check all that apply

abailly-akamai · 2023-12-21T16:46:17Z

packages/manager/src/components/SelectionCard/CardBase.tsx

@@ -16,7 +16,7 @@ export interface CardBaseProps {
  headingDecoration?: JSX.Element;
  renderIcon?: () => JSX.Element;
  renderVariant?: () => JSX.Element | null;
-  subheadings: (string | undefined)[];
+  subheadings: (JSX.Element | string | undefined)[];


This allows adding the chip to the selection card on mobile

abailly-akamai · 2023-12-21T16:56:48Z

packages/manager/src/features/components/PlansPanel/utils.ts

+
+  return !!availability;
+};
+


This is the meat of the PR. Made sure to have early returns so we don't loop over regionAvailabilities for no reasons

github-actions · 2023-12-21T17:44:42Z

Coverage Report: ✅
Base Coverage: 79.14%
Current Coverage: 79.2%

coliu-akamai

code review ✅

will be reviewing the functionality and following the steps listed above next 👍

coliu-akamai · 2023-12-21T18:40:04Z

packages/manager/src/features/Kubernetes/KubernetesPlansPanel/KubernetesPlanContainer.tsx

  }, [
    disabled,
    getTypeCount,
    onAdd,
    onSelect,
    plans,
    selectedId,
-    selectedRegionID,
+    selectedRegionId,


saw an eslint error for missing regionAvailabilities from dependency list

Suggested change

selectedRegionId,

regionAvailabilities,

selectedRegionId,

coliu-akamai · 2023-12-21T19:00:54Z

packages/manager/src/features/components/PlansPanel/utils.test.ts

+    expect(result).toBe(false);
+  });
+
+  it('should return false if plan or selectedRegionId is falsy', () => {


Suggested change

it('should return false if plan or selectedRegionId is falsy', () => {

it('should return false if selectedRegionId is falsy', () => {

coliu-akamai · 2023-12-21T19:22:44Z

packages/manager/src/features/components/PlansPanel/utils.test.ts

+
+    expect(result).toBe(false);
+  });
+


can we add a test for non matching planId:

it('should return false if no matching regionAvailability is found (based on planId)', () => { const result = getPlanSoldOutStatus({ plan: mockPlan, regionAvailabilities: [ { available: true, plan: otherMockPlan.id, region: 'us-east-1' }, ], selectedRegionId: mockSelectedRegionId, }); expect(result).toBe(false); });

Good idea - added!

coliu-akamai · 2023-12-21T21:49:33Z

a couple of things caught my eye:

at some screen widths, the sold out chip goes onto a new line however these screenwidths may not be very common viewport widths so might not be too much of an issue? (~975-1100px when the side nav is open, neglible when side nav is closed ~960-970px)
For the Kubernetes Plans panel, we need to make sure to disable the actions on the row (but still have the chip be clickable for accessibility I think):

Screen.Recording.2023-12-21.at.4.46.13.PM.mov

mjac0bs

✅ Confirmed sold out chips in either the Dedicated CPU or Shared CPU tabs
✅ Confirmed mobile and desktop styling
✅ Confirmed light and dark mode (ignore known visual defect for disabled radios which will be released at the same time)
✅ Confirmed chip doesn't show if panel is disabled
✅ Test coverage looks good

Connie had some good feedback. Agreed about the weird experience of being able to add a kube cluster that is Sold Out; we should disable those buttons.

Otherwise, a small piece of feedback on the disabled table row styling, which differs between Linode and Kube: This is more noticeable in light mode. Kube styling seems preferable to me, where the disabled row is lighter than the other rows. Can we make that consistent with the Linode plans table?

Linode:

Kube:

abailly-akamai · 2024-01-02T18:36:58Z

@mjac0bs addressed all the feedback - pls take another look when you get a chance, ty!

bnussman-akamai

Functionality and logic is looking good! Left a few optional comments/questions

bnussman-akamai · 2024-01-02T18:35:39Z

packages/manager/src/constants.ts

+
+// Sold Out plans
+export const PLAN_IS_SOLD_OUT_COPY =
+  'This plan has no availability for the selected region. Please select a smaller plan or the same plan in another region.';


Suggested change

'This plan has no availability for the selected region. Please select a smaller plan or the same plan in another region.';

'This plan has no availability for the selected region. Please select a different plan or the same plan in another region.';

Was "smaller" picked because all plan above a certain size will always be sold out? (is it possible for a Dedicated 32GB to be sold out but a Dedicated 64GB to be in stock)

It feels weird to me to assume the user should pick a smaller plan.

This was the recommended copy (and yes if it is based on the host's availability so theoretically a bigger plan would never be available), but i do prefer using different as well so I made that update 👍

bnussman-akamai · 2024-01-02T18:39:52Z

packages/manager/src/features/Kubernetes/KubernetesPlansPanel/KubernetesPlanSelection.tsx

+              <TooltipIcon
+                data-testid="sold-out-chip"
+                icon={<Chip label="Sold Out" />}
+                status="other"
+                text={PLAN_IS_SOLD_OUT_COPY}
+              />


Should we use a raw Tooltip and Chip here?

I'm not sure if TooltipIcon is the best fit here because of the styles and the fact that it renders a button.

Fair enough - I added a regular tooltip with Chip inside for both instance - also moved the tooltip to the SelectionCard on mobile

bnussman-akamai · 2024-01-02T18:44:19Z

packages/manager/src/features/components/PlansPanel/utils.ts

+/**
+ * Utility to map Premium & GPU plans to a selected region's availability.
+ */
+export const getPlanSoldOutStatus = ({


Optional naming / comment changes. I like using "is" for boolean things

Suggested change

/**

* Utility to map Premium & GPU plans to a selected region's availability.

*/

export const getPlanSoldOutStatus = ({

/**

* Utility to determine if a plan is sold out based on a region's availability.

*/

export const getIsPlanSoldOut = ({

Suggested change

/**

* Utility to map Premium & GPU plans to a selected region's availability.

*/

export const getPlanSoldOutStatus = ({

/**

* Utility to map Premium & GPU plans to a selected region's availability.

*/

export const getPlanSoldOutStatus = ({

Agreed, done as well!

bnussman-akamai · 2024-01-02T18:44:37Z

packages/manager/src/features/components/PlansPanel/utils.ts

@@ -109,6 +111,41 @@ export const getRegionsWithCapability = (
  return arrayToList(withCapability ?? []);
 };

+interface PlanSoldOutStatus {


Optional

Suggested change

interface PlanSoldOutStatus {

interface PlanSoldOutStatusOptions {

mjac0bs · 2024-01-02T22:50:34Z

Thanks for the congruent disabled row treatment!

One more thing to fix with Kube: in selection card mode only, the buttons aren't disabled when the plan is sold out.

Screen.Recording.2024-01-02.at.2.44.48.PM.mov

abailly-akamai · 2024-01-03T20:42:36Z

@mjac0bs feedback addressed as well!

mjac0bs

Thanks, Alban! Verified light and dark mode both still look good and that rows (and their buttons) are all disabled at various screen sizes once all feedback was addressed.

One tiny nitpick: with the tooltip/chip changes, it looks like we lost a little margin between the plan label and the chip - not sure if that was intentional. They look a little close together and could benefit from a bit of spacing, in my opinion.

bnussman-akamai · 2024-01-04T15:02:46Z

packages/manager/src/features/Kubernetes/KubernetesPlansPanel/KubernetesPlanContainer.tsx


-  const shouldDisplayNoRegionSelectedMessage = !selectedRegionID;
+  const { data: regionAvailabilities } = useRegionsAvailabilitiesQuery(


Rather than using the useRegionsAvailabilitiesQuery, can we just fetch the availability for selectedRegionId?

Yeah good idea it's available and will make for smaller requests on RegionSelect change - done here:
7adfe39

abailly-akamai self-assigned this Dec 20, 2023

abailly-akamai added the Work in Progress label Dec 20, 2023

abailly-akamai force-pushed the M3-7373 branch from 35848b0 to 8063a57 Compare December 21, 2023 15:43

abailly-akamai commented Dec 21, 2023

View reviewed changes

abailly-akamai marked this pull request as ready for review December 21, 2023 17:21

abailly-akamai requested review from a team as code owners December 21, 2023 17:21

abailly-akamai requested review from jdamore-linode, mjac0bs and cliu-akamai and removed request for a team December 21, 2023 17:21

abailly-akamai removed the Work in Progress label Dec 21, 2023

abailly-akamai requested a review from coliu-akamai December 21, 2023 17:24

coliu-akamai added Analytics Relating to Analytics migration project or Adobe Analytics Sold Out Plans and removed Analytics Relating to Analytics migration project or Adobe Analytics labels Dec 21, 2023

coliu-akamai reviewed Dec 21, 2023

View reviewed changes

coliu-akamai added the Ready for Review label Dec 21, 2023

mjac0bs reviewed Dec 29, 2023

View reviewed changes

abailly-akamai force-pushed the M3-7373 branch 2 times, most recently from a02f47f to 8a2fa6e Compare January 2, 2024 17:49

abailly-akamai requested review from mjac0bs and coliu-akamai January 2, 2024 18:34

bnussman-akamai reviewed Jan 2, 2024

View reviewed changes

Initial commit - work in progress

710526e

abailly-akamai added 8 commits January 3, 2024 14:27

Add chip and tooltip

2421126

move to util + tests

6cba44d

Handle kubes

19d9bae

Increase test coverage

9374ae6

Disabled panels & changeset

25da220

e2e fix

909e440

Feedback

1dd7d5f

Disabled table rows styling

cd9be32

abailly-akamai force-pushed the M3-7373 branch from 6e5d79a to cd9be32 Compare January 3, 2024 19:27

abailly-akamai added 2 commits January 3, 2024 15:36

feedback

06380f2

feedback 2

bf5c876

mjac0bs approved these changes Jan 3, 2024

View reviewed changes

feedback - add spacing before chip

3edb84b

bnussman-akamai approved these changes Jan 4, 2024

View reviewed changes

bnussman-akamai reviewed Jan 4, 2024

View reviewed changes

Use useRegionsAvailabilityQuery instead

7adfe39

mjac0bs added Approved Multiple approvals and ready to merge! and removed Ready for Review labels Jan 4, 2024

abailly-akamai merged commit b2d5bc8 into linode:develop Jan 5, 2024
18 checks passed

This was referenced Jan 18, 2024

Release v1.110.0 - release → staging #10081

Closed

Release v1.110.0 - release → staging #10082

Merged

Release v1.110.0 - staging → master #10092

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [M3-7373] - Sold Out Chips #10013

feat: [M3-7373] - Sold Out Chips #10013

abailly-akamai commented Dec 20, 2023 •

edited

Loading

abailly-akamai Dec 21, 2023

abailly-akamai Dec 21, 2023

github-actions bot commented Dec 21, 2023 •

edited

Loading

coliu-akamai left a comment

coliu-akamai Dec 21, 2023

abailly-akamai Jan 2, 2024

coliu-akamai Dec 21, 2023

abailly-akamai Jan 2, 2024

coliu-akamai Dec 21, 2023

abailly-akamai Jan 2, 2024

coliu-akamai commented Dec 21, 2023

mjac0bs left a comment

abailly-akamai commented Jan 2, 2024

bnussman-akamai left a comment

bnussman-akamai Jan 2, 2024

abailly-akamai Jan 3, 2024

bnussman-akamai Jan 2, 2024

abailly-akamai Jan 3, 2024

bnussman-akamai Jan 2, 2024

abailly-akamai Jan 3, 2024

bnussman-akamai Jan 2, 2024

mjac0bs commented Jan 2, 2024

abailly-akamai commented Jan 3, 2024

mjac0bs left a comment

bnussman-akamai Jan 4, 2024

abailly-akamai Jan 4, 2024

	it('should return false if plan or selectedRegionId is falsy', () => {
	it('should return false if selectedRegionId is falsy', () => {

	'This plan has no availability for the selected region. Please select a smaller plan or the same plan in another region.';
	'This plan has no availability for the selected region. Please select a different plan or the same plan in another region.';

	interface PlanSoldOutStatus {
	interface PlanSoldOutStatusOptions {


		const shouldDisplayNoRegionSelectedMessage = !selectedRegionID;
		const { data: regionAvailabilities } = useRegionsAvailabilitiesQuery(

feat: [M3-7373] - Sold Out Chips #10013

feat: [M3-7373] - Sold Out Chips #10013

Conversation

abailly-akamai commented Dec 20, 2023 • edited Loading

Description 📝

Changes 🔄

Preview 📷

Linode Creation Flow

Kubernetes Creation Flow

How to test 🧪

Prerequisites

Verification steps

As an Author I have considered 🤔

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 21, 2023 • edited Loading

coliu-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coliu-akamai commented Dec 21, 2023

mjac0bs left a comment

Choose a reason for hiding this comment

abailly-akamai commented Jan 2, 2024

bnussman-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjac0bs commented Jan 2, 2024

abailly-akamai commented Jan 3, 2024

mjac0bs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abailly-akamai commented Dec 20, 2023 •

edited

Loading

github-actions bot commented Dec 21, 2023 •

edited

Loading