web-app icon indicating copy to clipboard operation
web-app copied to clipboard

WEB-95 Fix problems with GLIM application: saved total, "Moratorium", "Overdue Charges""

Open JaySoni1 opened this issue 4 months ago • 12 comments

Changes Made :-

-Fixed GLIM loan application total calculation and ensured correct parent/child payload structure. -Made Moratorium and Overdue Charges fields configurable and editable in the UI and API payload.

WEB :- 95

Summary by CodeRabbit

  • New Features

    • Edit overdue charge amounts directly in loan accounts.
    • Explicit inclusion of application/group identifiers for account creation flows.
  • Bug Fixes

    • Safer rendering with guarded access to charge fields.
    • More consistent amount/currency display and improved charge handling.
  • Accessibility

    • Added aria-labels to many inputs and controls.
    • Standardized button types across steps and dialogs.
  • Refactor

    • Simplified charge payloads and removed deprecated fields for cleaner data.
  • Chores

    • Added i18n provider and updated UI imports for module consistency.

✏️ Tip: You can customize this high-level summary in your review settings.

JaySoni1 avatar Oct 23 '25 18:10 JaySoni1

[!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

Refactors GLIM account payload construction and request flow (new setData signature, simplified charges, submit removal, notify signature change); adds overdue-charge inline amount editing; improves accessibility (ARIA labels and explicit button types) across loan stepper templates; adjusts material imports and registers I18nService in AppModule.

Changes

Cohort / File(s) Summary
GLIM Account Creation
src/app/loans/glim-account/create-glim-account/create-glim-account.component.ts
setData() signature changed to setData(applicationId, client, totalLoan, isFirst, isLast); adds applicationId/groupId, conditional isParentAccount/lastApplication; charges simplified to { chargeId, amount, currency }; removes moratorium/deprecated fields; submit() removed; buildRequestData() now generates applicationId, computes first/last flags and calls setData(...) with reference; notify() signature changed to notify(body, data) and appends serialized data to error messages.
Loan Charges Management (TS + Template)
src/app/loans/loans-account-stepper/loans-account-charges-step/loans-account-charges-step.component.ts,
src/app/loans/loans-account-stepper/loans-account-charges-step/loans-account-charges-step.component.html
Template: buttons updated with type="button" and aria-label, currency displayed conditionally; Cancel button rendering guarded by *ngIf="loanId" (merge-conflict resolved to use button type). TS: added editOverdueChargeAmount(charge) opening a FormDialog to edit amount, replaces matching overdue charge in overDueChargesDataSource, reassigns array and marks form non-pristine.
Loan Terms Accessibility
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html
Added aria-label to many inputs and explicit type="button" on action controls (add/remove/stepper); no logic changes.
Loan Preview Rendering
src/app/loans/loans-account-stepper/loans-account-preview-step/loans-account-preview-step.component.html
Replaced charge.chargeTimeType.value usages with optional chaining charge.chargeTimeType?.value and guarded date-related checks.
Authentication & Core
src/app/login/login-form/login-form.component.ts,
src/app/app.module.ts
Added MatIconButton and MatCheckboxModule to login component imports; registered I18nService in AppModule providers.

Sequence Diagram(s)

(omitted — changes do not introduce a multi-component sequential flow requiring a diagram)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • openMF/web-app#2690 — Similar changes to how charge objects are constructed and normalized in loan request payloads.
  • openMF/web-app#2822 — Overlapping modifications to charges mapping and charge ID handling in CreateGlimAccountComponent.
  • openMF/web-app#2662 — Directly related edits to CreateGlimAccountComponent request-building, notify signature, and submit flow.

Suggested reviewers

  • IOhacker
  • alberto-art3ch
  • steinwinde

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 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: fixing GLIM application issues including saved total calculation, Moratorium field handling, and Overdue Charges functionality.
✨ 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

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

coderabbitai[bot] avatar Oct 23 '25 18:10 coderabbitai[bot]

Hi @JaySoni1 , I'm very surprised that you were even able to test your changes - have you tested them? I downloaded your PR and tried to test it. However, when I chose "Applications" - "GLIM Application" from the hamburger menu, I got a Javascript error in the console:

ERROR NullInjectorError: R3InjectorError(Standalone[_CreateGlimAccountComponent])[_I18nService -> _I18nService -> _I18nService -> _I18nService]: NullInjectorError: No provider for _I18nService! at NullInjector.get (core.mjs:1600:21) at R3Injector.get (core.mjs:2130:27) at R3Injector.get (core.mjs:2130:27) at R3Injector.get (core.mjs:2130:27) at R3Injector.get (core.mjs:2130:27) at ChainedInjector.get (core.mjs:16828:32) at lookupTokenUsingModuleInjector (core.mjs:4955:31) at getOrCreateInjectable (core.mjs:5001:10) at Module.ɵɵdirectiveInject (core.mjs:16875:17) at NodeInjectorFactory.CreateGlimAccountComponent_Factory [as factory] (create-glim-account.component.ts:44:40)

Only after making changes in i18n.service.ts, I was able to open the form for GLIM application. Have you not met this problem?

When I finally get to the "Active Client Members" tab/screen, I'm getting a series of Javascript errors

ERROR TypeError: Cannot read properties of undefined (reading 'displaySymbol') at LoansAccountPreviewStepComponent_div_216_td_14_Template (loans-account-preview-step.component.html:265:11) at executeTemplate (core.mjs:12074:5) at refreshView (core.mjs:13675:7) at detectChangesInView (core.mjs:13887:5) at detectChangesInViewIfAttached (core.mjs:13849:3) at detectChangesInEmbeddedViews (core.mjs:13807:7) at refreshView (core.mjs:13703:5) at detectChangesInView (core.mjs:13887:5) at detectChangesInViewIfAttached (core.mjs:13849:3) at detectChangesInComponent (core.mjs:13837:3)

In the "loansAccount.charges" array, the charge seems to lack a currency, which causes the error, because the "displaySymbol" is retrieved from the currency. I'm not 100% sure this problem is related to your changes, but it looks to me as if it does. Because I'm not seeing this Javascript error when testing based on the code in the dev branch (but I see other Javascript errors). From what I can tell, it is the above Javascript error that causes the submit/next button to be disabled. I can't save a GLIM application, based on your PR.

steinwinde avatar Oct 29 '25 13:10 steinwinde

Hi @steinwinde, Thank you for your feedback. I have now addressed the issues you mentioned. Yes, I did test the GLIM application flow earlier and did not encounter the erros at that time. However, after your review, I re-tested the code and was able to find the errors . I have now resolved the errors , please review .

JaySoni1 avatar Oct 29 '25 22:10 JaySoni1

@JaySoni1 I did some digging into the GLIM application process on the backend. My understanding is as follows:

  • You have to generate your own applicationId which serves to link the different loan applications into one GLIM.
  • You have to make one request for each child loan (m_loan table). Each request should use the same applicationId.
  • In the first request, you pass isParentAccount: true to create the parent account (glim_accounts table).
  • In the last request, you pass lastApplication: true to close the GLIM (accepting_child will change to 0 in the glim_accounts table).
  • Since we are using the /batches endpoint, we have to make sure that the requests are processed in order (or at least the first and last request should not be processed out of order). To do this, you can make each request dependent on the one before it, using the reference field. It is not entirely clear to me if this is absolutely necessary, but it seems like a safe choice.

All of this is reflected in the following code change in create-glim-account.component.ts:

  setData(applicationId: number, client: any, totalLoan: number, isFirst: boolean, isLast: boolean): any {
    const locale = this.settingsService.language.code;
    const dateFormat = this.settingsService.dateFormat;
    // const monthDayFormat = 'dd MMMM';
    const data = {
      ...this.loansAccount,
      charges: this.loansAccount.charges.map((charge: any) => ({
        chargeId: charge.id,
        amount: charge.amount
      })),
      clientId: client.id,
      totalLoan: totalLoan,
      loanType: 'glim',
      applicationId: applicationId,
      lastApplication: isLast,
      amortizationType: 1,
      principal: client.principal,
      syncDisbursementWithMeeting: false,
      expectedDisbursementDate: this.dateUtils.formatDate(this.loansAccount.expectedDisbursementDate, dateFormat),
      submittedOnDate: this.dateUtils.formatDate(this.loansAccount.submittedOnDate, dateFormat),
      dateFormat,
      // monthDayFormat,
      locale
    };
    data.groupId = this.loansAccountTemplate.group.id;

    if (isFirst) {
      // Note: Only add isParentAccount = true for the first request.
      // Setting isParentAccount = false for subsequent requests is not handled correctly by Fineract.
      data.isParentAccount = true;
    }

    delete data.principalAmount;
    // TODO: 2025-03-17: Apparently (?!) unsupported for GLIM
    delete data.allowPartialPeriodInterestCalculation;
    delete data.multiDisburseLoan;
    delete data.isFloatingInterestRate;

    return JSON.stringify(data);
  }

  /** Request Body Data */
  buildRequestData(): any[] {
    const requestData = [];
    const memberSelected = this.selectedMembers?.selectedMembers ?? [];
    const totalLoan = this.totalLoanAmount();

    // Create a random 10-digit applicationId for the GLIM
    const applicationId = Math.floor(1000000000 + Math.random() * 9000000000);

    for (let index = 0; index < memberSelected.length; index++) {
      const isFirst = index === 0;
      const isLast = index === memberSelected.length - 1;

      requestData.push({
        requestId: index,
        reference: index === 0 ? null : index - 1, // Make sure requests are executed in order
        method: 'POST',
        relativeUrl: 'loans',
        body: this.setData(applicationId, memberSelected[index], totalLoan, isFirst, isLast)
      });
    }
    return requestData;
  }

This only changes about a dozen lines from the dev branch. Of course, this does not fix the issues with "Moratorium" and "Overdue Charges", but at least the accounts are created in the correct way.

Please let me know if you have a different understanding of the process or API.

rhopman avatar Oct 30 '25 13:10 rhopman

@rhopman Thank you for your detailed summary and for reviewing the GLIM application process. I have updated the PR . I have done the following things in the updated PR :-

-I have generated a unique 10-digit applicationId and use it for all requests in the batch, linking all child loans to the same GLIM. -Each child loan request uses the same applicationId. -The first request includes isParentAccount: true to create the parent account and the last request includes lastApplication: true to close the GLIM. -I have used the reference field in each request (except the first) to ensure the batch is processed in order, as you suggested.

If you have any furthur suggestions please let me know .

JaySoni1 avatar Oct 31 '25 00:10 JaySoni1

@steinwinde Thank you for your feedback. I have not yet discussed these with @bharathcgowda or @IOhacke but I will reach out to them to confirm the necessity and placement of the new input fields I agree with your suggestion that these fields should be moved under the Moratorium section and that their labels should use translation keys. I will also update English texts along with the above changes .

JaySoni1 avatar Oct 31 '25 00:10 JaySoni1

@IOhacker I will resolve the conflict and will update the PR soon

JaySoni1 avatar Nov 08 '25 22:11 JaySoni1

@JaySoni1 it is not working this is the error, please make sure it is being tested using the latest Apache Fineract: { "developerMessage": "The request was invalid. This typically will happen due to validation errors which are provided.", "httpStatusCode": "400", "defaultUserMessage": "Validation errors exist.", "userMessageGlobalisationCode": "validation.msg.validation.errors.exist", "errors": [ { "defaultUserMessage": "The parameter moratoriumPrincipal is not supported.", "parameterName": "moratoriumPrincipal", "developerMessage": "The parameter moratoriumPrincipal is not supported.", "userMessageGlobalisationCode": "error.msg.parameter.unsupported", "args": [ { "value": "moratoriumPrincipal" } ] }, { "defaultUserMessage": "The parameter moratoriumInterest is not supported.", "parameterName": "moratoriumInterest", "developerMessage": "The parameter moratoriumInterest is not supported.", "userMessageGlobalisationCode": "error.msg.parameter.unsupported", "args": [ { "value": "moratoriumInterest" } ] } ] }

IOhacker avatar Dec 02 '25 01:12 IOhacker

Ok @IOhacker I will solve this and will update the PR soon.

Thank you for the review

JaySoni1 avatar Dec 02 '25 17:12 JaySoni1

@JaySoni1 prettier message

IOhacker avatar Dec 15 '25 00:12 IOhacker

Ok @IOhacker I will update the PR soon

JaySoni1 avatar Dec 15 '25 00:12 JaySoni1

@IOhacker I have udated this PR please review

JaySoni1 avatar Dec 15 '25 17:12 JaySoni1

@JaySoni1 there is one conflict here

IOhacker avatar Dec 22 '25 14:12 IOhacker

@JaySoni1 could you fix the conflic?

IOhacker avatar Dec 28 '25 15:12 IOhacker

@IOhacker I will fix this will update you soon

JaySoni1 avatar Dec 28 '25 19:12 JaySoni1