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

feat: [M3-7252] - Only show regions that support VPCs in VPC Create page #9787

Merged
merged 4 commits into from
Oct 17, 2023

Conversation

coliu-akamai
Copy link
Contributor

@coliu-akamai coliu-akamai commented Oct 12, 2023

Description 📝

  • For the VPC Create page, rather than showing all regions, we only show the regions that support VPCs in the region select
  • NOTE: Hiding the disabled Tokyo region from this page as well, since 1) Tokyo doesn't support VPCs yet, and 2) the tooltip for disabled Tokyo region mentions using the Osaka region instead, but Osaka also doesn't support VPCs and doesn't show up

Preview 📷

Before After
image image

How to test 🧪

Prerequisites

  • Using the dev environment, with vpc admin tags, navigate to the VPC Create page

Verification steps

  • Verify that the region select on the VPC Create page only contains the following regions (these are the ones that have the VPC capability, based on the API return object)
    • Mumbai, India
    • Toronto, Ontario, CAN
    • Sydney, NSW, Australia
    • Newark, NJ, USA
    • London, England, UK

Tests

  • Verify that the vpc-create-spec tests pass:
    yarn cy:run -s "cypress/e2e/core/vpc/vpc-create.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

@coliu-akamai coliu-akamai added the VPC Relating to VPC project label Oct 12, 2023
@coliu-akamai coliu-akamai self-assigned this Oct 12, 2023
@coliu-akamai coliu-akamai marked this pull request as ready for review October 12, 2023 21:24
@coliu-akamai coliu-akamai requested a review from a team as a code owner October 12, 2023 21:24
@coliu-akamai coliu-akamai requested review from jdamore-linode and jaalah-akamai and removed request for a team October 12, 2023 21:24
@jdamore-linode
Copy link
Contributor

Hey Connie!

The only relevant test failure here is vpc-create.spec.ts. I didn't look too closely, but I'm guessing judging by the changes that the tests are tripping up since they choose a region at random (also line 275), and are probably trying to create VPCs using regions that aren't in the select now.

Couple ways to fix:

I have no real preference. To run the test locally:

yarn && yarn build && yarn start:manager:ci, and then

yarn cy:run -s "cypress/e2e/core/vpc/vpc-create.spec.ts"

(Bear in mind that it might still pass as-is, if the test chooses a region that happens to have the VPC capability!)

@coliu-akamai
Copy link
Contributor Author

Thanks @jdamore-linode! Looking into the fixes you recommended

Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

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

E2E test passes ✅
Only the expected regions show up as options ✅
Code LGTM ✅

@coliu-akamai coliu-akamai added the Add'tl Approval Needed Waiting on another approval! label Oct 16, 2023
Copy link
Contributor

@hana-akamai hana-akamai left a comment

Choose a reason for hiding this comment

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

Verified that only the expected regions show up and that the E2E test passed 🧹

@hana-akamai hana-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Oct 17, 2023
Copy link
Contributor

@jdamore-linode jdamore-linode left a comment

Choose a reason for hiding this comment

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

Thanks for the test fix @coliu-akamai! Looks good!

@coliu-akamai coliu-akamai merged commit f35a923 into linode:develop Oct 17, 2023
11 checks passed
abailly-akamai pushed a commit to abailly-akamai/manager that referenced this pull request Oct 17, 2023
…age (linode#9787)

* filter to regions that support VPCs for VPCCreate

* hide disabled Tokyo from
VPC create

* Added changeset: Only show regions that support VPCs in VPC Create page

* mocked regions to include vpc capabilities
@coliu-akamai coliu-akamai deleted the feat-m3-7252 branch December 19, 2023 04:57
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! VPC Relating to VPC project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants