Skip to content

feat: [M3-6973] - Add DC-specific pricing Object Storage overages#9813

Merged
mjac0bs merged 20 commits intolinode:developfrom
mjac0bs:M3-6973-pt-2-obj-dc-pricing
Oct 20, 2023
Merged

feat: [M3-6973] - Add DC-specific pricing Object Storage overages#9813
mjac0bs merged 20 commits intolinode:developfrom
mjac0bs:M3-6973-pt-2-obj-dc-pricing

Conversation

@mjac0bs
Copy link
Contributor

@mjac0bs mjac0bs commented Oct 18, 2023

Description 📝

Object storage has hardcoded prices throughout the codebase. The service is a flat $5/month. Beyond the first 250GB storage and 1TB outbound transfer, it's and $0.02/GB/month. We need to update this due to pricing changes for DC-specific overage rates and clarification that outbound transfer is not added to DC-specific pools, only the global pool.

The only time a user sees the Enable Object Storage notice is the first time they create a bucket or access key and must opt-in to the service.

  • For this reason, we are adding region-specific overage pricing to the Create Bucket drawer.

Note: Access keys are not currently tied to regions.

Changes 🔄

  • Overage prices on the Create Bucket drawer reflect accurate prices for dynamic region pricing.
  • Copy updates are made in the Enable Object Storage modal to match the mocks for enabling via both bucket and key.
  • Cleaned up dcSpecificPricing feature flag in the Enable OBJ Modal and related tests, since initial DC-specific pricing work has been released, to avoid extra conditional logic. DC-specific pricing behavior is now the default.
  • Updated tests and added a new unit test for a new component.

Preview 📷

Description Prod This Branch
Enable Object Modal Screenshot 2023-10-18 at 11 26 56 AM Screenshot 2023-10-18 at 11 16 30 AM Screenshot 2023-10-18 at 11 17 49 AM Screenshot 2023-10-18 at 11 16 54 AM Screenshot 2023-10-18 at 11 17 33 AM
Create Bucket Drawer Screenshot 2023-10-18 at 11 14 25 AM Screenshot 2023-10-18 at 11 02 18 AM Screenshot 2023-10-18 at 11 02 26 AM Screenshot 2023-10-18 at 11 02 34 AM
Create Bucket Drawer Tooltip N/A Screenshot 2023-10-18 at 11 02 44 AM Screenshot 2023-10-18 at 11 02 56 AM Screenshot 2023-10-18 at 1 07 51 PM

How to test 🧪

Prerequisites

(How to setup test environment)

  • Check out this PR.
  • Turn on the objDCSpecificPricing feature flag.
  • If Object Storage already enabled, go to /account/settings and cancel it.

Verification steps

  • With the feature flags on:
    • Confirm that the copy of the modal matches the copy in the screenshots above for each scenario, which matches the mocks attached to the internal ticket.
    • Create your first access key and enable OBJ.
    • Create your first bucket and enable OBJ.
    • Confirm that overage pricing displays in the Create Bucket drawer, matches the mocks attached to the internal ticket, and updates dynamically depending on cluster region selection.
    • Create Bucket in Sao Paulo.
    • Create Bucket in Jakarta.
    • Create Bucket in any other region (will use base pricing).

Verify unit tests pass:

yarn test EnableObjectStorageModal OveragePricing CreateBucketDrawer

Verify Cypress test passes:

yarn cy:run -s "cypress/e2e/core/objectStorage/enable-object-storage.spec.ts"

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@mjac0bs mjac0bs force-pushed the M3-6973-pt-2-obj-dc-pricing branch from 2d298bd to 66487b0 Compare October 18, 2023 18:04
Comment on lines +147 to +149
{flags.objDcSpecificPricing && clusterRegion?.[0]?.id && (
<OveragePricing regionId={clusterRegion?.[0]?.id} />
)}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OveragePricing is a new component with the helper text and text tooltips about OBJ storage and network transfer overages. It will display in the Create Bucket drawer once a region has been selected.

Comment on lines +37 to +61
it('Renders a tooltip for DC-specific transfer pools for a region with price increases', async () => {
const { findByRole, getByText } = renderWithTheme(
<OveragePricing regionId="br-gru" />
);

fireEvent.mouseEnter(getByText('network transfer pool for this region'));

const tooltip = await findByRole(/tooltip/);

expect(tooltip).toBeInTheDocument();
expect(getByText(DC_SPECIFIC_TRANSFER_POOLS_TOOLTIP_TEXT)).toBeVisible();
});

it('Renders a tooltip for global network transfer pools for a region without price increases', async () => {
const { findByRole, getByText } = renderWithTheme(
<OveragePricing regionId="us-east" />
);

fireEvent.mouseEnter(getByText('global network transfer pool'));

const tooltip = await findByRole(/tooltip/);

expect(tooltip).toBeInTheDocument();
expect(getByText(GLOBAL_TRANSFER_POOL_TOOLTIP_TEXT)).toBeVisible();
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While enable-object-storage.spec.ts does test for the presence of the overage pricing helper text in the flow, I didn't go so far as to check whether the tooltip displays correctly and so we cover that in a unit test here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the adjustments in this file are just to account for my removal of the dcSpecificPricing feature flag from the component.

The component now displays DC-specific pricing logic by default and will display the copy changes covered in this ticket if objDcSpecificPricing flag is enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few UI changes in this file that I did NOT feature flag: updating the title of the modal and increasing the font size and spacing of the text. These are general improvements, unrelated to the move from beta to real OBJ DC-specific pricing, so there's no reason to hide them behind the objDcSpecificPricing feature flag.

// with special pricing during beta period, then cancel.
ui.dialog
.findByTitle('Just to confirm...')
.findByTitle('Enable Object Storage')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I commented in another spot, I did not hide this title change behind a feature flag because it's a general improvement, so these tests needed updating in multiple spots.

// Click "Enable Object Storage".
ui.button
.findByTitle('Enable Object Storage')
.findByAttribute('data-qa-enable-obj', 'true')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that the modal title and primary action button match, this selector needed to be changed to be more specific, or the test would fail.

Comment on lines 521 to +525
ui.dialog
.findByTitle('Just to confirm...')
.findByTitle('Access Keys')
.should('be.visible')
.within(() => {
// Confirm that pricing and cancellation explanations are present, but
// DC-specific information is hidden.
confirmModalContents();
ui.button
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff looks weird here, but I think it is just because I deleted the test that checked that old modal copy displayed when the dcSpecificPricing feature flag was off - now that this code has been released, we won't be turning it off.

@mjac0bs mjac0bs marked this pull request as ready for review October 19, 2023 14:40
@mjac0bs mjac0bs requested a review from a team as a code owner October 19, 2023 14:40
@mjac0bs mjac0bs requested review from abailly-akamai, bnussman-akamai, coliu-akamai and jdamore-linode and removed request for a team October 19, 2023 14:40
Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work - just left a couple questions

matches mocks ✅
Created your first access key and enable OBJ. ✅
Created your first bucket and enable OBJ. ✅
Overage pricing display ✅
Create Bucket in Jakarta. ✅
Create Bucket in any other region ✅
Unit test ✅
e2e ✅

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great so far! (Much better than the original proposals for obj pricing UX)

Kind of unrelated to the pricing stuff but it feels weird that Label isn't marked as (required) but Region is (required)

Screenshot 2023-10-20 at 11 13 08 AM

@mjac0bs
Copy link
Contributor Author

mjac0bs commented Oct 20, 2023

@jdamore-linode I am confused -- I could swear I got the enable-object-storage spec passing yesterday. Now I'm seeing it consistently fail when trying to find a message in the OBJ modal that is visible. Text matching isn't exact. Any idea what we're missing?

Screenshot 2023-10-20 at 9 01 18 AM

EDIT: It was the curly apostrophe (&rsquo;). I swapped out ' in the modal to match our use of curly apostrophes and then missed that in the test -- had to update the substring to ignore that character.

Passing now:
Screenshot 2023-10-20 at 10 51 33 AM

@mjac0bs
Copy link
Contributor Author

mjac0bs commented Oct 20, 2023

Kind of unrelated to the pricing stuff but it feels weird that Label isn't marked as (required) but Region is (required)

@bnussman-akamai Yeah, agreed. We made Region (required) in the first OBJ pricing release to provide user feedback because we disabled the create button unless a region is selected. But now the mismatch is weird, so addressed this in fe7479e. I left the label on the Access Key flow as is, with the thought that of course the only form field in a form would be required; it's implied.

Screenshot 2023-10-20 at 9 17 04 AM

Worth noting in this PR that the only reason we're passing regionId to the Enable modal now is because we need a way to differentiate between whether we're in the access key flow or the bucket flow for a small copy difference: To create your first [access key / bucket ].... I kept the regionId prop as the differentiator since it exists for buckets and not access keys. I could rename this prop (and revert form (required) UX + disabled button), but I think the earlier feedback can be beneficial to the user.

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work as always @mjac0bs

@mjac0bs mjac0bs added Approved Multiple approvals and ready to merge! and removed Ready for Review labels Oct 20, 2023
@mjac0bs
Copy link
Contributor Author

mjac0bs commented Oct 20, 2023

The CI e2e failure is once again due to the 504 issue on that 4th runner, so that subset of tests didn't run. I ran them locally using the handy command Joe shared with us, and all passed, so I am merging this now:

image

@mjac0bs mjac0bs merged commit e159bc6 into linode:develop Oct 20, 2023
@mjac0bs mjac0bs added the Object Storage deals with Object Storage label Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge! DC-Specific Pricing Object Storage deals with Object Storage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants