Skip to content

Conversation

@budaidev
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.

@budaidev budaidev force-pushed the FINERACT-2326/fix-delinquency-pause-calculation branch 2 times, most recently from 637ac4d to f0c390e Compare October 14, 2025 07:23
@budaidev budaidev marked this pull request as ready for review October 14, 2025 12:04
final long delinquentDays = overdueDays - graceDays;
if (delinquentDays > 0) {
calculateDelinquentDays(effectiveDelinquencyList, businessDate, collectionData, delinquentDays);
Long calculatedDelinquentDays = delinquentDays - pausedDays;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd love to extract this logic into a method that can be reused, otherwise I fear this duplicated (triplicated) logic would be changed in the future only in 1 place, not at the rest of them.

The pausedDays retrieval can also be included here instead and in the extracted method.

If that's done, I think we're good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I tried to keep the changes minimal, but a refactor like this certainly makes sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@budaidev budaidev force-pushed the FINERACT-2326/fix-delinquency-pause-calculation branch from f0c390e to fcbe686 Compare October 14, 2025 20:50
@budaidev budaidev requested a review from galovics October 15, 2025 05:43
@budaidev budaidev force-pushed the FINERACT-2326/fix-delinquency-pause-calculation branch 2 times, most recently from 311237b to c77fe2c Compare October 15, 2025 12:05
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

@budaidev budaidev marked this pull request as draft October 15, 2025 13:32
@budaidev budaidev force-pushed the FINERACT-2326/fix-delinquency-pause-calculation branch 20 times, most recently from 7d584e3 to 0245ba2 Compare October 20, 2025 11:51
@budaidev budaidev force-pushed the FINERACT-2326/fix-delinquency-pause-calculation branch 2 times, most recently from a2979b5 to a8ea2bf Compare October 21, 2025 09:21
@budaidev budaidev marked this pull request as ready for review October 21, 2025 11:48
@budaidev budaidev force-pushed the FINERACT-2326/fix-delinquency-pause-calculation branch from 9d961a7 to 26795d8 Compare October 22, 2025 09:43
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 d37b452 into apache:develop Oct 22, 2025
37 of 38 checks passed
@adamsaghy adamsaghy deleted the FINERACT-2326/fix-delinquency-pause-calculation branch October 22, 2025 11:17
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.

4 participants