WEB-748: Improve email validation and fix translation namespace consi…#3180
WEB-748: Improve email validation and fix translation namespace consi…#3180parth-sharma-10 wants to merge 1 commit intoopenMF:devfrom
Conversation
|
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:
Note
|
| Cohort / File(s) | Summary |
|---|---|
Client component (template + logic) src/app/clients/client-stepper/client-general-step/client-general-step.component.ts, src/app/clients/client-stepper/client-general-step/client-general-step.component.html |
Replaced Validators.email with Validators.pattern(emailRegex) where emailRegex is derived from environment.EXTERNAL_EMAIL_REGEX or a default. Template error check changed from errors?.email to hasError('pattern') && value and the error translation key updated. |
Runtime env assets src/assets/env.js, src/assets/env.template.js |
Added EXTERNAL_EMAIL_REGEX property to the client-side runtime env object (window['env']) initialized to '' with a comment describing optional override. |
Angular environment configs src/environments/environment.ts, src/environments/environment.prod.ts |
Added EXTERNAL_EMAIL_REGEX property to exported environment objects (default/fallback regex present in source). |
Documentation README.md |
Added "External Email Validation Configuration" section documenting EXTERNAL_EMAIL_REGEX, default fallback, example, and Docker Compose snippet. |
Sequence Diagram(s)
sequenceDiagram
participant Browser
participant EnvJS as Runtime env (window['env'])
participant Angular as Build env (environment.ts)
participant Component as ClientForm
Browser->>EnvJS: load env.js (sets EXTERNAL_EMAIL_REGEX)
Browser->>Angular: bootstrap app (app reads environment.EXTERNAL_EMAIL_REGEX or '')
Angular->>Component: call setClientForm() (derive emailRegex = environment.EXTERNAL_EMAIL_REGEX || DEFAULT)
Component->>Component: apply Validators.pattern(emailRegex) to emailAddress
Browser->>Component: user enters email
Component->>Component: validate input -> pattern check
Component->>Browser: template shows error if hasError('pattern') && value
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- WEB-600 feat:External National ID System integration for client creat… #3145 — Modifies the same ClientGeneralStepComponent (.ts and .html) affecting form validation and initialization; likely related.
Suggested reviewers
- IOhacker
🚥 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 accurately summarizes the main changes: improving email validation and fixing translation namespace inconsistency, which are the primary objectives of this 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 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.
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/clients/client-stepper/client-general-step/client-general-step.component.ts`:
- Around line 134-140: The emailAddress FormControl in
ClientGeneralStepComponent was made required (Validators.required) which changes
behavior; remove Validators.required so the control remains optional and keep
only the pattern validator so format is enforced only when a value is present,
or if requirement depends on client type add a conditional required validator
(e.g., add/remove Validators.required at runtime based on client type) instead
of a static required in the control definition.
src/app/clients/client-stepper/client-general-step/client-general-step.component.ts
Show resolved
Hide resolved
IOhacker
left a comment
There was a problem hiding this comment.
The email must be optional
200636e to
a1279ff
Compare
@IOhacker I have updated the PR. The email is now optional. |
src/app/clients/client-stepper/client-general-step/client-general-step.component.html
Outdated
Show resolved
Hide resolved
a1279ff to
6b6ada1
Compare
src/app/clients/client-stepper/client-general-step/client-general-step.component.ts
Outdated
Show resolved
Hide resolved
6b6ada1 to
65be4bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 382-385: The fenced code block containing the environment variable
declaration EXTERNAL_EMAIL_REGEX should include a language specifier to satisfy
markdownlint MD040; update the block opening from ``` to ```bash (or another
appropriate language like sh) so the snippet reads as a shell-style code block
and linter warnings are resolved.
In `@src/assets/env.template.js`:
- Around line 124-125: The new env.template.js entry for
window['env']['EXTERNAL_EMAIL_REGEX'] currently hardcodes an empty string,
preventing runtime substitution; change it to use the same envsubst placeholder
pattern as other keys (e.g., '$EXTERNAL_EMAIL_REGEX') so Docker/nginx envsubst
can inject the environment variable at deploy time and the app can read
EXTERNAL_EMAIL_REGEX from loadedEnv; update the
window['env']['EXTERNAL_EMAIL_REGEX'] assignment to mirror the placeholder style
used by window['env']['EXTERNAL_NATIONAL_ID_REGEX'] and other entries.
In `@src/environments/environment.prod.ts`:
- Line 19: Production environment value EXTERNAL_EMAIL_REGEX is hard-coded to an
empty string; update it to read the runtime-loaded value like the other entries
(use loadedEnv['EXTERNAL_EMAIL_REGEX'] || '') so it follows the bracket-notation
pattern used elsewhere (see externalNationalIdRegex for reference) and becomes
runtime-configurable.
In `@src/environments/environment.ts`:
- Line 22: EXTERNAL_EMAIL_REGEX is hardcoded to an empty string so it never
reads runtime values; update the environment export to read from loadedEnv by
changing EXTERNAL_EMAIL_REGEX to use loadedEnv.EXTERNAL_EMAIL_REGEX || '' so the
runtime-injected window.env value is used (match the pattern used by other keys
like externalNationalIdRegex).
8df2b24 to
f010ffa
Compare
|
Hi @IOhacker, I’ve addressed the comments. Could you please take another look when you get a chance? Thanks! |
alberto-art3ch
left a comment
There was a problem hiding this comment.
Please review my comment
src/app/clients/client-stepper/client-general-step/client-general-step.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/assets/env.js`:
- Around line 114-116: The runtime EXTERNAL_EMAIL_REGEX is never read from
window.env because environment.EXTERNAL_EMAIL_REGEX is hardcoded; update
environment.ts and environment.prod.ts so EXTERNAL_EMAIL_REGEX is assigned from
loadedEnv.EXTERNAL_EMAIL_REGEX with a fallback to the current regex (i.e.,
EXTERNAL_EMAIL_REGEX: loadedEnv.EXTERNAL_EMAIL_REGEX ||
'<current-default-regex>'), update env.template.js (and env.js if used at
build/runtime) to expose the variable like window['env']['EXTERNAL_EMAIL_REGEX']
= '$EXTERNAL_EMAIL_REGEX'; this will allow the component
client-general-step.component.ts (which reads environment.EXTERNAL_EMAIL_REGEX)
to pick up runtime overrides.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a363965a-fcb7-427d-993a-46e31a548104
📒 Files selected for processing (7)
README.mdsrc/app/clients/client-stepper/client-general-step/client-general-step.component.htmlsrc/app/clients/client-stepper/client-general-step/client-general-step.component.tssrc/assets/env.jssrc/assets/env.template.jssrc/environments/environment.prod.tssrc/environments/environment.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/app/clients/client-stepper/client-general-step/client-general-step.component.html
- src/environments/environment.ts
- src/environments/environment.prod.ts
- README.md
- src/assets/env.template.js
- src/app/clients/client-stepper/client-general-step/client-general-step.component.ts
4112116 to
3437368
Compare
alberto-art3ch
left a comment
There was a problem hiding this comment.
Please one last simple comment
src/app/clients/client-stepper/client-general-step/client-general-step.component.ts
Outdated
Show resolved
Hide resolved
- Keep email field optional - Improve format validation using stricter regex - Align translation namespace with errors.* structure Fix: Make email optional and adjust validation logic Move email regex to environment configuration for reuse Make EXTERNAL_EMAIL_REGEX runtime configurable via environment Remove unused DEFAULT_EMAIL_REGEX constant
01e5ce1 to
fadadde
Compare
WEB-748
This PR improves validation reliability for the Email Address field in the Create Client → General Step.
The existing implementation relied on Angular’s default Validators.email, which permits loosely formatted email inputs. Validation has been strengthened to enforce stricter formatting standards and ensure better data quality.
Additionally, the translation namespace for the email validation message has been corrected to align with the application's established errors.* structure, ensuring consistent i18n behavior across the form.
Summary by CodeRabbit