Skip to content

Commit

Permalink
FINERACT-2081: Fix inconsistent transaction processing order
Browse files Browse the repository at this point in the history
  • Loading branch information
@adamsaghy
adamsaghy committed Nov 11, 2024
1 parent 832b84f commit 39da688
Show file tree
Hide file tree
Showing 21 changed files with 74 additions and 86 deletions.
11 changes: 0 additions & 11 deletions ...rg/apache/fineract/infrastructure/core/domain/AbstractAuditableWithUTCDateTimeCustom.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
import org.apache.fineract.infrastructure.core.service.DateUtils;
import org.springframework.data.domain.Auditable;
import org.springframework.data.jpa.domain.AbstractAuditable;

Expand Down Expand Up @@ -81,11 +80,6 @@ public Optional<OffsetDateTime> getCreatedDate() {
return Optional.ofNullable(createdDate);
}

@NotNull
public OffsetDateTime getCreatedDateTime() {
return getCreatedDate().orElseGet(DateUtils::getAuditOffsetDateTime);
}

@Override
@NotNull
public Optional<Long> getLastModifiedBy() {
Expand All @@ -97,9 +91,4 @@ public Optional<Long> getLastModifiedBy() {
public Optional<OffsetDateTime> getLastModifiedDate() {
return Optional.ofNullable(lastModifiedDate);
}

@NotNull
public OffsetDateTime getLastModifiedDateTime() {
return getLastModifiedDate().orElseGet(DateUtils::getAuditOffsetDateTime);
}
}
20 changes: 20 additions & 0 deletions fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.time.temporal.ChronoUnit;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import org.apache.fineract.infrastructure.core.data.ApiParameterError;
import org.apache.fineract.infrastructure.core.domain.FineractPlatformTenant;
import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
Expand Down Expand Up @@ -425,4 +426,23 @@ private static DateTimeFormatter getDateTimeFormatter(String format, Locale loca
}
return formatter;
}

/**
* Comparing dates. Null will be considered as last elements
*
* @param first
* @param second
* @return
*/
public static int compareWithNullsLast(LocalDate first, LocalDate second) {
return first == null ? (second == null ? 0 : 1) : (second == null ? -1 : first.compareTo(second));
}

public static int compareWithNullsLast(@NotNull Optional<OffsetDateTime> first, @NotNull Optional<OffsetDateTime> second) {
return DateUtils.compareWithNullsLast(first.orElse(null), second.orElse(null));
}

public static int compareWithNullsLast(OffsetDateTime first, OffsetDateTime second) {
return first == null ? (second == null ? 0 : 1) : (second == null ? -1 : first.compareTo(second));
}
}
2 changes: 1 addition & 1 deletion ...src/main/java/org/apache/fineract/portfolio/calendar/data/CalendarHistoryDataWrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public CalendarHistoryDataWrapper(final Set<CalendarHistory> calendarHistoryList

@Override
public int compare(CalendarHistory calendarHistory1, CalendarHistory calendarHistory2) {
return DateUtils.compare(calendarHistory1.getEndDate(), calendarHistory2.getEndDate());
return DateUtils.compareWithNullsLast(calendarHistory1.getEndDate(), calendarHistory2.getEndDate());
}
};
this.calendarHistoryList.sort(orderByDate);
Expand Down
16 changes: 8 additions & 8 deletions fineract-e2e-tests-runner/src/test/resources/features/EMICalculation.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3824,8 +3824,8 @@ Feature: EMI calculation and repayment schedule checks for interest bearing loan
| Transaction date | Transaction Type | Amount | Principal | Interest | Fees | Penalties | Loan Balance | Reverted | Replayed |
| 01 January 2024 | Disbursement | 1000.0 | 0.0 | 0.0 | 0.0 | 0.0 | 1000.0 | false | false |
| 01 February 2024 | Repayment | 255.14 | 246.75 | 8.39 | 0.0 | 0.0 | 753.25 | false | false |
| 09 February 2024 | Payout Refund | 1000.0 | 743.23 | 1.63 | 0.0 | 0.0 | 10.02 | false | false |
| 09 February 2024 | Interest Refund | 10.02 | 10.02 | 0.0 | 0.0 | 0.0 | 0.0 | false | false |
| 09 February 2024 | Payout Refund | 1000.0 | 753.25 | 1.63 | 0.0 | 0.0 | 0.0 | false | false |
| 09 February 2024 | Interest Refund | 10.02 | 0.0 | 0.0 | 0.0 | 0.0 | 0.0 | false | false |
When Admin sets the business date to "10 February 2024"
When Admin makes Credit Balance Refund transaction on "10 February 2024" with 255.14 EUR transaction amount
Then Loan Repayment schedule has 4 periods, with the following data for periods:
Expand All @@ -3842,8 +3842,8 @@ Feature: EMI calculation and repayment schedule checks for interest bearing loan
| Transaction date | Transaction Type | Amount | Principal | Interest | Fees | Penalties | Loan Balance | Reverted | Replayed |
| 01 January 2024 | Disbursement | 1000.0 | 0.0 | 0.0 | 0.0 | 0.0 | 1000.0 | false | false |
| 01 February 2024 | Repayment | 255.14 | 246.75 | 8.39 | 0.0 | 0.0 | 753.25 | false | false |
| 09 February 2024 | Payout Refund | 1000.0 | 743.23 | 1.63 | 0.0 | 0.0 | 10.02 | false | false |
| 09 February 2024 | Interest Refund | 10.02 | 10.02 | 0.0 | 0.0 | 0.0 | 0.0 | false | false |
| 09 February 2024 | Payout Refund | 1000.0 | 753.25 | 1.63 | 0.0 | 0.0 | 0.0 | false | false |
| 09 February 2024 | Interest Refund | 10.02 | 0.0 | 0.0 | 0.0 | 0.0 | 0.0 | false | false |
| 10 February 2024 | Credit Balance Refund | 255.14 | 0.0 | 0.0 | 0.0 | 0.0 | 0.0 | false | false |
| 10 February 2024 | Accrual | 10.02 | 0.0 | 10.02 | 0.0 | 0.0 | 0.0 | false | false |
Then Loan status will be "CLOSED_OBLIGATIONS_MET"
Expand Down Expand Up @@ -3960,8 +3960,8 @@ Feature: EMI calculation and repayment schedule checks for interest bearing loan
| 01 January 2024 | Disbursement | 500.0 | 0.0 | 0.0 | 0.0 | 0.0 | 500.0 | false | false |
| 07 January 2024 | Disbursement | 500.0 | 0.0 | 0.0 | 0.0 | 0.0 | 1000.0 | false | false |
| 01 February 2024 | Repayment | 255.14 | 247.57 | 7.57 | 0.0 | 0.0 | 752.43 | false | false |
| 09 February 2024 | Merchant Issued Refund | 1000.0 | 743.23 | 1.63 | 0.0 | 0.0 | 9.2 | false | false |
| 09 February 2024 | Interest Refund | 9.2 | 9.2 | 0.0 | 0.0 | 0.0 | 0.0 | false | false |
| 09 February 2024 | Merchant Issued Refund | 1000.0 | 752.43 | 1.63 | 0.0 | 0.0 | 0.0 | false | false |
| 09 February 2024 | Interest Refund | 9.2 | 0.0 | 0.0 | 0.0 | 0.0 | 0.0 | false | false |
When Admin sets the business date to "10 February 2024"
When Admin makes Credit Balance Refund transaction on "10 February 2024" with 255.14 EUR transaction amount
Then Loan Repayment schedule has 4 periods, with the following data for periods:
Expand All @@ -3980,8 +3980,8 @@ Feature: EMI calculation and repayment schedule checks for interest bearing loan
| 01 January 2024 | Disbursement | 500.0 | 0.0 | 0.0 | 0.0 | 0.0 | 500.0 | false | false |
| 07 January 2024 | Disbursement | 500.0 | 0.0 | 0.0 | 0.0 | 0.0 | 1000.0 | false | false |
| 01 February 2024 | Repayment | 255.14 | 247.57 | 7.57 | 0.0 | 0.0 | 752.43 | false | false |
| 09 February 2024 | Merchant Issued Refund | 1000.0 | 743.23 | 1.63 | 0.0 | 0.0 | 9.2 | false | false |
| 09 February 2024 | Interest Refund | 9.2 | 9.2 | 0.0 | 0.0 | 0.0 | 0.0 | false | false |
| 09 February 2024 | Merchant Issued Refund | 1000.0 | 752.43 | 1.63 | 0.0 | 0.0 | 0.0 | false | false |
| 09 February 2024 | Interest Refund | 9.2 | 0.0 | 0.0 | 0.0 | 0.0 | 0.0 | false | false |
| 10 February 2024 | Credit Balance Refund | 255.14 | 0.0 | 0.0 | 0.0 | 0.0 | 0.0 | false | false |
| 10 February 2024 | Accrual | 9.2 | 0.0 | 9.2 | 0.0 | 0.0 | 0.0 | false | false |
Then Loan status will be "CLOSED_OBLIGATIONS_MET"
2 changes: 0 additions & 2 deletions fineract-e2e-tests-runner/src/test/resources/features/LoanRepayment.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3942,8 +3942,6 @@ Feature: LoanRepayment
| 24 June 2024 | Disbursement | 100.0 | 0.0 | 0.0 | 0.0 | 0.0 | 500.0 |
| 10 September 2024 | Merchant Issued Refund | 200.0 | 200.0 | 0.0 | 0.0 | 0.0 | 300.0 |

# Needs to fix Merchant issued Refund - Interest refund order
@Skip
Scenario: Verify the relationship for Interest Refund transaction after repayment by reverting related transaction
When Admin sets the business date to "30 January 2024"
When Admin creates a client with random data
Expand Down
5 changes: 3 additions & 2 deletions ...n/java/org/apache/fineract/portfolio/delinquency/validator/LoanDelinquencyActionData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.time.LocalDate;
import java.time.OffsetDateTime;
import lombok.Data;
import org.apache.fineract.infrastructure.core.service.DateUtils;
import org.apache.fineract.portfolio.delinquency.domain.DelinquencyAction;
import org.apache.fineract.portfolio.delinquency.domain.LoanDelinquencyAction;

Expand All @@ -44,8 +45,8 @@ public LoanDelinquencyActionData(LoanDelinquencyAction loanDelinquencyAction) {

loanDelinquencyAction.getCreatedBy().ifPresent(this::setCreatedById);
loanDelinquencyAction.getLastModifiedBy().ifPresent(this::setUpdatedById);
this.createdOn = loanDelinquencyAction.getCreatedDateTime();
this.lastModifiedOn = loanDelinquencyAction.getLastModifiedDateTime();
this.createdOn = loanDelinquencyAction.getCreatedDate().orElse(DateUtils.getAuditOffsetDateTime());
this.lastModifiedOn = loanDelinquencyAction.getLastModifiedDate().orElse(DateUtils.getAuditOffsetDateTime());
}

}
5 changes: 1 addition & 4 deletions ...t-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/data/DisbursementData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,7 @@ public boolean isDisbursed() {

@Override
public int compareTo(final DisbursementData obj) {
if (obj == null) {
return -1;
}
return DateUtils.compare(obj.expectedDisbursementDate, this.expectedDisbursementDate);
return DateUtils.compareWithNullsLast(obj.expectedDisbursementDate, this.expectedDisbursementDate);
}

public boolean isDueForDisbursement(LoanScheduleType loanScheduleType, final LocalDate fromDate, final LocalDate toDate) {
Expand Down
17 changes: 1 addition & 16 deletions ...-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransaction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1041,22 +1041,7 @@ public void setLoanReAgeParameter(LoanReAgeParameter loanReAgeParameter) {
}

public boolean happenedBefore(LoanTransaction loanTransaction) {
// compare transaction date, creation date and then transaction id
if (DateUtils.isBefore(getTransactionDate(), loanTransaction.getTransactionDate())) {
return true;
}
if (DateUtils.isEqual(getTransactionDate(), loanTransaction.getTransactionDate())) {
if (DateUtils.isBefore(getCreatedDateTime(), loanTransaction.getCreatedDateTime())) {
return true;
}
if (DateUtils.isEqual(getCreatedDateTime(), loanTransaction.getCreatedDateTime())) {
if (getId().compareTo(loanTransaction.getId()) < 0) {
return true;
}
}
}

return false;
return LoanTransactionComparator.INSTANCE.compare(this, loanTransaction) < 0;
}

public boolean isOverPaid() {
Expand Down
22 changes: 14 additions & 8 deletions ...main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransactionComparator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,30 +22,36 @@
import org.apache.fineract.infrastructure.core.service.DateUtils;

/**
* Sort loan transactions by transaction date, created date and transaction type placing
* Sort loan transactions by transaction date, submitted date, created date, transaction type placing, lastly compare
* ids
*/
public class LoanTransactionComparator implements Comparator<LoanTransaction> {

public static final LoanTransactionComparator INSTANCE = new LoanTransactionComparator();

@Override
public int compare(final LoanTransaction o1, final LoanTransaction o2) {
int result = DateUtils.compare(o1.getTransactionDate(), o2.getTransactionDate());
int result = DateUtils.compareWithNullsLast(o1.getTransactionDate(), o2.getTransactionDate());
if (result != 0) {
return result;
}
// For transactions bearing the same transaction date, we sort transactions based on created date (when
// available) after which sorting for waivers takes place
result = o1.getCreatedDate().isPresent()
? (o2.getCreatedDate().isPresent() ? DateUtils.compare(o1.getCreatedDateTime(), o2.getCreatedDateTime()) : -1)
: (o2.getCreatedDate().isPresent() ? 1 : 0);
// Accrual activity as last
if (o1.isAccrualActivity() != o2.isAccrualActivity()) {
return o1.isAccrualActivity() ? 1 : -1;
}
result = DateUtils.compareWithNullsLast(o1.getSubmittedOnDate(), o2.getSubmittedOnDate());
if (result != 0) {
return result;
}
result = DateUtils.compareWithNullsLast(o1.getCreatedDate(), o2.getCreatedDate());
if (result != 0) {
return result;
}
// equal transaction dates
// income posting takes priority
if (o1.isIncomePosting() != o2.isIncomePosting()) {
return o1.isIncomePosting() ? -1 : 1;
}
// waive takes priority
if (o1.isWaiver() != o2.isWaiver()) {
return o1.isWaiver() ? -1 : 1;
}
Expand Down
16 changes: 2 additions & 14 deletions ...account/domain/transactionprocessor/impl/AdvancedPaymentScheduleTransactionProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleProcessingWrapper;
import org.apache.fineract.portfolio.loanaccount.domain.LoanTermVariations;
import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionComparator;
import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionRelation;
import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionRelationTypeEnum;
import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
Expand Down Expand Up @@ -471,7 +472,7 @@ private Map<AllocationType, Money> adjustOriginalAllocationWithFormerChargebacks
List<LoanTransaction> chargebacks = allTransactions.stream().filter(LoanTransaction::isChargeback).toList();

// let's figure out the original transaction for these chargebacks, and order them by ascending order
Comparator<LoanTransaction> comparator = loanTransactionDateComparator();
Comparator<LoanTransaction> comparator = LoanTransactionComparator.INSTANCE;
List<LoanTransaction> chargebacksForTheSameOriginal = chargebacks.stream()
.filter(tr -> findChargebackOriginalTransaction(tr, ctx) == originalTransaction
&& comparator.compare(tr, chargeBackTransaction) < 0)
Expand All @@ -486,19 +487,6 @@ private Map<AllocationType, Money> adjustOriginalAllocationWithFormerChargebacks
return allocation;
}

@NotNull
private Comparator<LoanTransaction> loanTransactionDateComparator() {
return (tr1, tr2) -> {
if (tr1.getTransactionDate().compareTo(tr2.getTransactionDate()) != 0) {
return tr1.getTransactionDate().compareTo(tr2.getTransactionDate());
} else if (tr1.getSubmittedOnDate().compareTo(tr2.getSubmittedOnDate()) != 0) {
return tr1.getSubmittedOnDate().compareTo(tr2.getSubmittedOnDate());
} else {
return tr1.getCreatedDateTime().compareTo(tr2.getCreatedDateTime());
}
};
}

private void recognizeAmountsAfterChargeback(MonetaryCurrency currency, LocalDate localDate,
LoanRepaymentScheduleInstallment installment, Map<AllocationType, Money> chargebackAllocation) {
Money principal = chargebackAllocation.get(PRINCIPAL);
Expand Down
8 changes: 4 additions & 4 deletions ...ache/fineract/portfolio/loanaccount/domain/transactionprocessor/impl/ChangeOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ private OffsetDateTime getCreatedDateTime() {
} else if (loanCharge.isPresent() && loanCharge.get().getCreatedDate().isPresent()) {
return loanCharge.get().getCreatedDate().get();
} else if (loanTransaction.isPresent()) {
return loanTransaction.get().getCreatedDateTime();
return loanTransaction.get().getCreatedDate().orElse(null);
} else {
throw new RuntimeException("Either charge with createdDate or transaction created datetime should be present");
}
Expand All @@ -117,15 +117,15 @@ private OffsetDateTime getCreatedDateTime() {
@Override
@SuppressFBWarnings(value = "EQ_COMPARETO_USE_OBJECT_EQUALS", justification = "TODO: fix this! See: https://stackoverflow.com/questions/2609037/findbugs-how-to-solve-eq-compareto-use-object-equals")
public int compareTo(@NotNull ChangeOperation o) {
int datePortion = DateUtils.compare(this.getEffectiveDate(), o.getEffectiveDate());
int datePortion = DateUtils.compareWithNullsLast(this.getEffectiveDate(), o.getEffectiveDate());
if (datePortion == 0) {
final boolean isAccrual = isAccrualActivity();
if (isAccrual != o.isAccrualActivity()) {
return isAccrual ? 1 : -1;
}
int submittedDate = DateUtils.compare(getSubmittedOnDate(), o.getSubmittedOnDate());
int submittedDate = DateUtils.compareWithNullsLast(getSubmittedOnDate(), o.getSubmittedOnDate());
if (submittedDate == 0) {
return DateUtils.compare(getCreatedDateTime(), o.getCreatedDateTime(), null);
return DateUtils.compareWithNullsLast(getCreatedDateTime(), o.getCreatedDateTime());
}
return submittedDate;
}
Expand Down
2 changes: 1 addition & 1 deletion .../fineract/portfolio/loanaccount/domain/transactionprocessor/impl/ChangeOperationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ private ChangeOperation createTransaction(String transactionDate, String submitt
LoanTransaction transaction = Mockito.mock(LoanTransaction.class);
Mockito.when(transaction.getSubmittedOnDate()).thenReturn(LocalDate.parse(submittedDate));
Mockito.when(transaction.getTransactionDate()).thenReturn(LocalDate.parse(transactionDate));
Mockito.when(transaction.getCreatedDateTime()).thenReturn(OffsetDateTime.parse(creationDateTime));
Mockito.when(transaction.getCreatedDate()).thenReturn(Optional.of(OffsetDateTime.parse(creationDateTime)));
return new ChangeOperation(transaction);
}

Expand Down
7 changes: 5 additions & 2 deletions ...rg/apache/fineract/infrastructure/campaigns/sms/service/SmsCampaignDomainServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import jakarta.annotation.PostConstruct;
import java.io.IOException;
import java.security.InvalidParameterException;
import java.time.OffsetDateTime;
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.Collection;
Expand All @@ -40,6 +41,7 @@
import org.apache.fineract.infrastructure.campaigns.sms.domain.SmsCampaignRepository;
import org.apache.fineract.infrastructure.campaigns.sms.exception.SmsRuntimeException;
import org.apache.fineract.infrastructure.campaigns.sms.serialization.SmsCampaignValidator;
import org.apache.fineract.infrastructure.core.service.DateUtils;
import org.apache.fineract.infrastructure.event.business.BusinessEventListener;
import org.apache.fineract.infrastructure.event.business.domain.client.ClientActivateBusinessEvent;
import org.apache.fineract.infrastructure.event.business.domain.client.ClientRejectBusinessEvent;
Expand Down Expand Up @@ -330,9 +332,10 @@ private HashMap<String, Object> processRepaymentDataForSms(final LoanTransaction
smsParams.put("loanOfficerId", -1);
}

OffsetDateTime creationDate = loanTransaction.getCreatedDate().orElse(DateUtils.getAuditOffsetDateTime());
smsParams.put("repaymentAmount", loanTransaction.getAmount(loan.getCurrency()));
smsParams.put("RepaymentDate", loanTransaction.getCreatedDateTime().toLocalDate().format(dateFormatter));
smsParams.put("RepaymentTime", loanTransaction.getCreatedDateTime().toLocalTime().format(timeFormatter));
smsParams.put("RepaymentDate", creationDate.toLocalDate().format(dateFormatter));
smsParams.put("RepaymentTime", creationDate.toLocalTime().format(timeFormatter));

if (loanTransaction.getPaymentDetail() != null) {
smsParams.put("receiptNumber", loanTransaction.getPaymentDetail().getReceiptNumber());
Expand Down
Loading

0 comments on commit 39da688

Please sign in to comment.