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 2022/09/01 12:16:00 UTC
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2550: Loan Delinquency Classification job
adamsaghy commented on code in PR #2550:
URL: https://github.com/apache/fineract/pull/2550#discussion_r960579769
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/DelinquencyBucketsIntegrationTest.java:
##########
@@ -271,19 +282,41 @@ public void testLoanClassificationJob() {
loanTransactionHelper.disburseLoanWithNetDisbursalAmount(operationDate, loanId, principalAmount);
// When
+ // Run first time the Job
final String jobName = "Loan Delinquency Classification";
schedulerJobHelper.executeAndAwaitJob(jobName);
- final GetLoansLoanIdResponse getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanId);
- log.info("Loan Delinquency Range {}", getLoansLoanIdResponse.getDelinquencyRange().getClassification());
+ // Get loan details expecting to have not a delinquency classification
+ GetLoansLoanIdResponse getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanId);
+ final GetDelinquencyRangesResponse firstTestCase = getLoansLoanIdResponse.getDelinquencyRange();
+ log.info("Loan Delinquency Range is null {}", (firstTestCase == null));
+ GetLoansLoanIdRepaymentSchedule getLoanRepaymentSchedule = getLoansLoanIdResponse.getRepaymentSchedule();
+ if (getLoanRepaymentSchedule != null) {
+ log.info("Loan with {} periods", getLoanRepaymentSchedule.getPeriods().size());
+ for (GetLoansLoanIdRepaymentPeriod period : getLoanRepaymentSchedule.getPeriods()) {
+ log.info("Period number {} for due date {}", period.getPeriod(), period.getDueDate());
+ }
+ }
+
+ // Move the Business date to get older the loan and to have an overdue loan
+ businessDate = businessDate.plusDays(40);
+ log.info("Current date {}", businessDate);
+ BusinessDateHelper.updateBusinessDate(requestSpec, responseSpec, BusinessDateType.COB_DATE, businessDate);
+ // Run Second time the Job
+ schedulerJobHelper.executeAndAwaitJob(jobName);
+
+ // Get loan details expecting to have a delinquency classification
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanId);
+ final GetDelinquencyRangesResponse secondTestCase = getLoansLoanIdResponse.getDelinquencyRange();
+ log.info("Loan Delinquency Range is {}", secondTestCase.getClassification());
Review Comment:
Use assertion please to validate whether it does not have delinquencyClassification!
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/DelinquencyBucketsIntegrationTest.java:
##########
@@ -271,19 +282,41 @@ public void testLoanClassificationJob() {
loanTransactionHelper.disburseLoanWithNetDisbursalAmount(operationDate, loanId, principalAmount);
// When
+ // Run first time the Job
final String jobName = "Loan Delinquency Classification";
schedulerJobHelper.executeAndAwaitJob(jobName);
- final GetLoansLoanIdResponse getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanId);
- log.info("Loan Delinquency Range {}", getLoansLoanIdResponse.getDelinquencyRange().getClassification());
+ // Get loan details expecting to have not a delinquency classification
+ GetLoansLoanIdResponse getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanId);
+ final GetDelinquencyRangesResponse firstTestCase = getLoansLoanIdResponse.getDelinquencyRange();
+ log.info("Loan Delinquency Range is null {}", (firstTestCase == null));
+ GetLoansLoanIdRepaymentSchedule getLoanRepaymentSchedule = getLoansLoanIdResponse.getRepaymentSchedule();
+ if (getLoanRepaymentSchedule != null) {
+ log.info("Loan with {} periods", getLoanRepaymentSchedule.getPeriods().size());
+ for (GetLoansLoanIdRepaymentPeriod period : getLoanRepaymentSchedule.getPeriods()) {
+ log.info("Period number {} for due date {}", period.getPeriod(), period.getDueDate());
+ }
+ }
+
+ // Move the Business date to get older the loan and to have an overdue loan
+ businessDate = businessDate.plusDays(40);
+ log.info("Current date {}", businessDate);
+ BusinessDateHelper.updateBusinessDate(requestSpec, responseSpec, BusinessDateType.COB_DATE, businessDate);
+ // Run Second time the Job
+ schedulerJobHelper.executeAndAwaitJob(jobName);
+
+ // Get loan details expecting to have a delinquency classification
+ getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanId);
+ final GetDelinquencyRangesResponse secondTestCase = getLoansLoanIdResponse.getDelinquencyRange();
+ log.info("Loan Delinquency Range is {}", secondTestCase.getClassification());
// Then
assertNotNull(delinquencyBucketResponse);
assertNotNull(getLoanProductsProductResponse);
+ assertNull(firstTestCase);
Review Comment:
you can move this assertion to fail fast
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -282,26 +282,28 @@ private Map<String, Object> lookUpDelinquencyRange(final Loan loan, final Delinq
Map<String, Object> changes = new HashMap<>();
if (ageDays <= 0) { // No Delinquency
- log.debug("Loan {} without delinquency range with {} days", loan.getId(), ageDays);
+ log.info("Loan {} without delinquency range with {} days", loan.getId(), ageDays);
changes = setLoanDelinquencyTag(loan, null);
} else {
// Sort the ranges based on the minAgeDays
- List<DelinquencyRange> ranges = sortDelinquencyRangesByMinAge(delinquencyBucket.getRanges());
+ final List<DelinquencyRange> ranges = sortDelinquencyRangesByMinAge(delinquencyBucket.getRanges());
- for (DelinquencyRange delinquencyRange : ranges) {
- if (delinquencyRange.getMaximumAgeDays() == null) {
+ for (final DelinquencyRange delinquencyRange : ranges) {
+ if (delinquencyRange.getMaximumAgeDays() == null) { // Last Range in the Bucket
if (delinquencyRange.getMinimumAgeDays() <= ageDays) {
- log.debug("Loan {} with delinquency range {} with {} days", loan.getId(), delinquencyRange.getClassification(),
+ log.info("Loan {} with delinquency range {} with {} days", loan.getId(), delinquencyRange.getClassification(),
ageDays);
changes = setLoanDelinquencyTag(loan, delinquencyRange.getId());
+ log.info("A Loan {} with range {}", loan.getId(), changes.get("current"));
break;
}
} else {
if (delinquencyRange.getMinimumAgeDays() <= ageDays && delinquencyRange.getMaximumAgeDays() >= ageDays) {
- log.debug("Loan {} with delinquency range {} with {} days", loan.getId(), delinquencyRange.getClassification(),
+ log.info("Loan {} with delinquency range {} with {} days", loan.getId(), delinquencyRange.getClassification(),
Review Comment:
Please do not log at info level this. It is better with debug. (IMO)
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -282,26 +282,28 @@ private Map<String, Object> lookUpDelinquencyRange(final Loan loan, final Delinq
Map<String, Object> changes = new HashMap<>();
if (ageDays <= 0) { // No Delinquency
- log.debug("Loan {} without delinquency range with {} days", loan.getId(), ageDays);
+ log.info("Loan {} without delinquency range with {} days", loan.getId(), ageDays);
Review Comment:
Please do not log at info level this. It is better with debug. (IMO)
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -282,26 +282,28 @@ private Map<String, Object> lookUpDelinquencyRange(final Loan loan, final Delinq
Map<String, Object> changes = new HashMap<>();
if (ageDays <= 0) { // No Delinquency
- log.debug("Loan {} without delinquency range with {} days", loan.getId(), ageDays);
+ log.info("Loan {} without delinquency range with {} days", loan.getId(), ageDays);
changes = setLoanDelinquencyTag(loan, null);
} else {
// Sort the ranges based on the minAgeDays
- List<DelinquencyRange> ranges = sortDelinquencyRangesByMinAge(delinquencyBucket.getRanges());
+ final List<DelinquencyRange> ranges = sortDelinquencyRangesByMinAge(delinquencyBucket.getRanges());
- for (DelinquencyRange delinquencyRange : ranges) {
- if (delinquencyRange.getMaximumAgeDays() == null) {
+ for (final DelinquencyRange delinquencyRange : ranges) {
+ if (delinquencyRange.getMaximumAgeDays() == null) { // Last Range in the Bucket
if (delinquencyRange.getMinimumAgeDays() <= ageDays) {
- log.debug("Loan {} with delinquency range {} with {} days", loan.getId(), delinquencyRange.getClassification(),
+ log.info("Loan {} with delinquency range {} with {} days", loan.getId(), delinquencyRange.getClassification(),
ageDays);
changes = setLoanDelinquencyTag(loan, delinquencyRange.getId());
+ log.info("A Loan {} with range {}", loan.getId(), changes.get("current"));
break;
}
} else {
if (delinquencyRange.getMinimumAgeDays() <= ageDays && delinquencyRange.getMaximumAgeDays() >= ageDays) {
- log.debug("Loan {} with delinquency range {} with {} days", loan.getId(), delinquencyRange.getClassification(),
+ log.info("Loan {} with delinquency range {} with {} days", loan.getId(), delinquencyRange.getClassification(),
ageDays);
changes = setLoanDelinquencyTag(loan, delinquencyRange.getId());
+ log.info("B Loan {} with range {}", loan.getId(), changes.get("current"));
Review Comment:
Please do not log at info level this. It is better with debug. (IMO)
What is "B Loan"? :)
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/serialization/LoanProductDataValidator.java:
##########
@@ -278,7 +278,7 @@ public void validateForCreate(final String json) {
final Long delinquencyBucketId = this.fromApiJsonHelper.extractLongNamed(LoanProductConstants.DELINQUENCY_BUCKET_PARAM_NAME,
element);
baseDataValidator.reset().parameter(LoanProductConstants.DELINQUENCY_BUCKET_PARAM_NAME).value(delinquencyBucketId)
- .ignoreIfNull().integerGreaterThanZero();
+ .integerGreaterThanZero();
Review Comment:
integerGreaterThanZero will not throw error if the value is null, but it shall fail... Please add "notNull()"
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/DelinquencyBucketsIntegrationTest.java:
##########
@@ -271,19 +282,41 @@ public void testLoanClassificationJob() {
loanTransactionHelper.disburseLoanWithNetDisbursalAmount(operationDate, loanId, principalAmount);
// When
+ // Run first time the Job
final String jobName = "Loan Delinquency Classification";
schedulerJobHelper.executeAndAwaitJob(jobName);
- final GetLoansLoanIdResponse getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanId);
- log.info("Loan Delinquency Range {}", getLoansLoanIdResponse.getDelinquencyRange().getClassification());
+ // Get loan details expecting to have not a delinquency classification
+ GetLoansLoanIdResponse getLoansLoanIdResponse = loanTransactionHelper.getLoan(requestSpec, responseSpec, loanId);
+ final GetDelinquencyRangesResponse firstTestCase = getLoansLoanIdResponse.getDelinquencyRange();
+ log.info("Loan Delinquency Range is null {}", (firstTestCase == null));
Review Comment:
Use assertion please to validate whether it does not have delinquencyClassification!
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/jobs/setloandelinquencytags/SetLoanDelinquencyTagsTasklet.java:
##########
@@ -36,16 +36,19 @@
@RequiredArgsConstructor
public class SetLoanDelinquencyTagsTasklet implements Tasklet {
- private final LoanProductRepository loanProductRepository;
private final DelinquencyWritePlatformService delinquencyWritePlatformService;
private final LoanReadPlatformService loanReadPlatformService;
@Override
public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) throws Exception {
+ ThreadLocalContextUtil.setActionContext(ActionContext.COB);
+ log.info("Run job for date {}", DateUtils.getBusinessLocalDate());
Review Comment:
This should be debug level logging (IMO)
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -282,26 +282,28 @@ private Map<String, Object> lookUpDelinquencyRange(final Loan loan, final Delinq
Map<String, Object> changes = new HashMap<>();
if (ageDays <= 0) { // No Delinquency
- log.debug("Loan {} without delinquency range with {} days", loan.getId(), ageDays);
+ log.info("Loan {} without delinquency range with {} days", loan.getId(), ageDays);
changes = setLoanDelinquencyTag(loan, null);
} else {
// Sort the ranges based on the minAgeDays
- List<DelinquencyRange> ranges = sortDelinquencyRangesByMinAge(delinquencyBucket.getRanges());
+ final List<DelinquencyRange> ranges = sortDelinquencyRangesByMinAge(delinquencyBucket.getRanges());
- for (DelinquencyRange delinquencyRange : ranges) {
- if (delinquencyRange.getMaximumAgeDays() == null) {
+ for (final DelinquencyRange delinquencyRange : ranges) {
+ if (delinquencyRange.getMaximumAgeDays() == null) { // Last Range in the Bucket
if (delinquencyRange.getMinimumAgeDays() <= ageDays) {
- log.debug("Loan {} with delinquency range {} with {} days", loan.getId(), delinquencyRange.getClassification(),
+ log.info("Loan {} with delinquency range {} with {} days", loan.getId(), delinquencyRange.getClassification(),
ageDays);
changes = setLoanDelinquencyTag(loan, delinquencyRange.getId());
+ log.info("A Loan {} with range {}", loan.getId(), changes.get("current"));
Review Comment:
Please do not log at info level this. It is better with debug. (IMO)
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -282,26 +282,28 @@ private Map<String, Object> lookUpDelinquencyRange(final Loan loan, final Delinq
Map<String, Object> changes = new HashMap<>();
if (ageDays <= 0) { // No Delinquency
- log.debug("Loan {} without delinquency range with {} days", loan.getId(), ageDays);
+ log.info("Loan {} without delinquency range with {} days", loan.getId(), ageDays);
changes = setLoanDelinquencyTag(loan, null);
} else {
// Sort the ranges based on the minAgeDays
- List<DelinquencyRange> ranges = sortDelinquencyRangesByMinAge(delinquencyBucket.getRanges());
+ final List<DelinquencyRange> ranges = sortDelinquencyRangesByMinAge(delinquencyBucket.getRanges());
- for (DelinquencyRange delinquencyRange : ranges) {
- if (delinquencyRange.getMaximumAgeDays() == null) {
+ for (final DelinquencyRange delinquencyRange : ranges) {
+ if (delinquencyRange.getMaximumAgeDays() == null) { // Last Range in the Bucket
if (delinquencyRange.getMinimumAgeDays() <= ageDays) {
- log.debug("Loan {} with delinquency range {} with {} days", loan.getId(), delinquencyRange.getClassification(),
+ log.info("Loan {} with delinquency range {} with {} days", loan.getId(), delinquencyRange.getClassification(),
Review Comment:
Please do not log at info level this. It is better with debug. (IMO)
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/jobs/setloandelinquencytags/SetLoanDelinquencyTagsTasklet.java:
##########
@@ -36,16 +36,19 @@
@RequiredArgsConstructor
public class SetLoanDelinquencyTagsTasklet implements Tasklet {
- private final LoanProductRepository loanProductRepository;
private final DelinquencyWritePlatformService delinquencyWritePlatformService;
private final LoanReadPlatformService loanReadPlatformService;
@Override
public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) throws Exception {
+ ThreadLocalContextUtil.setActionContext(ActionContext.COB);
Review Comment:
This is not a COB job, please do not set the ActionContext to COB
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/jobs/setloandelinquencytags/SetLoanDelinquencyTagsTasklet.java:
##########
@@ -36,16 +36,19 @@
@RequiredArgsConstructor
public class SetLoanDelinquencyTagsTasklet implements Tasklet {
- private final LoanProductRepository loanProductRepository;
private final DelinquencyWritePlatformService delinquencyWritePlatformService;
private final LoanReadPlatformService loanReadPlatformService;
@Override
public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) throws Exception {
+ ThreadLocalContextUtil.setActionContext(ActionContext.COB);
+ log.info("Run job for date {}", DateUtils.getBusinessLocalDate());
Collection<LoanScheduleDelinquencyData> loanScheduleDelinquencyData = this.loanReadPlatformService
.retrieveScheduleDelinquencyData(DateUtils.getBusinessLocalDate());
- log.debug("Were found {} items", loanScheduleDelinquencyData.size());
+ log.info("Were found {} items", loanScheduleDelinquencyData.size());
for (LoanScheduleDelinquencyData loanDelinquencyData : loanScheduleDelinquencyData) {
+ log.info("Processing Loan {} with due date {} and {} overdue days", loanDelinquencyData.getLoanId(),
Review Comment:
This should be debug level logging (IMO)
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/jobs/setloandelinquencytags/SetLoanDelinquencyTagsTasklet.java:
##########
@@ -36,16 +36,19 @@
@RequiredArgsConstructor
public class SetLoanDelinquencyTagsTasklet implements Tasklet {
- private final LoanProductRepository loanProductRepository;
private final DelinquencyWritePlatformService delinquencyWritePlatformService;
private final LoanReadPlatformService loanReadPlatformService;
@Override
public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) throws Exception {
+ ThreadLocalContextUtil.setActionContext(ActionContext.COB);
+ log.info("Run job for date {}", DateUtils.getBusinessLocalDate());
Collection<LoanScheduleDelinquencyData> loanScheduleDelinquencyData = this.loanReadPlatformService
.retrieveScheduleDelinquencyData(DateUtils.getBusinessLocalDate());
- log.debug("Were found {} items", loanScheduleDelinquencyData.size());
+ log.info("Were found {} items", loanScheduleDelinquencyData.size());
Review Comment:
This should be debug level logging (IMO)
--
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