-
Notifications
You must be signed in to change notification settings - Fork 849
WEB-642: Loan originator view with optional fields #3114
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
WEB-642: Loan originator view with optional fields #3114
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Loan Originator Components src/app/organization/loan-originators/edit-loan-originator/edit-loan-originator.component.ts, src/app/organization/loan-originators/view-loan-originator/view-loan-originator.component.html |
Made form property nullable and added optional chaining (?.) for originatorType and channelType property access to handle potentially undefined values safely. Added Back button to view template with router navigation and translation support. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
- WEB-642: Add Loan Originators management #3095: Directly modifies the same EditLoanOriginatorComponent and related templates.
Suggested reviewers
- adamsaghy
🚥 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 summarizes the main change: making the loan originator view handle optional fields (Channel Type and Originator Type) safely through null handling. |
| 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.
Comment @coderabbitai help to get the list of available commands and usage tips.
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: 1
🤖 Fix all issues with AI agents
In
`@src/app/organization/loan-originators/edit-loan-originator/edit-loan-originator.component.ts`:
- Line 38: The field loanOriginatorForm is declared as Nullable but submit()
directly accesses this.loanOriginatorForm.value; change loanOriginatorForm to
use definite assignment (remove | null and add the ! assertion in its
declaration) since the form is initialized in ngOnInit, or alternatively add a
null-guard at the start of submit() (e.g., if (!this.loanOriginatorForm)
return;), ensuring submit() and any other methods (like in submit()) never
access this.loanOriginatorForm when it's null; update the declaration and/or
guard in the submit() method accordingly (references: loanOriginatorForm,
ngOnInit, submit()).
🧹 Nitpick comments (1)
src/app/organization/loan-originators/edit-loan-originator/edit-loan-originator.component.ts (1)
51-58: Consider moving route data subscription out of the constructor.Subscribing to route data in the constructor works but is not the idiomatic Angular pattern. More importantly, this subscription is never unsubscribed, which is a minor memory leak if the component is destroyed and recreated. Consider using
takeUntilDestroyed()(available since Angular 16) or moving tongOnInitwith aDestroyRef.
|
|
||
| /** Loan Originator form. */ | ||
| loanOriginatorForm: UntypedFormGroup; | ||
| loanOriginatorForm: UntypedFormGroup | null = null; |
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.
Nullable loanOriginatorForm accessed without null guard in submit().
Making the form UntypedFormGroup | null on Line 38 but accessing this.loanOriginatorForm.value on Line 104 without a null check is inconsistent and will fail with strict null checks or cause an NPE if submit() is ever called before initialization.
Either add a guard in submit() or use the definite assignment assertion (!) since the form is always initialized in ngOnInit:
Option A (preferred): Use definite assignment instead of nullable
- loanOriginatorForm: UntypedFormGroup | null = null;
+ loanOriginatorForm!: UntypedFormGroup;Option B: Add null guard in submit()
submit() {
+ if (!this.loanOriginatorForm) return;
const loanOriginatorFormData = this.loanOriginatorForm.value;As per coding guidelines, src/app/**: "verify … strict type safety".
Also applies to: 104-104
🤖 Prompt for AI Agents
In
`@src/app/organization/loan-originators/edit-loan-originator/edit-loan-originator.component.ts`
at line 38, The field loanOriginatorForm is declared as Nullable but submit()
directly accesses this.loanOriginatorForm.value; change loanOriginatorForm to
use definite assignment (remove | null and add the ! assertion in its
declaration) since the form is initialized in ngOnInit, or alternatively add a
null-guard at the start of submit() (e.g., if (!this.loanOriginatorForm)
return;), ensuring submit() and any other methods (like in submit()) never
access this.loanOriginatorForm when it's null; update the declaration and/or
guard in the submit() method accordingly (references: loanOriginatorForm,
ngOnInit, submit()).
adamsaghy
left a 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.
LGTM
Description
There was an issue in the Loan Originator view due optional fields: Channel Type and Originator Type
WEB-642
Screenshots, if any
Screen.Recording.2026-02-09.at.10.09.45.a.m.mov
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
New Features
Bug Fixes