Skip to content

NO-ISSUE: Localise validation messages#3379

Merged
openshift-merge-bot[bot] merged 2 commits intoopenshift-assisted:masterfrom
jgyselov:validation_schema_untangling
Feb 8, 2026
Merged

NO-ISSUE: Localise validation messages#3379
openshift-merge-bot[bot] merged 2 commits intoopenshift-assisted:masterfrom
jgyselov:validation_schema_untangling

Conversation

@jgyselov
Copy link
Contributor

@jgyselov jgyselov commented Feb 3, 2026

  • Split the libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts file into multiple
  • Add translations for validations used in /cim

Summary by CodeRabbit

  • New Features

    • Validation across forms now uses localized messages (networking, CIDR/IP, DNS, MAC, host/network naming, SSH keys, proxies, pull secret, NTP, file uploads).
  • Bug Fixes

    • Improved and consistent validation behavior: stricter CIDR/IP/host-prefix checks, uniqueness enforcement (DNS/IP/SSH keys), proxy requirement handling, file type/size and VIP/address constraints, and SNO host-selection length enforcement.
  • Chores

    • Added extensive English translation entries for validation and guidance messages.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 3, 2026
@openshift-ci-robot
Copy link

@jgyselov: This pull request explicitly references no jira issue.

Details

In response to this:

  • Split the libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts file into multiple
  • Add translations for validations used in /cim

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.

@openshift-ci openshift-ci bot requested review from jkilzi and rawagner February 3, 2026 15:34
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 3, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 3, 2026

📝 Walkthrough

Walkthrough

Centralizes and modularizes form validation into new i18n-aware validators under libs/.../validationSchemas, removes the legacy monolithic formik validation bundle, threads the translation function (t) through validators and Formik usages, updates imports/exports, extends English translations, and adjusts related tests.

Changes

Cohort / File(s) Summary
Localization updates
libs/locales/lib/en/translation.json
Adds many ai: translation keys covering CIDR/IP/subnet, DNS, proxy, SSH key, file, MAC, host/network naming, and other validation messages.
Legacy bundle removed & UI barrel updated
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (deleted), libs/ui-lib/lib/common/components/ui/formik/index.tsx
Deletes the monolithic validation file and removes its re-exports from the UI/formik barrel.
New validation library (barrel + re-exports)
libs/ui-lib/lib/common/validationSchemas/index.tsx, libs/ui-lib/lib/common/index.ts, libs/ui-lib/lib/common/validationSchemas/*
Adds modular validation modules (address, clusterDetails, hostname, networking, ntp, proxy, pullSecret, ssh, regexes, utils) and a barrel; re-exports from common index.
Address & IP validators
.../validationSchemas/addressValidation.tsx
Implements IP/CIDR/mac/VIP/subnet/host-prefix validations with localized messages and cross-field checks (overlap, subnet membership, broadcast/network checks).
Networking & dual-stack validators
.../validationSchemas/networkingValidation.tsx
Adds cluster/machine/service network schemas, dual-stack/version-aware checks, IPv4-only enforcement, and host-prefix integration; accepts t.
Proxy, no-proxy, NTP, SSH, pull-secret
.../validationSchemas/proxyValidation.tsx, .../ntpValidation.tsx, .../sshValidation.tsx, .../pullSecretValidation.tsx
Adds i18n-capable validators: HTTP(S) proxy parsing & pairing, no-proxy list parsing, NTP sources list validation, SSH single/multi-key validation, and pull-secret JSON structure check.
Regexes & utilities
.../validationSchemas/regexes.tsx, .../validationSchemas/utils.tsx, libs/ui-lib/lib/common/utils.ts
Introduces shared regex constants, IP/DNS helpers, alwaysRequired helper, path index extractor; converts some message constants to t-based functions (e.g., getMaxFileSizeMessage/getIncorrectFileTypeMessage).
Component integrations (CIM/OCM/OCM forms & modals, common components)
libs/ui-lib/lib/cim/..., libs/ui-lib/lib/ocm/..., libs/ui-lib/lib/common/components/...
Threads t into many validationSchema constructors and Formik usages (networking, discovery image, infraEnv, modals, static IP views, manifests, cluster details), updates imports to new validation modules, and adapts validationSchema signatures.
Tests & minor assertions
libs/ui-lib/lib/common/_validationSchemas.test.ts, libs/ui-lib-tests/cypress/...
Updates tests to initialize schemas with t, adjust expected localized messages, and fixes a manifest test assertion spacing.

Sequence Diagram(s)

sequenceDiagram
  participant User as UI Component (Formik)
  participant T as i18n (t)
  participant V as Validation module (validationSchemas/*)
  participant Store as Form State / Submit

  User->>T: useTranslation() -> t
  User->>V: build validationSchema(values, t)
  V-->>User: Yup schema (with localized messages)
  User->>Store: submit(values)
  User->>V: schema.validate(values)
  V-->>User: validation result / errors (localized via t)
  alt valid
    User->>Store: proceed submit
  else invalid
    User-->>User: show localized errors
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

lgtm

Suggested reviewers

  • rawagner
  • jkilzi
  • ElayAharoni

Poem

I hop through schemas, soft and spry,
Threading "t" so errors speak and sigh.
CIDRs, keys, and proxies dance in tune,
Validations sing beneath the moon.
A rabbit's cheer — small paws, big boon! 🐇

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding localization support for validation messages. It accurately reflects the primary objective of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 3, 2026
@jgyselov jgyselov requested a review from LiorSoffer February 3, 2026 15:37
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
libs/ui-lib/lib/cim/components/Agent/BMCForm.tsx (1)

159-166: ⚠️ Potential issue | 🟠 Major

Use functional when instead of object form

Biome flags object literals with then in when. Replace with a functional callback:

Refactor
- macAddress: macAddressValidationSchema(t).when('name', {
-   is: (name: string) => !!name,
-   then: (schema) => schema.required(t('ai:MAC has to be specified')),
- }),
+ macAddress: macAddressValidationSchema(t).when('name', ([name], schema) =>
+   name ? schema.required(t('ai:MAC has to be specified')) : schema,
+ ),

- name: Yup.string().when('macAddress', {
-   is: (mac: string) => !!mac,
-   then: (schema) => schema.required(t('ai:Name has to be specified')),
- }),
+ name: Yup.string().when('macAddress', ([mac], schema) =>
+   mac ? schema.required(t('ai:Name has to be specified')) : schema,
+ ),
libs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/YamlView/yamlViewValidationSchema.tsx (1)

23-75: ⚠️ Potential issue | 🟡 Minor

Localize validation messages
Hardcoded strings in required, max, and matches should be wrapped in t(...) for translation.

Suggested diff
-  .required('Required field')
+  .required(t('ai:Required field'))
...
-        .max(15, 'Interface name must be 15 characters at most.')
+        .max(15, t('ai:Interface name must be 15 characters at most.'))
...
-        .matches(/^\S+$/, 'Interface name can not contain spaces.')
+        .matches(/^\S+$/, t('ai:Interface name can not contain spaces.'))
libs/ui-lib/lib/common/validationSchemas/_validationSchemas.test.ts (1)

376-383: ⚠️ Potential issue | 🟡 Minor

Add trailing periods to expected error strings in tests
In _validationSchemas.test.ts (lines 381, 423, 439), update expected messages to include trailing periods:

Fix expected strings
- 'First network has to be IPv4 subnet'
+ 'First network has to be IPv4 subnet.'
...
- 'ai:Maximum number of {{field}} subnets in dual stack is 2'
+ 'ai:Maximum number of {{field}} subnets in dual stack is 2.'
🤖 Fix all issues with AI agents
In `@libs/locales/lib/en/translation.json`:
- Line 386: Update the translation entry with key "ai:File type is not
supported. File type must be yaml, yml ,json , yaml.patch. or yml.patch." to
remove extra spaces and the stray period so both the key and value read with
consistent punctuation and spacing (e.g., "File type is not supported. File type
must be yaml, yml, json, yaml.patch or yml.patch."). Ensure you edit the exact
translation key and its value (the string in
libs/locales/lib/en/translation.json) to use the corrected spacing and
punctuation.

In `@libs/ui-lib/lib/cim/components/modals/EditSSHKeyModal.tsx`:
- Around line 24-28: The validationSchema currently passes a hardcoded string to
.required(); change it to use the translation function by replacing the literal
with t('ai:An SSH key is required to debug hosts as they register.') so the
sshPublicKey field error is localized; update the validationSchema (and any call
sites using TFunction t) to call
sshPublicKeyValidationSchema(t).required(t('ai:An SSH key is required to debug
hosts as they register.')) ensuring the message uses the ai namespace
translation key.

In `@libs/ui-lib/lib/common/validationSchemas/addressValidation.tsx`:
- Around line 110-114: Update the validation schemas to accept a translation
function and use localized messages: modify hostSubnetValidationSchema to accept
(t: TFunction) and change the notOneOf message to t('ai:Host subnet must be
selected.'), update ipBlockValidationSchema to replace .required('A value is
required.') with .required(t('ai:Required field')), and update
bmcAddressValidationSchema to change .required() to .required(t('ai:Required
field')); ensure the function signatures and callers are adjusted to pass a
TFunction (t) where these schemas are constructed or exported.

In `@libs/ui-lib/lib/common/validationSchemas/clusterDetailsValidation.tsx`:
- Around line 54-75: The Yup .when usage with an object containing
then/otherwise triggers the linter; refactor the conditional to the callback
form by replacing .when('useRedHatDnsService', { is: ..., then: ..., otherwise:
... }) with .when('useRedHatDnsService', (useRedHatDnsService, schema) => { if
(useRedHatDnsService === true) return schema.test('is-name-unique',
clusterNameValidationMessagesList.NOT_UNIQUE, (value?: string) => { const
clusterFullName = `${value || ''}.${baseDnsDomain}`; return !value ||
!usedClusterNames.includes(clusterFullName); }); return
schema.test('is-name-unique', clusterNameValidationMessagesList.NOT_UNIQUE,
(value?: string) => { return validateUniqueName ? !value ||
!usedClusterNames.includes(value) : true; }); }); so the logic around
useRedHatDnsService, clusterFullName, usedClusterNames and validateUniqueName
remains identical but uses the callback signature to satisfy the noThenProperty
rule.

In `@libs/ui-lib/lib/common/validationSchemas/constants.tsx`:
- Around line 69-70: Update the user-facing message returned by
getIncorrectFileTypeMessage to correct spacing and punctuation and list allowed
file types clearly; replace the current string in getIncorrectFileTypeMessage
(which takes t: TFunction) with a properly formatted message such as "File type
is not supported. Supported file types: yaml, yml, json, yaml.patch, yml.patch."
so the translation key receives the cleaned, human-readable text.

In `@libs/ui-lib/lib/common/validationSchemas/networkingValidation.tsx`:
- Around line 47-110: dualStackValidationSchema currently uses hardcoded test
messages—replace them with localized messages using the provided t parameter
(wrap the three test message strings: the 'dual-stack-ipv4' message, the
`Provided ${field} subnets must be unique.` message, and the `When two ${field}
are provided, one must be IPv4 and the other IPv6.` message) so they call t(...)
and include the field where needed; also change IPv4ValidationSchema from a
plain Yup.array() export into a function that accepts t: TFunction (e.g. export
const IPv4ValidationSchema = (t: TFunction) => ...) and use t('ai:All network
subnets must be IPv4.') as the test error message while still invoking
allSubnetsIPv4(values); update any call sites that import IPv4ValidationSchema
to call it with t.

In `@libs/ui-lib/lib/common/validationSchemas/proxyValidation.tsx`:
- Around line 19-37: The current Yup test registered as 'http-proxy-validation'
on the validation constant rejects https URLs for both fields; update the test
function (the arrow passed into Yup.string().test) to inspect the Yup test
context (this.path or this.parent) to distinguish the field name: when the path
ends with or equals 'httpProxy' require the value to start with 'http://' only,
and when the path is 'httpsProxy' allow values that start with either 'http://'
or 'https://'; keep the empty-value shortcut, still validate by constructing new
URL(value) inside a try/catch, and adjust the validation message text
accordingly so clients see the correct allowed schemes for each field.

In `@libs/ui-lib/lib/common/validationSchemas/utils.tsx`:
- Around line 5-12: The isIPorDN function currently calls match on a possibly
undefined value and uses match instead of test; update isIPorDN to immediately
return false if value is falsy, then use dnsRegex.test(value) to check
DNS_NAME_REGEX, and otherwise return the result of Address4.isValid(value) ||
Address6.isValid(value); reference the isIPorDN function and DNS_NAME_REGEX,
Address4.isValid, Address6.isValid when making the change.

In
`@libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx`:
- Around line 86-116: getValidationSchema uses hardcoded messages for pullSecret
and rendezvousIp (and the IP max length message); replace those literal strings
with calls to the i18n function t so validation messages are localized.
Specifically, in getValidationSchema change the pullSecret .required('Required
field') to .required(t('some.key.pullSecretRequired')) and replace the
rendezvousIp .max(45, 'IP address must be at most 45 characters') and the test
message 'Not a valid IP address' with t('some.key.ipTooLong', { count: 45 }) and
t('some.key.invalidIp') (or appropriate keys you maintain), keeping
ipValidationSchema(t) as-is; ensure the chosen translation keys exist in your
locales.

@jgyselov jgyselov force-pushed the validation_schema_untangling branch from 605f770 to f51ec5b Compare February 3, 2026 16:21
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)

84-96: ⚠️ Potential issue | 🟡 Minor

Update hostSubnetValidationSchema and machineNetworksValidationSchema to support i18n.

machineNetworksValidationSchema is a constant (unlike clusterNetworksValidationSchema and serviceNetworkValidationSchema, which are functions accepting t). The underlying issue is that hostSubnetValidationSchema contains a hardcoded error message 'Host subnet must be selected.' that is not translated.

Refactor hostSubnetValidationSchema to accept the translation function t and use it for the error message, then convert machineNetworksValidationSchema to a function so it can pass t to hostSubnetValidationSchema, matching the pattern used by the other network validation schemas.

🤖 Fix all issues with AI agents
In `@libs/ui-lib/lib/common/utils.ts`:
- Around line 62-65: Two call sites in customManifestsValidationSchema.tsx are
still passing the function reference getMaxFileSizeMessage instead of calling it
with the translation function; update both .test('not-big-file', ...) usages to
call getMaxFileSizeMessage(t) so the translation function is provided (i.e.,
change .test('not-big-file', getMaxFileSizeMessage, validateFileSize) to
.test('not-big-file', getMaxFileSizeMessage(t), validateFileSize)), following
the working example in yamlViewValidationSchema.tsx and keeping validateFileSize
unchanged.

In `@libs/ui-lib/lib/common/validationSchemas/networkingValidation.tsx`:
- Around line 55-62: The validator function that checks primary network CIDR
(the arrow function using openshiftVersion and
isMajorMinorVersionEqualOrGreater) can throw when values is an empty array
because it accesses values?.[0].cidr without guarding the element; fix it by
first reading the primary CIDR with a null-safe access (e.g., const primaryCidr
= values?.[0]?.cidr) and then use primaryCidr in the existing checks (use
!!primaryCidr && isCIDR.v4(...) / isCIDR.v6(...) as appropriate), ensuring you
reference the same openshiftVersion and isMajorMinorVersionEqualOrGreater logic
and isCIDR checks.

In `@libs/ui-lib/lib/common/validationSchemas/ntpValidation.tsx`:
- Around line 11-17: The validator currently treats whitespace-only comma
strings as non-empty and fails because trimCommaSeparatedList(value) can be
empty; update the anonymous validator so after calling
trimCommaSeparatedList(value) you check if the resulting string is empty (or
split and filter out empty/whitespace-only entries) and return true for that
case, otherwise split on ',' and validate each non-empty entry with isIPorDN;
reference trimCommaSeparatedList and isIPorDN when locating the change.
- Around line 23-28: The uniqueness validator currently doesn't handle the case
where trimCommaSeparatedList(value) returns an empty string; update the arrow
function validator to first call trimCommaSeparatedList(value), return true if
that trimmed string is empty, then split that trimmed string by ',' and compare
arr.length to new Set(arr).size to validate uniqueness (reference the existing
trimCommaSeparatedList utility and the inline (value?: string) => { ... }
validator).

In
`@libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/components/customManifestsValidationSchema.tsx`:
- Around line 69-91: Replace hardcoded validation strings in the Yup schema with
localized calls using t('ai:...'): change 'Required' -> t('ai:required'),
'Number of characters must be 1-255' ->
t('ai:number-of-characters-must-be-1-255') and wrap INCORRECT_FILENAME and
UNIQUE_FOLDER_FILENAME with t(...) (e.g., t(INCORRECT_FILENAME) or if those are
keys replace with t('ai:incorrect-filename') and t('ai:unique-folder-filename')
per codebase convention). Also fix the getMaxFileSizeMessage usage by invoking
it with the i18n function: replace getMaxFileSizeMessage with
getMaxFileSizeMessage(t) in both places (the test call on line with then and the
otherwise branch), keeping existing tests validateFileSize and validateFileType
and preserving uniqueManifestFiles(...) usage.
- Around line 77-88: The conditional in the Yup schema for manifestYaml uses
filename.includes('patch') which misdetects names like "dispatch.yaml"; update
the when predicate on manifestYaml (the function that checks 'filename') to
guard for undefined and use a suffix check such as filename?.endsWith('.patch')
(or a callback form that receives the field value and returns
filename?.endsWith('.patch')) so only files ending with ".patch" skip the
file-content validation in the manifestYaml schema.

In
`@libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmDiscoveryImageConfigForm.tsx`:
- Around line 18-21: The imported type Proxy from
'@openshift-assisted/types/assisted-installer-service' shadows the global Proxy
which Biome flags; change the import to alias it (e.g., import { Proxy as
ServiceProxy }) and update all usages in this file (e.g., types, function
signatures, variables) to the new alias (ServiceProxy) so the linter no longer
conflicts with the global Proxy.

In
`@libs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/YamlView/yamlViewValidationSchema.tsx`:
- Around line 23-27: The required message in networkYamlValidationSchema is
hardcoded; change the Yup.string().required('Required field') call to use the
i18n function t instead (e.g., Yup.string().required(t('validation.required'))
or the project's existing required-message key) so the required error message is
translated; update the required invocation in the networkYamlValidationSchema
function accordingly.
🧹 Nitpick comments (4)
libs/ui-lib/lib/common/components/clusterConfiguration/DiscoveryImageConfigForm.tsx (1)

21-26: Consider simplifying the AlertFormikError import path.

The import path ../../../common/components/ui goes up three levels to lib/ and back down to common/components/ui. Since this file is already located in lib/common/components/clusterConfiguration/, a simpler path would be ../ui.

That said, if this longer path is intentional for consistency with the broader module reorganization in this PR, feel free to keep it as-is.

✨ Optional simplification
-import { AlertFormikError } from '../../../common/components/ui';
+import { AlertFormikError } from '../ui';
libs/ui-lib/lib/common/components/clusterWizard/clusterDetailsValidation.ts (1)

100-126: Consider extracting common validation fields.

The name and baseDnsDomain validation is duplicated in both branches. You could extract these to reduce repetition:

♻️ Optional refactor to reduce duplication
 export const getClusterDetailsValidationSchema = ({
   usedClusterNames,
   pullSecretSet,
   validateUniqueName,
   isOcm,
   t,
 }: {
   usedClusterNames: string[];
   pullSecretSet?: boolean;
   validateUniqueName?: boolean;
   isOcm?: boolean;
   t: TFunction;
 }) =>
   Yup.lazy((values: { baseDnsDomain: string; isSNODevPreview: boolean }) => {
+    const commonFields = {
+      name: nameValidationSchema(
+        t,
+        usedClusterNames,
+        values.baseDnsDomain,
+        validateUniqueName,
+        isOcm,
+      ),
+      baseDnsDomain: isOcm
+        ? baseDomainValidationSchema(t).required(t('ai:Required field'))
+        : dnsNameValidationSchema(t).required(t('ai:Required field')),
+    };
+
     if (pullSecretSet) {
-      return Yup.object({
-        name: nameValidationSchema(
-          t,
-          usedClusterNames,
-          values.baseDnsDomain,
-          validateUniqueName,
-          isOcm,
-        ),
-        baseDnsDomain: isOcm
-          ? baseDomainValidationSchema(t).required(t('ai:Required field'))
-          : dnsNameValidationSchema(t).required(t('ai:Required field')),
-      });
+      return Yup.object(commonFields);
     }
-    return Yup.object({
-      name: nameValidationSchema(
-        t,
-        usedClusterNames,
-        values.baseDnsDomain,
-        validateUniqueName,
-        isOcm,
-      ),
-      baseDnsDomain: isOcm
-        ? baseDomainValidationSchema(t).required(t('ai:Required field'))
-        : dnsNameValidationSchema(t).required(t('ai:Required field')),
+    return Yup.object({
+      ...commonFields,
       pullSecret: pullSecretValidationSchema(t).required(t('ai:Required field')),
       diskEncryptionTangServers: Yup.array().when('diskEncryptionMode', {
         // ... rest unchanged
       }),
     });
   });
libs/ui-lib/lib/common/validationSchemas/clusterDetailsValidation.tsx (1)

91-96: Consider using a simpler regex for single-character validation.

The code matches individual characters (trimmed[0], trimmed[trimmed.length - 1]) against NAME_START_END_REGEX, which is designed for full strings (/^[a-z0-9](.*[a-z0-9])?$/). While this works functionally, it's semantically unclear. A simpler pattern like /^[a-z0-9]$/ would be more explicit for single-character checks.

libs/ui-lib/lib/cim/components/Agent/BMCForm.tsx (1)

159-166: Biome noThenProperty lint warning.

Lines 161 and 165 use Yup's .when() with object syntax containing then properties, which triggers the Biome lint rule. This is standard Yup API usage and the localization changes are correct. The lint refactoring could be addressed in a follow-up if desired.

@LiorSoffer
Copy link
Contributor

/retest

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 5, 2026
@jgyselov jgyselov force-pushed the validation_schema_untangling branch from f51ec5b to 96cd061 Compare February 5, 2026 08:30
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 5, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
libs/ui-lib/lib/cim/components/Agent/BMCForm.tsx (1)

154-166: ⚠️ Potential issue | 🟠 Major

Fix Biome noThenProperty by switching Yup.when to callback form.

The then property in the options object conflicts with Biome's noThenProperty rule. Yup 1.4.0 supports the callback form as an alternative, which avoids the property name conflict while preserving behavior.

♻️ Proposed fix
-          macAddress: macAddressValidationSchema(t).when('name', {
-            is: (name: string) => !!name,
-            then: (schema) => schema.required(t('ai:MAC has to be specified')),
-          }),
-          name: Yup.string().when('macAddress', {
-            is: (name: string) => !!name,
-            then: () => Yup.string().required(t('ai:Name has to be specified')),
-          }),
+          macAddress: macAddressValidationSchema(t).when('name', (name, schema) =>
+            name ? schema.required(t('ai:MAC has to be specified')) : schema,
+          ),
+          name: Yup.string().when('macAddress', (mac, schema) =>
+            mac ? schema.required(t('ai:Name has to be specified')) : schema,
+          ),
🤖 Fix all issues with AI agents
In `@libs/ui-lib/lib/cim/components/ClusterDeployment/use-networking-formik.ts`:
- Around line 56-77: The Yup .when calls for clusterNetworks and serviceNetworks
use the object form with then/otherwise; change them to the callback form and
fix the parameter signature to accept the first arg as an array destructure: use
when('stackType', ([stackType], schema) => stackType === IPV4_STACK ?
schema.concat(IPv4ValidationSchema) :
schema.concat(dualStackValidationSchema(t('ai:cluster network') or t('ai:service
network'), t, openshiftVersion))) so that clusterNetworksValidationSchema(t) and
serviceNetworkValidationSchema(t) are concatenated correctly with
IPv4ValidationSchema or dualStackValidationSchema based on IPV4_STACK; update
both clusterNetworks and serviceNetworks when usages accordingly.

In `@libs/ui-lib/lib/common/utils.ts`:
- Around line 62-65: The displayed max file size is using MAX_FILE_SIZE_BYTES
while validation allows MAX_FILE_SIZE_BYTES * MAX_FILE_SIZE_OFFSET_FACTOR;
update getMaxFileSizeMessage to compute and display the real enforced limit
(multiply MAX_FILE_SIZE_BYTES by MAX_FILE_SIZE_OFFSET_FACTOR or reuse the same
computed limit used in validateFileSize) so the message matches validation
(refer to getMaxFileSizeMessage, validateFileSize, MAX_FILE_SIZE_BYTES, and
MAX_FILE_SIZE_OFFSET_FACTOR).

In
`@libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts`:
- Around line 97-107: The when() calls for clusterNetworks and serviceNetworks
must use Yup 1.4.0's callback signature where the first parameter is an array of
values; update clusterNetworksValidationSchema(...).when and
serviceNetworkValidationSchema(...).when to use (values, schema) and access the
stackType via values[0] (e.g., values[0] === IPV4_STACK ?
schema.concat(IPv4ValidationSchema) :
schema.concat(dualStackValidationSchema(..., t, openshiftVersion))). Ensure you
replace the old (stackType, schema) parameters with (values, schema) in both
clusterNetworks and serviceNetworks handlers and keep the same concat logic and
references to IPV4_STACK, IPv4ValidationSchema, dualStackValidationSchema, t,
and openshiftVersion.
🧹 Nitpick comments (3)
libs/ui-lib/lib/cim/components/modals/CimConfiguration/CimConfigurationModal.tsx (1)

103-111: Localization looks good; consider using interpolation for maintainability.

The i18n approach for validation messages is correct and aligns with the PR objectives. One optional improvement: the minimum values are hardcoded in translation strings ("1Gi", "10Gi"). If the constants MIN_DB_VOL_SIZE, MIN_FS_VOL_SIZE, or MIN_IMG_VOL_SIZE change, the messages would become out of sync.

♻️ Optional: Use interpolation with constants
   dbVolSize: Yup.number()
-    .min(MIN_DB_VOL_SIZE, t('ai:Minimal value is 1Gi'))
+    .min(MIN_DB_VOL_SIZE, t('ai:Minimal value is {{min}}Gi', { min: MIN_DB_VOL_SIZE }))
     .required(t('ai:Required field')),
   fsVolSize: Yup.number()
-    .min(MIN_FS_VOL_SIZE, t('ai:Minimal value is 1Gi'))
+    .min(MIN_FS_VOL_SIZE, t('ai:Minimal value is {{min}}Gi', { min: MIN_FS_VOL_SIZE }))
     .required(t('ai:Required field')),
   imgVolSize: Yup.number()
-    .min(MIN_IMG_VOL_SIZE, t('ai:Minimal value is 10Gi'))
+    .min(MIN_IMG_VOL_SIZE, t('ai:Minimal value is {{min}}Gi', { min: MIN_IMG_VOL_SIZE }))
     .required(t('ai:Required field')),

This would require updating the locale file to use:

"ai:Minimal value is {{min}}Gi": "Minimal value is {{min}}Gi"
libs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/YamlView/YamlView.tsx (1)

10-18: OCM validation schemas should stay non-localized unless requirements changed.

This OCM path now threads t into yamlViewValidationSchema, which introduces translation keys and the translation workflow. If OCM still doesn’t require localized validation messages, consider keeping this schema non‑i18n or confirm the requirement before proceeding.

Based on learnings: Do not localize validation messages in OpenShift Console Management (OCM) components under libs/ui-lib/lib/ocm. Hardcoded strings are acceptable in these files since translations are not required for these validations. When adding/updating validation messages in this path, keep strings stable and avoid triggering translation workflows for these specific components.

libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)

2-2: OCM validation schemas should stay non-localized unless requirements changed.

OCM validations typically remain hardcoded; threading t here introduces translation keys and the translation workflow. Consider keeping these schemas non‑i18n or confirm the requirement before proceeding.

Based on learnings: Do not localize validation messages in OpenShift Console Management (OCM) components under libs/ui-lib/lib/ocm. Hardcoded strings are acceptable in these files since translations are not required for these validations. When adding/updating validation messages in this path, keep strings stable and avoid triggering translation workflows for these specific components.

Also applies to: 73-96

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 8, 2026
@openshift-ci
Copy link

openshift-ci bot commented Feb 8, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jgyselov, LiorSoffer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit cda6370 into openshift-assisted:master Feb 8, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants