feat(CRED-100): Adds penalty calculation and repayments logic for back dated transactions #79

hamzafarooqX · 2025-09-04T13:04:13Z

…d transactions

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.

…d transactions

caasiremos · 2025-09-05T05:33:20Z

Fix merge conflict

…ased-on-transaction-date

...a/com/crediblex/fineract/portfolio/loanaccount/api/CredibleXLoanTransactionsApiResource.java

Napho · 2025-09-10T07:07:56Z

...a/com/crediblex/fineract/portfolio/loanaccount/api/CredibleXLoanTransactionsApiResource.java

+        // Default to current date if no transactionDate is provided
+        LocalDate transactionDate;
+        if (transactionDateDateParam == null) {
+            transactionDate = LocalDate.now();


Use local date of tenant not local date.now

...java/com/crediblex/fineract/portfolio/loanaccount/domain/CredibleXLoanPenaltyCalculator.java

Napho · 2025-09-10T07:18:52Z

...java/com/crediblex/fineract/portfolio/loanaccount/domain/CredibleXLoanPenaltyCalculator.java

+
+        LocalDate firstPendingInstallmentDate = loanInstallments.stream()
+                .filter(p -> p.status != ExtendedLoanSchedulePeriodData.Status.PAID
+                        && p.status != ExtendedLoanSchedulePeriodData.Status.SCHEDULED


Shouldn't disbursment be part of the status not included?

@Napho - Didn't get this one! Can you please elaborate ?

Napho · 2025-09-10T07:38:56Z

.../fineract/portfolio/loanaccount/service/CustomLoanWritePlatformServiceJpaRepositoryImpl.java

+//            return date != null ? date.toLocaleString() : null;
+//        }
+
+        public String loanPaymentsSummarySchema() {


You need to keep with the fineract coding standards, thse queries need to go to a read class or an actual jpa repository class

Moved this logic to a separate DAO!

Napho · 2025-09-10T08:37:37Z

.../fineract/portfolio/loanaccount/service/CustomLoanWritePlatformServiceJpaRepositoryImpl.java

+        return super.makeLoanRepaymentWithChargeRefundChargeType(repaymentTransactionType, loanId, command, isRecoveryRepayment, chargeRefundChargeType);
+    }
+
+    private static final class LoanRepaymentsSummaryMapper implements RowMapper<LoanSchedulePeriodData> {


Move this out of the write platform service class

Napho · 2025-09-10T08:38:20Z

.../fineract/portfolio/loanaccount/service/CustomLoanWritePlatformServiceJpaRepositoryImpl.java

+
+        final LoanCurrencyDataMapper loanCurrencyDataMapper = new LoanCurrencyDataMapper();
+        final String loanCurrencySql = loanCurrencyDataMapper.loanPaymentsSummarySchema();
+        final CurrencyData currency = this.jdbcTemplate.queryForObject(loanCurrencySql, loanCurrencyDataMapper, loanId);


When you fetch a loan object it wil have a currency data, you dont need to fetch one here

Removed the currency access logic and accessed it via assembled loan!

…ased-on-transaction-date

…ion date

hamzafarooqX · 2025-09-16T10:55:24Z

.../fineract/portfolio/loanaccount/service/CustomLoanWritePlatformServiceJpaRepositoryImpl.java

+
+        if (result != null && result.getResourceId() != null && result.getResourceId() > 0L) {
+            this.applyOverdueChargesForSingleLoan(loanId);
+        }
+        return result;
+    }
+
+    @Transactional(propagation = Propagation.REQUIRES_NEW)
+    public void applyOverdueChargesForSingleLoan(Long loanId) {
+        final Long penaltyWaitPeriodValue = configurationDomainService.retrievePenaltyWaitPeriod();
+        final Boolean backdatePenalties = configurationDomainService.isBackdatePenaltiesEnabled();
+
+        final Collection<OverdueLoanScheduleData> installmentsForLoan =
+                this.credibleXLoanReadPlatformService.retrieveOverdueInstallmentsForLoan(
+                        loanId, penaltyWaitPeriodValue, backdatePenalties);
+
+        if (installmentsForLoan.isEmpty()) {
+            return;
+        }
+
+        try {
+            this.credibleXLoanChargeWritePlatformService.applyOverdueChargesForLoan(loanId, installmentsForLoan);
+
+        } catch (final PlatformApiDataValidationException e) {
+            // Validation errors (e.g. bad data, config issues)
+            for (final ApiParameterError error : e.getErrors()) {
+                log.error("Apply overdue charges failed for loan {} with validation error: {}",
+                        loanId, error.getDeveloperMessage(), e);
+            }
+            throw e; // rethrow so caller knows it failed
+


@Napho - This is the code that I have added to apply penalties to a loan after repayment is actually done. This code runs that I have confirmed. But the changes do not reflect!

…ased-on-transaction-date

Copilot

Pull Request Overview

This pull request implements penalty calculation and repayment logic for back-dated transactions in the Apache Fineract system. The changes add functionality to calculate penalties up to a specific transaction date rather than the posting date, handle penalty disabling during repayments, and provide API endpoints for retrieving penalty information.

Adds penalty calculation logic for back-dated transactions with configurable wait periods
Implements penalty disabling mechanism when processing repayments
Introduces new API endpoint for retrieving accrued penalties up to a given date

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
LoanWritePlatformServiceJpaRepositoryImpl.java	Changes field visibility from private to protected for inheritance
LoanReadPlatformServiceImpl.java	Makes MusoniOverdueLoanScheduleMapper class public for external access
LoanTransactionsApiResource.java	Updates field visibility to protected for inheritance
CustomLoanWritePlatformServiceJpaRepositoryImpl.java	Implements custom repayment logic with penalty calculation and charge management
CustomLoanChargeReadPlatformServiceImpl.java	Provides custom loan charge reading functionality
CredXLoanReadPlatformServiceImpl.java	Extends loan reading service with penalty calculation capabilities
LoanRepaymentsSummaryDAO.java	Data access object for loan repayment summary queries
CustomLoanChargeRepository.java	Repository for custom loan charge operations
OverdueChargeHelper.java	Helper class for applying overdue charges
CredibleXLoanPenaltyCalculator.java	Core penalty calculation logic
ExtendedLoanSchedulePeriodData.java	Extended data class with status field
BackdatedRepaymentPenaltyDTO.java	Data transfer object for penalty information
CredibleXLoanTransactionsApiResource.java	API resource for penalty-related endpoints
dependencies.gradle	Adds required dependencies for JAX-RS and Swagger annotations

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-18T15:06:52Z

...a/com/crediblex/fineract/portfolio/loanaccount/service/CredXLoanReadPlatformServiceImpl.java

+            // This is a disbursement period has nusla
+            // ll period value


The comment has a typo 'nusla' and is incorrectly split across two lines. It should read 'null' on a single line.

Suggested change

// This is a disbursement period has nusla

// ll period value

// This is a disbursement period and has null period value

Copilot · 2025-09-18T15:06:53Z

...java/com/crediblex/fineract/portfolio/loanaccount/domain/CredibleXLoanPenaltyCalculator.java

+                .filter(p -> p.status != ExtendedLoanSchedulePeriodData.Status.PAID
+                        && p.status != ExtendedLoanSchedulePeriodData.Status.SCHEDULED
+                        && p.status != ExtendedLoanSchedulePeriodData.Status.DUE)


[nitpick] The status comparison logic is inverted and unclear. Consider using a positive filter with explicit status checks like p.status == Status.OVERDUE || p.status == Status.PARTIAL_PAID for better readability.

Suggested change

.filter(p -> p.status != ExtendedLoanSchedulePeriodData.Status.PAID

&& p.status != ExtendedLoanSchedulePeriodData.Status.SCHEDULED

&& p.status != ExtendedLoanSchedulePeriodData.Status.DUE)

.filter(p -> p.status == ExtendedLoanSchedulePeriodData.Status.OVERDUE

|| p.status == ExtendedLoanSchedulePeriodData.Status.PARTIAL_PAID)

Copilot · 2025-09-18T15:06:53Z

custom/crediblex/portfolio/loanaccount/dependencies.gradle

+    implementation 'jakarta.ws.rs:jakarta.ws.rs-api:3.1.0'
+    implementation "io.swagger.core.v3:swagger-annotations:2.2.12"
+    implementation "org.springframework.security:spring-security-core:6.1.2"


[nitpick] Hard-coded dependency versions should be managed centrally. Consider using version variables or a dependency management platform to ensure consistency across the project.

Copilot · 2025-09-18T15:06:53Z

.../fineract/portfolio/loanaccount/service/CustomLoanWritePlatformServiceJpaRepositoryImpl.java

+        } catch (Exception e) {
+            // Catch-all for unexpected exceptions
+            log.error("Apply overdue charges failed for loan {}", loanId, e);
+            throw new RuntimeException("Unexpected error while applying overdue charges for loan " + loanId, e);


[nitpick] Generic Exception catch-all should be avoided. Consider catching specific exceptions or using a more specific exception type rather than wrapping in RuntimeException.

Suggested change

} catch (Exception e) {

// Catch-all for unexpected exceptions

log.error("Apply overdue charges failed for loan {}", loanId, e);

throw new RuntimeException("Unexpected error while applying overdue charges for loan " + loanId, e);

CRED-100: Adds penalty calculation and repayments logic for back date…

a3d1a16

…d transactions

hamzafarooqX requested a review from Napho September 4, 2025 13:04

hamzafarooqX force-pushed the CRED-100-net-outstanding-to-get-updated-based-on-transaction-date branch 2 times, most recently from ca962d4 to a3d1a16 Compare September 8, 2025 08:32

hamzafarooqX added 2 commits September 8, 2025 13:47

Merge branch 'develop' into CRED-100-net-outstanding-to-get-updated-b…

41f4118

…ased-on-transaction-date

CRED-100: Adds deleted files and other logic needed

da6ca59

Napho requested changes Sep 10, 2025

View reviewed changes

hamzafarooqX added 7 commits September 10, 2025 15:45

CRED-100: Address PR feedback partial

3ff9f73

CRED-100: Removes data access logic from loan write service

c4ad564

Merge branch 'develop' into CRED-100-net-outstanding-to-get-updated-b…

d6a082d

…ased-on-transaction-date

CRED-100: Adds logic to club principal and interest as per trasansact…

02b3de3

…ion date

CRED-100: adds code to appply penalties and fix penatlty disabling logic

903bc40

CRED-100: Adds overdue charge helper

d5aee6b

CRED-100: Fixes code

098db6b

hamzafarooqX commented Sep 16, 2025

View reviewed changes

hamzafarooqX added 7 commits September 17, 2025 15:12

Merge branch 'develop' into CRED-100-net-outstanding-to-get-updated-b…

43752c8

…ased-on-transaction-date

CRED-100: Fix formatting issues

7ead930

CRED-100: Fix more formatting issues

1e6e210

CRED-100: Fix formatting issue

fa0658d

CRED-100: Fix import all issue

0294e17

CRED-100: Verifies all spotless checks

3df16aa

CRED-100: Fetch outstanding principal on repayment screen

51bc5ca

Copilot AI review requested due to automatic review settings September 18, 2025 15:05

Copilot AI reviewed Sep 18, 2025

View reviewed changes

		// This is a disbursement period has nusla
		// ll period value

	// This is a disbursement period has nusla
	// ll period value
	// This is a disbursement period and has null period value

feat(CRED-100): Adds penalty calculation and repayments logic for back dated transactions #79

Are you sure you want to change the base?

feat(CRED-100): Adds penalty calculation and repayments logic for back dated transactions #79

Uh oh!

Conversation

hamzafarooqX commented Sep 4, 2025

Description

Checklist

Uh oh!

caasiremos commented Sep 5, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!