Add input validation support to flow components#518
Open
SajidMannikeri17 wants to merge 1 commit into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (23)
✅ Files skipped from review due to trivial changes (6)
🚧 Files skipped from review as they are similar to previous changes (15)
📝 WalkthroughWalkthroughAdds ValidationRule and FieldError types, client utilities (evaluateValidationRule, buildValidatorFromRules) with i18n fallback keys, updates translation bundles, re-exports new APIs, and wires client/server field-level validation into React flow components to surface per-field errors and touched state. ChangesInput validation flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/react/src/components/presentation/auth/SignIn/v2/BaseSignIn.tsx (1)
64-70: 💤 Low valueJSDoc comment could clarify the server error lifecycle.
The inline documentation states that
fieldErrorsreflects both client-side and server-side errors, but it doesn't explain when server errors are cleared. Consider adding a note that server errors persist until the next submission or until the field is edited (depending on implementation).🤖 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 `@packages/react/src/components/presentation/auth/SignIn/v2/BaseSignIn.tsx` around lines 64 - 70, Update the JSDoc for the fieldErrors block in BaseSignIn to explicitly document the server error lifecycle: state that fieldErrors combines client-side rule evaluation and server-side failures (from data.fieldErrors), that full FieldError[] is on the raw response and mirrored to the serverFieldErrors prop, and add a sentence stating when server-side errors are cleared (e.g., they persist until the next form submission or until the user edits the associated field depending on the existing implementation of BaseSignIn's submit/field-change handlers). Reference the fieldErrors identifier and the serverFieldErrors prop so readers can locate the related state/prop handling.packages/react/src/components/presentation/auth/InviteUser/v2/BaseInviteUser.tsx (1)
435-452: 💤 Low valueComplex nested validation logic can be simplified.
Lines 435-452 have nested conditionals: if required & empty → error, else → email validation, then rule validation (if no error yet). The
if (value && !errors[comp.ref])check at line 443 suggests defensive coding, buterrors[comp.ref]won't be set yet in this context since we're building the errors object.Consider restructuring for clarity:
♻️ Optional refactor for clarity
const value: any = formValues[comp.ref]; if (comp.required && (!value || value.trim() === '')) { errors[comp.ref] = `${comp.label || comp.ref} is required`; + return; // Skip further validation for empty required fields } else { // Email validation if (comp.type === 'EMAIL_INPUT' && value && !/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(value)) { errors[comp.ref] = 'Please enter a valid email address'; + return; // Skip rule validation if email format is invalid } // Evaluate declarative validation rules from meta.components[].validation. - if (value && !errors[comp.ref]) { + if (value) { const ruleValidator = buildValidatorFromRules(comp.validation); if (ruleValidator) { const message = ruleValidator(value); if (message) { errors[comp.ref] = message; } } } }This makes the validation precedence explicit: required → email format → rules.
🤖 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 `@packages/react/src/components/presentation/auth/InviteUser/v2/BaseInviteUser.tsx` around lines 435 - 452, The validation block in BaseInviteUser.tsx is overly nested and uses a redundant guard (checking value && !errors[comp.ref]) which is unnecessary because errors is being built here; refactor the logic in the loop that inspects each comp so it enforces precedence explicitly: first check required and set errors[comp.ref] if missing, then if comp.type === 'EMAIL_INPUT' validate the email format and set errors[comp.ref] if invalid, and finally if value exists and no error yet call buildValidatorFromRules(comp.validation) and apply the returned ruleValidator to set errors[comp.ref] if it returns a message; keep references to comp.ref, comp.label, comp.type, buildValidatorFromRules and the errors object to locate and update the code.packages/react/src/components/presentation/auth/AcceptInvite/v2/BaseAcceptInvite.tsx (1)
491-502: 💤 Low valueValidation logic flow may allow both required and rule errors to be set.
Lines 491-502 check
comp.requiredfirst, then evaluate rules. However, ifcomp.required && !value, the function returns early, but theelse if (value)block at line 493 means rules are only evaluated when there's a value. The structure is correct, but the code would be clearer with an explicit early return after setting the required error.♻️ Optional refactor for clarity
const value: any = formValues[comp.ref]; if (comp.required && (!value || value.trim() === '')) { errors[comp.ref] = t('validations.required.field.error'); - } else if (value) { + return; // Skip rule evaluation for empty required fields + } + if (value) { // Evaluate declarative validation rules from meta.components[].validation. const ruleValidator = buildValidatorFromRules(comp.validation); if (ruleValidator) { const message = ruleValidator(value); if (message) { errors[comp.ref] = t(message); } } }This makes it explicit that we don't evaluate rules for empty required fields.
🤖 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 `@packages/react/src/components/presentation/auth/AcceptInvite/v2/BaseAcceptInvite.tsx` around lines 491 - 502, The required-field branch may be ambiguous; after setting errors[comp.ref] when comp.required && (!value || value.trim() === ''), short-circuit so we don't run buildValidatorFromRules/ ruleValidator for empty values — add an explicit early exit (e.g., continue the loop or return from the enclosing function) immediately after setting the required error so rules are not evaluated for empty required fields and t(...) is only called once.
🤖 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
`@packages/react/src/components/presentation/auth/AcceptInvite/v2/BaseAcceptInvite.tsx`:
- Around line 293-308: The useEffect that reads server FieldError entries from
currentFlow (FieldError) currently merges them into state via setFormErrors(prev
=> ({...prev,...errors})) which leaves stale server errors; change setFormErrors
and setTouchedFields to replace the server error state when responseFieldErrors
exist (e.g., call setFormErrors(errors) and setTouchedFields(touched)) so
server-provided errors do not persist across responses; if you need to preserve
client-side validation errors, track them separately or merge only client-side
errors back into state after replacing server errors.
In
`@packages/react/src/components/presentation/auth/InviteUser/v2/BaseInviteUser.tsx`:
- Around line 284-299: The useEffect that processes server FieldError objects
(referencing currentFlow, FieldError, and useEffect) merges the new server
errors into existing form state via setFormErrors((prev) => ({...prev,
...errors})) and similarly for setTouchedFields, which leaves stale errors;
change the update to replace the server-provided error state instead of merging
(e.g., setFormErrors to set only the new errors and setTouchedFields to set only
the new touched map), ensuring you still preserve non-server local form state
elsewhere if needed.
In `@packages/react/src/components/presentation/auth/SignIn/v2/BaseSignIn.tsx`:
- Around line 385-397: The effect that applies serverFieldErrors should also
clear injected errors when serverFieldErrors becomes null/empty: update the
useEffect around serverFieldErrors to, when serverFieldErrors is falsy or length
=== 0, call setFormErrors({}) and clear touched flags for any previously-set
server error fields by calling setFormTouched(key, false) for each key (you can
read previous keys from current formErrors or track them in a ref) so stale
messages/touched state are removed; keep the existing logic for populating
errors when serverFieldErrors exists and adjust the effect dependencies to
include formErrors or the ref used to track previous keys.
---
Nitpick comments:
In
`@packages/react/src/components/presentation/auth/AcceptInvite/v2/BaseAcceptInvite.tsx`:
- Around line 491-502: The required-field branch may be ambiguous; after setting
errors[comp.ref] when comp.required && (!value || value.trim() === ''),
short-circuit so we don't run buildValidatorFromRules/ ruleValidator for empty
values — add an explicit early exit (e.g., continue the loop or return from the
enclosing function) immediately after setting the required error so rules are
not evaluated for empty required fields and t(...) is only called once.
In
`@packages/react/src/components/presentation/auth/InviteUser/v2/BaseInviteUser.tsx`:
- Around line 435-452: The validation block in BaseInviteUser.tsx is overly
nested and uses a redundant guard (checking value && !errors[comp.ref]) which is
unnecessary because errors is being built here; refactor the logic in the loop
that inspects each comp so it enforces precedence explicitly: first check
required and set errors[comp.ref] if missing, then if comp.type ===
'EMAIL_INPUT' validate the email format and set errors[comp.ref] if invalid, and
finally if value exists and no error yet call
buildValidatorFromRules(comp.validation) and apply the returned ruleValidator to
set errors[comp.ref] if it returns a message; keep references to comp.ref,
comp.label, comp.type, buildValidatorFromRules and the errors object to locate
and update the code.
In `@packages/react/src/components/presentation/auth/SignIn/v2/BaseSignIn.tsx`:
- Around line 64-70: Update the JSDoc for the fieldErrors block in BaseSignIn to
explicitly document the server error lifecycle: state that fieldErrors combines
client-side rule evaluation and server-side failures (from data.fieldErrors),
that full FieldError[] is on the raw response and mirrored to the
serverFieldErrors prop, and add a sentence stating when server-side errors are
cleared (e.g., they persist until the next form submission or until the user
edits the associated field depending on the existing implementation of
BaseSignIn's submit/field-change handlers). Reference the fieldErrors identifier
and the serverFieldErrors prop so readers can locate the related state/prop
handling.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 021eeeb6-b26e-4ba3-9e3a-4e980b34fc2e
📒 Files selected for processing (23)
.changeset/input-validation-flow-inputs.mdpackages/i18n/src/models/i18n.tspackages/i18n/src/translations/en-US.tspackages/i18n/src/translations/fr-FR.tspackages/i18n/src/translations/hi-IN.tspackages/i18n/src/translations/ja-JP.tspackages/i18n/src/translations/pt-BR.tspackages/i18n/src/translations/pt-PT.tspackages/i18n/src/translations/si-LK.tspackages/i18n/src/translations/ta-IN.tspackages/i18n/src/translations/te-IN.tspackages/javascript/src/index.tspackages/javascript/src/models/v2/embedded-flow-v2.tspackages/javascript/src/utils/__tests__/buildValidatorFromRules.test.tspackages/javascript/src/utils/__tests__/evaluateValidationRule.test.tspackages/javascript/src/utils/v2/buildValidatorFromRules.tspackages/javascript/src/utils/v2/evaluateValidationRule.tspackages/react/src/components/presentation/auth/AcceptInvite/v2/BaseAcceptInvite.tsxpackages/react/src/components/presentation/auth/InviteUser/v2/BaseInviteUser.tsxpackages/react/src/components/presentation/auth/Recovery/v2/BaseRecovery.tsxpackages/react/src/components/presentation/auth/SignIn/v2/BaseSignIn.tsxpackages/react/src/components/presentation/auth/SignIn/v2/SignIn.tsxpackages/react/src/components/presentation/auth/SignUp/v2/BaseSignUp.tsx
Signed-off-by: Sajid Mannikeri <[email protected]>
SajidMannikeri17
force-pushed
the
feat/input-validation
branch
from
207a59f to
a78d19b
Compare
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
1 participant
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.
Purpose
SDK support for declarative input validation rules on flow prompts, paired
with the backend feature in asgardeo/thunder#2410.
Today, a flow's prompt inputs can carry
validationrules in the flow definitionand the server returns
data.fieldErrorswhen those rules fail. This PR makesthat feature usable from
@asgardeo/reactconsumers — flow authors put rules oninput components, the SDK runs them client-side for instant feedback, and the SDK
surfaces server-side validation errors into the same render state used for
required-field errors.
Approach
Three layers, mirroring the design in the discussion:
@asgardeo/javascript— new framework-agnostic primitives:ValidationRule,ValidationRuleType,FieldErrortypes.evaluateValidationRuleandbuildValidatorFromRulesutilities.EmbeddedFlowComponent.validationandEmbeddedFlowResponseData.fieldErrorsextensions on the v2 flow models.@asgardeo/react— wires the primitives into the auth components:BaseSignIn,BaseSignUp,BaseAcceptInvite,BaseInviteUser,BaseRecovery— client-side rule evaluation viauseForm, server-sidefieldErrorsprojection viauseEffect.SignInRenderProps.fieldErrors— exposes server-side validation errors to render-prop consumers (custom UI flows).@asgardeo/i18n— default validation message keys (validation.pattern.invalid,validation.minLength.invalid,validation.maxLength.invalid) added to all locale bundles.Backward compatibility
All new fields are optional. Existing consumers without
validationrules behaveexactly as before. No package version bumps required at the consumer level; a
consumer pinned to the previous SDK version can upgrade without code changes.
Tests
evaluateValidationRule,5 for
buildValidatorFromRules).Related Issues
Related PRs
Checklist
Security checks
Summary by CodeRabbit
New Features
Tests
Documentation