Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: [M3-7138] - Fix Create Volume Drawer static pricing copy and misplaced helper text #9683

Merged
merged 3 commits into from
Sep 15, 2023

Conversation

mjac0bs
Copy link
Contributor

@mjac0bs mjac0bs commented Sep 14, 2023

Description 📝

This PR cleans up some copy for static Volume pricing ($0.10/GB per month) that was previously missed when creating a volume from a Linode's Storage tab. It also corrects a bug with the size field helper text related to when it should display.

These issues were found/fixed in #9601, but due to that PR's size and amount of refactoring, should be fixed in their own PR.

Preview 📷

Prod This Branch
Screenshot 2023-09-14 at 3 21 40 PM image Screenshot 2023-09-14 at 3 30 01 PM Screenshot 2023-09-14 at 3 29 07 PM
Screenshot 2023-09-14 at 3 21 21 PM Screenshot 2023-09-14 at 3 30 15 PM

How to test 🧪

  1. How to setup test environment?
    Check out this PR.
  2. How to reproduce the issue (if applicable)?
  • Go to http://localhost:3000/linodes/{id}/storage and click the Create Volume button.
  • Observe the hardcoded pricing ($0.10/GB) in the helper text.
  • Toggle the DC pricing feature flag on.
  • Observe the hardcoded pricing ($0.10/GB) in the helper text.
  • Observe the floating size field helper text "The size of the new volume in GB".
  • Observe the floating size field helper text prompting region selection, which should not display at all in this drawer (we already have the linode's region).
  • Create a volume and observe this floating helper text is also present when resizing a volume.
  1. How to verify changes?
  • Go to http://localhost:3000/linodes/{id}/storage.
  • Click the Create Volume button.
  • Notice that the floating helper text is gone.
  • Observe the hardcoded pricing ($0.10/GB) in the helper text.
  • Toggle the DC pricing feature flag on.
  • Observe there is not hardcoded pricing ($0.10/GB) in the helper text.
  • Create a volume and observe this floating helper text is not present when resizing a volume.
  • Check http://localhost:3000/volumes/create and confirm that the size field helper text displays a price when the feature flag is off or once a region has been selected. When the DC pricing feature flag is on and a region is not selected, confirm text is "Select a region to see cost per month.".
  1. How to run Unit or E2E tests?
    The Create Volume Drawer relies on getDynamicVolumePrice, which already has a unit test. (yarn test getDynamicVolumePrice)

@mjac0bs mjac0bs added Bug Fixes for regressions or bugs DC-Specific Pricing labels Sep 14, 2023
@mjac0bs mjac0bs self-assigned this Sep 14, 2023
@mjac0bs mjac0bs marked this pull request as ready for review September 14, 2023 22:47
{resize || isFromLinode ? (
'The size of the new volume in GB.'
) : (
{resize || isFromLinode ? null : (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change was also made in the big Volumes refactor PR.

I don't see reason why we'd want to keep this text; it was not displaying in the correct place in the drawers.

I also updated the name to priceDisplayText because legacyHelperText felt misleading. We still want the price to display, even after DC pricing changes, as long as a region is selected.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. I must have missed this one when working on volumes.

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 catch! Looking good

Tested copy and display on the linode storage tab and volume creation with and without the flag.

{resize || isFromLinode ? (
'The size of the new volume in GB.'
) : (
{resize || isFromLinode ? null : (
Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. I must have missed this one when working on volumes.

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.

Looks good! Thanks for breaking out this fix into its own PR!

@mjac0bs mjac0bs added the Approved Multiple approvals and ready to merge! label Sep 15, 2023
@mjac0bs mjac0bs merged commit 8fb6c2d into linode:develop Sep 15, 2023
11 checks passed
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! Bug Fixes for regressions or bugs DC-Specific Pricing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants