WEB-425 Field validation for required field missing in "Bulk Loan Reassignment" page

[JaySoni1]

Changes Made :-

Added required field validation and error message for the "Office" field on the Bulk Loan Reassignment page to ensure consistent user feedback for missing required fields.

WEB-425

Before :-

After :-

Summary by CodeRabbit

• New Features
  • Office selection is now required when reassigning bulk loans; validation errors display if omitted.
  • Assignment Date now shows a helpful placeholder and defaults to the current business date.
  • Submit button is explicitly set as a form submit and remains disabled until the form is valid.

[coderabbitai]

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)

Validation error: Unrecognized key(s) in object: 'pre_merge_checks'

⚙️ Configuration instructions

• Please see the configuration documentation for more information.
• You can also validate your configuration using the online YAML validator.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Walkthrough

Added a required officeId FormControl to the bulk loan reassignment form, updated the template to bind the selector to that control and show validation errors, set the assignment date default to business date, added a placeholder for the date input, and marked the submit button as type="submit".

Changes

Cohort / File(s)	Summary
Bulk Loan Reassignment component src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.ts, src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.html	Added required officeId FormControl to the form group and included it in submit payload; changed assignment date default to settingsService.businessDate; bound officeId in template mat-select and added mat-error display; added placeholder to assignment date input; set submit button type="submit" and preserved disabled state tied to form validity and permissions.

Sequence Diagram(s)

(omitted — changes are limited to form fields, validation and template bindings; no new control-flow interactions warranting a sequence diagram)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Check form initialization and validation for officeId in ...component.ts.
• Verify template binding, mat-error conditions, and type="submit" in ...component.html.
• Confirm assignment date default change aligns with other business-date usages.

Suggested reviewers

• IOhacker
• gkbishnoi07

Pre-merge checks and finishing touches

✅ 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 change: adding field validation for a required field (Office) in the Bulk Loan Reassignment component, which aligns with the actual modifications in both HTML and TypeScript files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)

src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.html (1)

132-148: CRITICAL: Invalid nested button elements.

A submit button is nested inside another button element, creating invalid HTML. Browsers will not render nested buttons correctly, and this will break form submission functionality.

Apply this diff to fix the structure by removing the outer button wrapper and keeping only the submit button:

-        <button
-          mat-raised-button
-          color="primary"
-          [disabled]="!bulkLoanForm.valid"
-          *mifosxHasPermission="'BULKREASSIGN_LOAN'"
-        >
-          {{ 'labels.buttons.Submit' | translate }}
-          <button
-            type="submit"
-            mat-raised-button
-            color="primary"
-            [disabled]="!bulkLoanForm.valid"
-            *mifosxHasPermission="'BULKREASSIGN_LOAN'"
-          >
-            {{ 'labels.buttons.Submit' | translate }}
-          </button>
-        </button></mat-card-actions
-      >
+        <button
+          type="submit"
+          mat-raised-button
+          color="primary"
+          [disabled]="!bulkLoanForm.valid"
+          *mifosxHasPermission="'BULKREASSIGN_LOAN'"
+        >
+          {{ 'labels.buttons.Submit' | translate }}
+        </button>
+      </mat-card-actions>

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 36fb5f2 and f3d0bec.

📒 Files selected for processing (3)

• src/app/configuration-wizard/configuration-wizard.component.ts (0 hunks)
• src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.html (3 hunks)
• src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.ts (1 hunks)

💤 Files with no reviewable changes (1)

• src/app/configuration-wizard/configuration-wizard.component.ts

🧰 Additional context used

📓 Path-based instructions (1)

src/app/**

⚙️ CodeRabbit configuration file

src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.

Files:

• src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.ts
• src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.html

🔇 Additional comments (2)

src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.ts (1)

82-85: LGTM! Office field validation added correctly.

The required validator on officeId properly enforces validation for the Office field, aligning with the PR objective to provide consistent user feedback.

src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.html (1)

8-8: LGTM! Form control binding added correctly.

The formControlName="officeId" binding properly connects the Office select field to the form control defined in the component.

[alberto-art3ch]

Please review my comments

[alberto-art3ch]

There is a way to set this date as the current business date in the backend, Has It other constraints? Like max or min value

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.ts (2)

102-107: Consider cleaning up subscriptions to prevent memory leaks.

The service subscriptions at lines 102, 116, and 156 lack cleanup logic. While not introduced by this PR, adding takeUntil or storing subscriptions for cleanup in ngOnDestroy would align with the coding guidelines for clean observable patterns and prevent potential memory leaks.

As per coding guidelines.

Also applies to: 116-119, 156-158

---

81-94: Consider migrating to typed forms for better type safety.

The form uses UntypedFormGroup and UntypedFormBuilder. Since Angular 19 supports typed forms, consider upgrading to FormGroup<{...}> and FormBuilder for compile-time type checking of form values, which aligns with the coding guidelines' emphasis on strict type safety.

As per coding guidelines.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9b950bd and 9dccbf7.

📒 Files selected for processing (2)

• src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.html (3 hunks)
• src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.ts (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.html

🧰 Additional context used

📓 Path-based instructions (1)

src/app/**

⚙️ CodeRabbit configuration file

src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.

Files:

• src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.ts

🔇 Additional comments (2)

src/app/organization/bulk-loan-reassignmnet/bulk-loan-reassignmnet.component.ts (2)

82-85: LGTM! Office validation successfully added.

The officeId form control is correctly added with required validation, addressing the PR objective. The implementation follows the same pattern as other required fields in the form.

---

87-87: Verified: businessDate initialization approach is consistent across the application.

The settingsService.businessDate is retrieved from localStorage via convertToDate(), creating a dependency on localStorage.getItem('mifosXServerDate') being populated before component initialization. However, this pattern is used consistently across 130+ components in the codebase and represents an application-wide architectural decision rather than an issue specific to this change.

The change from new Date() to this.settingsService.businessDate is appropriate and aligns with business logic (using the configured business date instead of system time). If localStorage initialization fails, it affects the entire application, not just this component.

[alberto-art3ch]

LGTM

[JaySoni1]

@alberto-art3ch Thank You for the review