Obj label feature venky april 09 - #657
agorthi-akamai merged 9 commits into
Conversation
|
run_cypress |
There was a problem hiding this comment.
Pull request overview
Adds a consistent display format for Object Storage bucket hostnames across CloudPulse by converting bucket.region.linodeobjects.tld into bucket[region] in UI labels and updating related tests.
Changes:
- Introduced
formatObjectStorageUrl()utility to format Object Storage bucket hostnames for display. - Applied formatting to Object Storage resource labels in
getDimensionName()andCloudPulseResourcesSelect. - Updated unit tests and Cypress selectors/assertions to match the new formatted bucket labels.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
packages/manager/src/features/CloudPulse/Utils/utils.ts |
Adds formatObjectStorageUrl() utility for Object Storage label formatting. |
packages/manager/src/features/CloudPulse/Utils/utils.test.ts |
Adds unit tests validating URL formatting behavior. |
packages/manager/src/features/CloudPulse/Utils/CloudPulseWidgetUtils.ts |
Formats Object Storage dimension labels via formatObjectStorageUrl(). |
packages/manager/src/features/CloudPulse/shared/CloudPulseResourcesSelect.tsx |
Formats Object Storage resource option labels in the resources selector. |
packages/manager/cypress/e2e/core/cloudpulse/preference/object-strorage-service-preference-verification.spec.ts |
Updates assertions/selectors to expect bucket[region] formatting. |
packages/manager/cypress/e2e/core/cloudpulse/object-strorage-widget-verification.spec.ts |
Updates typing/selection to use formatted bucket label text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Need at least 3 parts for a valid object storage URL | ||
| // e.g., 'bucket.region.linodeobjects.com' has 4 parts minimum | ||
| if (parts.length < 4) { | ||
| return url; // Return original if format doesn't match | ||
| } | ||
|
|
||
| // Remove 'linodeobjects' and 'com' from the end | ||
| const withoutDomain = parts.slice(0, -2); | ||
|
|
There was a problem hiding this comment.
The inline comments don’t match the implementation and intended behavior: the function requires at least 4 dot-separated parts (not 3), and it slices off the last two parts regardless of what they are (so it’s not specifically removing linodeobjects + com). Please update the comments/JSDoc to reflect the actual rule, and consider adding a guard that the second-to-last segment is linodeobjects (otherwise a non-object-storage hostname with 4+ segments would be reformatted unexpectedly).
| const filteredResources = | ||
| filterUsingDependentFilters(resources, xFilter) ?? []; | ||
| if (resourceType !== 'objectstorage') { | ||
| return filteredResources; | ||
| } | ||
|
|
||
| return filteredResources.map((resource: CloudPulseResources) => { | ||
| if (resource.label) { | ||
| return { | ||
| ...resource, |
There was a problem hiding this comment.
This maps over all object storage resources and overwrites resource.label with a formatted value. Since label is also used downstream for things like selection bookkeeping and preference/display labels, mutating it can introduce collisions and makes it harder to reason about identity vs display. Consider keeping the raw label intact and deriving a separate display label (e.g., format in renderOption / getOptionLabel, and/or when building the labels array passed to the filter-change callback) while continuing to use id as the stable identifier.
| * This is used to track the open state of the autocomplete and useRef optimizes the re-renders that this component goes through and it is used for below | ||
| * When the autocomplete is already closed, we should publish the resources on clear action and deselect action as well since onclose will not be triggered at that time | ||
| * When the autocomplete is open, we should publish any resources on clear action until the autocomplete is close | ||
| */ | ||
| const isAutocompleteOpen = React.useRef(false); // Ref to track the open state of Autocomplete | ||
|
|
||
| const getResourcesList = React.useMemo<CloudPulseResources[]>(() => { | ||
| return filterUsingDependentFilters(resources, xFilter) ?? []; | ||
| }, [resources, xFilter]); | ||
| const filteredResources = | ||
| filterUsingDependentFilters(resources, xFilter) ?? []; | ||
| if (resourceType !== 'objectstorage') { | ||
| return filteredResources; | ||
| } | ||
|
|
||
| return filteredResources.map((resource: CloudPulseResources) => { | ||
| if (resource.label) { | ||
| return { | ||
| ...resource, |
There was a problem hiding this comment.
New behavior is introduced for resourceType === 'objectstorage' (formatting displayed labels). There’s already a CloudPulseResourcesSelect.test.tsx suite in the repo; adding a unit test to assert that object storage options render with the formatted label (and that selection still returns the original id) would help prevent regressions.
…/agorthi-akamai/manager into obj-label-feature_venky_April_09
|
run_cypress |
|
run_cypress |
|
run_cypress |
Description 📝
Highlight the Pull Request's context and intentions.
Changes 🔄
List any change(s) relevant to the reviewer.
Scope 🚢
Upon production release, changes in this PR will be visible to:
Target release date 🗓️
Please specify a release date (and environment, if applicable) to guarantee timely review of this PR. If exact date is not known, please approximate and update it as needed.
Preview 📷
Include a screenshot
<img src="" />or video<video src="" />of the change.🔒 Use the Mask Sensitive Data setting for security.
💡 For changes requiring multiple steps to validate, prefer a video for clarity.
How to test 🧪
Prerequisites
(How to setup test environment)
Reproduction steps
(How to reproduce the issue, if applicable)
Verification steps
(How to verify changes)
Author Checklists
As an Author, to speed up the review process, I considered 🤔
Check all that apply
As an Author, before moving this PR from Draft to Open, I confirmed ✅