Skip to content

Conversation

@alberto-art3ch
Copy link
Contributor

@alberto-art3ch alberto-art3ch commented Jul 22, 2025

Description

We need the calculations with CI to reflect the right amount even when the Allow approval / disbursal above loan applied amount setting enabled and look like that:

Disbursement + CI <= Approved (including over applied amount when enabled)

FINERACT-2181

Checklist

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

  • Write the commit message as per https://github.com/apache/fineract/#pull-requests
  • 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 coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/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
  • Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)

FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2181/capitalized-income-calculation-with-approved-over-applied-amount-enabled branch from 48df271 to 2c3eef1 Compare July 22, 2025 03:22
@alberto-art3ch alberto-art3ch changed the title FINERACT-2181: Capitalized Income transaction template considering ov… FINERACT-2396: Capitalized Income transaction template considering ov… Jul 22, 2025
@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2181/capitalized-income-calculation-with-approved-over-applied-amount-enabled branch from 2c3eef1 to 0956bf9 Compare July 22, 2025 03:28
@alberto-art3ch alberto-art3ch changed the title FINERACT-2396: Capitalized Income transaction template considering ov… FINERACT-2326: Capitalized Income transaction template considering ov… Jul 22, 2025
import org.springframework.stereotype.Component;

@Component
public final class LoanMaximumAmountValidator {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not call this a validator as it doesn't really validate anything. Please rename it and maybe move it out of the serialization package as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! I've renamed this class

final BigDecimal maxAppliedAmount = getOverAppliedMax(loan);
final BigDecimal maxAppliedAmount = loanMaximumAmountValidator.getOverAppliedMax(loan);
if (newTotal.compareTo(maxAppliedAmount) > 0) {
baseDataValidator.reset().parameter("transactionAmount").failWithCode("exceeds.approved.amount",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we consider changing the error code? We aren't really comparing the transaction amount to the approved amount in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, this is a part of legacy code, I've just change from the local method to the other class

@Cocoa-Puffs
Copy link
Contributor

The description references FINERACT-2181, but since we have already released 1.12.0 it should reference FINERACT-2326. Same with the branch.

@adamsaghy adamsaghy changed the title FINERACT-2326: Capitalized Income transaction template considering ov… FINERACT-2326: Capitalized Income transaction template considering over applied amount when enabled Jul 30, 2025
@adamsaghy
Copy link
Contributor

@alberto-art3ch Please rebase your PR!

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2181/capitalized-income-calculation-with-approved-over-applied-amount-enabled branch from 1e6c6f3 to 0d935c1 Compare July 30, 2025 19:00
@alberto-art3ch
Copy link
Contributor Author

@alberto-art3ch Please rebase your PR!

@adamsaghy PR rebased

Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adamsaghy adamsaghy merged commit 54f055d into apache:develop Jul 31, 2025
39 checks passed
@adamsaghy adamsaghy deleted the FINERACT-2181/capitalized-income-calculation-with-approved-over-applied-amount-enabled branch July 31, 2025 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants