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

upcoming: [M3-7978] - RegionSelect disabled option API updates #10373

Merged
merged 12 commits into from
Apr 18, 2024

Conversation

abailly-akamai
Copy link
Contributor

@abailly-akamai abailly-akamai commented Apr 12, 2024

Description πŸ“

This PR allows customizing the disabled RegionSelect option. Previously the disabling of a region (and its associated tooltip) we controlled statically, only based on the region capabilities.

With this update, we keep defaulting to the capability based disabling, and allow a custom handler to disable a region based on whatever logic needed. In this case, it is for the PlacementGroupCreateDrawer to allow disabling a region based on the maximum_pgs_per_customer region limit.

Changes πŸ”„

  • Add new handleDisabledRegion prop to RegionSelect
  • Change unavailable front end code to take a new disabledProps so we can configure custom tooltip messages and dimensions as well
  • Modify getOptionDisabled to reflect the new disabledProps prop
  • Improve naming conventions and prop destructuring
  • Update PlacementGroupCreateDrawer to pass new option filtering logic based on maximum_pgs_per_customer
  • Update relevant tests

Preview πŸ“·

Before After
Screenshot 2024-04-12 at 12 04 26 Screenshot 2024-04-12 at 12 21 08
Legacy
Screenshot 2024-04-12 at 11 39 50

How to test πŸ§ͺ

Verification steps

ℹ️ Use PROD
Legacy (no regression)

  • Best is to create a new account (can be employee account) so the region restrictions are applied
  • Confirm Washington, DC is still showing as a disabled region.
  • Check no other regression with the RegionSelect & the RegionMultiSelect through the app

ℹ️ Use ALPHA
New Behavior (test the new PlacementGroupCreateDrawer behavior)

  • Have 5 Placement Groups in London, UK
  • Try creating a new one
  • Confirm behavior and styles

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

@abailly-akamai abailly-akamai self-assigned this Apr 12, 2024
@abailly-akamai abailly-akamai changed the title upcoming: [M3-7978] upcoming: [M3-7978] - RegionSelect disabled option API updates Apr 12, 2024
@abailly-akamai abailly-akamai marked this pull request as ready for review April 12, 2024 16:32
@abailly-akamai abailly-akamai requested a review from a team as a code owner April 12, 2024 16:32
@abailly-akamai abailly-akamai requested review from dwiley-akamai, hana-linode, cpathipa and carrillo-erik and removed request for a team April 12, 2024 16:32
Copy link

github-actions bot commented Apr 12, 2024

Coverage Report: βœ…
Base Coverage: 81.8%
Current Coverage: 81.82%

Copy link
Contributor

@hana-linode hana-linode left a comment

Choose a reason for hiding this comment

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

I am not seeing the Washington, DC region disabled without the placement-group tag πŸ€”

image

@abailly-akamai
Copy link
Contributor Author

@hana-linode sorry I had forgotten the logic behind this. It's only customer tag based. Basically at the time of the region getting limited availability, the admin script looks at if the user has had any resource in said data-center in the last 3 months. If so, user can continue using that region to create resources.

What I would recommend is create a new account (can be employee account) in order to have that region disabled

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.

I created a fresh dev account but don't see IAD listed as a region at all. Am I missing something?

Screenshot

Screenshot 2024-04-15 at 1 55 12β€―PM

@abailly-akamai
Copy link
Contributor Author

@dwiley-akamai would have to be a PROD account

@dwiley-akamai
Copy link
Contributor

@dwiley-akamai would have to be a PROD account

Gotcha, thanks for clarifying and updating the PR description

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.

No regressions observed for legacy use cases βœ…
Observed outlined behavior for PlacementGroupCreateDrawer use case βœ…

Copy link
Contributor

@cpathipa cpathipa left a comment

Choose a reason for hiding this comment

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

LGTM!

@hana-linode hana-linode added the Approved Multiple approvals and ready to merge! label Apr 17, 2024
@abailly-akamai abailly-akamai merged commit 2f71529 into linode:develop Apr 18, 2024
18 checks passed
mjac0bs pushed a commit to mjac0bs/manager that referenced this pull request Apr 18, 2024
…e#10373)

* save progress

* save progress

* save progress

* Save progress

* fix tests

* code cleanup

* Improve coverage

* moar cleanup

* Added changeset: RegionSelect disabled option API updates

* Address feedback

* Address feedback 2

* fix test
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! Placement Groups
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants