fix: [M3-7741] - Hide error notices for $0 regions in Resize Pool and Add a Node Pool drawers #10157

mjac0bs · 2024-02-07T00:15:06Z

Description 📝

This PR supports $0 region pricing by correctly showing pricing without errors on the LKE Details page. It also fixes the UI for invalid (null or undefined) prices in the Resize Pool drawer.

Currently, once a user creates a LKE cluster in a region where pricing is $0 (currently: Madrid, which is in LA):

When trying to resize pool, the UI displays a confusing 0 in place of the "Current pool" and "Resized pool" calculations, the pricing error is displayed, but a user is actually still able to complete the resize because the enhanced number input is still clickable and can be incremented, at which point the user can save changes.

When trying to add a node pool, after adding at lease one node, the UI displays a pricing error and pricing display text with $--.-- placeholder pricing in it. The user can still save changes.

Changes 🔄

Resize Pool:

The user is able to resize pool for an LKE cluster in a region where pricing is $0 and sees no error notice.
The user is not able to resize pool for an LKE cluster with pricing that is null or undefined.
If pricing is null or undefined, correctly display placeholder copy
- Current pool: $--.–-/month (N node(s) at $--.--/month)
- Resized pool: $--.–-/month (N node(s) at $--.--/month)
  and the pricing error rather than just 0.

Add a Node Pool:

The user is able to add nodes to an LKE cluster in a region where pricing is $0 and sees no error notice.
The user is not able to add nodes to an LKE cluster with pricing that is null or undefined.
If pricing is null or undefined, correctly display placeholder copy
- This pool will add $--.--/month (N node(s) at $--.--/month) to this cluster.

Preview 📷

Description	Before	After
Resize Pool: Region with $0 pricing (currently `Madrid`)
Resize Pool: `undefined` or `null` pricing
Add a Node Pool: Region with $0 pricing (currently `Madrid`)

How to test 🧪

Prerequisites

(How to setup test environment)

Check out this PR and yarn dev.
Create a LKE cluster with es-madrid as its region.

Reproduction steps

(How to reproduce the issue, if applicable)

Resize Pool:

Navigate to the cluster's details page and click Resize Pool.
Observe the described/screenshotted issues with the drawer.

Add a Node Pool:

Navigate to the cluster's details page and click Add a Node Pool.
Add at least one node to the pool (press the + icon)
Observe the described/screenshotted issues with the drawer.

Verification steps

(How to verify changes)

On this branch, follow the repro steps and observe that there are no errors for a region with $0 pricing when Resizing Pool or Adding a Node Pool.
To test behavior with pricing that is undefined (same behavior as null), block network requests to the linode type (example: v4/linode/types/g6-standard-1) or manually edit the values of pricePerNode and totalMonthlyPrice (here, here).
Confirm test passes with added coverage for resizing and adding to node pool in $0 regions:

yarn cy:run -s "cypress/e2e/core/kubernetes/lke-update.spec.ts"

As an Author I have considered 🤔

Check all that apply

…ol drawers

mjac0bs · 2024-02-07T16:08:47Z

...er/src/features/Kubernetes/KubernetesClusterDetail/NodePoolsDisplay/ResizeNodePoolDrawer.tsx

            <Typography className={classes.summary}>
              {`Resized pool: $${renderMonthlyPriceToCorrectDecimalPlace(
-                updatedCount * pricePerNode
+                !isInvalidPricePerNode ? updatedCount * pricePerNode : undefined


Because the pricePerNode could be null or undefined, we can't multiply a non-number. renderMonthlyPriceToCorrectDecimalPlace() has logic to render a non-numeric price as $--.--.

mjac0bs · 2024-02-07T16:11:08Z

...er/src/features/Kubernetes/KubernetesClusterDetail/NodePoolsDisplay/ResizeNodePoolDrawer.tsx

-            variant="error"
+
+          {nodePool.count &&
+            (isInvalidPricePerNode || isInvalidTotalMonthlyPrice) && (


Only real change in this section is the updated bools to determine whether to display the pricing error notice or not - the rest of the diff is due to indentation.

mjac0bs · 2024-02-07T16:15:29Z

...er/src/features/Kubernetes/KubernetesClusterDetail/NodePoolsDisplay/ResizeNodePoolDrawer.tsx

+      {isLoadingTypes ? (
+        <CircleProgress />
+      ) : (
+        <form
+          onSubmit={(e: React.ChangeEvent<HTMLFormElement>) => {
+            e.preventDefault();
+            handleSubmit();
+          }}
+        >
+          <div className={classes.section}>


Most of this diff is an indentation change caused by fixing an issue where the loading spinner was displaying above the form, instead of in its place, while isLoadingTypes is true. This was visible when I blocked network requests as described in the testing steps.

Before After

Screen.Recording.2024-02-07.at.9.01.57.AM.mov

Screen.Recording.2024-02-07.at.9.01.12.AM.mov

mjac0bs · 2024-02-07T16:17:27Z

...c/features/Kubernetes/KubernetesClusterDetail/NodePoolsDisplay/ResizeNodePoolDrawer.test.tsx

Changes in the file are to account for the loading state that is now present before form data is rendered.

mjac0bs · 2024-02-07T16:19:07Z

...er/src/features/Kubernetes/KubernetesClusterDetail/NodePoolsDisplay/ResizeNodePoolDrawer.tsx

+  const isInvalidPricePerNode = !pricePerNode && pricePerNode !== 0;
+  const isInvalidTotalMonthlyPrice =
+    !totalMonthlyPrice && totalMonthlyPrice !== 0;
+


undefined and null prices are invalid, but 0 is allowed.

Might be good to have this clarification as a comment in the code

I second this. Am also thinking you could consolidate this logic with the addNodePool drawer logic in a util with your comments.

Done in 6b86e9a.

…e and Add Node Pools

github-actions · 2024-02-07T21:33:24Z

Coverage Report: ✅
Base Coverage: 81.11%
Current Coverage: 81.11%

dwiley-akamai

Observed the behavior described in the Changes section for each circumstance (opted to manually modify the values of pricePerNode and totalMonthlyPrice) ✅

lke-update.spec.ts passed ✅

dwiley-akamai · 2024-02-08T00:11:39Z

...er/src/features/Kubernetes/KubernetesClusterDetail/NodePoolsDisplay/ResizeNodePoolDrawer.tsx

+  const isInvalidPricePerNode = !pricePerNode && pricePerNode !== 0;
+  const isInvalidTotalMonthlyPrice =
+    !totalMonthlyPrice && totalMonthlyPrice !== 0;
+


Might be good to have this clarification as a comment in the code

abailly-akamai

Looks good and confirmed the fixes shown in the description ✅

Thanks for the improved coverage as well!

Bonus point if you wanna do that minor refactor

abailly-akamai · 2024-02-08T14:07:50Z

...er/src/features/Kubernetes/KubernetesClusterDetail/NodePoolsDisplay/ResizeNodePoolDrawer.tsx

+  const isInvalidPricePerNode = !pricePerNode && pricePerNode !== 0;
+  const isInvalidTotalMonthlyPrice =
+    !totalMonthlyPrice && totalMonthlyPrice !== 0;
+


I second this. Am also thinking you could consolidate this logic with the addNodePool drawer logic in a util with your comments.

… Add a Node Pool drawers (linode#10157) * Allow -zsh LKE prices without error notices in Resize Pool and Add Pool drawers * Fix loading spinner displaying above what was supposed to be loading * Fix conditional to render notice if either price is invalid * Add test coverage * Added changeset: Hide error notices for /bin/sh regions for LKE Resize and Add Node Pools * Fix changeset wording * Address feedback: use invalid price util

…xes for release (#10177) * fix: [M3-7741] - Hide error notices for $0 regions in Resize Pool and Add a Node Pool drawers (#10157) * Allow -zsh LKE prices without error notices in Resize Pool and Add Pool drawers * Fix loading spinner displaying above what was supposed to be loading * Fix conditional to render notice if either price is invalid * Add test coverage * Added changeset: Hide error notices for /bin/sh regions for LKE Resize and Add Node Pools * Fix changeset wording * Address feedback: use invalid price util * fix: [M3-7746] - Fix $0 region price error in "Enable All Backups" drawer (#10161) * Remove error indicator for Linodes in $0 regions * Fix $0 total price display issue * Cover $0 pricing cases in Cypress backup tests * Add BackupLinodeRow tests to account for error states and $0 regions * Add unit tests for BackupDrawer component * fix: [M3-7747] - Fix Linode Migration dialog hidden $0 price (#10166) * Add unit tests for MigrationPricing component * Accounting for $0 prices in MigrationPricing component * fix: [M3-7739] - Fix error when enabling backups for Linodes in regions with $0 price (#10153) * Fix error when enabling backups for Linodes in regions with $0 price * Add unit tests for EnableBackupsDialog --------- Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com> * Replace "toBeDisabled" with "toHaveAttribute" assertion --------- Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com>

Allow -zsh LKE prices without error notices in Resize Pool and Add Po…

21916de

…ol drawers

mjac0bs self-assigned this Feb 7, 2024

mjac0bs added 2 commits February 7, 2024 09:00

Fix loading spinner displaying above what was supposed to be loading

2dd11af

Fix conditional to render notice if either price is invalid

0af12a7

mjac0bs commented Feb 7, 2024

View reviewed changes

mjac0bs added 2 commits February 7, 2024 10:54

Add test coverage

314e3f9

Added changeset: Hide error notices for /bin/sh regions for LKE Resiz…

7a1c274

…e and Add Node Pools

mjac0bs changed the title ~~fix: [M3-7741] - Allow $0 LKE prices without error notices in Resize Pool and Add a Node Pool drawers~~ fix: [M3-7741] - Hide error notices for $0 regions in Resize Pool and Add a Node Pool drawers Feb 7, 2024

mjac0bs marked this pull request as ready for review February 7, 2024 18:00

mjac0bs requested review from a team as code owners February 7, 2024 18:00

mjac0bs requested review from cliu-akamai, dwiley-akamai and abailly-akamai and removed request for a team February 7, 2024 18:00

mjac0bs added the Ready for Review label Feb 7, 2024

jdamore-linode approved these changes Feb 7, 2024

View reviewed changes

mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Feb 7, 2024

Fix changeset wording

e17a427

dwiley-akamai approved these changes Feb 8, 2024

View reviewed changes

abailly-akamai approved these changes Feb 8, 2024

View reviewed changes

bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Feb 8, 2024

mjac0bs removed the Ready for Review label Feb 8, 2024

Address feedback: use invalid price util

6b86e9a

mjac0bs merged commit 49ea132 into linode:develop Feb 8, 2024
18 checks passed

cpathipa mentioned this pull request Feb 12, 2024

Release v1.112.0 - release → staging #10174

Closed

mjac0bs mentioned this pull request Feb 12, 2024

fix: [M3-7739, M3-7741, M3-7746, M3-7747] - Cherry pick zero price fixes for release #10177

Merged

12 tasks

This was referenced Feb 12, 2024

Release v1.112.0 - release → staging #10178

Merged

Release v1.112.0 - staging → master #10180

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: [M3-7741] - Hide error notices for $0 regions in Resize Pool and Add a Node Pool drawers #10157

fix: [M3-7741] - Hide error notices for $0 regions in Resize Pool and Add a Node Pool drawers #10157

mjac0bs commented Feb 7, 2024 •

edited

Loading

mjac0bs Feb 7, 2024

mjac0bs Feb 7, 2024 •

edited by dwiley-akamai

Loading

mjac0bs Feb 7, 2024

mjac0bs Feb 7, 2024

mjac0bs Feb 7, 2024

dwiley-akamai Feb 8, 2024

abailly-akamai Feb 8, 2024

mjac0bs Feb 8, 2024

github-actions bot commented Feb 7, 2024 •

edited

Loading

dwiley-akamai left a comment

dwiley-akamai Feb 8, 2024

abailly-akamai left a comment

abailly-akamai Feb 8, 2024

fix: [M3-7741] - Hide error notices for $0 regions in Resize Pool and Add a Node Pool drawers #10157

fix: [M3-7741] - Hide error notices for $0 regions in Resize Pool and Add a Node Pool drawers #10157

Conversation

mjac0bs commented Feb 7, 2024 • edited Loading

Description 📝

Changes 🔄

Preview 📷

How to test 🧪

Prerequisites

Reproduction steps

Verification steps

As an Author I have considered 🤔

Choose a reason for hiding this comment

mjac0bs Feb 7, 2024 • edited by dwiley-akamai Loading

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

github-actions bot commented Feb 7, 2024 • edited Loading

dwiley-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abailly-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjac0bs commented Feb 7, 2024 •

edited

Loading

mjac0bs Feb 7, 2024 •

edited by dwiley-akamai

Loading

github-actions bot commented Feb 7, 2024 •

edited

Loading