FINERACT-1958-Configuration-down-payment #3355
Conversation
| private boolean enableDownPayment; | ||
|
|
||
| @Column(name = "disbursed_amount_percentage_down_payment", scale = 2, precision = 5) | ||
| private BigDecimal disbursedAmountPercentageDownPayment; |
There was a problem hiding this comment.
i am not too fond of this naming. Maybe disbursedAmountPercentageForDownPayment?
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/LoanProduct.java
Show resolved
Hide resolved
| } | ||
|
|
||
| @Test | ||
| public void loanProductCreationWithDownPaymentConfigurationTest() { |
There was a problem hiding this comment.
Can you please have the following extra tests?
- Down payment is not enabled, but percentage is set.
- Down payment is enabled, but percentage is 0 or 101
- Down payment was set and percentage also. After update loan product and turn off down payment. What is the result? What is the value of the percentage of down payment?
ruchiD
force-pushed
the
FINERACT-1958-Configuration-downpayment
branch
2 times, most recently
from
0ef01fd
to
f152f50
Compare
| .extractBigDecimalWithLocaleNamed(LoanProductConstants.DISBURSED_AMOUNT_PERCENTAGE_DOWN_PAYMENT, element); | ||
| baseDataValidator.reset().parameter(LoanProductConstants.DISBURSED_AMOUNT_PERCENTAGE_DOWN_PAYMENT) | ||
| .value(disbursedAmountPercentageDownPayment).notLessThanMin(BigDecimal.ONE) | ||
| .notGreaterThanMax(BigDecimal.valueOf(100)).scaleNotGreaterThan(Integer.valueOf(6)); |
There was a problem hiding this comment.
Integer.valueOf(6) can be simplified: 6 ;)
| // Loan Product without enable down payment configuration | ||
| final GetLoanProductsProductIdResponse getLoanProductsProductResponse = createLoanProduct(loanTransactionHelper, | ||
| delinquencyBucketId); | ||
| assertNotNull(getLoanProductsProductResponse); |
There was a problem hiding this comment.
Please check whether the result has "enabledDownPayment=false"
| // Modify Loan Product to update enable down payment configuration | ||
| PutLoanProductsProductIdResponse loanProductModifyResponse = updateLoanProduct(loanTransactionHelper, | ||
| getLoanProductsProductResponse.getId()); | ||
| assertNotNull(loanProductModifyResponse); |
There was a problem hiding this comment.
Please check whether the result has "enabledDownPayment=true"
| assertEquals(0, loanDetails.getDisbursedAmountPercentageForDownPayment().compareTo(disbursedAmountPercentageForDownPayment)); | ||
|
|
||
| // Modify Loan Product to update enable down payment configuration | ||
| PutLoanProductsProductIdResponse loanProductModifyResponse = updateLoanProduct(loanTransactionHelper, |
There was a problem hiding this comment.
Down payment is already enabled...
There was a problem hiding this comment.
Hi Adam, In this test case i changed disbursedAmount percentage and not enable downpayment config. Earlier for LoanProduct it was 12.5 and loan account was created with 12.5 and then i changed percentage to 25 but loan account has percentage 12.5 with which it was created.
| // verify Loan product configuration change | ||
| GetLoanProductsProductIdResponse getLoanProductsProductResponse_1 = loanTransactionHelper.getLoanProduct(loanProductId); | ||
| assertNotNull(getLoanProductsProductResponse_1); | ||
| assertEquals(enableDownPayment, getLoanProductsProductResponse_1.getEnableDownPayment()); |
There was a problem hiding this comment.
Down payment was already enabled, so it should be disabled in this test case, no?
There was a problem hiding this comment.
Hi Adam, In this test case i changed disbursedAmount percentage and not enable downpayment config. Earlier for LoanProduct it was 12.5 and loan account was created with 12.5 and then i changed percentage to 25 but loan account has percentage 12.5 with which it was created.
There was a problem hiding this comment.
I see. I was wondering whether we should have a test case where the down payment was enabled and % was set, but later it got disabled. Are we removing the % or leave it AS-IS?
There was a problem hiding this comment.
We are leaving downpayment AS-IS.
There was a problem hiding this comment.
Is it not failing on the validation?
I recall if the down payment percentage is passed, but the down payment is not enabled, there is a validation to catch this situations, no?
There was a problem hiding this comment.
Yes. There is a validation for this situation. I am checking if enableDownPayment is false then downPayment percentage element should not be passed.
I am not setting that to null in db if that is so.
ruchiD
force-pushed
the
FINERACT-1958-Configuration-downpayment
branch
from
f152f50
to
b3cc8af
Compare
Description
Added fields for down-payment configuration for loan product.
Validations
Swagger changes
Integration test
Describe the changes made and why they were made.
Ignore if these details are present on the associated Apache Fineract JIRA ticket.
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Write the commit message as per 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.