Skip to content

Conversation

@oleksii-novikov-onix
Copy link
Contributor

Description

Describe the changes made and why they were made.

Ignore if these details are present on the associated Apache Fineract JIRA ticket.

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.

@oleksii-novikov-onix oleksii-novikov-onix force-pushed the FINERACT-2354/introduce-loan-reaging-preview branch 3 times, most recently from 8d5cc27 to 61b0528 Compare October 6, 2025 06:23
@adamsaghy
Copy link
Contributor

@oleksii-novikov-onix Please rebase.

@oleksii-novikov-onix oleksii-novikov-onix force-pushed the FINERACT-2354/introduce-loan-reaging-preview branch from 61b0528 to 377db88 Compare October 15, 2025 11:27
@oleksii-novikov-onix
Copy link
Contributor Author

@oleksii-novikov-onix Please rebase.

@adamsaghy @MarianaDmytrivBinariks Done

@oleksii-novikov-onix oleksii-novikov-onix force-pushed the FINERACT-2354/introduce-loan-reaging-preview branch from 377db88 to ff13fb2 Compare October 23, 2025 11:41
@MarianaDmytrivBinariks MarianaDmytrivBinariks force-pushed the FINERACT-2354/introduce-loan-reaging-preview branch 2 times, most recently from cbb30b1 to 3a8af86 Compare October 24, 2025 10:54
@oleksii-novikov-onix oleksii-novikov-onix marked this pull request as ready for review October 27, 2025 08:53
Comment on lines 2893 to 2904
AtomicReference<Money> outstandingPrincipalBalance = new AtomicReference<>(Money.zero(currency));
final LocalDate startDate = loanTransaction.getLoanReAgeParameter().getStartDate();
final LocalDate endDate = calculateReAgedInstallmentEndDate(loanTransaction.getLoanReAgeParameter());
final AtomicReference<Money> outstandingPrincipalBalance = new AtomicReference<>(Money.zero(currency));
installments.forEach(i -> {
Money principalOutstanding = i.getPrincipalOutstanding(currency);
if (principalOutstanding.isGreaterThanZero()) {
outstandingPrincipalBalance.set(outstandingPrincipalBalance.get().add(principalOutstanding));
i.addToPrincipal(loanTransaction.getTransactionDate(), principalOutstanding.negated());
final boolean shouldInclude = !i.isAdditional() || i.getDueDate().isBefore(startDate)
|| (!i.getDueDate().isBefore(startDate) && i.getDueDate().isBefore(endDate));
if (shouldInclude) {
Money principalOutstanding = i.getPrincipalOutstanding(currency);
if (principalOutstanding.isGreaterThanZero()) {
outstandingPrincipalBalance.set(outstandingPrincipalBalance.get().add(principalOutstanding));
i.addToPrincipal(loanTransaction.getTransactionDate(), principalOutstanding.negated());
}
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need these changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was added to handle negative balances. I will review the logic again

@Parameter(hidden = true) final String apiRequestBodyAsJson, @Context final UriInfo uriInfo) {
final LoanScheduleData result = previewReAgeScheduleInternal(loanId, null, apiRequestBodyAsJson);
final ApiRequestJsonSerializationSettings settings = this.apiRequestParameterHelper.process(uriInfo.getQueryParameters());
return this.loanScheduleToApiJsonSerializer.serialize(settings, result, new HashSet<>());
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to serialize. Simply return the LoanScheduleData result. Also you can remove the ApiResponses annotation as well in that case.

}
}

@POST
Copy link
Contributor

Choose a reason for hiding this comment

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

I think GET would be better here. Lets provide the parameters as query params.

}

@Transactional(readOnly = true)
public LoanScheduleData previewReAge(final Long loanId, final JsonCommand command) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need command. It's a read operation.

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.

Please kindly see my review!

@MarianaDmytrivBinariks MarianaDmytrivBinariks force-pushed the FINERACT-2354/introduce-loan-reaging-preview branch from 3a8af86 to c3b04b9 Compare October 27, 2025 09:44
@oleksii-novikov-onix oleksii-novikov-onix force-pushed the FINERACT-2354/introduce-loan-reaging-preview branch from c3b04b9 to 5d47942 Compare October 27, 2025 10:59
@oleksii-novikov-onix oleksii-novikov-onix force-pushed the FINERACT-2354/introduce-loan-reaging-preview branch from 5d47942 to cc7407d Compare October 28, 2025 09:48
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