change, test: [M3-7073, M3-7176] - Remove `dcSpecificPricing` and `objDcSpecificPricing` feature flags #9881

mjac0bs · 2023-11-07T16:47:56Z

Description 📝

Both phases of feature have been released. We want to make our changes permanent by removing the feature flag logic from our code.

Changes 🔄

List any change relevant to the reviewer.

Removes the dcSpecificPricing and objDcSpecificPricing feature flags from /src code, updating component logic and utils as necessary
Updates a handful of unit tests to remove feature flag logic
Updates the following e2e tests to remove feature flag logic:
- volumes/create-volume.spec.ts
- linodes/migrate-linode.spec.ts
- linodes/create-linode.spec.ts
- linodes/backup-linode.spec.ts
- linodes/clone-linode.spec.ts
- kubernetes/lke-update.spec.ts
- kubernetes/lke-create.spec.ts
- objectStorage/enable-object-storage.spec.ts
- nodebalancers/smoke-create-nodebal.spec.ts
- billing/billing-invoices.spec.ts

Preview 📷

Before	After

How to test 🧪

Prerequisites

(How to setup test environment)

Check out this PR.
As part of testing these flows, you'll want to create resources in Jakarta, ID or Sao Paulo, BR and compare that to a region without DC-specific prices (e.g. Newark, NJ).

Verification steps

(How to verify changes)

Verify that the feature flags are not listed in the dev tools and are now returning false via Launch Darkly in the development environment (check the network requests)
Verify that the following features are all still visible:
- In the Kube Create flow and Linode create/clone flows region selection: DC Specific Pricing Notices and Docs Links (#9572)
- For invoices from October onwards, a region column displays. Otherwise, the region column is hidden. (#9597)
- In the Linode Create flows, prices are not displayed until a region selection is made (#9598)
- In the Linode migration flow, a price display is shown if there is a difference in migration-from and migration-to region pricing (#9570)
- In the Kube Create flow, prices are not displayed until a region selection is made and on the Kube details page, dynamically calculated prices are displaying correctly (#9606)
- With a linode on your account in Jakarta or Sao Paulo, Monthly Network Transfer Pool dialog and linode usage displays include a DC-specific transfer pool (#9620), (#9692)
- In the NodeBalancer create flow, prices are calculated dynamically and displaying correctly (#9566)
- Add DC-specific pricing to Kubernetes HA (#9568)
- For Linode Backups (Create flow, Linode Details page, Enable Backups drawer), prices are calculated dynamically and displaying correctly (#9588)
- In the Volumes Create flows (from Volumes Landing and Linode Details), prices are calculated dynamically and displaying correctly (#9569), (#9683)
- In the OBJ Create flows (access key and buckets), the Create Bucket drawer displays correct overages and the Enable Object Storage modal contains correct, non-beta copy (#9813)
Verify that all unit tests pass
Verify that all e2es listed above pass

Confirm the following e2es pass:

yarn cy:run -s "cypress/e2e/core/volumes/create-volume.spec.ts,cypress/e2e/core/linodes/migrate-linode.spec.ts,cypress/e2e/core/linodes/create-linode.spec.ts,cypress/e2e/core/linodes/backup-linode.spec.ts,cypress/e2e/core/linodes/clone-linode.spec.ts,cypress/e2e/core/kubernetes/lke-update.spec.ts,cypress/e2e/core/kubernetes/lke-create.spec.ts,cypress/e2e/core/objectStorage/enable-object-storage.spec.ts,cypress/e2e/core/nodebalancers/smoke-create-nodebal.spec.ts,cypress/e2e/core/billing/billing-invoices.spec.ts"

As an Author I have considered 🤔

Check all that apply

…nd summary assertions out of table context

mjac0bs · 2023-11-09T16:32:01Z

packages/manager/src/features/ObjectStorage/EnableObjectStorageModal.tsx

-        {renderRegionPricingText(regionLabel ?? 'this region')}
+        <StyledTypography>
+          To create your first {regionId ? 'bucket' : 'access key'}, you need to
+          enable Object Storage.{' '}
+        </StyledTypography>


Now that we're not trying to render so many different versions of copy in the modal, I cleaned up the function and replaced it with the Typography directly.

mjac0bs · 2023-11-09T16:35:20Z

packages/manager/src/features/components/PlansPanel/PlanSelection.test.tsx

+  it('renders the table row with unknown prices if a region is not selected', () => {
+    const { container } = renderWithTheme(
+      wrapWithTableBody(
+        <PlanSelection
+          idx={0}
+          isCreate={true}
+          onSelect={() => jest.fn()}
+          type={mockPlan}
+        />
+      )
+    );
+
+    expect(container.querySelector('[data-qa-plan-row]')).toBeInTheDocument();
+    expect(container.querySelector('[data-qa-plan-name]')).toHaveTextContent(
+      mockPlan.heading
+    );
+    expect(container.querySelector('[data-qa-monthly]')).toHaveTextContent(
+      '$--.--'
+    );
+    expect(container.querySelector('[data-qa-hourly]')).toHaveTextContent(
+      '$--.--'
+    );
+  });
+


This test is added twice because it tests in both desktop and mobile view.

Have you considered using a test helper util to resize instead?

https://github.com/linode/manager/blob/develop/packages/manager/src/utilities/testHelpers.tsx#L59

Looking back at my comment, it was a little misleading. The tests are similar, but test slightly different things, since the desktop view tests that the unknown prices appear within a table and the mobile view tests that the unknown prices appear within cards.

The tests do already use the resizeScreenSize util to test the desktop and mobile display of plan items. I could break the unknown price test cases out into a separate test, but I don't see how that would make much difference, since we'd still want to test that the UI displays unknown prices in both views.

mjac0bs · 2023-11-09T16:36:47Z

packages/manager/src/features/components/PlansPanel/PlanSelection.tsx

+  // DC Dynamic price logic - DB creation flow is out of scope
+  const isDatabaseCreateFlow = location.pathname.includes('/databases/create');
+  const price: PriceObject | undefined = !isDatabaseCreateFlow
+    ? getLinodeRegionPrice(type, selectedRegionId)
+    : type.price;


This logic is necessary because the PlanSelection is used in the Database Create flow. Since it is out of scope for DC-specific pricing, we do not depend on a region selection to show prices; they are shown when the page loads.

mjac0bs · 2023-11-09T16:38:11Z

packages/manager/src/utilities/pricing/linodes.ts

@@ -15,8 +15,8 @@ import type { ExtendedType } from 'src/utilities/extendType';
 * @returns pricing information for this specific linode type in a region
 */
 export const getLinodeRegionPrice = (


Consolidated these two tests into one since getPrice was a wrapper for getLinodeRegionPrice.

mjac0bs · 2023-11-09T16:39:08Z

packages/manager/cypress/e2e/core/billing/billing-invoices.spec.ts

The changes in this file are due to consolidating tests.

bnussman-akamai

Awesome clean up! The diff is crazy!

All pricing I've check looks good! While this sits in develop, I'll remain vigilat

packages/manager/src/features/Kubernetes/KubernetesClusterDetail/UpgradeClusterDialog.tsx

abailly-akamai

Huge cleanup! nice work

code looks good and no UI regression noticed where I tested ✅
unit tests all passed ✅
clone-linode e2e failure seems legit ❌ (failed locally consistently)

packages/manager/cypress/e2e/core/billing/billing-invoices.spec.ts

abailly-akamai · 2023-11-13T14:48:54Z

packages/manager/src/features/components/PlansPanel/PlanSelection.test.tsx

+  it('renders the table row with unknown prices if a region is not selected', () => {
+    const { container } = renderWithTheme(
+      wrapWithTableBody(
+        <PlanSelection
+          idx={0}
+          isCreate={true}
+          onSelect={() => jest.fn()}
+          type={mockPlan}
+        />
+      )
+    );
+
+    expect(container.querySelector('[data-qa-plan-row]')).toBeInTheDocument();
+    expect(container.querySelector('[data-qa-plan-name]')).toHaveTextContent(
+      mockPlan.heading
+    );
+    expect(container.querySelector('[data-qa-monthly]')).toHaveTextContent(
+      '$--.--'
+    );
+    expect(container.querySelector('[data-qa-hourly]')).toHaveTextContent(
+      '$--.--'
+    );
+  });
+


Have you considered using a test helper util to resize instead?

https://github.com/linode/manager/blob/develop/packages/manager/src/utilities/testHelpers.tsx#L59

packages/manager/src/utilities/pricing/linodes.ts

mjac0bs · 2023-11-13T18:16:10Z

@abailly-akamai - Should be ready for another look!

clone-linode e2e failure seems legit ❌ (failed locally consistently)

This was a legit failure but was not due to this PR. It was fixed in #9890, which was recently merged, so I merged in the latest develop accordingly.

abailly-akamai

Thanks for addressing the feedback! All looks good per my previous review and test suites all passing - Ship it 🚢 !

…jDcSpecificPricing` feature flags (linode#9881) * Remove flag from Migrate dialog and Region selection * Remove flag from MNTP dialog and Linode Details Network tab * Remove flag from backups * Remove flag from invoices * Remove flag from Volumes and NodeBalancers * Remove OBJ flag from OBJ * Remove flag from linodes and kubernetes * Remove flags from dev tools and feature flags list * Remove flag on lke tests; fix HA in Create Cluster flow * Remove flags in Migrate test, OBJ test, and Volumes test * Remove flags in backup, clone, and create linode tests * Remove flags in nodebalancer test * Remove flag from billing test and consolidate... with failure WIP * Add changesets * Fix initial unknown prices for Database Create flow * Remove duplicate instances of "$5.00" by updating mock, move header and summary assertions out of table context * Make changelogs consistent * Address feedback: simplify dialog copy * Address feedback: make variable name more clear --------- Co-authored-by: Joe D'Amore <jdamore@linode.com>

mjac0bs added 6 commits November 6, 2023 20:11

Remove flag from Migrate dialog and Region selection

6ed1a5a

Remove flag from MNTP dialog and Linode Details Network tab

22e4cc9

Remove flag from backups

3893e96

Remove flag from invoices

9c0bc9e

Remove flag from Volumes and NodeBalancers

a3a0afd

Remove OBJ flag from OBJ

bae9f60

mjac0bs added Work in Progress DC-Specific Pricing labels Nov 7, 2023

mjac0bs self-assigned this Nov 7, 2023

mjac0bs added 8 commits November 7, 2023 10:17

Remove flag from linodes and kubernetes

b66ffcd

Remove flags from dev tools and feature flags list

ec8d4b5

Remove flag on lke tests; fix HA in Create Cluster flow

87abd33

Remove flags in Migrate test, OBJ test, and Volumes test

b927262

Remove flags in backup, clone, and create linode tests

17eb5a8

Remove flags in nodebalancer test

a14d9c4

Remove flag from billing test and consolidate... with failure WIP

940467f

Add changesets

f1ceb4b

mjac0bs requested a review from jdamore-linode November 7, 2023 21:53

mjac0bs and others added 3 commits November 9, 2023 07:18

Fix initial unknown prices for Database Create flow

01ef08b

Remove duplicate instances of "$5.00" by updating mock, move header a…

807176e

…nd summary assertions out of table context

Make changelogs consistent

a408898

mjac0bs commented Nov 9, 2023

View reviewed changes

mjac0bs removed the Work in Progress label Nov 9, 2023

mjac0bs marked this pull request as ready for review November 9, 2023 16:40

mjac0bs requested a review from a team as a code owner November 9, 2023 16:40

mjac0bs requested review from bnussman-akamai and abailly-akamai and removed request for a team November 9, 2023 16:40

mjac0bs added the Ready for Review label Nov 9, 2023

bnussman-akamai approved these changes Nov 9, 2023

View reviewed changes

packages/manager/src/features/Kubernetes/KubernetesClusterDetail/UpgradeClusterDialog.tsx Outdated Show resolved Hide resolved

mjac0bs added 2 commits November 9, 2023 11:31

Address feedback: simplify dialog copy

3b3c8ed

Merge branch 'develop' into M3-7073-remove-dc-pricing-feature-flags

e8773e9

mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Nov 9, 2023

abailly-akamai reviewed Nov 13, 2023

View reviewed changes

mjac0bs added 2 commits November 13, 2023 07:48

Merge branch 'develop' into M3-7073-remove-dc-pricing-feature-flags

5b704d8

Address feedback: make variable name more clear

aeb7401

mjac0bs requested a review from abailly-akamai November 13, 2023 18:18

abailly-akamai approved these changes Nov 14, 2023

View reviewed changes

mjac0bs added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! Ready for Review labels Nov 14, 2023

mjac0bs merged commit 52f0f71 into linode:develop Nov 14, 2023
13 checks passed

mjac0bs mentioned this pull request Dec 11, 2023

Release v1.108.0 - staging → master #9986

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change, test: [M3-7073, M3-7176] - Remove `dcSpecificPricing` and `objDcSpecificPricing` feature flags #9881

change, test: [M3-7073, M3-7176] - Remove `dcSpecificPricing` and `objDcSpecificPricing` feature flags #9881

mjac0bs commented Nov 7, 2023 •

edited

Loading

mjac0bs Nov 9, 2023

mjac0bs Nov 9, 2023

abailly-akamai Nov 13, 2023

mjac0bs Nov 13, 2023

mjac0bs Nov 9, 2023 •

edited

Loading

mjac0bs Nov 9, 2023

mjac0bs Nov 9, 2023

bnussman-akamai left a comment

abailly-akamai left a comment

abailly-akamai Nov 13, 2023

mjac0bs commented Nov 13, 2023

abailly-akamai left a comment

change, test: [M3-7073, M3-7176] - Remove dcSpecificPricing and objDcSpecificPricing feature flags #9881

change, test: [M3-7073, M3-7176] - Remove dcSpecificPricing and objDcSpecificPricing feature flags #9881

Conversation

mjac0bs commented Nov 7, 2023 • edited Loading

Description 📝

Changes 🔄

Preview 📷

How to test 🧪

Prerequisites

Verification steps

As an Author I have considered 🤔

mjac0bs Nov 9, 2023

Choose a reason for hiding this comment

mjac0bs Nov 9, 2023

Choose a reason for hiding this comment

abailly-akamai Nov 13, 2023

Choose a reason for hiding this comment

mjac0bs Nov 13, 2023

Choose a reason for hiding this comment

mjac0bs Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

mjac0bs Nov 9, 2023

Choose a reason for hiding this comment

mjac0bs Nov 9, 2023

Choose a reason for hiding this comment

bnussman-akamai left a comment

Choose a reason for hiding this comment

abailly-akamai left a comment

Choose a reason for hiding this comment

abailly-akamai Nov 13, 2023

Choose a reason for hiding this comment

mjac0bs commented Nov 13, 2023

abailly-akamai left a comment

Choose a reason for hiding this comment

change, test: [M3-7073, M3-7176] - Remove `dcSpecificPricing` and `objDcSpecificPricing` feature flags #9881

change, test: [M3-7073, M3-7176] - Remove `dcSpecificPricing` and `objDcSpecificPricing` feature flags #9881

mjac0bs commented Nov 7, 2023 •

edited

Loading

mjac0bs Nov 9, 2023 •

edited

Loading