WEB-968: Memory leaks, bugs and build hygiene#3618
Closed
alberto-art3ch wants to merge 1 commit into
Closed
Conversation
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Build and package configuration cleanup angular.json, package.json |
Cypress architect targets removed from angular.json project definitions; Cypress dependency and related npm scripts removed from package.json. |
testRigor rule and testcase removal e2e/testRigor/rules/*, e2e/testRigor/testcases/*, e2e/testRigor/README.md, e2e/testRigor/run_testrigor_tests.sh |
All testRigor interaction rules (navigation, account/client/organization creation, login/logout), test cases (create/delete/edit workflows), and supporting documentation and runner scripts removed. |
Angular Component Lifecycle and RxJS Management
| Layer / File(s) | Summary |
|---|---|
Accounting module subscription teardown src/app/accounting/provisioning-entries/view-provisioning-entry/view-provisioning-entry.component.ts, src/app/accounting/search-journal-entry/search-journal-entry.component.ts |
Injects DestroyRef and applies takeUntilDestroyed to all filter valueChanges pipelines and table reload subscriptions in both components for automatic unsubscription on component destruction. |
Listing and audit view subscription teardown src/app/centers/centers.component.ts, src/app/groups/groups.component.ts, src/app/system/audit-trails/audit-trails.component.ts |
Adds DestroyRef injection and pipes all filter valueChanges, sort, and paginator subscriptions through takeUntilDestroyed to prevent memory leaks across three listing/audit components. |
Runtime Behavior and Component Configuration Updates
| Layer / File(s) | Summary |
|---|---|
Service null-safety and environment logic src/app/loans/loans.service.ts, src/environments/environment.ts |
Loan service now uses nullish coalescing (?? '[]') before JSON.parse to handle missing localStorage values; environment.preloadClients now evaluates to false only when explicitly set to false instead of defaulting to true. |
Share products component OnPush change detection src/app/products/share-products/dividends-share-product/dividends.components.ts |
Component now uses ChangeDetectionStrategy.OnPush with markForCheck() invocation after assigning dividend data from route to ensure rendering updates. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- openMF/web-app#3629: Overlapping Angular component migration to DestroyRef + takeUntilDestroyed RxJS subscription management pattern across multiple components.
Suggested reviewers
- IOhacker
- adamsaghy
- gkbishnoi07
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'WEB-968: Memory leaks, bugs and build hygiene' accurately captures the main technical cleanup objectives of the PR, covering memory fixes, bug corrections, and build improvements. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/environments/environment.ts (1)
65-65: ⚡ Quick winConsider checking for string
'false'to match other environment flag patterns.The fix correctly preserves boolean
false, but other environment flags in this file check for both string'false'and booleanfalse(see lines 91, 98). Ifwindow.env['preloadClients']could be the string'false', it will be treated as truthy with the current logic.♻️ Proposed fix to match established pattern
- preloadClients: loadedEnv['preloadClients'] !== false, + preloadClients: loadedEnv['preloadClients'] !== 'false' && loadedEnv['preloadClients'] !== false,This aligns with the defensive pattern used for
mifosInterbankTransfersEnabled(line 91) andmifosRemittanceEnabled(line 98).🤖 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 `@src/environments/environment.ts` at line 65, The preloadClients assignment currently only checks for boolean false; update the check on loadedEnv['preloadClients'] so it treats the string 'false' the same as boolean false (consistent with how mifosInterbankTransfersEnabled and mifosRemittanceEnabled are handled): ensure the expression excludes both 'false' (string) and false (boolean) when setting the preloadClients property.
🤖 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 `@src/app/loans/loans.service.ts`:
- Around line 827-829: getLoanDisbursementDetailsData currently parses
localStorage.getItem('disbursementData') which can yield null
(JSON.parse('null') -> null) but the method signature expects
DisbursementData[]; change the method to check the stored value and return an
empty array when missing or parsed result is null/invalid. Locate
getLoanDisbursementDetailsData and implement a guard that reads the raw string,
returns [] if raw is null/empty, otherwise JSON.parse and validate/cast to
DisbursementData[] (or default to []) before returning so callers always receive
an array.
In
`@src/app/products/share-products/dividends-share-product/dividends.components.ts`:
- Line 10: The component uses ChangeDetectionStrategy.OnPush but mutates
this.dividendData inside the route subscription in DividendsComponent so the
view won't update; inject ChangeDetectorRef (via constructor or inject()) into
the component and after you update dividendData in the route subscription call
cdr.markForCheck() (or cdr.detectChanges()) to trigger change detection;
alternatively refactor dividendData to an Observable and expose it to the
template via async pipe instead of imperative mutation.
---
Nitpick comments:
In `@src/environments/environment.ts`:
- Line 65: The preloadClients assignment currently only checks for boolean
false; update the check on loadedEnv['preloadClients'] so it treats the string
'false' the same as boolean false (consistent with how
mifosInterbankTransfersEnabled and mifosRemittanceEnabled are handled): ensure
the expression excludes both 'false' (string) and false (boolean) when setting
the preloadClients property.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: df7307e1-40e6-4a22-a5a7-969bd1153c07
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.jsonand included by**/*
📒 Files selected for processing (54)
angular.jsoncypress/e2e/audit-trails.cy.tscypress/e2e/spec.cy.tscypress/plugins/index.tscypress/support/commands.tscypress/support/e2e.tscypress/tsconfig.jsone2e/.eslintrc.jsone2e/cypress.jsone2e/cypress/plugins/index.tse2e/cypress/support/commands.tse2e/cypress/support/index.tse2e/cypress/tsconfig.jsone2e/testRigor/README.mde2e/testRigor/rules/close-popup.txte2e/testRigor/rules/create-account.txte2e/testRigor/rules/create-admin-user.txte2e/testRigor/rules/create-client.txte2e/testRigor/rules/create-organization.txte2e/testRigor/rules/delete-admin-user.txte2e/testRigor/rules/delete-client.txte2e/testRigor/rules/log-in-and-validate.txte2e/testRigor/rules/logout.txte2e/testRigor/rules/navigate-to-accounting-creation.txte2e/testRigor/rules/navigate-to-admin-users-management.txte2e/testRigor/rules/navigate-to-clients.txte2e/testRigor/rules/navigate-to-offices.txte2e/testRigor/rules/pick-birthdate.txte2e/testRigor/rules/pick-today-date.txte2e/testRigor/rules/validate-login-page.txte2e/testRigor/run_testrigor_tests.she2e/testRigor/testcases/create-account.txte2e/testRigor/testcases/create-admin-user.txte2e/testRigor/testcases/create-client.txte2e/testRigor/testcases/create-journal-entries.txte2e/testRigor/testcases/create-new-office.txte2e/testRigor/testcases/delete-admin-user.txte2e/testRigor/testcases/delete-client.txte2e/testRigor/testcases/edit-mandatory-fields-of-organization.txte2e/testRigor/testcases/edit-organization.txte2e/testRigor/testcases/invalid-login.txte2e/testRigor/testcases/login.txte2e/testRigor/testcases/logout.txte2e/tsconfig.e2e.jsone2e/tsconfig.jsonpackage.jsonsrc/app/accounting/provisioning-entries/view-provisioning-entry/view-provisioning-entry.component.tssrc/app/accounting/search-journal-entry/search-journal-entry.component.tssrc/app/centers/centers.component.tssrc/app/groups/groups.component.tssrc/app/loans/loans.service.tssrc/app/products/share-products/dividends-share-product/dividends.components.tssrc/app/system/audit-trails/audit-trails.component.tssrc/environments/environment.ts
💤 Files with no reviewable changes (46)
- e2e/testRigor/rules/navigate-to-accounting-creation.txt
- e2e/testRigor/testcases/create-admin-user.txt
- e2e/testRigor/rules/navigate-to-offices.txt
- e2e/testRigor/rules/close-popup.txt
- e2e/testRigor/rules/create-organization.txt
- e2e/testRigor/rules/navigate-to-admin-users-management.txt
- e2e/testRigor/rules/logout.txt
- e2e/testRigor/testcases/create-client.txt
- e2e/cypress/support/index.ts
- e2e/.eslintrc.json
- e2e/testRigor/README.md
- cypress/support/commands.ts
- e2e/testRigor/testcases/edit-mandatory-fields-of-organization.txt
- e2e/cypress/tsconfig.json
- e2e/tsconfig.json
- cypress/tsconfig.json
- e2e/tsconfig.e2e.json
- e2e/testRigor/testcases/edit-organization.txt
- e2e/testRigor/rules/delete-client.txt
- e2e/testRigor/testcases/logout.txt
- e2e/testRigor/testcases/delete-admin-user.txt
- e2e/testRigor/rules/delete-admin-user.txt
- e2e/cypress/plugins/index.ts
- e2e/testRigor/testcases/login.txt
- e2e/testRigor/rules/pick-today-date.txt
- e2e/testRigor/rules/validate-login-page.txt
- e2e/testRigor/testcases/create-account.txt
- e2e/cypress.json
- e2e/testRigor/testcases/invalid-login.txt
- e2e/testRigor/rules/log-in-and-validate.txt
- e2e/testRigor/testcases/create-new-office.txt
- e2e/testRigor/rules/create-client.txt
- e2e/testRigor/rules/navigate-to-clients.txt
- e2e/testRigor/rules/create-account.txt
- cypress/support/e2e.ts
- cypress/e2e/spec.cy.ts
- e2e/testRigor/testcases/delete-client.txt
- e2e/testRigor/rules/pick-birthdate.txt
- e2e/testRigor/run_testrigor_tests.sh
- cypress/e2e/audit-trails.cy.ts
- e2e/testRigor/rules/create-admin-user.txt
- e2e/testRigor/testcases/create-journal-entries.txt
- cypress/plugins/index.ts
- e2e/cypress/support/commands.ts
- angular.json
- package.json
alberto-art3ch
force-pushed
the
WEB-968/tech-debt-memory-leaks-bugs-build-hygiene
branch
2 times, most recently
from
b2e8ad4 to
4fd3640
Compare
adamsaghy
reviewed
Jun 2, 2026
Collaborator
There was a problem hiding this comment.
Is this okay to remove this and the test files?
Collaborator
Author
There was a problem hiding this comment.
In few words yes, because we are consolidation of the E2E stack into a single framework, in this case playwright, even we have the GA .github/workflows/playwright.yml
devvaansh
suggested changes
Jun 4, 2026
Member
There was a problem hiding this comment.
Cypress coverage is being deleted here, i think this can be migrated to playwright. what do you think about this, should i raise a follow up pr for the migration ?
Collaborator
Author
There was a problem hiding this comment.
Yes, we could start creating a set of PRs to cover the E2E Test cases
alberto-art3ch
force-pushed
the
WEB-968/tech-debt-memory-leaks-bugs-build-hygiene
branch
from
4fd3640 to
ec28a48
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@package.json`:
- Line 95: The `@cypress/schematic` dependency on line 95 of package.json is not
referenced anywhere in the codebase beyond package.json itself and appears to be
unused. Either remove the `"`@cypress/schematic`": "^2.5.2"` line from the
devDependencies section in package.json if Cypress is no longer part of the
project workflow, or if it is still needed, add a comment or documentation
explaining why it must be retained.
In `@src/app/loans/loans.service.ts`:
- Around line 852-853: getLoanDisbursementDetailsData currently calls JSON.parse
on localStorage.getItem and can throw on malformed JSON or return a non-array
value; update getLoanDisbursementDetailsData to wrap JSON.parse in a try/catch,
validate that the parsed value is an Array of DisbursementData (Array.isArray),
and return an empty array fallback if parsing fails or the value is not an
array; preserve typing by ensuring the function always returns
DisbursementData[] and optionally remove invalid stored value from localStorage
or ignore it silently.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c10f2c3-6df4-432d-b006-541abf0d04a6
⛔ Files ignored due to path filters (2)
.github/workflows/test.ymlis excluded by!.github/**and included by**/*package-lock.jsonis excluded by!**/package-lock.jsonand included by**/*
📒 Files selected for processing (54)
angular.jsoncypress/e2e/audit-trails.cy.tscypress/e2e/spec.cy.tscypress/plugins/index.tscypress/support/commands.tscypress/support/e2e.tscypress/tsconfig.jsone2e/.eslintrc.jsone2e/cypress.jsone2e/cypress/plugins/index.tse2e/cypress/support/commands.tse2e/cypress/support/index.tse2e/cypress/tsconfig.jsone2e/testRigor/README.mde2e/testRigor/rules/close-popup.txte2e/testRigor/rules/create-account.txte2e/testRigor/rules/create-admin-user.txte2e/testRigor/rules/create-client.txte2e/testRigor/rules/create-organization.txte2e/testRigor/rules/delete-admin-user.txte2e/testRigor/rules/delete-client.txte2e/testRigor/rules/log-in-and-validate.txte2e/testRigor/rules/logout.txte2e/testRigor/rules/navigate-to-accounting-creation.txte2e/testRigor/rules/navigate-to-admin-users-management.txte2e/testRigor/rules/navigate-to-clients.txte2e/testRigor/rules/navigate-to-offices.txte2e/testRigor/rules/pick-birthdate.txte2e/testRigor/rules/pick-today-date.txte2e/testRigor/rules/validate-login-page.txte2e/testRigor/run_testrigor_tests.she2e/testRigor/testcases/create-account.txte2e/testRigor/testcases/create-admin-user.txte2e/testRigor/testcases/create-client.txte2e/testRigor/testcases/create-journal-entries.txte2e/testRigor/testcases/create-new-office.txte2e/testRigor/testcases/delete-admin-user.txte2e/testRigor/testcases/delete-client.txte2e/testRigor/testcases/edit-mandatory-fields-of-organization.txte2e/testRigor/testcases/edit-organization.txte2e/testRigor/testcases/invalid-login.txte2e/testRigor/testcases/login.txte2e/testRigor/testcases/logout.txte2e/tsconfig.e2e.jsone2e/tsconfig.jsonpackage.jsonsrc/app/accounting/provisioning-entries/view-provisioning-entry/view-provisioning-entry.component.tssrc/app/accounting/search-journal-entry/search-journal-entry.component.tssrc/app/centers/centers.component.tssrc/app/groups/groups.component.tssrc/app/loans/loans.service.tssrc/app/products/share-products/dividends-share-product/dividends.components.tssrc/app/system/audit-trails/audit-trails.component.tssrc/environments/environment.ts
💤 Files with no reviewable changes (45)
- e2e/testRigor/README.md
- e2e/testRigor/rules/create-admin-user.txt
- e2e/testRigor/rules/pick-birthdate.txt
- e2e/testRigor/testcases/create-new-office.txt
- cypress/support/e2e.ts
- e2e/testRigor/testcases/logout.txt
- e2e/testRigor/testcases/login.txt
- e2e/testRigor/rules/create-client.txt
- e2e/testRigor/rules/validate-login-page.txt
- e2e/tsconfig.json
- e2e/.eslintrc.json
- cypress/support/commands.ts
- e2e/testRigor/testcases/delete-client.txt
- e2e/testRigor/testcases/edit-mandatory-fields-of-organization.txt
- cypress/plugins/index.ts
- e2e/testRigor/rules/create-account.txt
- e2e/testRigor/rules/logout.txt
- cypress/e2e/spec.cy.ts
- e2e/testRigor/testcases/invalid-login.txt
- e2e/tsconfig.e2e.json
- e2e/testRigor/rules/close-popup.txt
- e2e/testRigor/testcases/create-account.txt
- e2e/testRigor/rules/create-organization.txt
- e2e/testRigor/rules/delete-client.txt
- e2e/testRigor/testcases/edit-organization.txt
- e2e/cypress.json
- e2e/testRigor/rules/navigate-to-accounting-creation.txt
- e2e/testRigor/testcases/create-admin-user.txt
- e2e/testRigor/rules/log-in-and-validate.txt
- e2e/testRigor/testcases/create-journal-entries.txt
- e2e/testRigor/testcases/create-client.txt
- e2e/testRigor/rules/navigate-to-offices.txt
- e2e/cypress/support/index.ts
- e2e/cypress/support/commands.ts
- e2e/testRigor/rules/navigate-to-admin-users-management.txt
- e2e/testRigor/rules/pick-today-date.txt
- e2e/cypress/plugins/index.ts
- e2e/testRigor/rules/delete-admin-user.txt
- cypress/e2e/audit-trails.cy.ts
- e2e/testRigor/testcases/delete-admin-user.txt
- cypress/tsconfig.json
- e2e/testRigor/rules/navigate-to-clients.txt
- e2e/cypress/tsconfig.json
- e2e/testRigor/run_testrigor_tests.sh
- angular.json
🚧 Files skipped from review as they are similar to previous changes (6)
- src/app/groups/groups.component.ts
- src/environments/environment.ts
- src/app/centers/centers.component.ts
- src/app/accounting/provisioning-entries/view-provisioning-entry/view-provisioning-entry.component.ts
- src/app/accounting/search-journal-entry/search-journal-entry.component.ts
- src/app/system/audit-trails/audit-trails.component.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 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 `@package.json`:
- Line 95: The `@cypress/schematic` dependency on line 95 of package.json is not
referenced anywhere in the codebase beyond package.json itself and appears to be
unused. Either remove the `"`@cypress/schematic`": "^2.5.2"` line from the
devDependencies section in package.json if Cypress is no longer part of the
project workflow, or if it is still needed, add a comment or documentation
explaining why it must be retained.
In `@src/app/loans/loans.service.ts`:
- Around line 852-853: getLoanDisbursementDetailsData currently calls JSON.parse
on localStorage.getItem and can throw on malformed JSON or return a non-array
value; update getLoanDisbursementDetailsData to wrap JSON.parse in a try/catch,
validate that the parsed value is an Array of DisbursementData (Array.isArray),
and return an empty array fallback if parsing fails or the value is not an
array; preserve typing by ensuring the function always returns
DisbursementData[] and optionally remove invalid stored value from localStorage
or ignore it silently.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c10f2c3-6df4-432d-b006-541abf0d04a6
⛔ Files ignored due to path filters (2)
.github/workflows/test.ymlis excluded by!.github/**and included by**/*package-lock.jsonis excluded by!**/package-lock.jsonand included by**/*
📒 Files selected for processing (54)
angular.jsoncypress/e2e/audit-trails.cy.tscypress/e2e/spec.cy.tscypress/plugins/index.tscypress/support/commands.tscypress/support/e2e.tscypress/tsconfig.jsone2e/.eslintrc.jsone2e/cypress.jsone2e/cypress/plugins/index.tse2e/cypress/support/commands.tse2e/cypress/support/index.tse2e/cypress/tsconfig.jsone2e/testRigor/README.mde2e/testRigor/rules/close-popup.txte2e/testRigor/rules/create-account.txte2e/testRigor/rules/create-admin-user.txte2e/testRigor/rules/create-client.txte2e/testRigor/rules/create-organization.txte2e/testRigor/rules/delete-admin-user.txte2e/testRigor/rules/delete-client.txte2e/testRigor/rules/log-in-and-validate.txte2e/testRigor/rules/logout.txte2e/testRigor/rules/navigate-to-accounting-creation.txte2e/testRigor/rules/navigate-to-admin-users-management.txte2e/testRigor/rules/navigate-to-clients.txte2e/testRigor/rules/navigate-to-offices.txte2e/testRigor/rules/pick-birthdate.txte2e/testRigor/rules/pick-today-date.txte2e/testRigor/rules/validate-login-page.txte2e/testRigor/run_testrigor_tests.she2e/testRigor/testcases/create-account.txte2e/testRigor/testcases/create-admin-user.txte2e/testRigor/testcases/create-client.txte2e/testRigor/testcases/create-journal-entries.txte2e/testRigor/testcases/create-new-office.txte2e/testRigor/testcases/delete-admin-user.txte2e/testRigor/testcases/delete-client.txte2e/testRigor/testcases/edit-mandatory-fields-of-organization.txte2e/testRigor/testcases/edit-organization.txte2e/testRigor/testcases/invalid-login.txte2e/testRigor/testcases/login.txte2e/testRigor/testcases/logout.txte2e/tsconfig.e2e.jsone2e/tsconfig.jsonpackage.jsonsrc/app/accounting/provisioning-entries/view-provisioning-entry/view-provisioning-entry.component.tssrc/app/accounting/search-journal-entry/search-journal-entry.component.tssrc/app/centers/centers.component.tssrc/app/groups/groups.component.tssrc/app/loans/loans.service.tssrc/app/products/share-products/dividends-share-product/dividends.components.tssrc/app/system/audit-trails/audit-trails.component.tssrc/environments/environment.ts
💤 Files with no reviewable changes (45)
- e2e/testRigor/README.md
- e2e/testRigor/rules/create-admin-user.txt
- e2e/testRigor/rules/pick-birthdate.txt
- e2e/testRigor/testcases/create-new-office.txt
- cypress/support/e2e.ts
- e2e/testRigor/testcases/logout.txt
- e2e/testRigor/testcases/login.txt
- e2e/testRigor/rules/create-client.txt
- e2e/testRigor/rules/validate-login-page.txt
- e2e/tsconfig.json
- e2e/.eslintrc.json
- cypress/support/commands.ts
- e2e/testRigor/testcases/delete-client.txt
- e2e/testRigor/testcases/edit-mandatory-fields-of-organization.txt
- cypress/plugins/index.ts
- e2e/testRigor/rules/create-account.txt
- e2e/testRigor/rules/logout.txt
- cypress/e2e/spec.cy.ts
- e2e/testRigor/testcases/invalid-login.txt
- e2e/tsconfig.e2e.json
- e2e/testRigor/rules/close-popup.txt
- e2e/testRigor/testcases/create-account.txt
- e2e/testRigor/rules/create-organization.txt
- e2e/testRigor/rules/delete-client.txt
- e2e/testRigor/testcases/edit-organization.txt
- e2e/cypress.json
- e2e/testRigor/rules/navigate-to-accounting-creation.txt
- e2e/testRigor/testcases/create-admin-user.txt
- e2e/testRigor/rules/log-in-and-validate.txt
- e2e/testRigor/testcases/create-journal-entries.txt
- e2e/testRigor/testcases/create-client.txt
- e2e/testRigor/rules/navigate-to-offices.txt
- e2e/cypress/support/index.ts
- e2e/cypress/support/commands.ts
- e2e/testRigor/rules/navigate-to-admin-users-management.txt
- e2e/testRigor/rules/pick-today-date.txt
- e2e/cypress/plugins/index.ts
- e2e/testRigor/rules/delete-admin-user.txt
- cypress/e2e/audit-trails.cy.ts
- e2e/testRigor/testcases/delete-admin-user.txt
- cypress/tsconfig.json
- e2e/testRigor/rules/navigate-to-clients.txt
- e2e/cypress/tsconfig.json
- e2e/testRigor/run_testrigor_tests.sh
- angular.json
🚧 Files skipped from review as they are similar to previous changes (6)
- src/app/groups/groups.component.ts
- src/environments/environment.ts
- src/app/centers/centers.component.ts
- src/app/accounting/provisioning-entries/view-provisioning-entry/view-provisioning-entry.component.ts
- src/app/accounting/search-journal-entry/search-journal-entry.component.ts
- src/app/system/audit-trails/audit-trails.component.ts
🛑 Comments failed to post (2)
package.json (1)
95-95:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Verify whether `@cypress/schematic` is still referenced by schematics/workflows rg -n --hidden -g '!.git' -P '`@cypress/schematic`|cypress' angular.json package.json cypress.config.ts .github AGENTS.mdRepository: openMF/web-app
Length of output: 504
🏁 Script executed:
#!/bin/bash set -euo pipefail # 1) Exact usage of `@cypress/schematic` rg -n --hidden -g '!.git' '`@cypress/schematic`' . # 2) Cypress scripts usage (runtime/CLI) in package.json and CI rg -n --hidden -g '!.git' -S '"cypress' package.json rg -n --hidden -g '!.git' -S 'cypress (run|open)|npx cypress|cypress/|`@cypress/schematic`' package.json .github angular.json # 3) Print package.json around the dependency line for context # (line numbers will vary; use a small window around the match) match_line=$(rg -n '"`@cypress/schematic`"' package.json | cut -d: -f1 | head -n1) start=$((match_line-20)) end=$((match_line+20)) sed -n "${start},${end}p" package.jsonRepository: openMF/web-app
Length of output: 524
🏁 Script executed:
#!/bin/bash set -euo pipefail # 1) See if Cypress is referenced by scripts/CI rg -n --hidden -g '!.git' -S 'cypress' package.json .github angular.json cypress.config.ts AGENTS.md # 2) See if `@cypress/schematic` is referenced by commands (not just as a dependency) rg -n --hidden -g '!.git' '`@cypress/schematic`' .github angular.json package.json cypress.config.ts AGENTS.md # 3) Print package.json scripts section for context python3 - <<'PY' import json p='package.json' with open(p,'r',encoding='utf-8') as f: data=json.load(f) print(json.dumps(data.get('scripts',{}), indent=2)) PYRepository: openMF/web-app
Length of output: 2580
🏁 Script executed:
#!/bin/bash set -euo pipefail match_line=$(rg -n '"`@cypress/schematic`"' package.json | cut -d: -f1 | head -n1) start=$((match_line-15)) end=$((match_line+15)) sed -n "${start},${end}p" package.jsonRepository: openMF/web-app
Length of output: 1154
Remove or justify leftover
@cypress/schematicdependency.
@cypress/schematicis still inpackage.jsondevDependencies (package.json:95) but has no in-repo references beyondpackage.json/package-lock.json; if Cypress is no longer actively used (no Cypress npm scripts/CI wiring), drop it or document why it’s still needed.🤖 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 `@package.json` at line 95, The `@cypress/schematic` dependency on line 95 of package.json is not referenced anywhere in the codebase beyond package.json itself and appears to be unused. Either remove the `"`@cypress/schematic`": "^2.5.2"` line from the devDependencies section in package.json if Cypress is no longer part of the project workflow, or if it is still needed, add a comment or documentation explaining why it must be retained.src/app/loans/loans.service.ts (1)
852-853:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winHarden disbursement localStorage parsing to always return an array.
This still throws on malformed JSON and can return non-array JSON, which breaks downstream
.forEachconsumers expectingDisbursementData[].Proposed fix
getLoanDisbursementDetailsData(): DisbursementData[] { - return JSON.parse(localStorage.getItem('disbursementData') ?? '[]'); + const raw = localStorage.getItem('disbursementData'); + if (!raw) { + return []; + } + try { + const parsed = JSON.parse(raw); + return Array.isArray(parsed) ? (parsed as DisbursementData[]) : []; + } catch { + return []; + } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.getLoanDisbursementDetailsData(): DisbursementData[] { const raw = localStorage.getItem('disbursementData'); if (!raw) { return []; } try { const parsed = JSON.parse(raw); return Array.isArray(parsed) ? (parsed as DisbursementData[]) : []; } catch { return []; } }🤖 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 `@src/app/loans/loans.service.ts` around lines 852 - 853, getLoanDisbursementDetailsData currently calls JSON.parse on localStorage.getItem and can throw on malformed JSON or return a non-array value; update getLoanDisbursementDetailsData to wrap JSON.parse in a try/catch, validate that the parsed value is an Array of DisbursementData (Array.isArray), and return an empty array fallback if parsing fails or the value is not an array; preserve typing by ensuring the function always returns DisbursementData[] and optionally remove invalid stored value from localStorage or ignore it silently.
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
3 participants
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.
Description
A technical cleanup covering the next areas:
preloadClientsalways truthy inenvironment.tsJSON.parse(null)ingetLoanDisbursementDetailsDatahonobasic-ftptarChangeDetectionStrategy.OnPushcoverage across all componentsRelated issues and discussion
WEB-968
Screenshots, if any
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
Release Notes
Bug Fixes
Performance
Chores