You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2023/01/15 20:24:01 UTC
[GitHub] [fineract] josehernandezfintecheandomx opened a new pull request, #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
josehernandezfintecheandomx opened a new pull request, #2882:
URL: https://github.com/apache/fineract/pull/2882
… the last date
## Description
When a loan with multitranches have two (or more) disbursals on it, and we call the` /loans/{loanId}?command=undolastdisbursal` or `/loans/external-id/{externalId}?command=undolastdisbursal `API it will look for only the last DATE, not the last disbursal itself. So if the last two disbursal was on the same day, it will undo both of them.
[FINERACT-1854](https://issues.apache.org/jira/browse/FINERACT-1854)
## 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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073680744
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6341,41 +6349,19 @@ public Map<String, Object> undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
return actualChanges;
}
- /**
- * Reverse only disbursement, accruals, and repayments at disbursal transactions
- *
- * @param actualDisbursementDate
- * @return
- */
- public List<LoanTransaction> reverseExistingTransactionsTillLastDisbursal(LocalDate actualDisbursementDate) {
Review Comment:
Accrual and repayment at disbursal can be reversed even if it was after the last disbursement.
Please be more careful to not do any regression!
--
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
[GitHub] [fineract] adamsaghy merged pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #2882:
URL: https://github.com/apache/fineract/pull/2882
--
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
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073641960
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -4158,20 +4157,12 @@ public LocalDate getExpectedDisbursedOnLocalDateForTemplate() {
return expectedDisbursementDate;
}
- /*
- * Reason for derving
- */
-
public BigDecimal getDisburseAmountForTemplate() {
- BigDecimal principal = this.loanRepaymentScheduleDetail.getPrincipal().getAmount();
Review Comment:
Done
--
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
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by "josehernandezfintecheandomx (via GitHub)" <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1085526892
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6341,41 +6349,19 @@ public Map<String, Object> undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
return actualChanges;
}
- /**
- * Reverse only disbursement, accruals, and repayments at disbursal transactions
- *
- * @param actualDisbursementDate
- * @return
- */
- public List<LoanTransaction> reverseExistingTransactionsTillLastDisbursal(LocalDate actualDisbursementDate) {
Review Comment:
Done
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1086494570
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6301,26 +6307,31 @@ public Map<String, Object> undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List<Long> existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
+ validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
+
+ final Map<String, Object> actualChanges = new LinkedHashMap<>();
+ List<LoanTransaction> loanTransactions = retrieveListOfTransactionsByType(LoanTransactionType.DISBURSEMENT);
+ loanTransactions.sort(Comparator.comparing(LoanTransaction::getId));
+ final LoanTransaction lastDisbursalTransaction = loanTransactions.get(loanTransactions.size() - 1);
+ final LocalDate lastTransactionDate = lastDisbursalTransaction.getTransactionDate();
+
existingTransactionIds.addAll(findExistingTransactionIds());
existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
- final Map<String, Object> actualChanges = new LinkedHashMap<>();
- validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
- LocalDate actualDisbursementDate = null;
- LocalDate lastTransactionDate = getDisbursementDate();
- List<LoanTransaction> loanTransactions = retrieveListOfTransactionsExcludeAccruals();
+
+ loanTransactions = retrieveListOfTransactionsExcludeAccruals();
Collections.reverse(loanTransactions);
for (final LoanTransaction previousTransaction : loanTransactions) {
if (lastTransactionDate.isBefore(previousTransaction.getTransactionDate())
&& (previousTransaction.isRepaymentType() || previousTransaction.isWaiver() || previousTransaction.isChargePayment())) {
throw new UndoLastTrancheDisbursementException(previousTransaction.getId());
}
- if (previousTransaction.isDisbursement()) {
- lastTransactionDate = previousTransaction.getTransactionDate();
+ if (previousTransaction.getId().compareTo(lastDisbursalTransaction.getId()) < 0) {
break;
}
}
- actualDisbursementDate = lastTransactionDate;
- updateLoanToLastDisbursalState(actualDisbursementDate);
+ final LoanDisbursementDetails disbursementDetail = loan.getDisbursementDetails(actualDisbursementDate,
Review Comment:
What is the value of actualDisbursementDate? Shouldnt it be lastTransactionDate instead?
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1082702226
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6301,26 +6307,27 @@ public Map<String, Object> undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List<Long> existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
- existingTransactionIds.addAll(findExistingTransactionIds());
- existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
- final Map<String, Object> actualChanges = new LinkedHashMap<>();
validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
- LocalDate actualDisbursementDate = null;
- LocalDate lastTransactionDate = getDisbursementDate();
- List<LoanTransaction> loanTransactions = retrieveListOfTransactionsExcludeAccruals();
+
+ final Map<String, Object> actualChanges = new LinkedHashMap<>();
+ List<LoanTransaction> loanTransactions = retrieveListOfTransactionsByType(LoanTransactionType.DISBURSEMENT);
Review Comment:
This is wrong, we filter the transactions to have only the transactions which are disbursement, but at line 6323, we are iterating through on the same list and try to decide whether there was any transaction after the last disbursement which prevent to undo the disbursement... (any repayment happened, we cannot undo them)
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1086496664
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6344,38 +6355,31 @@ public Map<String, Object> undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
/**
* Reverse only disbursement, accruals, and repayments at disbursal transactions
*
- * @param actualDisbursementDate
+ * @param lastDisbursalTransaction
* @return
*/
- public List<LoanTransaction> reverseExistingTransactionsTillLastDisbursal(LocalDate actualDisbursementDate) {
- final List<LoanTransaction> reversedTransactions = new ArrayList<>();
+ public void reverseExistingTransactionsTillLastDisbursal(LoanTransaction lastDisbursalTransaction) {
for (final LoanTransaction transaction : this.loanTransactions) {
- if ((actualDisbursementDate.equals(transaction.getTransactionDate())
- || actualDisbursementDate.isBefore(transaction.getTransactionDate()))
+ if ((transaction.getId().compareTo(lastDisbursalTransaction.getId()) >= 0)
Review Comment:
We need to check the transactionDate as well as it was originally to avoid reverting a backdated 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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1087894698
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanApplicationUndoLastTrancheTest.java:
##########
@@ -129,6 +134,140 @@ public void loanApplicationUndoLastTranche() {
validateDisbursedAmount(disbursedAmount);
}
+ @Test
+ public void loanApplicationUndoLastTrancheToClose() {
+ final LocalDate todaysDate = Utils.getLocalDateOfTenant();
+ LocalDate transactionDate = LocalDate.of(todaysDate.getYear(), 1, 1);
+ String operationDate = Utils.dateFormatter.format(transactionDate);
+ LOG.info("Operation date {}", transactionDate);
+
+ final String proposedAmount = "5000";
+
+ // CREATE CLIENT
+ final Integer clientID = ClientHelper.createClient(this.requestSpec, this.responseSpec, "01 January 2014");
+ LOG.info("---------------------------------CLIENT CREATED WITH ID--------------------------------------------------- {}", clientID);
+
+ // CREATE LOAN MULTIDISBURSAL PRODUCT
+ final Integer loanProductID = this.loanTransactionHelper
+ .getLoanProductId(new LoanProductTestBuilder().withInterestTypeAsDecliningBalance().withTranches(true)
+ .withDisallowExpectedDisbursements(true).withInterestCalculationPeriodTypeAsRepaymentPeriod(true).build(null));
+ LOG.info("----------------------------------LOAN PRODUCT CREATED WITH ID------------------------------------------- {}",
+ loanProductID);
+
+ // APPLY FOR LOAN WITH TRANCHES
+ final Integer loanID = applyForLoanApplicationWithTranches(clientID, loanProductID, proposedAmount, new ArrayList<>());
+
+ LOG.info("-----------------------------------LOAN CREATED WITH LOANID------------------------------------------------- {}", loanID);
+
+ LOG.info("-----------------------------------APPROVE LOAN-----------------------------------------------------------");
+ this.loanTransactionHelper.approveLoan(operationDate, proposedAmount, loanID, null);
+
+ GetLoansLoanIdResponse getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.approved");
+
+ // DISBURSE A LOAN
+ loanTransactionHelper.disburseLoanWithTransactionAmount(operationDate, loanID, "500");
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ // VALIDATE THE LOAN IS ACTIVE STATUS
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.active");
+ loanTransactionHelper.evaluateLoanDisbursementDetails(getLoansLoanIdResponse, 1, Double.valueOf("500.00"));
+
+ // DISBURSE A LOAN (second)
+ transactionDate = transactionDate.plusDays(2);
+ operationDate = Utils.dateFormatter.format(transactionDate);
+ LOG.info("Operation date {}", transactionDate);
+ loanTransactionHelper.disburseLoanWithTransactionAmount(operationDate, loanID, "500");
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ // VALIDATE THE LOAN IS ACTIVE STATUS
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.active");
+ loanTransactionHelper.evaluateLoanDisbursementDetails(getLoansLoanIdResponse, 2, Double.valueOf("1000.00"));
+
+ // BACKDATE REPAYMENT
+ transactionDate = transactionDate.minusDays(1);
+ operationDate = Utils.dateFormatter.format(transactionDate);
+ LOG.info("Operation date {}", transactionDate);
+ Float amount = Float.valueOf("559.36");
+ PostLoansLoanIdTransactionsResponse loanIdTransactionsResponse = loanTransactionHelper.makeLoanRepayment(operationDate, amount,
+ loanID);
+ assertNotNull(loanIdTransactionsResponse);
+ LOG.info("Loan Transaction Id: {} {}", loanID, loanIdTransactionsResponse.getResourceId());
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ // VALIDATE THE LOAN IS ACTIVE STATUS
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.active");
+ loanTransactionHelper.evaluateLoanDisbursementDetails(getLoansLoanIdResponse, 2, Double.valueOf("1000.00"));
+ loanTransactionHelper.validateLoanTotalOustandingBalance(getLoansLoanIdResponse, Double.valueOf("500.00"));
+
+ // UNDO LAST TRANCHE
+ this.loanTransactionHelper.undoLastDisbursal(loanID);
+
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ // VALIDATE THE LOAN IS ACTIVE STATUS
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.active");
Review Comment:
it should be closed, no?
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1086499211
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanApplicationUndoLastTrancheTest.java:
##########
@@ -129,6 +132,62 @@ public void loanApplicationUndoLastTranche() {
validateDisbursedAmount(disbursedAmount);
}
+ @Test
+ public void loanApplicationUndoLastTrancheWithSameDate() {
+
+ final String proposedAmount = "5000";
+ final String approveDate = "01 March 2014";
+ final String disbursalDate = "01 March 2014";
+
+ // CREATE CLIENT
+ final Integer clientID = ClientHelper.createClient(this.requestSpec, this.responseSpec, "01 January 2014");
+ LOG.info("---------------------------------CLIENT CREATED WITH ID--------------------------------------------------- {}", clientID);
+
+ // CREATE LOAN MULTIDISBURSAL PRODUCT
+ final Integer loanProductID = this.loanTransactionHelper
+ .getLoanProductId(new LoanProductTestBuilder().withInterestTypeAsDecliningBalance().withTranches(true)
+ .withDisallowExpectedDisbursements(true).withInterestCalculationPeriodTypeAsRepaymentPeriod(true).build(null));
+ LOG.info("----------------------------------LOAN PRODUCT CREATED WITH ID------------------------------------------- {}",
+ loanProductID);
+
+ // APPLY FOR LOAN WITH TRANCHES
+ final Integer loanID = applyForLoanApplicationWithTranches(clientID, loanProductID, proposedAmount, new ArrayList<>());
+
+ LOG.info("-----------------------------------LOAN CREATED WITH LOANID------------------------------------------------- {}", loanID);
+
+ LOG.info("-----------------------------------APPROVE LOAN-----------------------------------------------------------");
+ this.loanTransactionHelper.approveLoan(approveDate, proposedAmount, loanID, null);
+
+ GetLoansLoanIdResponse getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.approved");
+
+ // DISBURSE A LOAN
+ loanTransactionHelper.disburseLoanWithTransactionAmount(disbursalDate, loanID, "1000");
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ // VALIDATE THE LOAN IS ACTIVE STATUS
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.active");
+ loanTransactionHelper.evaluateLoanDisbursementDetails(getLoansLoanIdResponse, 1, Double.valueOf("1000.00"));
+
+ // DISBURSE A LOAN (second)
+ loanTransactionHelper.disburseLoanWithTransactionAmount(disbursalDate, loanID, "2000");
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ // VALIDATE THE LOAN IS ACTIVE STATUS
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.active");
+ loanTransactionHelper.evaluateLoanDisbursementDetails(getLoansLoanIdResponse, 2, Double.valueOf("3000.00"));
+
+ // UNDO LAST TRANCHE
+ this.loanTransactionHelper.undoLastDisbursal(loanID);
+
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
Review Comment:
Please validate whether the loan balance is correct (1000)
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073518496
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6301,26 +6302,18 @@ public Map<String, Object> undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List<Long> existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
- existingTransactionIds.addAll(findExistingTransactionIds());
- existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
- final Map<String, Object> actualChanges = new LinkedHashMap<>();
validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
- LocalDate actualDisbursementDate = null;
- LocalDate lastTransactionDate = getDisbursementDate();
- List<LoanTransaction> loanTransactions = retrieveListOfTransactionsExcludeAccruals();
- Collections.reverse(loanTransactions);
- for (final LoanTransaction previousTransaction : loanTransactions) {
- if (lastTransactionDate.isBefore(previousTransaction.getTransactionDate())
- && (previousTransaction.isRepaymentType() || previousTransaction.isWaiver() || previousTransaction.isChargePayment())) {
- throw new UndoLastTrancheDisbursementException(previousTransaction.getId());
- }
- if (previousTransaction.isDisbursement()) {
- lastTransactionDate = previousTransaction.getTransactionDate();
- break;
- }
- }
- actualDisbursementDate = lastTransactionDate;
- updateLoanToLastDisbursalState(actualDisbursementDate);
+
+ final Map<String, Object> actualChanges = new LinkedHashMap<>();
Review Comment:
This behaviour looks different to me like it was.
Please correct me if i wrong, but priorly we were not let the customer undo the last disbursement if there was any repayment, waiver or charge transaction after it.
I dont see the same restriction with the new logic. Was it purposely?
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1086491536
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6301,26 +6307,31 @@ public Map<String, Object> undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List<Long> existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
+ validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
+
+ final Map<String, Object> actualChanges = new LinkedHashMap<>();
+ List<LoanTransaction> loanTransactions = retrieveListOfTransactionsByType(LoanTransactionType.DISBURSEMENT);
+ loanTransactions.sort(Comparator.comparing(LoanTransaction::getId));
+ final LoanTransaction lastDisbursalTransaction = loanTransactions.get(loanTransactions.size() - 1);
+ final LocalDate lastTransactionDate = lastDisbursalTransaction.getTransactionDate();
+
existingTransactionIds.addAll(findExistingTransactionIds());
existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
- final Map<String, Object> actualChanges = new LinkedHashMap<>();
- validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
Review Comment:
Please put this validation back. I dont see any reason to remove it.
--
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
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by "josehernandezfintecheandomx (via GitHub)" <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1087480999
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6301,26 +6307,31 @@ public Map<String, Object> undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List<Long> existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
+ validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
+
+ final Map<String, Object> actualChanges = new LinkedHashMap<>();
+ List<LoanTransaction> loanTransactions = retrieveListOfTransactionsByType(LoanTransactionType.DISBURSEMENT);
+ loanTransactions.sort(Comparator.comparing(LoanTransaction::getId));
+ final LoanTransaction lastDisbursalTransaction = loanTransactions.get(loanTransactions.size() - 1);
+ final LocalDate lastTransactionDate = lastDisbursalTransaction.getTransactionDate();
+
existingTransactionIds.addAll(findExistingTransactionIds());
existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
- final Map<String, Object> actualChanges = new LinkedHashMap<>();
- validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
- LocalDate actualDisbursementDate = null;
- LocalDate lastTransactionDate = getDisbursementDate();
- List<LoanTransaction> loanTransactions = retrieveListOfTransactionsExcludeAccruals();
+
+ loanTransactions = retrieveListOfTransactionsExcludeAccruals();
Collections.reverse(loanTransactions);
for (final LoanTransaction previousTransaction : loanTransactions) {
if (lastTransactionDate.isBefore(previousTransaction.getTransactionDate())
&& (previousTransaction.isRepaymentType() || previousTransaction.isWaiver() || previousTransaction.isChargePayment())) {
throw new UndoLastTrancheDisbursementException(previousTransaction.getId());
}
- if (previousTransaction.isDisbursement()) {
- lastTransactionDate = previousTransaction.getTransactionDate();
+ if (previousTransaction.getId().compareTo(lastDisbursalTransaction.getId()) < 0) {
break;
}
}
- actualDisbursementDate = lastTransactionDate;
- updateLoanToLastDisbursalState(actualDisbursementDate);
+ final LoanDisbursementDetails disbursementDetail = loan.getDisbursementDetails(actualDisbursementDate,
Review Comment:
`actualDisbursementDate` was replaced with `lastTransactionDate`
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073533927
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -4158,20 +4157,12 @@ public LocalDate getExpectedDisbursedOnLocalDateForTemplate() {
return expectedDisbursementDate;
}
- /*
- * Reason for derving
- */
-
public BigDecimal getDisburseAmountForTemplate() {
- BigDecimal principal = this.loanRepaymentScheduleDetail.getPrincipal().getAmount();
Review Comment:
I see. Thanks for letting me know.
Would you please raise a new story and PR with this change?
--
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
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073641584
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6301,26 +6302,18 @@ public Map<String, Object> undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List<Long> existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
- existingTransactionIds.addAll(findExistingTransactionIds());
- existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
- final Map<String, Object> actualChanges = new LinkedHashMap<>();
validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
- LocalDate actualDisbursementDate = null;
- LocalDate lastTransactionDate = getDisbursementDate();
- List<LoanTransaction> loanTransactions = retrieveListOfTransactionsExcludeAccruals();
- Collections.reverse(loanTransactions);
- for (final LoanTransaction previousTransaction : loanTransactions) {
- if (lastTransactionDate.isBefore(previousTransaction.getTransactionDate())
- && (previousTransaction.isRepaymentType() || previousTransaction.isWaiver() || previousTransaction.isChargePayment())) {
- throw new UndoLastTrancheDisbursementException(previousTransaction.getId());
- }
- if (previousTransaction.isDisbursement()) {
- lastTransactionDate = previousTransaction.getTransactionDate();
- break;
- }
- }
- actualDisbursementDate = lastTransactionDate;
- updateLoanToLastDisbursalState(actualDisbursementDate);
+
+ final Map<String, Object> actualChanges = new LinkedHashMap<>();
Review Comment:
Done
--
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
[GitHub] [fineract] josehernandezfintecheandomx commented on pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on PR #2882:
URL: https://github.com/apache/fineract/pull/2882#issuecomment-1387202314
> Kindly review my questions!
Comments attended
--
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
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by "josehernandezfintecheandomx (via GitHub)" <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1087480508
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6301,26 +6307,31 @@ public Map<String, Object> undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List<Long> existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
+ validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
+
+ final Map<String, Object> actualChanges = new LinkedHashMap<>();
+ List<LoanTransaction> loanTransactions = retrieveListOfTransactionsByType(LoanTransactionType.DISBURSEMENT);
+ loanTransactions.sort(Comparator.comparing(LoanTransaction::getId));
+ final LoanTransaction lastDisbursalTransaction = loanTransactions.get(loanTransactions.size() - 1);
+ final LocalDate lastTransactionDate = lastDisbursalTransaction.getTransactionDate();
+
existingTransactionIds.addAll(findExistingTransactionIds());
existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
- final Map<String, Object> actualChanges = new LinkedHashMap<>();
- validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
Review Comment:
Done
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1087538881
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6301,26 +6307,31 @@ public Map<String, Object> undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List<Long> existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
+ validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
+
+ final Map<String, Object> actualChanges = new LinkedHashMap<>();
+ List<LoanTransaction> loanTransactions = retrieveListOfTransactionsByType(LoanTransactionType.DISBURSEMENT);
+ loanTransactions.sort(Comparator.comparing(LoanTransaction::getId));
+ final LoanTransaction lastDisbursalTransaction = loanTransactions.get(loanTransactions.size() - 1);
+ final LocalDate lastTransactionDate = lastDisbursalTransaction.getTransactionDate();
+
existingTransactionIds.addAll(findExistingTransactionIds());
existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
- final Map<String, Object> actualChanges = new LinkedHashMap<>();
- validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
- LocalDate actualDisbursementDate = null;
- LocalDate lastTransactionDate = getDisbursementDate();
- List<LoanTransaction> loanTransactions = retrieveListOfTransactionsExcludeAccruals();
+
+ loanTransactions = retrieveListOfTransactionsExcludeAccruals();
Collections.reverse(loanTransactions);
for (final LoanTransaction previousTransaction : loanTransactions) {
if (lastTransactionDate.isBefore(previousTransaction.getTransactionDate())
&& (previousTransaction.isRepaymentType() || previousTransaction.isWaiver() || previousTransaction.isChargePayment())) {
throw new UndoLastTrancheDisbursementException(previousTransaction.getId());
}
- if (previousTransaction.isDisbursement()) {
- lastTransactionDate = previousTransaction.getTransactionDate();
+ if (previousTransaction.getId().compareTo(lastDisbursalTransaction.getId()) < 0) {
break;
}
}
- actualDisbursementDate = lastTransactionDate;
- updateLoanToLastDisbursalState(actualDisbursementDate);
+ final LoanDisbursementDetails disbursementDetail = loan.getDisbursementDetails(actualDisbursementDate,
Review Comment:
I see no changes. it is still using the "actualDisbursementDate"!
--
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
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073519202
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -4158,20 +4157,12 @@ public LocalDate getExpectedDisbursedOnLocalDateForTemplate() {
return expectedDisbursementDate;
}
- /*
- * Reason for derving
- */
-
public BigDecimal getDisburseAmountForTemplate() {
- BigDecimal principal = this.loanRepaymentScheduleDetail.getPrincipal().getAmount();
Review Comment:
When you get the disburse template you get a wrong amount after the first disbursement
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073679864
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6301,26 +6307,27 @@ public Map<String, Object> undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List<Long> existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
- existingTransactionIds.addAll(findExistingTransactionIds());
- existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
- final Map<String, Object> actualChanges = new LinkedHashMap<>();
validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
- LocalDate actualDisbursementDate = null;
- LocalDate lastTransactionDate = getDisbursementDate();
- List<LoanTransaction> loanTransactions = retrieveListOfTransactionsExcludeAccruals();
+
+ final Map<String, Object> actualChanges = new LinkedHashMap<>();
+ List<LoanTransaction> loanTransactions = retrieveListOfTransactionsByType(LoanTransactionType.DISBURSEMENT);
+ loanTransactions.sort(Comparator.comparing(LoanTransaction::getId));
+ final LoanTransaction lastDisbursalTransaction = loanTransactions.get(loanTransactions.size() - 1);
Review Comment:
This might not be true. There could be Accrual transaction or any other one as well.
Please make it sure you understand what the legacy logic was doing and not do any regression!
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073511151
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -4158,20 +4157,12 @@ public LocalDate getExpectedDisbursedOnLocalDateForTemplate() {
return expectedDisbursementDate;
}
- /*
- * Reason for derving
- */
-
public BigDecimal getDisburseAmountForTemplate() {
- BigDecimal principal = this.loanRepaymentScheduleDetail.getPrincipal().getAmount();
Review Comment:
How does this change relates to the "undo last disbursal function" fix?
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073533927
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -4158,20 +4157,12 @@ public LocalDate getExpectedDisbursedOnLocalDateForTemplate() {
return expectedDisbursementDate;
}
- /*
- * Reason for derving
- */
-
public BigDecimal getDisburseAmountForTemplate() {
- BigDecimal principal = this.loanRepaymentScheduleDetail.getPrincipal().getAmount();
Review Comment:
I see. Thanks for letting me know.
Would you please raise a new story and PR with this change?
We should focus to fix one issue / PR, it makes confusing and complex if more than one issue is addressed in one PR.
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1073679864
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6301,26 +6307,27 @@ public Map<String, Object> undoLastDisbursal(ScheduleGeneratorDTO scheduleGenera
List<Long> existingReversedTransactionIds, Loan loan) {
validateAccountStatus(LoanEvent.LOAN_DISBURSAL_UNDO_LAST);
- existingTransactionIds.addAll(findExistingTransactionIds());
- existingReversedTransactionIds.addAll(findExistingReversedTransactionIds());
- final Map<String, Object> actualChanges = new LinkedHashMap<>();
validateActivityNotBeforeClientOrGroupTransferDate(LoanEvent.LOAN_DISBURSAL_UNDO_LAST, getDisbursementDate());
- LocalDate actualDisbursementDate = null;
- LocalDate lastTransactionDate = getDisbursementDate();
- List<LoanTransaction> loanTransactions = retrieveListOfTransactionsExcludeAccruals();
+
+ final Map<String, Object> actualChanges = new LinkedHashMap<>();
+ List<LoanTransaction> loanTransactions = retrieveListOfTransactionsByType(LoanTransactionType.DISBURSEMENT);
+ loanTransactions.sort(Comparator.comparing(LoanTransaction::getId));
+ final LoanTransaction lastDisbursalTransaction = loanTransactions.get(loanTransactions.size() - 1);
Review Comment:
This might not be true. There could be Accrual transaction or any other one as well.
Please make it sure you understand what the legacy logic was doing and not do any regression!
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1086504033
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanApplicationUndoLastTrancheTest.java:
##########
@@ -129,6 +132,62 @@ public void loanApplicationUndoLastTranche() {
validateDisbursedAmount(disbursedAmount);
}
+ @Test
+ public void loanApplicationUndoLastTrancheWithSameDate() {
+
+ final String proposedAmount = "5000";
+ final String approveDate = "01 March 2014";
+ final String disbursalDate = "01 March 2014";
+
+ // CREATE CLIENT
+ final Integer clientID = ClientHelper.createClient(this.requestSpec, this.responseSpec, "01 January 2014");
+ LOG.info("---------------------------------CLIENT CREATED WITH ID--------------------------------------------------- {}", clientID);
+
+ // CREATE LOAN MULTIDISBURSAL PRODUCT
+ final Integer loanProductID = this.loanTransactionHelper
+ .getLoanProductId(new LoanProductTestBuilder().withInterestTypeAsDecliningBalance().withTranches(true)
+ .withDisallowExpectedDisbursements(true).withInterestCalculationPeriodTypeAsRepaymentPeriod(true).build(null));
+ LOG.info("----------------------------------LOAN PRODUCT CREATED WITH ID------------------------------------------- {}",
+ loanProductID);
+
+ // APPLY FOR LOAN WITH TRANCHES
+ final Integer loanID = applyForLoanApplicationWithTranches(clientID, loanProductID, proposedAmount, new ArrayList<>());
+
+ LOG.info("-----------------------------------LOAN CREATED WITH LOANID------------------------------------------------- {}", loanID);
+
+ LOG.info("-----------------------------------APPROVE LOAN-----------------------------------------------------------");
+ this.loanTransactionHelper.approveLoan(approveDate, proposedAmount, loanID, null);
+
+ GetLoansLoanIdResponse getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.approved");
+
+ // DISBURSE A LOAN
+ loanTransactionHelper.disburseLoanWithTransactionAmount(disbursalDate, loanID, "1000");
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ // VALIDATE THE LOAN IS ACTIVE STATUS
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.active");
+ loanTransactionHelper.evaluateLoanDisbursementDetails(getLoansLoanIdResponse, 1, Double.valueOf("1000.00"));
+
+ // DISBURSE A LOAN (second)
+ loanTransactionHelper.disburseLoanWithTransactionAmount(disbursalDate, loanID, "2000");
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ // VALIDATE THE LOAN IS ACTIVE STATUS
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.active");
+ loanTransactionHelper.evaluateLoanDisbursementDetails(getLoansLoanIdResponse, 2, Double.valueOf("3000.00"));
+
+ // UNDO LAST TRANCHE
+ this.loanTransactionHelper.undoLastDisbursal(loanID);
+
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ // VALIDATE THE LOAN IS ACTIVE STATUS
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.active");
+ loanTransactionHelper.evaluateLoanDisbursementDetails(getLoansLoanIdResponse, 1, Double.valueOf("1000.00"));
+ }
Review Comment:
Please write 1 more tests:
- Disburse the loan (1 Jan, 500) -> Loan balance 500
- Disburse the second time (3 Jan, 500) -> Loan balance 1000
- Do a backdated repayment to (2 Jan, 500) -> Loan balance is 500
- Undo last disbursement which should happen and the repayment transaction is untouched -> Loan is closed
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1086504033
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanApplicationUndoLastTrancheTest.java:
##########
@@ -129,6 +132,62 @@ public void loanApplicationUndoLastTranche() {
validateDisbursedAmount(disbursedAmount);
}
+ @Test
+ public void loanApplicationUndoLastTrancheWithSameDate() {
+
+ final String proposedAmount = "5000";
+ final String approveDate = "01 March 2014";
+ final String disbursalDate = "01 March 2014";
+
+ // CREATE CLIENT
+ final Integer clientID = ClientHelper.createClient(this.requestSpec, this.responseSpec, "01 January 2014");
+ LOG.info("---------------------------------CLIENT CREATED WITH ID--------------------------------------------------- {}", clientID);
+
+ // CREATE LOAN MULTIDISBURSAL PRODUCT
+ final Integer loanProductID = this.loanTransactionHelper
+ .getLoanProductId(new LoanProductTestBuilder().withInterestTypeAsDecliningBalance().withTranches(true)
+ .withDisallowExpectedDisbursements(true).withInterestCalculationPeriodTypeAsRepaymentPeriod(true).build(null));
+ LOG.info("----------------------------------LOAN PRODUCT CREATED WITH ID------------------------------------------- {}",
+ loanProductID);
+
+ // APPLY FOR LOAN WITH TRANCHES
+ final Integer loanID = applyForLoanApplicationWithTranches(clientID, loanProductID, proposedAmount, new ArrayList<>());
+
+ LOG.info("-----------------------------------LOAN CREATED WITH LOANID------------------------------------------------- {}", loanID);
+
+ LOG.info("-----------------------------------APPROVE LOAN-----------------------------------------------------------");
+ this.loanTransactionHelper.approveLoan(approveDate, proposedAmount, loanID, null);
+
+ GetLoansLoanIdResponse getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.approved");
+
+ // DISBURSE A LOAN
+ loanTransactionHelper.disburseLoanWithTransactionAmount(disbursalDate, loanID, "1000");
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ // VALIDATE THE LOAN IS ACTIVE STATUS
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.active");
+ loanTransactionHelper.evaluateLoanDisbursementDetails(getLoansLoanIdResponse, 1, Double.valueOf("1000.00"));
+
+ // DISBURSE A LOAN (second)
+ loanTransactionHelper.disburseLoanWithTransactionAmount(disbursalDate, loanID, "2000");
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ // VALIDATE THE LOAN IS ACTIVE STATUS
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.active");
+ loanTransactionHelper.evaluateLoanDisbursementDetails(getLoansLoanIdResponse, 2, Double.valueOf("3000.00"));
+
+ // UNDO LAST TRANCHE
+ this.loanTransactionHelper.undoLastDisbursal(loanID);
+
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ // VALIDATE THE LOAN IS ACTIVE STATUS
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.active");
+ loanTransactionHelper.evaluateLoanDisbursementDetails(getLoansLoanIdResponse, 1, Double.valueOf("1000.00"));
+ }
Review Comment:
Please write 1 more tests:
- Disburse the loan (1 Jan, 500) -> Loan balance 500
- Disburse the second time (3 Jan, 500) -> Loan balance 1000
- Do a backdated repayment to (2 Jan, 500) -> Loan balance is 500
- Undo last disbursement which should happen and the repayment transaction is untouched -> Loan is closed
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2882: FINERACT-1854: Undo last disbursal function deletes all disbursals on…
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #2882:
URL: https://github.com/apache/fineract/pull/2882#discussion_r1086499211
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanApplicationUndoLastTrancheTest.java:
##########
@@ -129,6 +132,62 @@ public void loanApplicationUndoLastTranche() {
validateDisbursedAmount(disbursedAmount);
}
+ @Test
+ public void loanApplicationUndoLastTrancheWithSameDate() {
+
+ final String proposedAmount = "5000";
+ final String approveDate = "01 March 2014";
+ final String disbursalDate = "01 March 2014";
+
+ // CREATE CLIENT
+ final Integer clientID = ClientHelper.createClient(this.requestSpec, this.responseSpec, "01 January 2014");
+ LOG.info("---------------------------------CLIENT CREATED WITH ID--------------------------------------------------- {}", clientID);
+
+ // CREATE LOAN MULTIDISBURSAL PRODUCT
+ final Integer loanProductID = this.loanTransactionHelper
+ .getLoanProductId(new LoanProductTestBuilder().withInterestTypeAsDecliningBalance().withTranches(true)
+ .withDisallowExpectedDisbursements(true).withInterestCalculationPeriodTypeAsRepaymentPeriod(true).build(null));
+ LOG.info("----------------------------------LOAN PRODUCT CREATED WITH ID------------------------------------------- {}",
+ loanProductID);
+
+ // APPLY FOR LOAN WITH TRANCHES
+ final Integer loanID = applyForLoanApplicationWithTranches(clientID, loanProductID, proposedAmount, new ArrayList<>());
+
+ LOG.info("-----------------------------------LOAN CREATED WITH LOANID------------------------------------------------- {}", loanID);
+
+ LOG.info("-----------------------------------APPROVE LOAN-----------------------------------------------------------");
+ this.loanTransactionHelper.approveLoan(approveDate, proposedAmount, loanID, null);
+
+ GetLoansLoanIdResponse getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.approved");
+
+ // DISBURSE A LOAN
+ loanTransactionHelper.disburseLoanWithTransactionAmount(disbursalDate, loanID, "1000");
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ // VALIDATE THE LOAN IS ACTIVE STATUS
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.active");
+ loanTransactionHelper.evaluateLoanDisbursementDetails(getLoansLoanIdResponse, 1, Double.valueOf("1000.00"));
+
+ // DISBURSE A LOAN (second)
+ loanTransactionHelper.disburseLoanWithTransactionAmount(disbursalDate, loanID, "2000");
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
+ // VALIDATE THE LOAN IS ACTIVE STATUS
+ loanTransactionHelper.validateLoanStatus(getLoansLoanIdResponse, "loanStatusType.active");
+ loanTransactionHelper.evaluateLoanDisbursementDetails(getLoansLoanIdResponse, 2, Double.valueOf("3000.00"));
+
+ // UNDO LAST TRANCHE
+ this.loanTransactionHelper.undoLastDisbursal(loanID);
+
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanID);
+ assertNotNull(getLoansLoanIdResponse);
Review Comment:
Please validate whether the loan balance is correct (1000)
--
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