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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
