You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fineract.apache.org by "adamsaghy (via GitHub)" <gi...@apache.org> on 2026/07/09 07:55:03 UTC
[PR] FINERACT-2455: Simplify reset logic [fineract]
adamsaghy opened a new pull request, #6109:
URL: https://github.com/apache/fineract/pull/6109
## 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 [our guidelines](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#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](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made.
- [ ] Follow our [coding conventions](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
- [ ] [This PR must not be a "code dump"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute).
Your assigned reviewer(s) will follow our [guidelines for code reviews](https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3553072957
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanActiveBreachResetResolver.java:
##########
@@ -39,71 +37,25 @@ public class WorkingCapitalLoanActiveBreachResetResolver {
private final WorkingCapitalLoanBreachActionRepository breachActionRepository;
public Optional<WorkingCapitalLoanBreachAction> findLatestActiveReset(final Long workingCapitalLoanId) {
- return findLatestActiveReset(workingCapitalLoanId, null);
- }
-
- public Optional<LocalDate> findLatestActiveResetDate(final Long workingCapitalLoanId) {
- return findLatestActiveReset(workingCapitalLoanId).map(WorkingCapitalLoanBreachAction::getStartDate);
- }
-
- public Optional<WorkingCapitalLoanBreachAction> findLatestActiveReset(final Long workingCapitalLoanId,
- final Long maxActionIdExclusive) {
- if (workingCapitalLoanId == null) {
- return Optional.empty();
- }
- final List<WorkingCapitalLoanBreachAction> actions = loadActions(workingCapitalLoanId, maxActionIdExclusive);
- final Optional<Long> latestActiveResetId = findLatestActiveResetId(actions);
- return latestActiveResetId
- .flatMap(activeResetId -> actions.stream().filter(action -> Objects.equals(action.getId(), activeResetId)).findFirst());
+ List<WorkingCapitalLoanBreachAction> filtereredList = breachActionRepository.findByLoanAndActionType(workingCapitalLoanId,
+ List.of(WorkingCapitalLoanBreachActionType.RESET, WorkingCapitalLoanBreachActionType.UNDO_RESET));
+ Deque<WorkingCapitalLoanBreachAction> stack = new ArrayDeque<>();
+ filtereredList.forEach(action -> {
+ if (action.getAction().equals(WorkingCapitalLoanBreachActionType.RESET)) {
+ stack.push(action);
+ } else if (action.getAction().equals(WorkingCapitalLoanBreachActionType.UNDO_RESET)) {
+ stack.pop();
Review Comment:
business rules does not allow such situation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "mariiaKraievska (via GitHub)" <gi...@apache.org>.
mariiaKraievska commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3550694442
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/repository/WorkingCapitalLoanBreachActionRepository.java:
##########
@@ -41,4 +41,33 @@ select case when count(action) > 0 then true else false end from WorkingCapitalL
""")
boolean isBreachDisabledAsOf(@Param("loanId") Long loanId, @Param("date") LocalDate date);
+ @Query("""
+ select resetAction from WorkingCapitalLoanBreachAction resetAction
+ where resetAction.workingCapitalLoan.id = :workingCapitalLoanId
+ and resetAction.action = org.apache.fineract.portfolio.workingcapitalloan.domain.WorkingCapitalLoanBreachActionType.RESET
+ and resetAction.id = (
+ select max(latestReset.id) from WorkingCapitalLoanBreachAction latestReset
+ where latestReset.workingCapitalLoan.id = :workingCapitalLoanId
+ and latestReset.action = org.apache.fineract.portfolio.workingcapitalloan.domain.WorkingCapitalLoanBreachActionType.RESET
+ )
+ and not exists (
+ select undoResetAction.id from WorkingCapitalLoanBreachAction undoResetAction
+ where undoResetAction.workingCapitalLoan.id = :workingCapitalLoanId
+ and undoResetAction.action = org.apache.fineract.portfolio.workingcapitalloan.domain.WorkingCapitalLoanBreachActionType.UNDO_RESET
+ and undoResetAction.id > resetAction.id
+ )
+ """)
+ Optional<WorkingCapitalLoanBreachAction> findLatestActiveReset(@Param("workingCapitalLoanId") Long workingCapitalLoanId);
Review Comment:
Could we revisit this query? It currently checks activity only for the latest RESET id, so after undoing the latest reset it may return no active reset even when an earlier RESET is still active (LIFO case, @TestRailId:C85431).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "mariiaKraievska (via GitHub)" <gi...@apache.org>.
mariiaKraievska commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3550822046
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanBreachScheduleServiceImpl.java:
##########
@@ -148,22 +148,21 @@ public boolean evaluateBreachOnDate(final WorkingCapitalLoanBreachSchedule perio
@Override
public void applyRepayment(final Long loanId, final LocalDate transactionDate, final BigDecimal amount) {
- if (loanId == null || transactionDate == null || amount == null) {
- return;
- }
if (isBreachEvaluationDisabled(loanId, DateUtils.getBusinessLocalDate())) {
log.debug("Skipping breach schedule repayment update for WC loan {} - breach evaluation is disabled", loanId);
return;
}
- final Optional<LocalDate> lastResetDate = activeBreachResetResolver.findLatestActiveResetDate(loanId);
- if (lastResetDate.isPresent() && transactionDate.isBefore(lastResetDate.get())) {
- log.debug("Ignoring backdated repayment on {} for WC loan {} breach schedule (last reset on {})", transactionDate, loanId,
- lastResetDate.get());
- return;
- }
+
final Optional<WorkingCapitalLoanBreachSchedule> currentPeriod = repository
.findByLoanIdAndFromDateLessThanEqualAndToDateGreaterThanEqual(loanId, transactionDate, transactionDate);
- currentPeriod.ifPresent(period -> applyRepayment(period, amount, loanId));
+ currentPeriod.ifPresent(period -> {
+ if (period.isReset()) {
Review Comment:
Do we set/clear period.reset anywhere during reset/undo? Since this guard depends on period.isReset(), but I couldn’t find corresponding setReset(true/false) updates Iʼm worring if this guard is effective in practice
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "mariiaKraievska (via GitHub)" <gi...@apache.org>.
mariiaKraievska commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3552358740
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanBreachScheduleServiceImpl.java:
##########
@@ -306,35 +306,65 @@ public void recalculatePeriodsForPauses(final WorkingCapitalLoan loan) {
}
@Override
- public void reEvaluateAfterEnable(final WorkingCapitalLoan loan, final LocalDate enableDate) {
- final Optional<WorkingCapitalBreach> breachOpt = getBreachConfig(loan);
- if (breachOpt.isEmpty()) {
+ public void reprocessBreachSchedule(final WorkingCapitalLoan loan) {
+ final LocalDate businessDate = DateUtils.getBusinessLocalDate();
+ generateNextPeriodIfNeeded(loan, businessDate);
+ List<WorkingCapitalLoanBreachSchedule> breachPeriods = resetAllPeriodsForReprocessing(loan.getId());
+
+ final List<TransactionDateAndAmountHolder> transactionDateAndAmountHolderList = transactionRepository
+ .fetchTransactionDateAndAmount(loan.getId(), LoanTransactionType.getRepaymentLikeTransactionTypes());
Review Comment:
Same question for breach: reprocess uses transactionAmount here, but incremental updates in WorkingCapitalLoanWritePlatformServiceImpl still use principalPortion (line 778, line 1031). Would aligning on transactionAmount everywhere make sense?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "mariiaKraievska (via GitHub)" <gi...@apache.org>.
mariiaKraievska commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3550573612
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanBreachScheduleServiceImpl.java:
##########
@@ -306,32 +304,51 @@ public void recalculatePeriodsForPauses(final WorkingCapitalLoan loan) {
}
@Override
- public void reEvaluateAfterEnable(final WorkingCapitalLoan loan, final LocalDate enableDate) {
- final Optional<WorkingCapitalBreach> breachOpt = getBreachConfig(loan);
- if (breachOpt.isEmpty()) {
+ public void reprocessDelinquencySchedule(final WorkingCapitalLoan loan) {
+ final LocalDate businessDate = DateUtils.getBusinessLocalDate();
+ generateNextPeriodIfNeeded(loan, businessDate);
+ List<WorkingCapitalLoanBreachSchedule> breachPeriods = resetAllPeriodsForReprocessing(loan.getId());
+
+ final List<TransactionDateAndAmountHolder> transactionDateAndAmountHolderList = transactionRepository
+ .fetchTransactionDateAndAmount(loan.getId(), LoanTransactionType.getRepaymentLikeTransactionTypes());
+ if (transactionDateAndAmountHolderList.isEmpty()) {
+ breachPeriods.forEach(period -> {
+ recomputeBreach(period, businessDate);
+ });
return;
}
- generateNextPeriodIfNeeded(loan, enableDate);
- final List<WorkingCapitalLoanBreachSchedule> periods = repository.findByLoanIdOrderByPeriodNumberAsc(loan.getId());
- final Optional<LocalDate> lastResetDate = activeBreachResetResolver.findLatestActiveResetDate(loan.getId());
- periods.stream().filter(period -> isNotNullifiedByActiveReset(period, lastResetDate)).forEach(period -> {
- rebuildPaidAmountFromTransactions(period, lastResetDate);
- recomputeBreach(period, enableDate);
+
+ List<TransactionDateAndAmountHolder> remappedTransactionDateAndAmountHolderList = new ArrayList<>();
+ breachPeriods.forEach(period -> {
+ BigDecimal sumAmount = transactionDateAndAmountHolderList.stream().parallel()
+ .filter(holder -> DateUtils.isDateInRangeInclusive(holder.transactionDate(), period.getFromDate(), period.getToDate()))
+ .map(TransactionDateAndAmountHolder::transactionAmount).reduce(BigDecimal.ZERO, BigDecimal::add);
+ remappedTransactionDateAndAmountHolderList.add(new TransactionDateAndAmountHolder(period.getToDate(), sumAmount));
});
- repository.saveAll(periods);
- log.debug("Re-evaluated breach schedule for WC loan {} after enable as of {}", loan.getId(), enableDate);
+
+ remappedTransactionDateAndAmountHolderList.forEach(transactionDateAndAmountHolder -> {
+ applyRepayment(loan.getId(), transactionDateAndAmountHolder.transactionDate(),
+ transactionDateAndAmountHolder.transactionAmount());
+ });
+
+ evaluateExpiredPeriods(loan, breachPeriods);
}
- private boolean isNotNullifiedByActiveReset(final WorkingCapitalLoanBreachSchedule period, final Optional<LocalDate> lastResetDate) {
- return lastResetDate.map(resetDate -> !period.getToDate().isBefore(resetDate)).orElse(true);
+ private List<WorkingCapitalLoanBreachSchedule> resetAllPeriodsForReprocessing(final Long loanId) {
+ final List<WorkingCapitalLoanBreachSchedule> periods = repository.findByLoanIdOrderByPeriodNumberAsc(loanId);
+ for (final WorkingCapitalLoanBreachSchedule period : periods) {
+ period.setPaidAmount(BigDecimal.ZERO);
+ period.setOutstandingAmount(period.getMinPaymentAmount());
+ period.setBreach(null);
+ }
+ return periods;
}
- private void rebuildPaidAmountFromTransactions(final WorkingCapitalLoanBreachSchedule period, final Optional<LocalDate> lastResetDate) {
- final LocalDate fromDate = lastResetDate.filter(resetDate -> resetDate.isAfter(period.getFromDate())).orElse(period.getFromDate());
- final BigDecimal paidAmount = transactionRepository.sumBreachRelevantPaid(period.getLoan().getId(),
- LoanTransactionType.getRepaymentLikeTransactionTypes(), fromDate, period.getToDate());
- period.setPaidAmount(paidAmount);
- period.setOutstandingAmount(period.getMinPaymentAmount().subtract(paidAmount).max(BigDecimal.ZERO));
+ private void evaluateExpiredPeriods(WorkingCapitalLoan loan, List<WorkingCapitalLoanBreachSchedule> breachPeriods) {
Review Comment:
loan is not used in this method, I believe it can be removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "budaidev (via GitHub)" <gi...@apache.org>.
budaidev commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3550080466
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/repository/WorkingCapitalLoanTransactionRepository.java:
##########
@@ -89,4 +90,11 @@ BigDecimal sumBreachRelevantPaid(@Param("wcLoanId") Long wcLoanId,
@Param("transactionTypes") Collection<LoanTransactionType> transactionTypes, @Param("fromDate") LocalDate fromDate,
@Param("toDate") LocalDate toDate);
+ @Query("""
+ SELECT t.transactionDate, SUM(t.transactionAmount)
+ FROM WorkingCapitalLoanTransaction t
+ WHERE t.reversed = FALSE
+ GROUP BY t.transactionDate
+ """)
+ List<TransactionDateAndAmountHolder> fetchTransactionDateAndAmount();
Review Comment:
Also one addition: this table holds all transactions, but the previous implementetion filtered to the isRepaymentType, so if we use this directly we might diverge
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "mariiaKraievska (via GitHub)" <gi...@apache.org>.
mariiaKraievska commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3550519298
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanBreachScheduleServiceImpl.java:
##########
@@ -306,32 +304,51 @@ public void recalculatePeriodsForPauses(final WorkingCapitalLoan loan) {
}
@Override
- public void reEvaluateAfterEnable(final WorkingCapitalLoan loan, final LocalDate enableDate) {
- final Optional<WorkingCapitalBreach> breachOpt = getBreachConfig(loan);
- if (breachOpt.isEmpty()) {
+ public void reprocessDelinquencySchedule(final WorkingCapitalLoan loan) {
+ final LocalDate businessDate = DateUtils.getBusinessLocalDate();
+ generateNextPeriodIfNeeded(loan, businessDate);
+ List<WorkingCapitalLoanBreachSchedule> breachPeriods = resetAllPeriodsForReprocessing(loan.getId());
+
+ final List<TransactionDateAndAmountHolder> transactionDateAndAmountHolderList = transactionRepository
+ .fetchTransactionDateAndAmount(loan.getId(), LoanTransactionType.getRepaymentLikeTransactionTypes());
+ if (transactionDateAndAmountHolderList.isEmpty()) {
+ breachPeriods.forEach(period -> {
+ recomputeBreach(period, businessDate);
+ });
return;
}
- generateNextPeriodIfNeeded(loan, enableDate);
- final List<WorkingCapitalLoanBreachSchedule> periods = repository.findByLoanIdOrderByPeriodNumberAsc(loan.getId());
- final Optional<LocalDate> lastResetDate = activeBreachResetResolver.findLatestActiveResetDate(loan.getId());
- periods.stream().filter(period -> isNotNullifiedByActiveReset(period, lastResetDate)).forEach(period -> {
- rebuildPaidAmountFromTransactions(period, lastResetDate);
- recomputeBreach(period, enableDate);
+
+ List<TransactionDateAndAmountHolder> remappedTransactionDateAndAmountHolderList = new ArrayList<>();
+ breachPeriods.forEach(period -> {
+ BigDecimal sumAmount = transactionDateAndAmountHolderList.stream().parallel()
+ .filter(holder -> DateUtils.isDateInRangeInclusive(holder.transactionDate(), period.getFromDate(), period.getToDate()))
+ .map(TransactionDateAndAmountHolder::transactionAmount).reduce(BigDecimal.ZERO, BigDecimal::add);
+ remappedTransactionDateAndAmountHolderList.add(new TransactionDateAndAmountHolder(period.getToDate(), sumAmount));
});
- repository.saveAll(periods);
- log.debug("Re-evaluated breach schedule for WC loan {} after enable as of {}", loan.getId(), enableDate);
+
+ remappedTransactionDateAndAmountHolderList.forEach(transactionDateAndAmountHolder -> {
+ applyRepayment(loan.getId(), transactionDateAndAmountHolder.transactionDate(),
+ transactionDateAndAmountHolder.transactionAmount());
+ });
+
+ evaluateExpiredPeriods(loan, breachPeriods);
}
- private boolean isNotNullifiedByActiveReset(final WorkingCapitalLoanBreachSchedule period, final Optional<LocalDate> lastResetDate) {
- return lastResetDate.map(resetDate -> !period.getToDate().isBefore(resetDate)).orElse(true);
+ private List<WorkingCapitalLoanBreachSchedule> resetAllPeriodsForReprocessing(final Long loanId) {
+ final List<WorkingCapitalLoanBreachSchedule> periods = repository.findByLoanIdOrderByPeriodNumberAsc(loanId);
+ for (final WorkingCapitalLoanBreachSchedule period : periods) {
+ period.setPaidAmount(BigDecimal.ZERO);
+ period.setOutstandingAmount(period.getMinPaymentAmount());
+ period.setBreach(null);
+ }
+ return periods;
}
- private void rebuildPaidAmountFromTransactions(final WorkingCapitalLoanBreachSchedule period, final Optional<LocalDate> lastResetDate) {
- final LocalDate fromDate = lastResetDate.filter(resetDate -> resetDate.isAfter(period.getFromDate())).orElse(period.getFromDate());
- final BigDecimal paidAmount = transactionRepository.sumBreachRelevantPaid(period.getLoan().getId(),
- LoanTransactionType.getRepaymentLikeTransactionTypes(), fromDate, period.getToDate());
- period.setPaidAmount(paidAmount);
- period.setOutstandingAmount(period.getMinPaymentAmount().subtract(paidAmount).max(BigDecimal.ZERO));
+ private void evaluateExpiredPeriods(WorkingCapitalLoan loan, List<WorkingCapitalLoanBreachSchedule> breachPeriods) {
+ breachPeriods.stream().filter(period -> period.getBreach() == null).toList().forEach(period -> {
+ boolean criteriaMet = period.getPaidAmount().compareTo(period.getMinPaymentAmount()) >= 0;
+ period.setBreach(criteriaMet);
Review Comment:
It looks like breach is set to true when paidAmount >= minPaymentAmount, which might be the opposite of the expected behavior. Would it make sense to align this with recomputeBreach() logic for consistency?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "Cocoa-Puffs (via GitHub)" <gi...@apache.org>.
Cocoa-Puffs commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3549966827
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/repository/WorkingCapitalLoanTransactionRepository.java:
##########
@@ -89,4 +90,11 @@ BigDecimal sumBreachRelevantPaid(@Param("wcLoanId") Long wcLoanId,
@Param("transactionTypes") Collection<LoanTransactionType> transactionTypes, @Param("fromDate") LocalDate fromDate,
@Param("toDate") LocalDate toDate);
+ @Query("""
+ SELECT t.transactionDate, SUM(t.transactionAmount)
+ FROM WorkingCapitalLoanTransaction t
+ WHERE t.reversed = FALSE
+ GROUP BY t.transactionDate
+ """)
+ List<TransactionDateAndAmountHolder> fetchTransactionDateAndAmount();
Review Comment:
Should we filter for loan id here?
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanActiveBreachResetResolver.java:
##########
@@ -38,72 +33,26 @@ public class WorkingCapitalLoanActiveBreachResetResolver {
private final WorkingCapitalLoanBreachActionRepository breachActionRepository;
- public Optional<WorkingCapitalLoanBreachAction> findLatestActiveReset(final Long workingCapitalLoanId) {
- return findLatestActiveReset(workingCapitalLoanId, null);
+ private Optional<WorkingCapitalLoanBreachAction> findLatestActiveReset(final Long workingCapitalLoanId) {
+ return breachActionRepository.findLatestActiveReset(workingCapitalLoanId);
}
- public Optional<LocalDate> findLatestActiveResetDate(final Long workingCapitalLoanId) {
- return findLatestActiveReset(workingCapitalLoanId).map(WorkingCapitalLoanBreachAction::getStartDate);
- }
-
- public Optional<WorkingCapitalLoanBreachAction> findLatestActiveReset(final Long workingCapitalLoanId,
- final Long maxActionIdExclusive) {
+ public Optional<WorkingCapitalLoanBreachAction> findLatestReset(final Long workingCapitalLoanId) {
if (workingCapitalLoanId == null) {
return Optional.empty();
}
- final List<WorkingCapitalLoanBreachAction> actions = loadActions(workingCapitalLoanId, maxActionIdExclusive);
- final Optional<Long> latestActiveResetId = findLatestActiveResetId(actions);
- return latestActiveResetId
- .flatMap(activeResetId -> actions.stream().filter(action -> Objects.equals(action.getId(), activeResetId)).findFirst());
+ return breachActionRepository.findLatestReset(workingCapitalLoanId);
}
public boolean hasActiveReset(final Long workingCapitalLoanId) {
return findLatestActiveReset(workingCapitalLoanId).isPresent();
}
public boolean existsActiveResetInPeriod(final Long workingCapitalLoanId, final LocalDate fromDate, final LocalDate toDate) {
- if (workingCapitalLoanId == null || fromDate == null || toDate == null) {
- return false;
- }
- final List<WorkingCapitalLoanBreachAction> actions = loadActions(workingCapitalLoanId, null);
- final Set<Long> activeResetIds = findActiveResetIds(actions);
- return actions.stream().filter(action -> WorkingCapitalLoanBreachActionType.RESET.equals(action.getAction()))
- .filter(action -> action.getId() != null && activeResetIds.contains(action.getId()))
- .anyMatch(action -> isDateWithinPeriod(action.getStartDate(), fromDate, toDate));
- }
-
- private List<WorkingCapitalLoanBreachAction> loadActions(final Long workingCapitalLoanId, final Long maxActionIdExclusive) {
- return breachActionRepository.findByWorkingCapitalLoanIdOrderById(workingCapitalLoanId).stream()
- .filter(action -> action.getId() != null && (maxActionIdExclusive == null || action.getId() < maxActionIdExclusive))
- .toList();
- }
-
- private Optional<Long> findLatestActiveResetId(final List<WorkingCapitalLoanBreachAction> actions) {
- final Deque<Long> activeResetIds = buildActiveResetStack(actions);
- return activeResetIds.isEmpty() ? Optional.empty() : Optional.of(activeResetIds.peekLast());
+ AtomicBoolean exists = new AtomicBoolean(false);
+ findLatestActiveReset(workingCapitalLoanId).ifPresent(workingCapitalLoanBreachAction -> {
+ exists.set(DateUtils.isDateInRangeInclusive(workingCapitalLoanBreachAction.getStartDate(), fromDate, toDate));
+ });
+ return exists.get();
Review Comment:
Instead of the atomic boolean maybe we could do
`return findLatestActiveReset(workingCapitalLoanId).filter(a -> DateUtils.isDateInRangeInclusive(a.getStartDate(), fromDate, toDate)).isPresent();`
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanBreachScheduleServiceImpl.java:
##########
Review Comment:
I think we should add `period.setReset(false)`. If JPA generates the insert with reset = null explicitly instead of omitting the field, it will fail because of the non null constraint.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3550085713
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanBreachScheduleServiceImpl.java:
##########
Review Comment:
Not really, since the default value is false on the column
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3550070163
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/repository/WorkingCapitalLoanTransactionRepository.java:
##########
@@ -89,4 +90,11 @@ BigDecimal sumBreachRelevantPaid(@Param("wcLoanId") Long wcLoanId,
@Param("transactionTypes") Collection<LoanTransactionType> transactionTypes, @Param("fromDate") LocalDate fromDate,
@Param("toDate") LocalDate toDate);
+ @Query("""
+ SELECT t.transactionDate, SUM(t.transactionAmount)
+ FROM WorkingCapitalLoanTransaction t
+ WHERE t.reversed = FALSE
+ GROUP BY t.transactionDate
+ """)
+ List<TransactionDateAndAmountHolder> fetchTransactionDateAndAmount();
Review Comment:
Nice catch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3550559243
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanBreachScheduleServiceImpl.java:
##########
@@ -306,32 +304,51 @@ public void recalculatePeriodsForPauses(final WorkingCapitalLoan loan) {
}
@Override
- public void reEvaluateAfterEnable(final WorkingCapitalLoan loan, final LocalDate enableDate) {
- final Optional<WorkingCapitalBreach> breachOpt = getBreachConfig(loan);
- if (breachOpt.isEmpty()) {
+ public void reprocessDelinquencySchedule(final WorkingCapitalLoan loan) {
+ final LocalDate businessDate = DateUtils.getBusinessLocalDate();
+ generateNextPeriodIfNeeded(loan, businessDate);
+ List<WorkingCapitalLoanBreachSchedule> breachPeriods = resetAllPeriodsForReprocessing(loan.getId());
+
+ final List<TransactionDateAndAmountHolder> transactionDateAndAmountHolderList = transactionRepository
+ .fetchTransactionDateAndAmount(loan.getId(), LoanTransactionType.getRepaymentLikeTransactionTypes());
+ if (transactionDateAndAmountHolderList.isEmpty()) {
+ breachPeriods.forEach(period -> {
+ recomputeBreach(period, businessDate);
+ });
return;
}
- generateNextPeriodIfNeeded(loan, enableDate);
- final List<WorkingCapitalLoanBreachSchedule> periods = repository.findByLoanIdOrderByPeriodNumberAsc(loan.getId());
- final Optional<LocalDate> lastResetDate = activeBreachResetResolver.findLatestActiveResetDate(loan.getId());
- periods.stream().filter(period -> isNotNullifiedByActiveReset(period, lastResetDate)).forEach(period -> {
- rebuildPaidAmountFromTransactions(period, lastResetDate);
- recomputeBreach(period, enableDate);
+
+ List<TransactionDateAndAmountHolder> remappedTransactionDateAndAmountHolderList = new ArrayList<>();
+ breachPeriods.forEach(period -> {
+ BigDecimal sumAmount = transactionDateAndAmountHolderList.stream().parallel()
+ .filter(holder -> DateUtils.isDateInRangeInclusive(holder.transactionDate(), period.getFromDate(), period.getToDate()))
+ .map(TransactionDateAndAmountHolder::transactionAmount).reduce(BigDecimal.ZERO, BigDecimal::add);
+ remappedTransactionDateAndAmountHolderList.add(new TransactionDateAndAmountHolder(period.getToDate(), sumAmount));
});
- repository.saveAll(periods);
- log.debug("Re-evaluated breach schedule for WC loan {} after enable as of {}", loan.getId(), enableDate);
+
+ remappedTransactionDateAndAmountHolderList.forEach(transactionDateAndAmountHolder -> {
+ applyRepayment(loan.getId(), transactionDateAndAmountHolder.transactionDate(),
+ transactionDateAndAmountHolder.transactionAmount());
+ });
+
+ evaluateExpiredPeriods(loan, breachPeriods);
}
- private boolean isNotNullifiedByActiveReset(final WorkingCapitalLoanBreachSchedule period, final Optional<LocalDate> lastResetDate) {
- return lastResetDate.map(resetDate -> !period.getToDate().isBefore(resetDate)).orElse(true);
+ private List<WorkingCapitalLoanBreachSchedule> resetAllPeriodsForReprocessing(final Long loanId) {
+ final List<WorkingCapitalLoanBreachSchedule> periods = repository.findByLoanIdOrderByPeriodNumberAsc(loanId);
+ for (final WorkingCapitalLoanBreachSchedule period : periods) {
+ period.setPaidAmount(BigDecimal.ZERO);
+ period.setOutstandingAmount(period.getMinPaymentAmount());
+ period.setBreach(null);
+ }
+ return periods;
}
- private void rebuildPaidAmountFromTransactions(final WorkingCapitalLoanBreachSchedule period, final Optional<LocalDate> lastResetDate) {
- final LocalDate fromDate = lastResetDate.filter(resetDate -> resetDate.isAfter(period.getFromDate())).orElse(period.getFromDate());
- final BigDecimal paidAmount = transactionRepository.sumBreachRelevantPaid(period.getLoan().getId(),
- LoanTransactionType.getRepaymentLikeTransactionTypes(), fromDate, period.getToDate());
- period.setPaidAmount(paidAmount);
- period.setOutstandingAmount(period.getMinPaymentAmount().subtract(paidAmount).max(BigDecimal.ZERO));
+ private void evaluateExpiredPeriods(WorkingCapitalLoan loan, List<WorkingCapitalLoanBreachSchedule> breachPeriods) {
+ breachPeriods.stream().filter(period -> period.getBreach() == null).toList().forEach(period -> {
+ boolean criteriaMet = period.getPaidAmount().compareTo(period.getMinPaymentAmount()) >= 0;
+ period.setBreach(criteriaMet);
Review Comment:
Nice catch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3553069901
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/domain/WorkingCapitalLoanBreachSchedule.java:
##########
@@ -71,4 +71,18 @@ public class WorkingCapitalLoanBreachSchedule extends AbstractAuditableWithUTCDa
@Column(name = "breach")
private Boolean breach;
+ @Column(name = "reset", nullable = false)
+ private boolean reset;
+
+ public void reset() {
+ this.reset = true;
+ this.breach = null;
+ this.nearBreach = null;
+ this.outstandingAmount = null;
+ this.paidAmount = null;
+ }
+
+ public BigDecimal getMinPaymentAmount() {
+ return reset ? null : this.minPaymentAmount;
Review Comment:
safe guards are there
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "mariiaKraievska (via GitHub)" <gi...@apache.org>.
mariiaKraievska commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3550565775
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanBreachScheduleServiceImpl.java:
##########
@@ -306,32 +304,51 @@ public void recalculatePeriodsForPauses(final WorkingCapitalLoan loan) {
}
@Override
- public void reEvaluateAfterEnable(final WorkingCapitalLoan loan, final LocalDate enableDate) {
- final Optional<WorkingCapitalBreach> breachOpt = getBreachConfig(loan);
- if (breachOpt.isEmpty()) {
+ public void reprocessDelinquencySchedule(final WorkingCapitalLoan loan) {
+ final LocalDate businessDate = DateUtils.getBusinessLocalDate();
+ generateNextPeriodIfNeeded(loan, businessDate);
+ List<WorkingCapitalLoanBreachSchedule> breachPeriods = resetAllPeriodsForReprocessing(loan.getId());
+
+ final List<TransactionDateAndAmountHolder> transactionDateAndAmountHolderList = transactionRepository
+ .fetchTransactionDateAndAmount(loan.getId(), LoanTransactionType.getRepaymentLikeTransactionTypes());
+ if (transactionDateAndAmountHolderList.isEmpty()) {
+ breachPeriods.forEach(period -> {
+ recomputeBreach(period, businessDate);
+ });
return;
}
- generateNextPeriodIfNeeded(loan, enableDate);
- final List<WorkingCapitalLoanBreachSchedule> periods = repository.findByLoanIdOrderByPeriodNumberAsc(loan.getId());
- final Optional<LocalDate> lastResetDate = activeBreachResetResolver.findLatestActiveResetDate(loan.getId());
- periods.stream().filter(period -> isNotNullifiedByActiveReset(period, lastResetDate)).forEach(period -> {
- rebuildPaidAmountFromTransactions(period, lastResetDate);
- recomputeBreach(period, enableDate);
+
+ List<TransactionDateAndAmountHolder> remappedTransactionDateAndAmountHolderList = new ArrayList<>();
+ breachPeriods.forEach(period -> {
+ BigDecimal sumAmount = transactionDateAndAmountHolderList.stream().parallel()
+ .filter(holder -> DateUtils.isDateInRangeInclusive(holder.transactionDate(), period.getFromDate(), period.getToDate()))
+ .map(TransactionDateAndAmountHolder::transactionAmount).reduce(BigDecimal.ZERO, BigDecimal::add);
+ remappedTransactionDateAndAmountHolderList.add(new TransactionDateAndAmountHolder(period.getToDate(), sumAmount));
});
- repository.saveAll(periods);
- log.debug("Re-evaluated breach schedule for WC loan {} after enable as of {}", loan.getId(), enableDate);
+
+ remappedTransactionDateAndAmountHolderList.forEach(transactionDateAndAmountHolder -> {
+ applyRepayment(loan.getId(), transactionDateAndAmountHolder.transactionDate(),
+ transactionDateAndAmountHolder.transactionAmount());
+ });
+
+ evaluateExpiredPeriods(loan, breachPeriods);
}
- private boolean isNotNullifiedByActiveReset(final WorkingCapitalLoanBreachSchedule period, final Optional<LocalDate> lastResetDate) {
- return lastResetDate.map(resetDate -> !period.getToDate().isBefore(resetDate)).orElse(true);
+ private List<WorkingCapitalLoanBreachSchedule> resetAllPeriodsForReprocessing(final Long loanId) {
+ final List<WorkingCapitalLoanBreachSchedule> periods = repository.findByLoanIdOrderByPeriodNumberAsc(loanId);
+ for (final WorkingCapitalLoanBreachSchedule period : periods) {
+ period.setPaidAmount(BigDecimal.ZERO);
+ period.setOutstandingAmount(period.getMinPaymentAmount());
+ period.setBreach(null);
+ }
+ return periods;
}
- private void rebuildPaidAmountFromTransactions(final WorkingCapitalLoanBreachSchedule period, final Optional<LocalDate> lastResetDate) {
- final LocalDate fromDate = lastResetDate.filter(resetDate -> resetDate.isAfter(period.getFromDate())).orElse(period.getFromDate());
- final BigDecimal paidAmount = transactionRepository.sumBreachRelevantPaid(period.getLoan().getId(),
- LoanTransactionType.getRepaymentLikeTransactionTypes(), fromDate, period.getToDate());
- period.setPaidAmount(paidAmount);
- period.setOutstandingAmount(period.getMinPaymentAmount().subtract(paidAmount).max(BigDecimal.ZERO));
+ private void evaluateExpiredPeriods(WorkingCapitalLoan loan, List<WorkingCapitalLoanBreachSchedule> breachPeriods) {
+ breachPeriods.stream().filter(period -> period.getBreach() == null).toList().forEach(period -> {
+ boolean criteriaMet = period.getPaidAmount().compareTo(period.getMinPaymentAmount()) >= 0;
+ period.setBreach(criteriaMet);
Review Comment:
Also would it be safer to evaluate only expired periods here (for example, up to current business date)? I’m wondering if future/current periods might be receiving a non-null breach value earlier than expected
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "mariiaKraievska (via GitHub)" <gi...@apache.org>.
mariiaKraievska commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3550746329
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanBreachResetServiceImpl.java:
##########
@@ -91,8 +91,7 @@ public void resetBreach(final WorkingCapitalLoan loan, final WorkingCapitalLoanB
@Override
public void undoResetBreach(final WorkingCapitalLoan loan, final WorkingCapitalLoanBreachAction undoResetAction) {
- final WorkingCapitalLoanBreachAction latestReset = activeBreachResetResolver
- .findLatestActiveReset(loan.getId(), undoResetAction.getId()).orElseThrow();
+ final WorkingCapitalLoanBreachAction latestReset = activeBreachResetResolver.findLatestReset(loan.getId()).orElseThrow();
Review Comment:
Would it make sense here to resolve the latest active reset instead of the latest reset in general, so that in multi-reset flows we can still find a reset when the most recent one has already been undone?
If so, findLatestReset(...) may no longer be needed and could potentially be removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "mariiaKraievska (via GitHub)" <gi...@apache.org>.
mariiaKraievska commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3551012670
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanBreachScheduleServiceImpl.java:
##########
@@ -306,32 +304,51 @@ public void recalculatePeriodsForPauses(final WorkingCapitalLoan loan) {
}
@Override
- public void reEvaluateAfterEnable(final WorkingCapitalLoan loan, final LocalDate enableDate) {
- final Optional<WorkingCapitalBreach> breachOpt = getBreachConfig(loan);
- if (breachOpt.isEmpty()) {
+ public void reprocessDelinquencySchedule(final WorkingCapitalLoan loan) {
Review Comment:
Previously we ignored repayments with transactionDate < activeResetDate in applyRepayment, and rebuild after enable used reset-aware date ranges. That logic seems removed in this PR — is full replay without reset filtering intentional?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "mariiaKraievska (via GitHub)" <gi...@apache.org>.
mariiaKraievska commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3551034258
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanBreachScheduleServiceImpl.java:
##########
@@ -306,32 +304,51 @@ public void recalculatePeriodsForPauses(final WorkingCapitalLoan loan) {
}
@Override
- public void reEvaluateAfterEnable(final WorkingCapitalLoan loan, final LocalDate enableDate) {
- final Optional<WorkingCapitalBreach> breachOpt = getBreachConfig(loan);
- if (breachOpt.isEmpty()) {
+ public void reprocessDelinquencySchedule(final WorkingCapitalLoan loan) {
Review Comment:
Also I believe we should rename this method to reprocessBreachSchedule? reprocessDelinquencySchedule reads like delinquency logic, but this method updates breach schedule only.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "mariiaKraievska (via GitHub)" <gi...@apache.org>.
mariiaKraievska commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3552345946
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanDelinquencyRangeScheduleServiceImpl.java:
##########
@@ -152,42 +147,32 @@ public void applyRepayment(WorkingCapitalLoan loan, LocalDate transactionDate, B
@Override
public void reprocessDelinquencySchedule(final WorkingCapitalLoan loan) {
- final List<WorkingCapitalLoanTransaction> replayableTransactions = transactionRepository
- .findByWcLoan_IdOrderByTransactionDateAscIdAsc(loan.getId()).stream()
- .filter(txn -> !txn.isReversed() && txn.getTransactionType().isRepaymentType()).toList();
-
final LocalDate businessDate = DateUtils.getBusinessLocalDate();
- resetAllPeriodsForReprocessing(loan.getId());
generateNextPeriodIfNeeded(loan, businessDate);
-
- if (replayableTransactions.isEmpty()) {
- evaluateExpiredPeriods(loan, businessDate);
- delinquencyClassificationService.classifyDelinquency(loan, businessDate);
- return;
- }
-
- final Map<Long, WorkingCapitalLoanTransactionAllocation> allocationsByTxnId = allocationRepository
- .findByWcLoanTransactionIdIn(replayableTransactions.stream().map(WorkingCapitalLoanTransaction::getId).toList()).stream()
- .collect(Collectors.toMap(allocation -> allocation.getWcLoanTransaction().getId(), Function.identity()));
-
- for (final WorkingCapitalLoanTransaction txn : replayableTransactions) {
- final WorkingCapitalLoanTransactionAllocation allocation = allocationsByTxnId.get(txn.getId());
- if (allocation == null) {
- continue;
- }
- final BigDecimal principalPortion = allocation.getPrincipalPortion();
- if (principalPortion != null && principalPortion.compareTo(BigDecimal.ZERO) > 0) {
- allocateRepayment(loan, txn.getTransactionDate(), principalPortion);
- }
+ List<WorkingCapitalLoanDelinquencyRangeSchedule> delinquencyRangePeriods = resetAllPeriodsForReprocessing(loan.getId());
+
+ final List<TransactionDateAndAmountHolder> transactionDateAndAmountHolderList = transactionRepository
+ .fetchTransactionDateAndAmount(loan.getId(), LoanTransactionType.getRepaymentLikeTransactionTypes());
Review Comment:
This reprocess method now replays transactionAmount, while WorkingCapitalLoanWritePlatformServiceImpl.applyRepayment (line 775) still passes principalPortion to delinquency. Could we align both delinquency paths on transactionAmount?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "mariiaKraievska (via GitHub)" <gi...@apache.org>.
mariiaKraievska commented on PR #6109:
URL: https://github.com/apache/fineract/pull/6109#issuecomment-4926059110
Everything else LGTM!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "budaidev (via GitHub)" <gi...@apache.org>.
budaidev commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3552635457
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/domain/WorkingCapitalLoanBreachSchedule.java:
##########
@@ -71,4 +71,18 @@ public class WorkingCapitalLoanBreachSchedule extends AbstractAuditableWithUTCDa
@Column(name = "breach")
private Boolean breach;
+ @Column(name = "reset", nullable = false)
+ private boolean reset;
+
+ public void reset() {
+ this.reset = true;
+ this.breach = null;
+ this.nearBreach = null;
+ this.outstandingAmount = null;
+ this.paidAmount = null;
+ }
+
+ public BigDecimal getMinPaymentAmount() {
+ return reset ? null : this.minPaymentAmount;
Review Comment:
this might cause issue down the line, most probably in the WorkingCapitalLoanNearBreachEvaluationServiceImpl.evaluatePeriod, if there is no null check
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanActiveBreachResetResolver.java:
##########
@@ -39,71 +37,25 @@ public class WorkingCapitalLoanActiveBreachResetResolver {
private final WorkingCapitalLoanBreachActionRepository breachActionRepository;
public Optional<WorkingCapitalLoanBreachAction> findLatestActiveReset(final Long workingCapitalLoanId) {
- return findLatestActiveReset(workingCapitalLoanId, null);
- }
-
- public Optional<LocalDate> findLatestActiveResetDate(final Long workingCapitalLoanId) {
- return findLatestActiveReset(workingCapitalLoanId).map(WorkingCapitalLoanBreachAction::getStartDate);
- }
-
- public Optional<WorkingCapitalLoanBreachAction> findLatestActiveReset(final Long workingCapitalLoanId,
- final Long maxActionIdExclusive) {
- if (workingCapitalLoanId == null) {
- return Optional.empty();
- }
- final List<WorkingCapitalLoanBreachAction> actions = loadActions(workingCapitalLoanId, maxActionIdExclusive);
- final Optional<Long> latestActiveResetId = findLatestActiveResetId(actions);
- return latestActiveResetId
- .flatMap(activeResetId -> actions.stream().filter(action -> Objects.equals(action.getId(), activeResetId)).findFirst());
+ List<WorkingCapitalLoanBreachAction> filtereredList = breachActionRepository.findByLoanAndActionType(workingCapitalLoanId,
+ List.of(WorkingCapitalLoanBreachActionType.RESET, WorkingCapitalLoanBreachActionType.UNDO_RESET));
+ Deque<WorkingCapitalLoanBreachAction> stack = new ArrayDeque<>();
+ filtereredList.forEach(action -> {
+ if (action.getAction().equals(WorkingCapitalLoanBreachActionType.RESET)) {
+ stack.push(action);
+ } else if (action.getAction().equals(WorkingCapitalLoanBreachActionType.UNDO_RESET)) {
+ stack.pop();
Review Comment:
what happens if the stack is already empty?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #6109:
URL: https://github.com/apache/fineract/pull/6109
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Simplify reset logic [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #6109:
URL: https://github.com/apache/fineract/pull/6109#discussion_r3550161398
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/repository/WorkingCapitalLoanTransactionRepository.java:
##########
@@ -89,4 +90,11 @@ BigDecimal sumBreachRelevantPaid(@Param("wcLoanId") Long wcLoanId,
@Param("transactionTypes") Collection<LoanTransactionType> transactionTypes, @Param("fromDate") LocalDate fromDate,
@Param("toDate") LocalDate toDate);
+ @Query("""
+ SELECT t.transactionDate, SUM(t.transactionAmount)
+ FROM WorkingCapitalLoanTransaction t
+ WHERE t.reversed = FALSE
+ GROUP BY t.transactionDate
+ """)
+ List<TransactionDateAndAmountHolder> fetchTransactionDateAndAmount();
Review Comment:
Also nice catch: seems like i got distracted yesterday to miss all these filters :D
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org