upcoming: [M3-9065] - Build new Quotas Controls #11647
Conversation
abailly-akamai
force-pushed
the
M3-9065
branch
from
c76b59d to
04c43b6
Compare
abailly-akamai
requested review from
cpathipa and
hkhalil-akamai
and removed request for
a team
abailly-akamai
changed the title
|
Coverage Report: ✅ |
Cloud Manager UI test results🎉 508 passing tests on test run #8 ↗︎
|
| const { getByText } = renderWithTheme( | ||
| <QueryClientProvider client={queryClient}> | ||
| <Quotas /> | ||
| </QueryClientProvider> | ||
| ); |
There was a problem hiding this comment.
Our renderWithTheme function allows you to pass a custom queryClient in the options if needed. We may want to do that here
| const { getByText } = renderWithTheme( | |
| <QueryClientProvider client={queryClient}> | |
| <Quotas /> | |
| </QueryClientProvider> | |
| ); | |
| const { getByText } = renderWithTheme(<Quotas />, { queryClient }); |
| setSelectedLocation(null); | ||
| }; | ||
|
|
||
| return ( |
There was a problem hiding this comment.
We may want to break up this markup as it grows, similar to Linode create
| const globalOption = regionFactory.build({ | ||
| capabilities: [], | ||
| id: GLOBAL_QUOTA_VALUE, | ||
| label: GLOBAL_QUOTA_LABEL, | ||
| }); |
There was a problem hiding this comment.
I believe factories are not kosher for use in non-test related code.
It also doesn't make sense to have fields like country, status, etc for the global option. Perhaps we could design a new interface for the global option (that RegionSelect would accept)? Alternatively, we could add a boolean prop to RegionSelect like includeGlobalOption?: boolean.
There was a problem hiding this comment.
I believe factories are not kosher for use in non-test related code
I remember condemning this for another PR (it happens in a couple other places) but not really having a compelling argument that we should never either. Just satisfying types but it's not clean.
What you suggest is exactly what I was trying to avoid but I understand the concern - the global props is very specific to this feature, does not really belong as a prop for an edge case, and I wanted to avoid bloating the component further. That being said I can go that route if things are not clear enough
There was a problem hiding this comment.
the global props is very specific to this feature
Yes but we already are making lots of changes to RegionSelect in order to get this to work
There was a problem hiding this comment.
That being said I am not immovable on this issue, I'm fine with going this way.
There was a problem hiding this comment.
It's a fair comment. And DX documentation supersedes a hidden feature at a cost of an isolated case prop. I'll update accordingly 👍
| @@ -22,6 +22,12 @@ export const getRegionCountryGroup = (region: Region | undefined) => { | |||
| return 'Other'; | |||
| } | |||
|
|
|||
| // The global option should not be grouped | |||
| // An empty string here makes sense to avoid bigger refactors to an already complicated Component | |||
| if (region.id === 'global') { | |||
There was a problem hiding this comment.
'global' is now a magic string that isn't documented in any interface. See my comment above
Description 📝
This PR implements the controls for managing quotas in Cloud Manager, with a focus on filtering and displaying quota information based on service type and region/endpoint selection.
Changes
New Quotas Component Structure
Enhanced Region Selection
Types and Constants
UI Components
Implementation Details
API Integration
Testing
Type Safety
Note
The JSON data rendering in the page under the controls was made for development & review to verify data until the Quotas table is built - It'll be obviously removed
Preview 📷
How to test 🧪
Prerequisites
Verification steps
Author Checklists
As an Author, to speed up the review process, I considered 🤔
👀 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
As an Author, before moving this PR from Draft to Open, I confirmed ✅