OSAC-3599: [UI] Storage Backends list page - #130
Merged
openshift-merge-bot[bot] merged 12 commits intoAug 11, 2026
Merged
Conversation
|
@ElayAharoni: This pull request references OSAC-3599 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.0.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. |
WalkthroughThe PR replaces the Backends placeholder with a functional storage-backend list. It adds status display, create and edit navigation, delete confirmation, translations, tests, and mutable mock transport fixtures. ChangesStorage backend management
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant StorageManagementPage
participant StorageBackendsListPage
participant StorageBackendsTable
participant StorageBackendDeleteConfirmModal
Admin->>StorageManagementPage: open Backends tab
StorageManagementPage->>StorageBackendsListPage: render backend list
StorageBackendsListPage->>StorageBackendsTable: render loaded backends
Admin->>StorageBackendsTable: select delete action
StorageBackendsTable->>StorageBackendDeleteConfirmModal: open confirmation
StorageBackendDeleteConfirmModal-->>StorageBackendsListPage: report successful deletion
StorageBackendsListPage-->>Admin: display remaining backends
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
ElayAharoni
marked this pull request as ready for review
August 9, 2026 14:29
ElayAharoni
requested review from
batzionb and
rawagner
and removed request for
jhernand and
larsks
August 9, 2026 14:30
Contributor
|
rename state to status and move the column to be second after name |
ElayAharoni
force-pushed
the
OSAC-3599-storage-backends-list-page
branch
from
August 10, 2026 14:52
4a9fe01 to
9ae8da9
Compare
Contributor
|
screenshot is of tenanats page... |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
libs/ui-components/src/components/Storage/StorageBackendStatusLabel.tsx (1)
12-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a
switchfor the state mapping.Map
StorageBackendStatewith aswitch. This makes the default behavior explicit and avoids rebuilding a lookup object on each render.Proposed refactor
-const storageBackendStatusMap = (t: TFunction): Record<StorageBackendState, StatusLabelProps> => ({ - [StorageBackendState.READY]: { - status: 'ready', - text: t('Ready'), - }, - [StorageBackendState.UNSPECIFIED]: { - status: 'unspecified', - text: t('Unspecified'), - }, -}); +const getStorageBackendStatus = ( + state: StorageBackendState | undefined, + t: TFunction, +): StatusLabelProps => { + switch (state) { + case StorageBackendState.READY: + return { status: 'ready', text: t('Ready') }; + case StorageBackendState.UNSPECIFIED: + default: + return { status: 'unspecified', text: t('Unspecified') }; + } +}; - const statusMap = storageBackendStatusMap(t); - - const status = - state !== undefined ? statusMap[state] : statusMap[StorageBackendState.UNSPECIFIED]; + const status = getStorageBackendStatus(state, t);Based on learnings, prefer a
switchstatement when mapping a discriminant to component props.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/ui-components/src/components/Storage/StorageBackendStatusLabel.tsx` around lines 12 - 29, Replace the storageBackendStatusMap lookup and status selection in StorageBackendStatusLabel with a switch on state that returns the corresponding StatusLabelProps for READY and UNSPECIFIED, using UNSPECIFIED as the default. Keep translation through the existing useTranslation hook and preserve the rendered status behavior.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/ui-components/src/components/Storage/StorageBackendActionsMenu.tsx`:
- Around line 35-43: Update the MenuToggle in StorageBackendActionsMenu to
receive the controlled dropdown state via isExpanded={open}, using the same open
state managed by setOpen and passed to Control’s isOpen/onOpenChange, so its
aria-expanded value stays synchronized with the menu.
In
`@libs/ui-components/src/components/Storage/StorageBackendDeleteConfirmModal.test.tsx`:
- Around line 64-89: Update the test around StorageBackendDeleteConfirmModal so
the mocked deletion failure is triggered by the Delete action rather than being
present during render: make mutate set or return the FAILED_PRECONDITION error,
assert the exact server error is absent before clicking, then assert it appears
afterward and onSuccess remains uncalled.
---
Nitpick comments:
In `@libs/ui-components/src/components/Storage/StorageBackendStatusLabel.tsx`:
- Around line 12-29: Replace the storageBackendStatusMap lookup and status
selection in StorageBackendStatusLabel with a switch on state that returns the
corresponding StatusLabelProps for READY and UNSPECIFIED, using UNSPECIFIED as
the default. Keep translation through the existing useTranslation hook and
preserve the rendered status behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f7cbb20d-25c7-4311-b919-295f5d362f07
📒 Files selected for processing (13)
apps/app-frontend/src/shell/StorageRoutes.test.tsxlibs/i18n/locales/en/translation.jsonlibs/ui-components/src/components/Storage/StorageBackendActionsMenu.tsxlibs/ui-components/src/components/Storage/StorageBackendDeleteConfirmModal.test.tsxlibs/ui-components/src/components/Storage/StorageBackendDeleteConfirmModal.tsxlibs/ui-components/src/components/Storage/StorageBackendStatusLabel.test.tsxlibs/ui-components/src/components/Storage/StorageBackendStatusLabel.tsxlibs/ui-components/src/components/Storage/StorageBackendsTable.tsxlibs/ui-components/src/pages/admin/StorageBackendsListPage.test.tsxlibs/ui-components/src/pages/admin/StorageBackendsListPage.tsxlibs/ui-components/src/pages/admin/StorageManagementPage.test.tsxlibs/ui-components/src/pages/admin/StorageManagementPage.tsxlibs/ui-components/src/test-utils/createMockConnectTransport.ts
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Adds the delete-removal test path, which required fixing createMockConnectTransport's StorageBackends delete handler to actually remove the item from the fixtures array instead of no-op'ing, so a post-delete refetch reflects the deletion. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Also updates StorageRoutes.test.tsx, which independently asserted on the now-removed Backends placeholder text. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com>
- Defensively clone the storageBackends fixtures array in createMockConnectTransport so the delete handler's mutation cannot corrupt a shared fixture array reused across tests regardless of execution order. - Hide the "Create backend" button on a list-load error, matching the TenantListPage/IdentityProviderListPage convention. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com>
…column - Renamed StorageBackendStateLabel to StorageBackendStatusLabel to match the StatusLabel naming convention used by every other resource (Tenant, Cluster, IdentityProvider, Subnet, SecurityGroup, VirtualNetwork). - Renamed the table's "State" column header to "Status" and moved it to the second position, right after Name. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com>
main moved Storage under the new Infrastructure nav section (OSAC-3780), remounting StorageRoutes at /admin/infrastructure/storage/* instead of /admin/storage/*. Updates the remaining hardcoded absolute navigate() targets and test assertions in files this branch owns exclusively (Create-backend and Edit-backend navigation) to match; StorageManagementPage.tsx and its route registration were already updated by the rebase merge itself, since OSAC-3780 touched those pre-existing files directly. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com>
isExpanded was missing on StorageBackendActionsMenu's MenuToggle, so aria-expanded never reflected the controlled Dropdown's open/closed state. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Rebasing onto main (which merged OSAC-3604's real StorageTiersListPage
alongside our real StorageBackendsListPage in the same
StorageManagementPage.tsx) left two issues no textual merge conflict
caught:
- An unused StoragePlaceholder import (both tabs now render real
components, StoragePlaceholder is only needed by StorageRoutes.tsx's
create/edit routes).
- Two StorageManagementPage.test.tsx assertions written against the
Backends tab's old placeholder text ("Storage backends"), which no
longer renders now that the real list page is wired in. Updated to
check for the "Create backend" button instead, mirroring how the
same file already identifies the Tiers tab via "Create tier".
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
ElayAharoni
force-pushed
the
OSAC-3599-storage-backends-list-page
branch
from
August 11, 2026 08:55
71c3343 to
b6dabb6
Compare
rawagner
reviewed
Aug 11, 2026
Stop hiding the Create backend action when the backend list fails to load; per reviewer feedback, blocking it on a data-loading error isn't needed. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/ui-components/src/pages/admin/StorageBackendsListPage.tsx (1)
18-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the new error-state behavior with a regression test.
The button is now intentionally outside
ListPageBody, so it remains available whenusePrivateStorageBackends()returns an error. Add an error-state case inlibs/ui-components/src/pages/admin/StorageBackendsListPage.test.tsxthat assertsCreate backendremains rendered.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/ui-components/src/pages/admin/StorageBackendsListPage.tsx` around lines 18 - 27, Add an error-state regression test in StorageBackendsListPage.test.tsx that mocks usePrivateStorageBackends() to return an error and asserts the “Create backend” button remains rendered outside the failed list content. Reuse the existing test setup and rendering patterns in the StorageBackendsListPage test suite.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@libs/ui-components/src/pages/admin/StorageBackendsListPage.tsx`:
- Around line 18-27: Add an error-state regression test in
StorageBackendsListPage.test.tsx that mocks usePrivateStorageBackends() to
return an error and asserts the “Create backend” button remains rendered outside
the failed list content. Reuse the existing test setup and rendering patterns in
the StorageBackendsListPage test suite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5fb14510-6a39-4de3-abeb-616fa3583780
📒 Files selected for processing (1)
libs/ui-components/src/pages/admin/StorageBackendsListPage.tsx
rawagner
approved these changes
Aug 11, 2026
|
[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 |
This was referenced Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OSAC-3599: [UI] Storage Backends list page
Jira: https://redhat.atlassian.net/browse/OSAC-3599
Story type: [UI]
Summary
Adds the Backends list page under Storage admin management: a table of registered storage backends (name, provider, endpoint, state), a "Create backend" action, and a per-row Edit/Delete kebab. Replaces the placeholder previously left in
StorageManagementPage's Backends tab.Changes
StorageBackendStateLabel: mapsStorageBackendState(READY/UNSPECIFIED) to the sharedResourceStatusLabelStorageBackendsTable: plain PatternFly table with NAME/PROVIDER/ENDPOINT/STATE columns and a row-actions cellStorageBackendActionsMenu: row kebab — Edit navigates to the existing (placeholder) edit route, Delete opens a confirm modalStorageBackendDeleteConfirmModal: surfaces the server's delete error (e.g.FAILED_PRECONDITIONwhen a backend is still referenced by an activeStorageTier) verbatim, inline, without removing the row; hides the "Create backend" action when the list fails to loadStorageBackendsListPage: wiresusePrivateStorageBackends()to the table viaListPageBodyfor loading/error/empty statesStorageManagementPage's Backends tab, replacing theStoragePlaceholdercreateMockConnectTransport'sStorageBackends.deletetest stub, which was previously a stateless no-op — it now removes the deleted item from a defensively-cloned copy of the fixtures array, so tests can verify real query-invalidation-driven row removal without corrupting shared fixtures across test runsTesting
StorageBackendStateLabel(3 tests: READY/UNSPECIFIED/undefined mappings),StorageBackendDeleteConfirmModal(3 tests: success,FAILED_PRECONDITIONverbatim + no removal, cancel)StorageBackendsListPage(8 tests: columns, rows, state labels, empty state, create/edit navigation, delete-dialog open, delete-success row removal via real invalidation).StorageBackendActionsMenu/StorageBackendsTablehave no isolated tests, matching this codebase's existing convention (ClustersTable,IdentityProviderActionsMenuhave none) — covered transitively through the list page's tests.ClusterCreatePage.test.tsxparallel-worker timeout flake, untouched by this branch).Acceptance Criteria
usePrivateStorageBackends().StorageBackendStateLabelcomponent wrapping the sharedResourceStatusLabel, mapping READY to green "Ready" and UNSPECIFIED to grey "Unspecified."/admin/storage/backends/create./admin/storage/backends/:id/edit) and Delete.FAILED_PRECONDITIONerror verbatim, and the row remains in the table (no optimistic removal).Summary by CodeRabbit
New Features
Tests