OSAC-4026: [UI] Storage Tier details page - #146
Conversation
…ared module Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
|
@ElayAharoni: This pull request references OSAC-4026 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.1.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: Repository: osac-project/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
| <Alert variant="warning" isInline title={t('Unable to resolve backend names')}> | ||
| {t( | ||
| 'Backend IDs are shown in place of names until this recovers. This is separate from the normal fallback shown when a tier references a backend that no longer exists.', | ||
| )} | ||
| </Alert> |
There was a problem hiding this comment.
just render the error as we typically do
| <Alert variant="warning" isInline title={t('Unable to resolve backend names')}> | |
| {t( | |
| 'Backend IDs are shown in place of names until this recovers. This is separate from the normal fallback shown when a tier references a backend that no longer exists.', | |
| )} | |
| </Alert> | |
| <Alert variant="error" isInline title={t('Failed to fetch Storage backends')}> | |
| {getErrorMessage(backendsError)} | |
| </Alert> |
There was a problem hiding this comment.
Good catch — switched to the standard danger-alert-with-server-message pattern (getErrorMessage(backendsError)), matching StorageTierDeleteConfirmModal.tsx and friends. One tweak: Alert's variant is danger, not error — error is a valid HelperTextItem variant but not a valid PatternFly Alert variant. Used danger to match every other error alert in this codebase. Fixed in 9d2034d.
| const { data: backends = [], error: backendsError } = usePrivateStorageBackends( | ||
| { filter: storageBackendIdsFilter(backendIds) }, | ||
| { enabled: backendIds.length > 0 }, | ||
| ); | ||
| const backendsById = useMemo( | ||
| () => new Map(backends.map((backend) => [backend.id, backend])), | ||
| [backends], |
There was a problem hiding this comment.
Lets move this to StorageTierDetails.tsx. You are not doing anything with this data in this component
There was a problem hiding this comment.
Agreed, moved — StorageTierDetailsPage now only fetches the tier itself; StorageTierDetails owns fetching and resolving its own backend associations. Fixed in 9d2034d.
| import type { StorageTier } from '@osac/types/private'; | ||
| import { StorageProtocol } from '@osac/types/private'; | ||
|
|
||
| export const protocolLabel = (t: TFunction, protocol: StorageProtocol): string => { |
There was a problem hiding this comment.
Lets use Record to get TS validation that we handle all cases of StorageProtocol
| export const protocolLabel = (t: TFunction, protocol: StorageProtocol): string => { | |
| export const protocolLabel = (t: TFunction): Record<StorageProtocol, string> => { |
There was a problem hiding this comment.
Agreed — also brings this in line with StorageTierStatusLabel's existing Record-based map, which this was inconsistent with. Fixed in 0196c31.
…ol-label mapping Signed-off-by: Elay Aharoni <elayaha@gmail.com>
…rageTierDetails, render fetch errors verbatim Signed-off-by: Elay Aharoni <elayaha@gmail.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ElayAharoni, rawagner The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
OSAC-4026: [UI] Storage Tier details page
Jira: https://redhat.atlassian.net/browse/OSAC-4026
Story type: [UI]
Summary
Adds a
StorageTierDetailsPageat/admin/infrastructure/storage/tiers/:id, giving a Cloud Provider Admin a full view of a single storage tier's backend associations (protocol, bandwidth, quota, encryption) before assigning it to a tenant or troubleshooting one. Follows the existingBareMetalDetailsPage/BareMetalDetailssplit — the only prior codebase precedent composingResourceDetailHeader+ResourceDetailsPageLoading+ResourceDetailsPageError.Changes
StorageTierDetailsPage— data fetch viausePrivateStorageTier(id), loading/error/not-found branchingStorageTierDetails— presentational header + details card + backend-associations cardStorageTierBackendAssociationsTable— one row perspec.backendsentry (resolved name, protocol, bandwidth, quota, encryption)StorageTierDetailActionButtons— Edit (navigate) / Delete (reusesStorageTierDeleteConfirmModal)protocolLabel/uniqueBackendIdsout ofStorageTiersListPage.tsxinto a sharedstorageTierBackendResolution.tsmodule, used by both the list and details pagesStorageTiersListPage's Name column now links to the new details routetiers/:idinStorageRoutes.tsxonStorageTierGetoverride to the shared mock Connect transport (existing pattern for other RPCs was missing onGet)Testing
StorageTiersListPage.test.tsx(+1) andStorageRoutes.test.tsx(+1). Covers loading, full success rendering, status message present/absent, description present/absent, backend-name resolution and its raw-ID fallback, the resolution-failure warning banner, generic load error, not-found + return-to-list navigation, Edit navigation, delete success, and referential-integrity delete error.pnpm test) passes (two unrelated, pre-existing flaky timeouts observed under full-suite parallel load, both confirmed to pass in isolation — see validation report).Acceptance Criteria
/admin/infrastructure/storage/tiers/:idrenders a details page for the tier fetched viausePrivateStorageTier(id).StorageTiersListPagelinks to this route instead of rendering plain text.StorageTierStatusLabel), and the status message when present.spec.backends, with the backend name resolved the same wayStorageTiersListPageresolves it (falling back tobackendIdwhen resolution fails), its protocol, max read/write bandwidth, quota, and encryption enabled./admin/infrastructure/storage/tiers/:id/edit; Delete opensStorageTierDeleteConfirmModaland navigates back to the list on success. (Implemented as two standalone buttons rather than a kebab dropdown — matches this codebase's only existing detail-page-header precedent (BareMetalActionButtons) and preserves identical behavior; flagging for reviewer awareness.)ResourceDetailsPageLoading; an unknown/deleted ID or fetch failure usesResourceDetailsPageErrorwith a "Return to Storage Tiers" action.