FINERACT-2284: Enforce Charge Rounding using Money rounding rules by Dhanno98 · Pull Request #5940 · apache/fineract

Dhanno98 · 2026-06-04T17:24:59Z

Description

FINERACT-2284: Enforce charge rounding using Money rounding rules

This PR fixes charge amount handling across Fineract by ensuring that charge values are rounded using the application's configured currency rules before they are persisted or processed.

Previously, several charge flows stored or processed raw BigDecimal values directly. As a result, charge amounts could bypass currency rounding settings such as:

digitsAfterDecimal
inMultiplesOf

This led to inconsistent behavior between charge calculations and other monetary operations that already rely on Money.

This PR standardizes charge amount handling by using Money for charge rounding across all supported charge types:

Loan Charges
Savings Account Charges
Share Account Charges
Client Charges

In addition, if a charge amount rounds to zero after applying currency rules, the charge is no longer persisted or created.

Functional Changes

Loan Charges

Applied currency rounding when enforcing minimum and maximum charge caps.
Ensured installment fee calculations and charge updates persist rounded values.
Prevented zero-value loan charges from being persisted.
Prevented creation of zero-value tranche disbursement charges.

Savings Account Charges

Applied currency rounding through Money.
Prevented persistence of flat charges whose final rounded value is zero.

Share Account Charges

Applied currency rounding when deriving charge amounts.
Applied currency rounding when updating charge amounts for additional share transactions.
Prevented creation of charge transactions for charges whose rounded value is zero.
Prevented zero-value activation, purchase, and redemption charge transactions from being generated.

Client Charges

Applied currency rounding before charge creation.
Prevented persistence of client charges whose rounded value becomes zero after rounding.

API Documentation

Updated Swagger/OpenAPI schemas where required to improve request model documentation and examples.

Why This Change

Fineract already provides currency-specific monetary rules through Money, including support for:

Currency precision (digitsAfterDecimal)
Rounding increments (inMultiplesOf)

Charges should follow the same monetary rules as all other financial amounts.

By enforcing rounding consistently:

Charge calculations become predictable.
Persisted charge values match configured currency behavior.
Zero-value charges generated by rounding are avoided.
Charge processing remains consistent across all portfolio modules.

Test Coverage

Added integration test coverage for all supported charge types:

LoanChargeRoundingTest
SavingsAccountChargeRoundingTest
ShareAccountChargeRoundingTest
ClientChargeRoundingTest

The tests verify:

Charge amounts respect configured currency rounding rules.
inMultiplesOf and digitsAfterDecimal rounding is enforced.
Rounded values are persisted correctly.
Charges that round to zero are not created or persisted.

Verification

Executed the complete charge rounding integration test suite successfully.

Result: 44/44 tests passing.

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

Write the commit message as per our guidelines
Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
Create/update unit or integration tests for verifying the changes made.
Follow our coding conventions.
Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.

Your assigned reviewer(s) will follow our guidelines for code reviews.

…ings

adamsaghy · 2026-06-08T15:15:40Z

-                minMaxCap = this.minCap;
-                return minMaxCap;
-            }
+    public BigDecimal minimumAndMaximumCap(final BigDecimal value) {


I see no operations which warrants to use Money here... this method just simply comparing values, no calculations which might led to situations where rounding is needed...

adamsaghy · 2026-06-08T15:16:49Z

        update(loanCharge, chargeAmt, loanCharge.getDueLocalDate(), amount, loan.fetchNumberOfInstallmentsAfterExceptions(),
                totalChargeAmt);

+        // Skip zero-value charges


Adding 0 charge is incorrect and it should be stopped at validation and or during charge amount calculation. Silent return is misleading and hiding a potential issue!

adamsaghy · 2026-06-08T15:18:00Z

                            numberOfRepayments = loanCharge.getLoan().fetchNumberOfInstallmentsAfterExceptions();
                        }
-                        loanCharge.setAmount(amount.multiply(BigDecimal.valueOf(numberOfRepayments)));
+                        loanCharge.setAmount(loanCharge.minimumAndMaximumCap(amount.multiply(BigDecimal.valueOf(numberOfRepayments))));


Unrelated change? Why loanCharge.minimumAndMaximumCap is needed here? Charge amount calculation should cover the rounding, not this...

adamsaghy · 2026-06-08T15:18:20Z

+        @Schema(example = "en")
+        public String locale;
+        @Schema(example = "dd MMMM yyyy")
+        public String dateFormat;
+        @Schema(example = "01 January 2026")
+        public String activatedDate;
+        @Schema(example = "05 May 2026")
+        public String requestedDate;
+        @Schema(example = "01 January 2026")
+        public String closedDate;
+        @Schema(description = "Can represent either number of shares or transaction IDs")
+        public Object requestedShares;
+
        static final class PostAccountsRequestedShares {

            private PostAccountsRequestedShares() {}

            @Schema(example = "35")
            public Long id;
        }
-
-        public Set<PostAccountsRequestedShares> requestedShares;
    }


Unrelated changes?

adamsaghy · 2026-06-08T15:19:46Z


-    public static ClientCharge createNew(final Client client, final Charge charge, final JsonCommand command) {
-        BigDecimal amount = command.bigDecimalValueOfParameterNamed(ClientApiConstants.amountParamName);
+    public static ClientCharge createNew(final Client client, final Charge charge, final BigDecimal amount, final JsonCommand command) {


If you’ve started providing the fields as incoming arguments, don’t stop halfway. Remove the command parameter and ensure that the due date is provided as a parameter.

adamsaghy · 2026-06-08T15:20:01Z

            }

-            final ClientCharge clientCharge = ClientCharge.createNew(client, charge, command);
+            BigDecimal roundedAmount = calculateRoundedChargeAmount(charge, command);


Using Money here would be better, no?

adamsaghy · 2026-06-08T15:20:28Z

-            final ClientCharge clientCharge = ClientCharge.createNew(client, charge, command);
+            BigDecimal roundedAmount = calculateRoundedChargeAmount(charge, command);
+            if (roundedAmount.compareTo(BigDecimal.ZERO) == 0) {
+                return new CommandProcessingResultBuilder().withOfficeId(client.getOffice().getId()).withClientId(client.getId()).build();


Why to accept 0 amount charge? 0 amount charge is incorrect outcome!

adamsaghy · 2026-06-08T15:21:13Z

            LoanTrancheDisbursementCharge loanTrancheDisbursementCharge;
            ExternalId externalId = externalIdFactory.createFromCommand(command, "externalId");
            boolean isFirst = true;
+            boolean atLeastOneChargePersisted = false;


what are these flags for?

adamsaghy · 2026-06-08T15:21:46Z

+
+                    // if charge amount is rounded to zero, then continue and look for another disbursement
+                    if (isZeroCharge(tempCharge)) {
+                        continue;


0 amount charge sounds incorrect to me.

adamsaghy · 2026-06-08T15:21:56Z

+                    }
+
+                    // tempCharge was persisted so this is true
+                    atLeastOneChargePersisted = true;


what does it matter?

adamsaghy · 2026-06-08T15:22:10Z

            }
-            if (loanCharge == null) {
+            // No eligible disbursement
+            if (!eligibleDisbursementFound) {


I dont like this...

adamsaghy · 2026-06-08T15:22:19Z

            validateAddLoanCharge(loan, chargeDefinition, loanCharge);
            isAppliedOnBackDate = addCharge(loan, chargeDefinition, loanCharge);
+
+            // if charge amount is rounded to zero, then return early


Sounds incorrect result to me.

adamsaghy · 2026-06-08T15:22:41Z

+        if (!isZeroCharge(loanCharge)) {
+            loanCharge = this.loanChargeRepository.saveAndFlush(loanCharge);
+
+            // we want to apply charge transactions only for those loans charges that are applied when a loan is active


why to change this?

adamsaghy · 2026-06-08T15:22:50Z

-                        .notifyPostBusinessEvent(new LoanAccrualTransactionCreatedBusinessEvent(applyLoanChargeTransaction));
+
+        // If charge amount is not zero
+        if (!isZeroCharge(loanCharge)) {


I dont like this..

adamsaghy · 2026-06-08T15:23:07Z

        }
        final SavingsAccountCharge savingsAccountCharge = SavingsAccountCharge.createNewFromJson(savingsAccount, chargeDefinition, command);

+        if (isFlatCharge(savingsAccountCharge) && isZeroCharge(savingsAccountCharge, savingsAccount.getCurrency())) {


I dont like it... 0 amount charge is incorrect result.

adamsaghy

I stopped review at half time.. I dont like this approach, neither i think it is correct.

The introduced 0 amount handling is incorrect. 0 amount is incorrect outcome and charge cannot be created with this value, but nevertheless, we cannot accept the charge creation action and after not create the charge. This is incorrect and unwanted behaviour:

Add proper validation (first thing to do)
Calculate the amount, if it is incorrect (like 0 amount) -> Raise error and explain the issue!
Remove the unnecessary flags and checks... it makes the code hard to read and maintained...
Remove the unrelated changes, the PR should focus on 1 thing only!

Dhanno98 · 2026-06-10T14:45:37Z

@adamsaghy Thanks for your suggestions! I am working on the fix.

Dhanno98 force-pushed the FINERACT-2284/enforce-charge-rounding branch from f44e89e to 38bd72e Compare June 4, 2026 18:29

FINERACT-2284: Enforce Charge Rounding using Money inMultiplesOf sett…

8009f8c

…ings

Dhanno98 force-pushed the FINERACT-2284/enforce-charge-rounding branch from 38bd72e to 8009f8c Compare June 6, 2026 18:06

adamsaghy reviewed Jun 8, 2026

View reviewed changes

adamsaghy requested changes Jun 8, 2026

View reviewed changes

Conversation

Dhanno98 commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description