OSAC-3600: [UI] Create Storage Backend page - #136
Conversation
|
@ElayAharoni: This pull request references OSAC-3600 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: osac-project/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR replaces the storage backend creation placeholder with a validated creation form. It adds provider labels and validation messages, handles submission and navigation, updates storage-tier submission behavior, and expands related tests. ChangesStorage backend creation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
actor Operator
participant StorageBackendCreatePage
participant StorageBackendMutation
participant BackendListRoute
Operator->>StorageBackendCreatePage: enter backend details and submit
StorageBackendCreatePage->>StorageBackendMutation: send storage backend request
StorageBackendMutation-->>StorageBackendCreatePage: return success or creation error
StorageBackendCreatePage->>BackendListRoute: navigate after success
Possibly related PRs
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/pages/admin/StorageBackendCreatePage.tsx`:
- Around line 96-112: Update the storage-backend create flow in the onSubmit
handler to include the established CSRF token mechanism when calling mutate, and
ensure the corresponding /api/fulfillment request path validates that token
before processing the state-changing operation. Reuse existing token generation,
transport, and proxy validation utilities rather than introducing a parallel
implementation.
- Around line 96-112: Update the form submission flow around onSubmit to return
mutateAsync, and disable the Create action whenever the form isSubmitting or the
mutation isPending. Configure the shared production transport to include CSRF
protection alongside connectErrorInterceptor, then add regression coverage for
rapid repeated activation and rejected requests without a valid CSRF token.
🪄 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: bb9806b4-d817-408c-9362-6b575fcfdd0d
📒 Files selected for processing (5)
apps/app-frontend/src/shell/StorageRoutes.test.tsxapps/app-frontend/src/shell/StorageRoutes.tsxlibs/i18n/locales/en/translation.jsonlibs/ui-components/src/pages/admin/StorageBackendCreatePage.test.tsxlibs/ui-components/src/pages/admin/StorageBackendCreatePage.tsx
7e6dc55 to
4b31139
Compare
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>
…t guard, unsaved-changes prompt - Reuse the shared NameField component for the name input, restructuring Formik values/Yup schema from a flat name to nested metadata.name. - Drop the redundant provider oneOf validator (the Select already constrains the value); this also fixes a latent bug where an empty provider showed the wrong error message instead of "Provider is required". - Switch to mutateAsync and disable Create/Cancel while Formik's isSubmitting or the mutation's isPending is true, preventing duplicate submissions. - Mount LeaveFormConfirmation to warn on navigating away with unsaved changes; generalize its copy since it's no longer wizard-only. - Extract a hardcoded test password into a named fixture constant. - Widen the mock transport's onStorageBackendCreate override type to allow an async handler, matching what the router already supports. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com>
…onventions - Switch to mutateAsync and disable Create/Cancel while Formik's isSubmitting or the mutation's isPending is true, preventing duplicate submissions. - Reuse the shared NameField component, restructuring Formik values/Yup schema from a flat name to nested metadata.name. - Mount LeaveFormConfirmation to warn on navigating away with unsaved changes. - Widen the mock transport's onStorageTierCreate override type to allow an async handler, matching onStorageBackendCreate and what the router already supports. - Add a test verifying the Create/Cancel buttons disable during a pending submission. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com>
Formik's isSubmitting already covers the full window mutateAsync is in flight (it only resets once onSubmit's returned promise settles), so the separate isPending check on the Create/Cancel buttons was redundant. Applied the same simplification to StorageTierCreatePage for consistency. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com>
4513f68 to
285d2d3
Compare
Credentials are not required by the backend, but the create form forced username and password. Add a 'Use credentials' checkbox (checked by default): when unchecked, the credential fields are hidden, their validation is skipped, and the credentials object is omitted from the create request entirely. When checked, both username and password remain required, so credentials are always all-or-nothing by construction. Addresses review feedback that the UI should reflect the backend's optional credentials. Assisted-by: Claude Code <noreply@anthropic.com> 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 |
4bcc5df
into
osac-project:main
OSAC-3600: [UI] Create Storage Backend page
Jira: https://redhat.atlassian.net/browse/OSAC-3600
Story type: [UI]
Summary
Adds
StorageBackendCreatePage, a full-page Formik+Yup form for registering a new storage backend (name, provider, endpoint, description, credentials), and wires it into the existing/admin/infrastructure/storage/backends/createroute in place of the OSAC-3598 placeholder. Lets a Cloud Provider Admin register a backend without hand-constructing API/CLI calls.Changes
libs/ui-components/src/pages/admin/StorageBackendCreatePage.tsx(new) — full-page create form modeled onTenantCreatePage:name(RFC 1035 DNS-label validated),provider(Select constrained tovast/ceph/pure),endpoint(required),description(optional),credentials.username/credentials.password(required, password masked viatype="password"). Submits via the existinguseCreateStorageBackend()hook and navigates back to the backends list on success; a duplicate-nameALREADY_EXISTSresponse renders as a form-level error without navigating away.apps/app-frontend/src/shell/StorageRoutes.tsx—backends/createnow rendersStorageBackendCreatePageinstead of the placeholder.libs/i18n/locales/en/translation.json— new translation keys extracted viapnpm i18n.Testing
StorageBackendCreatePage.test.tsx(field rendering, provider option set, password masking, DNS-label validation, required-field validation, successful create+navigate with exact payload assertions, duplicate-name error without navigation) + 1 updated route test inStorageRoutes.test.tsx.osac-uihas no persisted integration/E2E suite; the create flow's true E2E coverage is owned by a futureosac-test-infra[QE]story.Acceptance Criteria
/admin/storage/backends/create(real path:/admin/infrastructure/storage/backends/create, renamed in OSAC-3598/3599) renders a full page with fields: name, provider, endpoint, description, credentials.username, credentials.password.nameis validated client-side as an RFC 1035 DNS label before submission.provideris a Select constrained to vast/ceph/pure — not a free-text field.credentials.passwordrenders as a masked input (type="password") and is never rendered as plain text.endpoint,credentials.username, andcredentials.passwordare required; submission is blocked client-side if any is empty./admin/infrastructure/storage/backends, where the new backend appears.ALREADY_EXISTSas a form-level error without navigating away.Summary by CodeRabbit
New Features
Bug Fixes