Aman-Mittal commented on code in PR #5827:
URL: https://github.com/apache/fineract/pull/5827#discussion_r3201164810
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/guarantor/service/GuarantorDomainServiceImpl.java:
##########
@@ -125,7 +125,18 @@ public void validateGuarantorBusinessRules(Loan loan) {
for (GuarantorFundingDetails guarantorFundingDetails :
fundingDetails) {
if (guarantorFundingDetails.getStatus().isActive() ||
guarantorFundingDetails.getStatus().isWithdrawn()
||
guarantorFundingDetails.getStatus().isCompleted()) {
- if (guarantor.isSelfGuarantee()) {
+ SavingsAccount savingsAccount =
guarantorFundingDetails.getLinkedSavingsAccount();
+ if (savingsAccount.isGroupAccount()) {
Review Comment:
Just trying to understand the intended business behavior here:
for client loans, if the borrower belongs to the same group whose savings
account is being used as guarantee, should that still be classified as an
external guarantee?
Right now it looks like only `loan.getGroupId()` is considered for
self-guarantee classification.
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/common/savings/SavingsAccountHelper.java:
##########
@@ -1302,6 +1303,31 @@ public static Integer openSavingsAccount(final
RequestSpecification requestSpec,
return savingsId;
}
+ // TODO: Rewrite to use fineract-client instead!
+ // Example:
org.apache.fineract.integrationtests.common.loans.LoanTransactionHelper.disburseLoan(java.lang.Long,
+ // org.apache.fineract.client.models.PostLoansLoanIdRequest)
Review Comment:
I think You should not write anymore test cases here.
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/guarantor/GuarantorTest.java:
##########
@@ -738,6 +738,49 @@ private Integer applyForLoanApplication(final Integer
clientID, final Integer lo
return this.loanTransactionHelper.getLoanId(loanApplicationJSON);
}
+ @SuppressWarnings({ "rawtypes", "unchecked" })
Review Comment:
Why we are suppressing warnings here
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/guarantor/service/GuarantorDomainServiceImpl.java:
##########
@@ -125,7 +125,18 @@ public void validateGuarantorBusinessRules(Loan loan) {
for (GuarantorFundingDetails guarantorFundingDetails :
fundingDetails) {
if (guarantorFundingDetails.getStatus().isActive() ||
guarantorFundingDetails.getStatus().isWithdrawn()
||
guarantorFundingDetails.getStatus().isCompleted()) {
- if (guarantor.isSelfGuarantee()) {
+ SavingsAccount savingsAccount =
guarantorFundingDetails.getLinkedSavingsAccount();
+ if (savingsAccount.isGroupAccount()) {
+ // Group loan with the same group's savings
account → self-guarantee
+ // Client loan or different group's savings →
external guarantee
+ if (loan.getGroupId() != null &&
loan.getGroupId().equals(savingsAccount.getGroupId())) {
Review Comment:
Right now code seems to be handling
Case | Classification
-- | --
Group loan + same group savings | self
Client loan + same group savings | external
Please confirm me about this case I am also new to this logic
Case | Classification
-- | --
Client belongs to same guarantor group | maybe self / partial self
--
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]
