-
Notifications
You must be signed in to change notification settings - Fork 336
Introduce new outbound provisioning connection and improve UX #9589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Introduce new outbound provisioning connection and improve UX #9589
Conversation
WalkthroughThis pull request introduces a new "Provisioning Connection V2" feature across the Identity and Access Management console. It adds feature flags to enable/disable the feature, creates a comprehensive wizard for creating outbound provisioning connections, adds form components for connector configuration, and updates existing connection edit flows to support the new provisioning connection type with conditional visibility logic. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/UI
participant Wizard as Outbound Provisioning<br/>Create Wizard
participant API as Connection API
participant Connectors as Provisioning<br/>Connectors Service
participant Backend as Backend
User->>Wizard: Open create wizard
Wizard->>Connectors: Fetch available connectors
Connectors->>Backend: GET /provisioning-connectors
Backend-->>Connectors: Return connector list
Connectors-->>Wizard: Return filtered metadata list
Wizard->>User: Show connector selection (Step 1)
User->>Wizard: Select connector
Wizard->>Connectors: Fetch connector metadata
Connectors->>Backend: GET /provisioning-connectors/{id}/metadata
Backend-->>Connectors: Return metadata
Connectors-->>Wizard: Return metadata with config schema
Wizard->>User: Show connector configuration form (Step 2)
User->>Wizard: Fill connection details & configuration
User->>Wizard: Submit wizard
Wizard->>API: POST create connection with outbound provisioners
API->>Backend: Create connection with payload
Backend-->>API: Return 201 + Location header
API-->>Wizard: Return created connection
Wizard->>User: Show success alert with connection ID
Wizard->>User: Close wizard & callback with ID
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@apps/console/src/public/deployment.config.json`:
- Line 337: The public sample config currently sets the boolean flag
enableProvisioningConnectionV2 to true while the production template
deployment.config.json.j2 defaults it to false; update the public sample to
match the production default by changing enableProvisioningConnectionV2 to false
(or alternatively update the template to true if the intended default is true)
so both artifacts use the same default behavior and avoid divergence between
sample and prod.
In
`@features/admin.connections.v1/components/create/outbound-provisioning-connection-create-wizard.tsx`:
- Around line 447-551: Replace all hardcoded user-facing English strings in the
generalDetailsPage JSX with i18n lookups via the t() function: update the
WizardPage validation message ("This is a required field") and both Field.Input
placeholders/labels ("Enter a name for the connection", "Connection name",
"Enter a description for the connection", "Description") to use t("...") keys;
replace the connector selection Heading text ("Select a provisioning connector")
and the SelectionCard fallbacks/loader strings ("Loading connector
configuration...", "No connector metadata available", and the duplicate "This is
a required field") with appropriate t() keys as well. Locate these in the
generalDetailsPage function and related JSX that renders Field.Input, Heading,
and SelectionCard (symbols: generalDetailsPage, Field.Input, Heading,
SelectionCard, outboundProvisioningConnectorsMetadataList) and swap the literal
strings for t("authenticationProvider:...") keys consistent with the file's
existing translation patterns. Ensure any validation messages inside the
validate and validation props also use t() instead of hardcoded text and keep
existing calls to setNextShouldBeDisabled unchanged.
- Around line 296-321: In handleFormSubmit, connector property checkbox
(boolean) values are being pushed raw; detect boolean values for each property
(use the existing loop over connectorMetaData.properties and the computed
fieldName/fieldValue) and convert them to string form ("true" or "false") before
adding to properties (mirror
OutboundProvisioningConnectorConfigForm.handleSubmit behavior of
String(!!fieldValue)); ensure defaultValue handling still applies and that
properties.push uses the stringified value for booleans.
- Around line 236-267: idpNameValidation currently initializes idpExist to false
so the uncached branch never calls the API; change the initialization to let
idpExist: boolean | undefined = undefined, keep the cache check using
idpNameValidationCache.current.value to assign idpExist when present, and then
let the existing if (idpExist === undefined) block call
ConnectionsManagementUtils.searchIdentityProviderName; finally ensure you still
set idpNameValidationCache.current = { state: idpExist, value } and call
setIsUserInputIdpNameAlreadyTaken(!!idpExist).
In
`@features/admin.connections.v1/components/edit/forms/outbound-provisioning-connectors/outbound-provisioning-connector-config-form.tsx`:
- Around line 104-105: The submit button visibility uses the raw readOnly prop
causing a mismatch with the computed isFieldReadOnly (which forces editable in
CREATE mode); update the submit button visibility logic to use isFieldReadOnly
(computed from mode === AuthenticatorSettingsFormModes.CREATE ? false :
readOnly) instead of readOnly so CREATE mode truly shows the submit action when
fields are editable.
🧹 Nitpick comments (11)
features/admin.connections.v1/components/edit/forms/outbound-provisioning-connectors/connector-config-form-fields.scss (1)
19-23: Consider using a theme token instead of hard-coded white.Line 19–23 hard-codes
background-color: white;, which can clash with dark themes or custom branding. A theme variable (if available) would keep the inputs consistent across themes.features/admin.core.v1/configs/app.ts (1)
432-432: Consider defaulting the new flag to false at the UI config layer.Line 432 currently passes through
undefinedwhen the flag isn’t present. A defaultfalsekeeps behavior consistent with the deployment config fallback and avoids downstream undefined checks.♻️ Suggested tweak
- enableProvisioningConnectionV2: window[ "AppUtils" ]?.getConfig()?.ui?.enableProvisioningConnectionV2, + enableProvisioningConnectionV2: + window[ "AppUtils" ]?.getConfig()?.ui?.enableProvisioningConnectionV2 ?? false,features/admin.connections.v1/components/edit/forms/general-details-form.tsx (1)
377-377: Inconsistent spacing:! isOutboundProvisioningConnection.Line 336 uses
!isOutboundProvisioningConnection(no space), but Line 377 uses! isOutboundProvisioningConnection(with a space). Align for consistency.🧹 Proposed fix
- && ! isOutboundProvisioningConnection + && !isOutboundProvisioningConnectionfeatures/admin.connections.v1/utils/provisioning-utils.ts (1)
51-61: Type assertion after shallow merge may mask missing properties.The
as OutboundProvisioningConnectorMetaDataInterfacecast on the merged object assumes the union ofconnectorandmetadatafields satisfies the full interface. Ifmetadataisundefined(no match found), the cast could hide the fact that required metadata fields are absent on the returned object.Consider whether a stricter approach (e.g., only spreading metadata when it's defined, or validating key fields) would be safer, or if callers already handle partial metadata gracefully.
features/admin.connections.v1/components/edit/forms/outbound-provisioning-connectors/outbound-provisioning-connector-config-form.tsx (1)
153-187: Side-effect inside render callback:formRef.current = formAssigning
formRef.currenton every render call (Line 157) inside the FinalFormrenderprop is a side effect during rendering, which can misbehave under React Strict Mode (double-invoked render) or concurrent features. Consider moving this into auseEffector using the form API reference differently.Additionally, the
PrimaryButtonat Line 171 is placed outside the<form>element (which ends at Line 169), yet carriestype="submit". Thetype="submit"attribute is ineffective outside a<form>. Since theonClickhandler callsform.submit()directly, the button works, but the attribute is misleading.Suggested fix: remove misleading type="submit"
<PrimaryButton - type="submit" + type="button" data-componentid={ `${componentId}-form-update-button` }features/admin.connections.v1/components/wizards/outbound-provisioning-connector-create-wizard.tsx (1)
515-517:defaultPropson function components is deprecated in React 19.This won't affect you on React 18.2, but if you plan to upgrade to React 19 (where
defaultPropson function components is removed), consider migrating to default parameter values in the destructuring.features/admin.connections.v1/components/create/outbound-provisioning-connection-create-wizard.tsx (2)
395-441:setTimeoutfor alert dismissal can cause state update on unmounted component.The three
setTimeout(() => setAlert(undefined), 4000)calls (Lines 412, 426, 437) may fire after the component has unmounted (e.g., if the wizard is closed within 4 seconds of an error), leading to a React "can't perform state update on unmounted component" warning and potential memory leak.Consider clearing the timeout on unmount or using a ref to track mount status.
120-129:templateprop is destructured but never used.The
templateprop is defined in the interface (Line 69) and destructured (Line 123-129 implicitly viaprops) but is never referenced in the component body. If it's intended for future use, consider removing it from destructuring to avoid confusion.features/admin.connections.v1/components/edit/forms/outbound-provisioning-connectors/connector-config-form-fields.tsx (3)
128-134: MoveFieldTypeenum outside the component body.The
FieldTypeenum is declared inside the component function, meaning a new enum object is created on every render. Since it's a static definition, move it to module scope for better performance and to allow reuse.Proposed change
Move the enum before the component definition (e.g., after line 37):
+/** + * Field types enum. + */ +enum FieldType { + CHECKBOX = "CheckBox", + RADIO = "Radio", + DROPDOWN = "DropDown", + TEXT = "Text", + PASSWORD = "Password" +} + /** * Shared metadata-driven field renderer for outbound provisioning connector config.And remove Lines 125-134 from inside the component.
79-88:dynamicValuesstate is not truly dynamic — only reflects initial values.The comment on Line 79 says it tracks "dynamic values for properties that need to listen to changes." However,
dynamicValuesis only updated wheninitialValuesprop changes (via theuseEffecton Line 84), not when the user interacts with the form. This means sub-property visibility that depends on a parent checkbox's live state won't work. If conditional sub-property rendering based on user interaction is needed, this state needs to be wired to form value changes.If this is intentional (sub-properties are always shown regardless of parent state), consider renaming
dynamicValuesto something likeresolvedInitialValuesto avoid confusion.
306-344: Replace__DEPRECATED__SelectFieldAdapterwithSelectFieldAdapter.The codebase already provides
SelectFieldAdapteras the non-deprecated alternative in the@wso2is/formpackage. It uses the same prop structure (acceptslabel,options,fullWidth,FormControlProps, etc.) and is already adopted in other parts of the codebase. Migrating away from the deprecated Material-UI-based adapter to the Oxygen-UI-basedSelectFieldAdapterwill ensure compatibility with future versions.
| "enableIdentityClaims": true, | ||
| "enableLegacySessionBoundTokenBehaviour": false, | ||
| "enableOldUIForEmailProvider": false, | ||
| "enableProvisioningConnectionV2": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file around line 337
cat -n apps/console/src/public/deployment.config.json | sed -n '330,345p'Repository: wso2/identity-apps
Length of output: 769
🏁 Script executed:
# Search for all occurrences of enableProvisioningConnectionV2 in the codebase
rg "enableProvisioningConnectionV2" -A 2 -B 2Repository: wso2/identity-apps
Length of output: 5371
🏁 Script executed:
# Check if there are other deployment config files with different values
fd "deployment.config" --type fRepository: wso2/identity-apps
Length of output: 654
🏁 Script executed:
# Check myaccount public config
cat -n apps/myaccount/src/public/deployment.config.json | sed -n '300,340p' | grep -A 5 -B 5 "enableProvisioningConnectionV2"Repository: wso2/identity-apps
Length of output: 44
🏁 Script executed:
# Check the template file to see the full context
cat -n apps/console/java/org.wso2.identity.apps.console.server.feature/resources/deployment.config.json.j2 | sed -n '250,290p'Repository: wso2/identity-apps
Length of output: 2113
🏁 Script executed:
# Search for any documentation or comments about this feature in issue trackers or comments
rg "provisioning.*v2" -i --type md --type txt -B 2 -A 2 | head -50Repository: wso2/identity-apps
Length of output: 44
🏁 Script executed:
# Search more broadly for myaccount config with enableProvisioningConnectionV2
rg "enableProvisioningConnectionV2" apps/myaccount/Repository: wso2/identity-apps
Length of output: 44
🏁 Script executed:
# Get more context around the template file setting
cat apps/console/java/org.wso2.identity.apps.console.server.feature/resources/deployment.config.json.j2 | sed -n '150,180p'Repository: wso2/identity-apps
Length of output: 1408
🏁 Script executed:
# Find the exact line in the template where enableProvisioningConnectionV2 is set
rg -n "enableProvisioningConnectionV2" apps/console/java/org.wso2.identity.apps.console.server.feature/resources/deployment.config.json.j2 -B 2 -A 2Repository: wso2/identity-apps
Length of output: 446
🏁 Script executed:
# Check if myaccount public config has any provisioning-related settings
cat apps/myaccount/src/public/deployment.config.json | grep -i provisionRepository: wso2/identity-apps
Length of output: 44
🏁 Script executed:
# Look for any git history or recent changes to understand intent
git log --oneline --all -- "*deployment.config.json" | head -20Repository: wso2/identity-apps
Length of output: 1273
Resolve config inconsistency between public sample and production template defaults.
Line 337 sets enableProvisioningConnectionV2 to true in the public sample config, but the production template (deployment.config.json.j2) defaults this to false when not explicitly configured (lines 2556–2560). Align these defaults to prevent confusion and behavioral differences between sample and production deployments.
🤖 Prompt for AI Agents
In `@apps/console/src/public/deployment.config.json` at line 337, The public
sample config currently sets the boolean flag enableProvisioningConnectionV2 to
true while the production template deployment.config.json.j2 defaults it to
false; update the public sample to match the production default by changing
enableProvisioningConnectionV2 to false (or alternatively update the template to
true if the intended default is true) so both artifacts use the same default
behavior and avoid divergence between sample and prod.
| const idpNameValidation: DebouncedFunc<(value: string) => void> = debounce( | ||
| async (value: string) => { | ||
| let idpExist: boolean = false; | ||
|
|
||
| if (idpNameValidationCache?.current?.value === value) { | ||
| idpExist = idpNameValidationCache?.current?.state; | ||
| } | ||
|
|
||
| if (idpExist === undefined) { | ||
| try { | ||
| idpExist = await ConnectionsManagementUtils.searchIdentityProviderName(value); | ||
| } catch (e) { | ||
| /** | ||
| * Ignore the error, as a failed identity provider search | ||
| * should not result in user blocking. However, if the | ||
| * identity provider name already exists, it will undergo | ||
| * validation from the backend, and any resulting errors | ||
| * will be displayed in the user interface. | ||
| */ | ||
| idpExist = false; | ||
| } | ||
|
|
||
| idpNameValidationCache.current = { | ||
| state: idpExist, | ||
| value | ||
| }; | ||
| } | ||
|
|
||
| setIsUserInputIdpNameAlreadyTaken(!!idpExist); | ||
| }, | ||
| 500 | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: IDP name uniqueness check API call is never reached.
idpExist is initialized to false (Line 238). When the value is not in the cache (Lines 240-242 are skipped), idpExist remains false. The condition on Line 244 (idpExist === undefined) is therefore always false for uncached values, so the searchIdentityProviderName API call on Line 246 is never executed. Every uncached name will be treated as available.
Proposed fix: initialize as undefined
- let idpExist: boolean = false;
+ let idpExist: boolean | undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const idpNameValidation: DebouncedFunc<(value: string) => void> = debounce( | |
| async (value: string) => { | |
| let idpExist: boolean = false; | |
| if (idpNameValidationCache?.current?.value === value) { | |
| idpExist = idpNameValidationCache?.current?.state; | |
| } | |
| if (idpExist === undefined) { | |
| try { | |
| idpExist = await ConnectionsManagementUtils.searchIdentityProviderName(value); | |
| } catch (e) { | |
| /** | |
| * Ignore the error, as a failed identity provider search | |
| * should not result in user blocking. However, if the | |
| * identity provider name already exists, it will undergo | |
| * validation from the backend, and any resulting errors | |
| * will be displayed in the user interface. | |
| */ | |
| idpExist = false; | |
| } | |
| idpNameValidationCache.current = { | |
| state: idpExist, | |
| value | |
| }; | |
| } | |
| setIsUserInputIdpNameAlreadyTaken(!!idpExist); | |
| }, | |
| 500 | |
| ); | |
| const idpNameValidation: DebouncedFunc<(value: string) => void> = debounce( | |
| async (value: string) => { | |
| let idpExist: boolean | undefined; | |
| if (idpNameValidationCache?.current?.value === value) { | |
| idpExist = idpNameValidationCache?.current?.state; | |
| } | |
| if (idpExist === undefined) { | |
| try { | |
| idpExist = await ConnectionsManagementUtils.searchIdentityProviderName(value); | |
| } catch (e) { | |
| /** | |
| * Ignore the error, as a failed identity provider search | |
| * should not result in user blocking. However, if the | |
| * identity provider name already exists, it will undergo | |
| * validation from the backend, and any resulting errors | |
| * will be displayed in the user interface. | |
| */ | |
| idpExist = false; | |
| } | |
| idpNameValidationCache.current = { | |
| state: idpExist, | |
| value | |
| }; | |
| } | |
| setIsUserInputIdpNameAlreadyTaken(!!idpExist); | |
| }, | |
| 500 | |
| ); |
🤖 Prompt for AI Agents
In
`@features/admin.connections.v1/components/create/outbound-provisioning-connection-create-wizard.tsx`
around lines 236 - 267, idpNameValidation currently initializes idpExist to
false so the uncached branch never calls the API; change the initialization to
let idpExist: boolean | undefined = undefined, keep the cache check using
idpNameValidationCache.current.value to assign idpExist when present, and then
let the existing if (idpExist === undefined) block call
ConnectionsManagementUtils.searchIdentityProviderName; finally ensure you still
set idpNameValidationCache.current = { state: idpExist, value } and call
setIsUserInputIdpNameAlreadyTaken(!!idpExist).
| const handleFormSubmit = (values: any): void => { | ||
|
|
||
| if (selectedConnectorId === null) return; | ||
| // Build provisioning properties from form values | ||
| const properties: any[] = []; | ||
|
|
||
| if (connectorMetaData?.properties) { | ||
| connectorMetaData.properties.forEach((property: any) => { | ||
| const fieldName: string = `connector_${property.key}`; | ||
| const fieldValue: any = values[fieldName]; | ||
|
|
||
| // Include property if it has a value or if it's a mandatory field | ||
| if (fieldValue !== undefined && fieldValue !== EMPTY_STRING) { | ||
| properties.push({ | ||
| key: property.key, | ||
| value: fieldValue | ||
| }); | ||
| } else if (property.defaultValue) { | ||
| // Include default value for fields without user input | ||
| properties.push({ | ||
| key: property.key, | ||
| value: property.defaultValue | ||
| }); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Boolean checkbox values are not stringified during submission.
In handleFormSubmit, checkbox field values (booleans) are passed directly without conversion. Compare with OutboundProvisioningConnectorConfigForm.handleSubmit (Lines 128-133 of the config form) which explicitly handles booleans with String(!!fieldValue). If the backend expects string "true"/"false" for boolean properties, this will submit raw boolean values instead.
Proposed fix
if (connectorMetaData?.properties) {
connectorMetaData.properties.forEach((property: any) => {
const fieldName: string = `connector_${property.key}`;
const fieldValue: any = values[fieldName];
- // Include property if it has a value or if it's a mandatory field
- if (fieldValue !== undefined && fieldValue !== EMPTY_STRING) {
+ if (property.type?.toUpperCase() === "BOOLEAN") {
+ properties.push({
+ key: property.key,
+ value: String(!!fieldValue)
+ });
+ } else if (fieldValue !== undefined && fieldValue !== EMPTY_STRING) {
properties.push({
key: property.key,
- value: fieldValue
+ value: String(fieldValue)
});
} else if (property.defaultValue) {🤖 Prompt for AI Agents
In
`@features/admin.connections.v1/components/create/outbound-provisioning-connection-create-wizard.tsx`
around lines 296 - 321, In handleFormSubmit, connector property checkbox
(boolean) values are being pushed raw; detect boolean values for each property
(use the existing loop over connectorMetaData.properties and the computed
fieldName/fieldValue) and convert them to string form ("true" or "false") before
adding to properties (mirror
OutboundProvisioningConnectorConfigForm.handleSubmit behavior of
String(!!fieldValue)); ensure defaultValue handling still applies and that
properties.push uses the stringified value for booleans.
| const generalDetailsPage = (): ReactElement => ( | ||
| <WizardPage | ||
| validate={ (values: any) => { | ||
| const errors: FormErrors = {}; | ||
|
|
||
| // Validate name field | ||
| errors.name = composeValidators(required, length(IDP_NAME_LENGTH))(values.name); | ||
| if (values?.name && isUserInputIdpNameAlreadyTaken) { | ||
| errors.name = t("authenticationProvider:forms.generalDetails.name.validations.duplicate"); | ||
| } | ||
| if (!FormValidation.isValidResourceName(values.name)) { | ||
| errors.name = t("authenticationProvider:templates.enterprise.validation.name"); | ||
| } | ||
|
|
||
| // Connector selection is mandatory | ||
| if (!selectedConnectorId) { | ||
| errors["connectorId"] = "This is a required field"; | ||
| } | ||
|
|
||
| setNextShouldBeDisabled(ifFieldsHave(errors)); | ||
|
|
||
| return errors; | ||
| } } | ||
| > | ||
| { /* General Details Section */ } | ||
| <Field.Input | ||
| data-testid={ `${componentId}-form-wizard-idp-name` } | ||
| ariaLabel="name" | ||
| inputType="resource_name" | ||
| name="name" | ||
| placeholder="Enter a name for the connection" | ||
| label="Connection name" | ||
| initialValue={ initialValues.name } | ||
| maxLength={ IDP_NAME_LENGTH.max } | ||
| minLength={ IDP_NAME_LENGTH.min } | ||
| required={ true } | ||
| width={ 15 } | ||
| format={ (values: any) => values.toString().trimStart() } | ||
| listen={ idpNameValidation } | ||
| validation={ (value: string) => { | ||
| let errors: string; | ||
|
|
||
| errors = composeValidators(required, length(IDP_NAME_LENGTH))(value); | ||
| if (value && isUserInputIdpNameAlreadyTaken) { | ||
| errors = t("authenticationProvider:forms.generalDetails.name.validations.duplicate"); | ||
| } | ||
| if (!FormValidation.isValidResourceName(value)) { | ||
| errors = t("authenticationProvider:templates.enterprise.validation.invalidName", { | ||
| idpName: value | ||
| }); | ||
| } | ||
|
|
||
| if (errors === EMPTY_STRING || errors === undefined) { | ||
| setNextShouldBeDisabled(false); | ||
| } | ||
|
|
||
| return errors; | ||
| } } | ||
| /> | ||
| <Field.Input | ||
| data-testid={ `${componentId}-form-wizard-idp-description` } | ||
| ariaLabel="description" | ||
| inputType="description" | ||
| name="description" | ||
| placeholder="Enter a description for the connection" | ||
| label="Description" | ||
| initialValue={ initialValues.description } | ||
| required={ false } | ||
| width={ 15 } | ||
| maxLength={ 300 } | ||
| minLength={ 0 } | ||
| type="textarea" | ||
| /> | ||
|
|
||
| { /* Connector Selection Section */ } | ||
| <div className="connector-selection"> | ||
| <Heading as="h5">Select a provisioning connector</Heading> | ||
| { | ||
| outboundProvisioningConnectorsMetadataList.map( | ||
| (connector: OutboundProvisioningConnectorMetaDataInterface) => ( | ||
| <SelectionCard | ||
| key={ connector.connectorId } | ||
| inline | ||
| image={ connector.icon } | ||
| size="small" | ||
| header={ connector.displayName || connector.name } | ||
| description={ connector.description } | ||
| selected={ selectedConnectorId === connector.connectorId } | ||
| onClick={ () => setSelectedConnectorId(connector.connectorId) } | ||
| imageSize="x30" | ||
| imageOptions={ { | ||
| relaxed: true, | ||
| square: false, | ||
| width: "auto" | ||
| } } | ||
| contentTopBorder={ false } | ||
| showTooltips={ true } | ||
| data-componentid={ `${componentId}-connector-${connector.name}-selection-card` } | ||
| /> | ||
| ) | ||
| ) | ||
| } | ||
| </div> | ||
| </WizardPage> | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple hardcoded English strings instead of i18n translations.
Several user-facing strings are hardcoded in English rather than using the t() translation function:
- Line 463:
"This is a required field" - Line 477:
"Enter a name for the connection" - Line 478:
"Connection name" - Line 511:
"Enter a description for the connection" - Line 512:
"Description" - Line 523:
"Select a provisioning connector" - Line 561:
"Loading connector configuration..." - Line 569:
"No connector metadata available" - Line 584:
"This is a required field"
These should use i18n keys for localization support, consistent with the rest of the codebase.
Also applies to: 557-602
🤖 Prompt for AI Agents
In
`@features/admin.connections.v1/components/create/outbound-provisioning-connection-create-wizard.tsx`
around lines 447 - 551, Replace all hardcoded user-facing English strings in the
generalDetailsPage JSX with i18n lookups via the t() function: update the
WizardPage validation message ("This is a required field") and both Field.Input
placeholders/labels ("Enter a name for the connection", "Connection name",
"Enter a description for the connection", "Description") to use t("...") keys;
replace the connector selection Heading text ("Select a provisioning connector")
and the SelectionCard fallbacks/loader strings ("Loading connector
configuration...", "No connector metadata available", and the duplicate "This is
a required field") with appropriate t() keys as well. Locate these in the
generalDetailsPage function and related JSX that renders Field.Input, Heading,
and SelectionCard (symbols: generalDetailsPage, Field.Input, Heading,
SelectionCard, outboundProvisioningConnectorsMetadataList) and swap the literal
strings for t("authenticationProvider:...") keys consistent with the file's
existing translation patterns. Ensure any validation messages inside the
validate and validation props also use t() instead of hardcoded text and keep
existing calls to setNextShouldBeDisabled unchanged.
| // In CREATE mode, fields should always be editable. | ||
| const isFieldReadOnly: boolean = mode === AuthenticatorSettingsFormModes.CREATE ? false : readOnly; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent use of readOnly vs isFieldReadOnly for button visibility.
Line 105 computes isFieldReadOnly to override readOnly in CREATE mode (fields are always editable). However, Line 170 uses the raw readOnly prop to control submit button visibility. If mode=CREATE and readOnly=true are passed simultaneously, fields will be editable but the submit button will be hidden — which seems unintentional.
Proposed fix
- { !readOnly && showSubmitButton && (
+ { !isFieldReadOnly && showSubmitButton && (Also applies to: 170-170
🤖 Prompt for AI Agents
In
`@features/admin.connections.v1/components/edit/forms/outbound-provisioning-connectors/outbound-provisioning-connector-config-form.tsx`
around lines 104 - 105, The submit button visibility uses the raw readOnly prop
causing a mismatch with the computed isFieldReadOnly (which forces editable in
CREATE mode); update the submit button visibility logic to use isFieldReadOnly
(computed from mode === AuthenticatorSettingsFormModes.CREATE ? false :
readOnly) instead of readOnly so CREATE mode truly shows the submit action when
fields are editable.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9589 +/- ##
=======================================
Coverage 55.88% 55.88%
=======================================
Files 42 42
Lines 1020 1020
Branches 231 254 +23
=======================================
Hits 570 570
Misses 450 450
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Purpose
Related Issues
Related PRs
Checklist
Security checks
Developer Checklist (Mandatory)
product-isissue to track any behavioral change or migration impact.Summary by CodeRabbit
Release Notes
New Features
Improvements