feat: [M3-6968] Add DC specific pricing info to migration modal #9570

abailly-akamai · 2023-08-21T15:49:08Z

Description 📝

Adds a price comparison when trying to migrate a linode and selecting a region with an DC increase.

Per the mockups, we're looking at aligning the old region a new region data in a row, as well as an optional pricing dataset underneath if the migration is set towards a DC with a price increase

Preview 📷

Before	After

How to test 🧪

Pull local code
Navigate to the linodes landing page or an individual linode
Use the action menu and click on "Migrate"
Play with the UI and confirm the test scenari outlined in the test file:

should render the initial ConfigureForm fields on modal open (no MigrationPricing)
shouldn't render the MigrationPricing component when the current Linode region is selected in the "New region" select
shouldn't render the MigrationPricing component when a region without price increase is selected in the "New region" select (anything else than br-gru & id-cgk)
should render the MigrationPricing component when a region with price increase is selected (ex: Sao Paulo br-gru)
should render the MigrationPricing component when a region with price decrease is selected (ex: Sao Paulo br-gru)
shouldn't render the MigrationPricingComponent for any case if the flag is disabled

packages/manager/.changeset/pr-9570-changed-1692743440177.md

packages/manager/src/features/Linodes/MigrateLinode/ConfigureForm.tsx

bnussman-akamai

General functionality and styling is looking great!

Feedback 🗣️
- Backups and hourly price does not look different between the two regions, does this need to be fixed?
- Volumes attached indentation is out of wack. (Out of scope but likely an easy fix to include here)
UX Concern 😰
- We show a backups price even if the Linode does not have backups enabled. I am worried this might scare users into thinking they have backups enabled 😟 when they really don't have it enabled. Is this a valid concern? Should we hide the backups price if it is not enabled? Should we somehow show that backups are not enabled?

abailly-akamai · 2023-08-24T16:11:05Z

@bnussman-akamai thx! still in draft, but valid points all around

hourly price was fixed, we needed 3 decimals
where is the backup price increase included? in the region_price[] object? I'll update the mock data
indentation - sure will fix!
agreed - we should not show backup price if disabled - learning about this feature while hacking at it because of missing context while being away. I would just not show anything.

mjac0bs · 2023-08-24T16:21:15Z

where is the backup price increase included? in the region_price[] object? I'll update the mock data

@abailly-akamai Pull in the latest develop and you'll have this!

agreed - we should not show backup price if disabled - learning about this feature while hacking at it because of missing context while being away. I would just not show anything.

This is a good call out and was a UX oversight when I made the ticket. Thanks for bringing it up @bnussman-akamai. Agreed that we shouldn't show backup price if disabled.

abailly-akamai · 2023-08-28T14:22:23Z

packages/manager/src/features/Linodes/MigrateLinode/ConfigureForm.tsx

+    currentLinodeType && getLinodeRegionPrice(currentLinodeType, currentRegion);
+  // TODO: DYNAMIC_PRICING we probably don't want to default to the current price in case something goes wrong,
+  // resulting in misleading pricing.
+  // we will need a way to handle an error for this case here and dynamicPricing.ts


@mjac0bs Should we make a ticket for this and reference it before we forget in how many places we need to handle this important case?

Yes, good thought, I made M3-7063 to account for this. We should display some error in lieu of the price comparison when selectedRegionPrice can't be calculated.

I wanted to clarify what you meant by dynamicPricing.ts. We're returning basePrice when there's no increase in our map, the feature flag is off, or we don't have regionId. Are we concerned about the regionId somehow getting set incorrectly or an issue with LD?

abailly-akamai · 2023-08-28T14:23:29Z

@bnussman-akamai @mjac0bs this is ready for another look - thanks!

mjac0bs

This is looking good with its various test scenarios! Just left a couple clarifying questions.

packages/manager/src/factories/types.ts

mjac0bs · 2023-08-28T21:03:06Z

packages/manager/src/features/Linodes/MigrateLinode/ConfigureForm.tsx

+    currentLinodeType && getLinodeRegionPrice(currentLinodeType, currentRegion);
+  // TODO: DYNAMIC_PRICING we probably don't want to default to the current price in case something goes wrong,
+  // resulting in misleading pricing.
+  // we will need a way to handle an error for this case here and dynamicPricing.ts


Yes, good thought, I made M3-7063 to account for this. We should display some error in lieu of the price comparison when selectedRegionPrice can't be calculated.

I wanted to clarify what you meant by dynamicPricing.ts. We're returning basePrice when there's no increase in our map, the feature flag is off, or we don't have regionId. Are we concerned about the regionId somehow getting set incorrectly or an issue with LD?

mjac0bs · 2023-08-28T21:09:05Z

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

+ * @param regionId The region to get the price for
+ * @returns backup pricing information for this specific linode type in a region
+ */
+export const getLinodeBackupPrice = (


Unit tests for this are in https://github.com/linode/manager/pull/9588/files#diff-fea3d37690e6818646d33bb4018e0417e793d218d2653a4d04d015c33a87f1f6R40-R90.

abailly-akamai · 2023-08-28T21:21:13Z

@mjac0bs thx will address this feedback but meanwhile, regarding #9570 (comment) and dynamic pricing, I am referring to this line: https://github.com/linode/manager/blob/develop/packages/manager/src/utilities/pricing/dynamicPricing.ts#L43

if regionId is undefined for whatever reason, we will not run the price comparison. We probably want to handle this case on its own and also display an error (or whatever way we want to handle it)

* Add dynamic pricing backups util functions * Update Enable Backups drawer with dynamic pricing * Update Backups tab placeholder and confirmation dialog with dynamic pricing * Update mocks for now * Revert mock updates after rebase * Add test for backup price util function * Add DC-specific pricing to Linode Create flow * Update cached regions file to include Jakarta and Sao Paulo * Feature flag changes * Improve consistency with price variable names * Update tests * Renamed util function for consistency with #9570 * Added changeset: Add DC-specific pricing to Linode backups * Update backups drawer total cost util function to use FF * Address feedback: && over ternary * Address feedback: util and types * Missed a spot

…de#9570) * feat: [M3-6968] Initial commit - set up current data * feat: [M3-6968] UI for migration dynamic pricing * feat: [M3-6968] styling adjustment * feat: [M3-6968] with with prop forwarding * feat: [M3-6968] ad unit test * feat: [M3-6968] improve unit test * feat: [M3-6968] post rebase fix * feat: [M3-6968] fix test * Added changeset: DC Dynamic pricing information for migration flow * feat: [M3-6968] improve display logic based on fedback * feat: [M3-6968] save work * feat: [M3-6968] save work * feat: [M3-6968] test update * Update packages/manager/.changeset/pr-9570-changed-1692743440177.md Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com> * Update packages/manager/.changeset/pr-9570-changed-1692743440177.md Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com> * feat: [M3-6968] cleanup * feat: [M3-6968] fix hourly display and improve test * feat: [M3-6968] handle backups * feat: [M3-6968] test for backups * feat: [M3-6968] cleanup 1 * feat: [M3-6968] fix identation issue * feat: [M3-6968] fix identation issue * feat: [M3-6968] fix failing test * feat: [M3-6968] small feedback * feat: [M3-6968] fix test --------- Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com>

) * Add dynamic pricing backups util functions * Update Enable Backups drawer with dynamic pricing * Update Backups tab placeholder and confirmation dialog with dynamic pricing * Update mocks for now * Revert mock updates after rebase * Add test for backup price util function * Add DC-specific pricing to Linode Create flow * Update cached regions file to include Jakarta and Sao Paulo * Feature flag changes * Improve consistency with price variable names * Update tests * Renamed util function for consistency with linode#9570 * Added changeset: Add DC-specific pricing to Linode backups * Update backups drawer total cost util function to use FF * Address feedback: && over ternary * Address feedback: util and types * Missed a spot

abailly-akamai self-assigned this Aug 21, 2023

abailly-akamai changed the title ~~feat: [M3-6968]~~ feat: [M3-6968] Add DC specific information to migration flow modal Aug 21, 2023

abailly-akamai changed the title ~~feat: [M3-6968] Add DC specific information to migration flow modal~~ feat: [M3-6968] Add DC specific pricing info to migration flow modal Aug 21, 2023

abailly-akamai changed the title ~~feat: [M3-6968] Add DC specific pricing info to migration flow modal~~ feat: [M3-6968] Add DC specific pricing info to migration modal Aug 21, 2023

mjac0bs added the DC-Specific Pricing label Aug 22, 2023

abailly-akamai force-pushed the feat-M3-6968 branch from 1428de0 to d3c9e21 Compare August 22, 2023 21:46

abailly-akamai marked this pull request as ready for review August 22, 2023 22:29

bnussman-akamai added Ready for Review Missing Changeset and removed Missing Changeset labels Aug 22, 2023

abailly-akamai requested review from bnussman-akamai and mjac0bs August 23, 2023 10:13

mjac0bs reviewed Aug 23, 2023

View reviewed changes

packages/manager/.changeset/pr-9570-changed-1692743440177.md Outdated Show resolved Hide resolved

packages/manager/.changeset/pr-9570-changed-1692743440177.md Outdated Show resolved Hide resolved

packages/manager/src/features/Linodes/MigrateLinode/ConfigureForm.tsx Show resolved Hide resolved

abailly-akamai force-pushed the feat-M3-6968 branch from cea5ee0 to a279b20 Compare August 23, 2023 20:14

abailly-akamai marked this pull request as draft August 23, 2023 22:24

bnussman-akamai reviewed Aug 24, 2023

View reviewed changes

abailly-akamai force-pushed the feat-M3-6968 branch from 439895f to e3e0674 Compare August 24, 2023 16:01

abailly-akamai added 11 commits August 26, 2023 19:22

feat: [M3-6968] Initial commit - set up current data

93fefcc

feat: [M3-6968] UI for migration dynamic pricing

e1d8c9a

feat: [M3-6968] styling adjustment

0b0525a

feat: [M3-6968] with with prop forwarding

e8ba766

feat: [M3-6968] ad unit test

19dbbe9

feat: [M3-6968] improve unit test

6cd7212

feat: [M3-6968] post rebase fix

9623ea2

feat: [M3-6968] fix test

21c8ec4

Added changeset: DC Dynamic pricing information for migration flow

c166d00

feat: [M3-6968] improve display logic based on fedback

1131c9d

feat: [M3-6968] save work

054131d

feat: [M3-6968] cleanup 1

7cc4608

abailly-akamai commented Aug 28, 2023

View reviewed changes

abailly-akamai marked this pull request as ready for review August 28, 2023 14:22

abailly-akamai requested review from mjac0bs and bnussman-akamai August 28, 2023 14:23

abailly-akamai added 3 commits August 28, 2023 10:35

feat: [M3-6968] fix identation issue

7f16474

feat: [M3-6968] fix identation issue

70c2e0d

feat: [M3-6968] fix failing test

2ffd41c

mjac0bs added a commit to mjac0bs/manager that referenced this pull request Aug 28, 2023

Renamed util function for consistency with linode#9570

2d1fe46

bnussman-akamai approved these changes Aug 28, 2023

View reviewed changes

bnussman-akamai added Add'tl Approval Needed Waiting on another approval! and removed Ready for Review labels Aug 28, 2023

mjac0bs approved these changes Aug 28, 2023

View reviewed changes

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

abailly-akamai added 2 commits August 29, 2023 08:53

feat: [M3-6968] small feedback

5a8e753

feat: [M3-6968] fix test

a9a2d33

abailly-akamai merged commit 392f9c2 into linode:develop Aug 29, 2023

mjac0bs added a commit to mjac0bs/manager that referenced this pull request Aug 29, 2023

Renamed util function for consistency with linode#9570

c4347e6

This was referenced Sep 5, 2023

Release v1.101.0 - release → staging #9627

Merged

Release v1.101.0 - staging → master #9633

Merged

mjac0bs mentioned this pull request Oct 12, 2023

feat: [M3-7063] - Provide dynamic price error handling for DC-specific pricing #9660

Merged

20 tasks

mjac0bs mentioned this pull request Nov 9, 2023

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

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [M3-6968] Add DC specific pricing info to migration modal #9570

feat: [M3-6968] Add DC specific pricing info to migration modal #9570

abailly-akamai commented Aug 21, 2023 •

edited

Loading

bnussman-akamai left a comment

abailly-akamai commented Aug 24, 2023 •

edited

Loading

mjac0bs commented Aug 24, 2023 •

edited

Loading

abailly-akamai Aug 28, 2023

mjac0bs Aug 28, 2023

abailly-akamai commented Aug 28, 2023

mjac0bs left a comment

mjac0bs Aug 28, 2023

mjac0bs Aug 28, 2023

abailly-akamai commented Aug 28, 2023

feat: [M3-6968] Add DC specific pricing info to migration modal #9570

feat: [M3-6968] Add DC specific pricing info to migration modal #9570

Conversation

abailly-akamai commented Aug 21, 2023 • edited Loading

Description 📝

Preview 📷

How to test 🧪

bnussman-akamai left a comment

Choose a reason for hiding this comment

abailly-akamai commented Aug 24, 2023 • edited Loading

mjac0bs commented Aug 24, 2023 • edited Loading

abailly-akamai Aug 28, 2023

Choose a reason for hiding this comment

mjac0bs Aug 28, 2023

Choose a reason for hiding this comment

abailly-akamai commented Aug 28, 2023

mjac0bs left a comment

Choose a reason for hiding this comment

mjac0bs Aug 28, 2023

Choose a reason for hiding this comment

mjac0bs Aug 28, 2023

Choose a reason for hiding this comment

abailly-akamai commented Aug 28, 2023

abailly-akamai commented Aug 21, 2023 •

edited

Loading

abailly-akamai commented Aug 24, 2023 •

edited

Loading

mjac0bs commented Aug 24, 2023 •

edited

Loading