feature(crr): add wizard shell and Configure step with source/destination/replication sections#1243
Conversation
Hello hervedombya,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list: |
![claude[bot]](https://avatars.github.com/in/1236702?s=60&v=4){kind=small}
|
LGTM — clean scaffold. One minor test hygiene nit: |
e96b205 to
31af7cd
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list: |
31af7cd to
c719e6d
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list: |
|
|
LGTM |
c719e6d to
37a8801
Compare
|
LGTM |
37a8801 to
9e5b6b5
Compare
Review by Claude Code |
9e5b6b5 to
7b4ef55
Compare
hervedombya
changed the title
| setValue(name, '', { shouldValidate: true, shouldDirty: true }); | ||
| return; | ||
| } | ||
| const reader = new FileReader(); |
There was a problem hiding this comment.
reader.onerror is not handled. If the dropped file cannot be read (binary, permission error, etc.), the user gets no feedback — the certificate field silently stays unchanged.
— Claude Code
|
|
||
| export const DestinationConnectionSection = ({ isCheckingConnection, onCheckConnection }: Props) => { | ||
| const { | ||
| control, |
There was a problem hiding this comment.
Typo: "Connexion" should be "Connection" — the rest of the UI is in English.
— Claude Code
| } | ||
| }; | ||
|
|
||
| const onContinue = handleSubmit(async (values) => { |
There was a problem hiding this comment.
onCheckConnection calls getValues() without triggering form validation first. Unlike onContinue (which is gated by handleSubmit), this sends the request even when required destination fields are empty or invalid, producing confusing server errors instead of form-level validation messages. Consider guarding with trigger() or at least checking isValid before firing.
— Claude Code
|
7b4ef55 to
e0f773f
Compare
| }); | ||
| }); | ||
|
|
||
| const onCheckConnection = async () => { |
There was a problem hiding this comment.
Unlike the Continue button which gates on !isValid, Check Connection has no guard — clicking it with empty fields sends blank credentials to the verify endpoint. Consider either disabling the button when destination fields are incomplete, or calling trigger() on the relevant fields before sending the request.
— Claude Code
|
9238fed to
b4e9889
Compare
| const reader = new FileReader(); | ||
| reader.onload = () => { | ||
| setValue(name, reader.result as string, { shouldValidate: true, shouldDirty: true }); | ||
| }; |
There was a problem hiding this comment.
reader.onerror is never set — if the FileReader fails (e.g. the browser cannot read the dropped file), the certificate field stays empty with no feedback to the user. Add an onerror handler that clears the field and shows a validation error.
— Claude Code
|
a8eb3cb to
4bfeebb
Compare
|
4bfeebb to
6ffc5b1
Compare
|
6ffc5b1 to
27b798b
Compare
|
27b798b to
69956ff
Compare
|
LGTM |
| .required() | ||
| .messages({ 'string.pattern.base': 'Paste a PEM-encoded certificate' }), | ||
|
|
||
| destinationAccountName: Joi.string().min(1).required(), |
There was a problem hiding this comment.
Can use accountNameValidationSchema in AccountCreate.tsx
| lastVerifiedRef.current = JSON.stringify(body); | ||
| }; | ||
|
|
||
| const onCheckConnection = async () => { |
There was a problem hiding this comment.
When click "Check Connection" on an empty form silently does nothing.
… Connection validation errors
| // biome-ignore-start lint/suspicious/noThenProperty: `then` is Joi's conditional schema branch, not a promise-like method | ||
| export const configureSchema = Joi.object<ConfigureFormValues>({ | ||
| accountNameType: Joi.string().valid('create', 'existing').required(), | ||
| accountName: Joi.string().min(1).required(), |
There was a problem hiding this comment.
accountNameValidationSchema is already imported but only applied to destinationAccountName. When accountNameType is create, the source account name passes with a single character or characters outside [\w+=,.@ -] that the API will reject. Add a conditional branch so new account names get the same validation:
```suggestion
accountName: Joi.when('accountNameType', {
is: 'create',
then: accountNameValidationSchema,
otherwise: Joi.string().min(1).required(),
}),
|
… name in create mode
| /> | ||
| </Stack> | ||
| {errorMessage && <ErrorText>{errorMessage}</ErrorText>} | ||
| </CertificateContainer> |
There was a problem hiding this comment.
The ErrorText is not programmatically associated with the TextArea — screen readers will not announce validation errors when the field is focused. The other form fields go through FormGroup which handles this, but since CertificateSection renders its own error display, it needs its own aria-describedby wiring. Add aria-describedby={errorMessage ? \${name}-error` : undefined}to the TextArea andid={`${name}-error`}` to ErrorText.
— Claude Code
|
Clean PR — well-structured wizard with proper conditional Joi validation, verify caching, and good integration test coverage. One minor accessibility issue: |
|
LGTM |
|
/approve |
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue ARTESCA-16787. Goodbye hervedombya. The following options are set: approve |
3 participants
Lands the CRR Provisioning Wizard's shell and its first step. Users open it from a
Start CRR Configurationbutton on the Locations page (only visible when theCRR_CONFIGURATORfeature flag is enabled) and are presented with a single Configure page grouped into three sections: Source (create or reuse an account on the source cluster), Destination Connection (network mode, URL or base domain + S3 endpoint, admin credentials, PEM certificate, destination account name), and Replication (an optional replication rule with source bucket, target bucket and prefix). A Check Connection button inside the Destination section validates the destination on demand and shows a success or error toast tied to the ARTESCA problem code. Continue silently re-verifies unless the last verify already covered the exact same connection values, and blocks progression with an error toast if that verify fails; a successful advance is transparent, as expected in a wizard flow. Apply actions and Summary land as placeholders and are filled in by the follow-up brick.Under the hood, the Configure step is composed of three small section components that share form state via
FormProvider, wrapping a strict Joi schema with conditional branches on the connection mode and the replication toggle. The PEM certificate input is a reusableCertificateSectioncopied from identity-ui's identity-provider form (drag-and-drop or paste) — long-term this belongs in@scality/core-uiso both apps share it.Deviation from the plan: F5 was originally scoped to just a Configure destination step; scope was widened during review to also cover the Source and Replication sections and the Locations-page entry point.
Next: the Apply actions step, which streams the destination-side setup chain and reports its progress.
screenshots :