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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
