You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "alberto-art3ch (via GitHub)" <gi...@apache.org> on 2025/08/03 00:20:00 UTC
[PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
alberto-art3ch opened a new pull request, #4916:
URL: https://github.com/apache/fineract/pull/4916
## Description
We are moving away from _loan.getLoanTransactions()_ calls in the codebase.
We should rely on JQPL queries to fetch only the most necessary and minimal data and have transactions lists which are provided as incoming parameter.
[FINERACT-2324](https://issues.apache.org/jira/browse/FINERACT-2324)
## 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.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "alberto-art3ch (via GitHub)" <gi...@apache.org>.
alberto-art3ch commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2265087468
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransaction.java:
##########
@@ -1023,4 +1029,13 @@ public boolean isBuyDownFeeAmortization() {
public boolean isBuyDownFeeAmortizationAdjustment() {
return LoanTransactionType.BUY_DOWN_FEE_AMORTIZATION_ADJUSTMENT.equals(this.typeOf);
}
+
+ public void markAsProcessed() {
Review Comment:
Done code updated!
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransaction.java:
##########
@@ -144,6 +145,9 @@ public class LoanTransaction extends AbstractAuditableWithUTCDateTimeCustom<Long
@OneToOne(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.LAZY, mappedBy = "loanTransaction")
private LoanReAgeParameter loanReAgeParameter;
+ @Transient
+ private boolean isProcessed = true;
Review Comment:
Done code updated!
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "alberto-art3ch (via GitHub)" <gi...@apache.org>.
alberto-art3ch commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2258825210
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionService.java:
##########
@@ -56,4 +60,35 @@ private Predicate<LoanTransaction> loanTransactionForReprocessingPredicate() {
|| !transaction.isNonMonetaryTransaction() || transaction.isContractTermination());
}
+ public boolean hasChargeOffTransaction(final Loan loan) {
+ return loanTransactionRepository.hasChargeOffTransaction(loan);
+ }
+
+ public boolean hasContractTerminationTransaction(final Loan loan) {
+ return loanTransactionRepository.hasContractTerminationTransaction(loan);
+ }
+
+ public boolean shouldRegenerateRepaymentSchedule(final Loan loan) {
+ final boolean hasChargeOffTransaction = hasChargeOffTransaction(loan);
+ final boolean hasContractTerminationTransaction = hasContractTerminationTransaction(loan);
+ return loan.isProgressiveSchedule()
+ && ((hasChargeOffTransaction && loan.hasAccelerateChargeOffStrategy()) || hasContractTerminationTransaction);
+ }
+
+ public Money calculateTotalPaidInRepayments(final Loan loan) {
+ final List<LoanTransactionType> loanTransactionPaymentTypes = List.of(LoanTransactionType.REPAYMENT, //
+ LoanTransactionType.MERCHANT_ISSUED_REFUND, //
+ LoanTransactionType.PAYOUT_REFUND, //
+ LoanTransactionType.GOODWILL_CREDIT, //
+ LoanTransactionType.CHARGE_REFUND, //
+ LoanTransactionType.CHARGE_ADJUSTMENT, //
+ LoanTransactionType.DOWN_PAYMENT, //
+ LoanTransactionType.INTEREST_PAYMENT_WAIVER, //
+ LoanTransactionType.INTEREST_REFUND, //
+ LoanTransactionType.CAPITALIZED_INCOME_ADJUSTMENT);
+ log.info("Amount {}", loanTransactionRepository.sumTotalAmountByLoanAndTransactionTypes(loan, loanTransactionPaymentTypes));
Review Comment:
Done! code updated
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanBalanceService.java:
##########
@@ -108,7 +111,7 @@ public void updateLoanSummaryDerivedFields(final Loan loan) {
public void refreshSummaryAndBalancesForDisbursedLoan(final Loan loan) {
final Money overpaidBy = calculateTotalOverpayment(loan);
loan.setTotalOverpaid(null);
- if (!overpaidBy.isLessThanZero()) {
+ if (!overpaidBy.isZero() && !overpaidBy.isLessThanZero()) {
Review Comment:
Done! code updated
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2260080878
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransaction.java:
##########
@@ -144,6 +145,9 @@ public class LoanTransaction extends AbstractAuditableWithUTCDateTimeCustom<Long
@OneToOne(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.LAZY, mappedBy = "loanTransaction")
private LoanReAgeParameter loanReAgeParameter;
+ @Transient
+ private boolean isProcessed = true;
Review Comment:
Please remove this! We dont want any transient field on any entities!
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "alberto-art3ch (via GitHub)" <gi...@apache.org>.
alberto-art3ch closed pull request #4916: FINERACT-2324: Remove getLoanTransactions from job related actions
URL: https://github.com/apache/fineract/pull/4916
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "alberto-art3ch (via GitHub)" <gi...@apache.org>.
alberto-art3ch commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2290889561
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionService.java:
##########
@@ -56,4 +66,37 @@ private Predicate<LoanTransaction> loanTransactionForReprocessingPredicate() {
|| !transaction.isNonMonetaryTransaction() || transaction.isContractTermination());
}
+ @Transactional(readOnly = true)
+ public boolean hasChargeOffTransaction(final Loan loan) {
+ return loanTransactionRepository.hasChargeOffTransaction(loan);
+ }
+
+ @Transactional(readOnly = true)
+ public boolean hasContractTerminationTransaction(final Loan loan) {
Review Comment:
Done! marked as private
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "alberto-art3ch (via GitHub)" <gi...@apache.org>.
alberto-art3ch commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2265087369
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -1348,10 +1347,10 @@ public CommandProcessingResult chargebackLoanTransaction(final Long loanId, fina
LoanTransactionRelationTypeEnum.CHARGEBACK);
this.loanTransactionRelationRepository.save(loanTransactionRelation);
- handleChargebackTransaction(loan, newTransaction);
-
newTransaction = this.loanTransactionRepository.saveAndFlush(newTransaction);
+ handleChargebackTransaction(loan, newTransaction);
Review Comment:
Done! code updated, the save as part of the method `handleChargebackTransaction` after reprocessing but before determine the transition
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2272927909
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanAccountDomainServiceJpa.java:
##########
@@ -689,8 +689,6 @@ public LoanTransaction creditBalanceRefund(final Loan loan, final LocalDate tran
loanRefundService.creditBalanceRefund(loan, newCreditBalanceRefundTransaction, existingTransactionIds,
existingReversedTransactionIds);
- newCreditBalanceRefundTransaction = this.loanTransactionRepository.saveAndFlush(newCreditBalanceRefundTransaction);
Review Comment:
If this transaction is not flushed, in the `businessEventNotifierService
.notifyPostBusinessEvent(new LoanCreditBalanceRefundPostBusinessEvent(newCreditBalanceRefundTransaction));` we wont have id...
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2260084111
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/AbstractLoanRepaymentScheduleTransactionProcessor.java:
##########
@@ -180,7 +180,7 @@ public ChangedTransactionDetail reprocessLoanTransactions(final LocalDate disbur
if (loanTransaction.isRepaymentLikeType() || loanTransaction.isInterestWaiver() || loanTransaction.isRecoveryRepayment()) {
// pass through for new transactions
- if (loanTransaction.getId() == null) {
+ if (loanTransaction.getId() == null || !loanTransaction.isProcessed()) {
Review Comment:
No need...
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "alberto-art3ch (via GitHub)" <gi...@apache.org>.
alberto-art3ch commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2290884842
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionService.java:
##########
@@ -56,4 +66,37 @@ private Predicate<LoanTransaction> loanTransactionForReprocessingPredicate() {
|| !transaction.isNonMonetaryTransaction() || transaction.isContractTermination());
}
+ @Transactional(readOnly = true)
+ public boolean hasChargeOffTransaction(final Loan loan) {
+ return loanTransactionRepository.hasChargeOffTransaction(loan);
+ }
+
+ @Transactional(readOnly = true)
+ public boolean hasContractTerminationTransaction(final Loan loan) {
+ return loanTransactionRepository.hasContractTerminationTransaction(loan);
+ }
+
+ @Transactional(readOnly = true)
+ public boolean shouldRegenerateRepaymentSchedule(final Loan loan) {
Review Comment:
Done! Code updated
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2260081573
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransaction.java:
##########
@@ -1023,4 +1029,13 @@ public boolean isBuyDownFeeAmortization() {
public boolean isBuyDownFeeAmortizationAdjustment() {
return LoanTransactionType.BUY_DOWN_FEE_AMORTIZATION_ADJUSTMENT.equals(this.typeOf);
}
+
+ public void markAsProcessed() {
Review Comment:
See above. Remove please!
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2272902087
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionService.java:
##########
@@ -24,19 +24,29 @@
import java.util.function.Predicate;
import java.util.stream.Collectors;
import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.apache.fineract.organisation.monetary.domain.Money;
import org.apache.fineract.portfolio.loanaccount.domain.Loan;
import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionComparator;
import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionRepository;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionType;
import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
+@Slf4j
@Service
@RequiredArgsConstructor
public class LoanTransactionService {
private final LoanTransactionRepository loanTransactionRepository;
+ public List<LoanTransaction> retrieveListOfTransactionsForReprocessing(final List<LoanTransaction> loanTransactions) {
Review Comment:
@alberto-art3ch Still not used anywhere!
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2272908253
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionService.java:
##########
@@ -56,4 +66,37 @@ private Predicate<LoanTransaction> loanTransactionForReprocessingPredicate() {
|| !transaction.isNonMonetaryTransaction() || transaction.isContractTermination());
}
+ @Transactional(readOnly = true)
+ public boolean hasChargeOffTransaction(final Loan loan) {
+ return loanTransactionRepository.hasChargeOffTransaction(loan);
+ }
+
+ @Transactional(readOnly = true)
+ public boolean hasContractTerminationTransaction(final Loan loan) {
+ return loanTransactionRepository.hasContractTerminationTransaction(loan);
+ }
+
+ @Transactional(readOnly = true)
+ public boolean shouldRegenerateRepaymentSchedule(final Loan loan) {
Review Comment:
Since we are already in transaction, we can drop the @Transaction(readOnly = true) flag. it will never start a new transaction...
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2272931116
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -1350,8 +1349,6 @@ public CommandProcessingResult chargebackLoanTransaction(final Long loanId, fina
handleChargebackTransaction(loan, newTransaction);
- newTransaction = this.loanTransactionRepository.saveAndFlush(newTransaction);
Review Comment:
if new transaction is not saved and flushed, there will be no id in the `businessEventNotifierService.notifyPostBusinessEvent(new LoanChargebackTransactionBusinessEvent(newTransaction));`
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "alberto-art3ch (via GitHub)" <gi...@apache.org>.
alberto-art3ch commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2290890801
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionService.java:
##########
@@ -56,4 +66,37 @@ private Predicate<LoanTransaction> loanTransactionForReprocessingPredicate() {
|| !transaction.isNonMonetaryTransaction() || transaction.isContractTermination());
}
+ @Transactional(readOnly = true)
+ public boolean hasChargeOffTransaction(final Loan loan) {
Review Comment:
Done! marked as private
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "budaidev (via GitHub)" <gi...@apache.org>.
budaidev commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2258446536
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionService.java:
##########
@@ -56,4 +60,35 @@ private Predicate<LoanTransaction> loanTransactionForReprocessingPredicate() {
|| !transaction.isNonMonetaryTransaction() || transaction.isContractTermination());
}
+ public boolean hasChargeOffTransaction(final Loan loan) {
+ return loanTransactionRepository.hasChargeOffTransaction(loan);
+ }
+
+ public boolean hasContractTerminationTransaction(final Loan loan) {
+ return loanTransactionRepository.hasContractTerminationTransaction(loan);
+ }
+
+ public boolean shouldRegenerateRepaymentSchedule(final Loan loan) {
+ final boolean hasChargeOffTransaction = hasChargeOffTransaction(loan);
+ final boolean hasContractTerminationTransaction = hasContractTerminationTransaction(loan);
+ return loan.isProgressiveSchedule()
+ && ((hasChargeOffTransaction && loan.hasAccelerateChargeOffStrategy()) || hasContractTerminationTransaction);
+ }
+
+ public Money calculateTotalPaidInRepayments(final Loan loan) {
+ final List<LoanTransactionType> loanTransactionPaymentTypes = List.of(LoanTransactionType.REPAYMENT, //
+ LoanTransactionType.MERCHANT_ISSUED_REFUND, //
+ LoanTransactionType.PAYOUT_REFUND, //
+ LoanTransactionType.GOODWILL_CREDIT, //
+ LoanTransactionType.CHARGE_REFUND, //
+ LoanTransactionType.CHARGE_ADJUSTMENT, //
+ LoanTransactionType.DOWN_PAYMENT, //
+ LoanTransactionType.INTEREST_PAYMENT_WAIVER, //
+ LoanTransactionType.INTEREST_REFUND, //
+ LoanTransactionType.CAPITALIZED_INCOME_ADJUSTMENT);
+ log.info("Amount {}", loanTransactionRepository.sumTotalAmountByLoanAndTransactionTypes(loan, loanTransactionPaymentTypes));
Review Comment:
I am not sure it is a good idea to leave this logic here in info level, since it will duplicate the calculation. Maybe extract it to a variable if we really need this information, but we can also put into debug level instead of info
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanBalanceService.java:
##########
@@ -108,7 +111,7 @@ public void updateLoanSummaryDerivedFields(final Loan loan) {
public void refreshSummaryAndBalancesForDisbursedLoan(final Loan loan) {
final Money overpaidBy = calculateTotalOverpayment(loan);
loan.setTotalOverpaid(null);
- if (!overpaidBy.isLessThanZero()) {
+ if (!overpaidBy.isZero() && !overpaidBy.isLessThanZero()) {
Review Comment:
Maybe use overpaidBy.isGreaterThanZero()?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2272911730
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionService.java:
##########
@@ -56,4 +66,37 @@ private Predicate<LoanTransaction> loanTransactionForReprocessingPredicate() {
|| !transaction.isNonMonetaryTransaction() || transaction.isContractTermination());
}
+ @Transactional(readOnly = true)
+ public boolean hasChargeOffTransaction(final Loan loan) {
+ return loanTransactionRepository.hasChargeOffTransaction(loan);
+ }
+
+ @Transactional(readOnly = true)
+ public boolean hasContractTerminationTransaction(final Loan loan) {
+ return loanTransactionRepository.hasContractTerminationTransaction(loan);
+ }
+
+ @Transactional(readOnly = true)
+ public boolean shouldRegenerateRepaymentSchedule(final Loan loan) {
+ final boolean hasChargeOffTransaction = hasChargeOffTransaction(loan);
Review Comment:
We can improve and check in 1 query whether charge-off or contract termination transaction exists... calling twice is not the best for performance.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "alberto-art3ch (via GitHub)" <gi...@apache.org>.
alberto-art3ch commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2265087442
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/AbstractLoanRepaymentScheduleTransactionProcessor.java:
##########
@@ -180,7 +180,7 @@ public ChangedTransactionDetail reprocessLoanTransactions(final LocalDate disbur
if (loanTransaction.isRepaymentLikeType() || loanTransaction.isInterestWaiver() || loanTransaction.isRecoveryRepayment()) {
// pass through for new transactions
- if (loanTransaction.getId() == null) {
+ if (loanTransaction.getId() == null || !loanTransaction.isProcessed()) {
Review Comment:
Done code updated!
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2260089137
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionService.java:
##########
@@ -24,19 +24,29 @@
import java.util.function.Predicate;
import java.util.stream.Collectors;
import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.apache.fineract.organisation.monetary.domain.Money;
import org.apache.fineract.portfolio.loanaccount.domain.Loan;
import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionComparator;
import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionRepository;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionType;
import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
+@Slf4j
@Service
@RequiredArgsConstructor
public class LoanTransactionService {
private final LoanTransactionRepository loanTransactionRepository;
+ public List<LoanTransaction> retrieveListOfTransactionsForReprocessing(final List<LoanTransaction> loanTransactions) {
Review Comment:
What is this? Not used anywhere! Please remove!
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2260092932
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -1348,10 +1347,10 @@ public CommandProcessingResult chargebackLoanTransaction(final Long loanId, fina
LoanTransactionRelationTypeEnum.CHARGEBACK);
this.loanTransactionRelationRepository.save(loanTransactionRelation);
- handleChargebackTransaction(loan, newTransaction);
-
newTransaction = this.loanTransactionRepository.saveAndFlush(newTransaction);
+ handleChargebackTransaction(loan, newTransaction);
Review Comment:
You CANNOT save and flush transaction before reprocessing!
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2272931116
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -1350,8 +1349,6 @@ public CommandProcessingResult chargebackLoanTransaction(final Long loanId, fina
handleChargebackTransaction(loan, newTransaction);
- newTransaction = this.loanTransactionRepository.saveAndFlush(newTransaction);
Review Comment:
if new transaction is not saved and flushed, there will be no id in the `businessEventNotifierService.notifyPostBusinessEvent(new LoanChargebackTransactionBusinessEvent(newTransaction));`
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2272905364
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionService.java:
##########
@@ -56,4 +66,37 @@ private Predicate<LoanTransaction> loanTransactionForReprocessingPredicate() {
|| !transaction.isNonMonetaryTransaction() || transaction.isContractTermination());
}
+ @Transactional(readOnly = true)
+ public boolean hasChargeOffTransaction(final Loan loan) {
Review Comment:
used only internally...
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionService.java:
##########
@@ -56,4 +66,37 @@ private Predicate<LoanTransaction> loanTransactionForReprocessingPredicate() {
|| !transaction.isNonMonetaryTransaction() || transaction.isContractTermination());
}
+ @Transactional(readOnly = true)
+ public boolean hasChargeOffTransaction(final Loan loan) {
+ return loanTransactionRepository.hasChargeOffTransaction(loan);
+ }
+
+ @Transactional(readOnly = true)
+ public boolean hasContractTerminationTransaction(final Loan loan) {
Review Comment:
used only internally...
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2324: Remove getLoanTransactions from job related actions [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4916:
URL: https://github.com/apache/fineract/pull/4916#discussion_r2272928846
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanAccountDomainServiceJpa.java:
##########
@@ -742,8 +740,6 @@ public LoanTransaction makeRefundForActiveLoan(Long accountId, CommandProcessing
newRefundTransaction.getTransactionDate());
loanRefundService.makeRefundForActiveLoan(loan, newRefundTransaction, existingTransactionIds, existingReversedTransactionIds);
- this.loanTransactionRepository.saveAndFlush(newRefundTransaction);
Review Comment:
Same as above
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org