You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "taskain7 (via GitHub)" <gi...@apache.org> on 2023/04/12 14:52:02 UTC
[GitHub] [fineract] taskain7 opened a new pull request, #3114: [FINERACT-1678] Exclude loan IDs from execution context parameter name fix
taskain7 opened a new pull request, #3114:
URL: https://github.com/apache/fineract/pull/3114
## Description
fixing the paratemer name for `Loan COB parameter`
--
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] taskain7 commented on a diff in pull request #3114: [FINERACT-1678] Exclude loan IDs from execution context parameter name fix
Posted by "taskain7 (via GitHub)" <gi...@apache.org>.
taskain7 commented on code in PR #3114:
URL: https://github.com/apache/fineract/pull/3114#discussion_r1165086339
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanRepository.java:
##########
@@ -91,8 +91,7 @@ public interface LoanRepository extends JpaRepository<Loan, Long>, JpaSpecificat
String FIND_ALL_NON_CLOSED_LOANS_BEHIND_OR_NULL_BY_LOAN_IDS = "select loan.id, loan.lastClosedBusinessDate from Loan loan where loan.id IN :loanIds and loan.loanStatus in (100,200,300,303,304) and (loan.lastClosedBusinessDate < :cobBusinessDate or "
+ "loan.lastClosedBusinessDate is null)";
- String FIND_ALL_NON_CLOSED_LOANS_BEHIND_OR_NULL_BY_MIN_AND_MAX_LOAN_ID = "select loan.id from Loan loan where loan.id BETWEEN :minLoanId and :maxLoanId and loan.loanStatus in (100,200,300,303,304) and (loan.lastClosedBusinessDate < "
- + ":cobBusinessDate or loan.lastClosedBusinessDate is null)";
+ String FIND_ALL_NON_CLOSED_LOANS_BEHIND_OR_NULL_BY_MIN_AND_MAX_LOAN_ID = "select loan.id from Loan loan where loan.id BETWEEN :minLoanId and :maxLoanId and loan.loanStatus in (100,200,300,303,304) and (:cobBusinessDate = loan.lastClosedBusinessDate or loan.lastClosedBusinessDate is NULL)";
Review Comment:
renamed
--
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] taskain7 commented on a diff in pull request #3114: [FINERACT-1678] Exclude loan IDs from execution context parameter name fix
Posted by "taskain7 (via GitHub)" <gi...@apache.org>.
taskain7 commented on code in PR #3114:
URL: https://github.com/apache/fineract/pull/3114#discussion_r1165075350
##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/LoanItemReader.java:
##########
@@ -49,7 +49,7 @@ public void beforeStep(@NotNull StepExecution stepExecution) {
loanIds = Collections.emptyList();
} else {
loanIds = loanRepository.findAllNonClosedLoansBehindOrNullByMinAndMaxLoanId(loanCOBParameter.getMinLoanId(),
- loanCOBParameter.getMaxLoanId(), ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE));
+ loanCOBParameter.getMaxLoanId(), ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE).minusDays(1));
Review Comment:
moved the `NUMBER_OF_DAYS_BEHIND` to `LoanCOBConstant` and use it here as well
--
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 #3114: [FINERACT-1678] Exclude loan IDs from execution context parameter name fix
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3114:
URL: https://github.com/apache/fineract/pull/3114#discussion_r1164569528
##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/LoanItemReader.java:
##########
@@ -49,7 +49,7 @@ public void beforeStep(@NotNull StepExecution stepExecution) {
loanIds = Collections.emptyList();
} else {
loanIds = loanRepository.findAllNonClosedLoansBehindOrNullByMinAndMaxLoanId(loanCOBParameter.getMinLoanId(),
- loanCOBParameter.getMaxLoanId(), ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE));
+ loanCOBParameter.getMaxLoanId(), ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE).minusDays(1));
Review Comment:
Also this one is not correct as it is ignoring any already locked loans (locked by INLINE COB for example)
--
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 #3114: [FINERACT-1678] Exclude loan IDs from execution context parameter name fix
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3114:
URL: https://github.com/apache/fineract/pull/3114#discussion_r1164557036
##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/ApplyLoanLockTasklet.java:
##########
@@ -65,7 +65,7 @@ public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull Chu
loanIds = Collections.emptyList();
} else {
loanIds = new ArrayList<>(loanRepository.findAllNonClosedLoansBehindOrNullByMinAndMaxLoanId(loanCOBParameter.getMinLoanId(),
- loanCOBParameter.getMaxLoanId(), ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE)));
+ loanCOBParameter.getMaxLoanId(), ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE).minusDays(1)));
Review Comment:
why -1 day?
--
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 #3114: [FINERACT-1678] Exclude loan IDs from execution context parameter name fix
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3114:
URL: https://github.com/apache/fineract/pull/3114#discussion_r1164577521
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanRepository.java:
##########
@@ -91,8 +91,7 @@ public interface LoanRepository extends JpaRepository<Loan, Long>, JpaSpecificat
String FIND_ALL_NON_CLOSED_LOANS_BEHIND_OR_NULL_BY_LOAN_IDS = "select loan.id, loan.lastClosedBusinessDate from Loan loan where loan.id IN :loanIds and loan.loanStatus in (100,200,300,303,304) and (loan.lastClosedBusinessDate < :cobBusinessDate or "
+ "loan.lastClosedBusinessDate is null)";
- String FIND_ALL_NON_CLOSED_LOANS_BEHIND_OR_NULL_BY_MIN_AND_MAX_LOAN_ID = "select loan.id from Loan loan where loan.id BETWEEN :minLoanId and :maxLoanId and loan.loanStatus in (100,200,300,303,304) and (loan.lastClosedBusinessDate < "
- + ":cobBusinessDate or loan.lastClosedBusinessDate is null)";
+ String FIND_ALL_NON_CLOSED_LOANS_BEHIND_OR_NULL_BY_MIN_AND_MAX_LOAN_ID = "select loan.id from Loan loan where loan.id BETWEEN :minLoanId and :maxLoanId and loan.loanStatus in (100,200,300,303,304) and (:cobBusinessDate = loan.lastClosedBusinessDate or loan.lastClosedBusinessDate is NULL)";
Review Comment:
the naming is confusing, it is not filtering by it is behind or not... it checks whether the date is matching
--
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 #3114: [FINERACT-1678] Exclude loan IDs from execution context parameter name fix
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #3114:
URL: https://github.com/apache/fineract/pull/3114
--
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 #3114: [FINERACT-1678] Exclude loan IDs from execution context parameter name fix
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3114:
URL: https://github.com/apache/fineract/pull/3114#discussion_r1164569528
##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/LoanItemReader.java:
##########
@@ -49,7 +49,7 @@ public void beforeStep(@NotNull StepExecution stepExecution) {
loanIds = Collections.emptyList();
} else {
loanIds = loanRepository.findAllNonClosedLoansBehindOrNullByMinAndMaxLoanId(loanCOBParameter.getMinLoanId(),
- loanCOBParameter.getMaxLoanId(), ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE));
+ loanCOBParameter.getMaxLoanId(), ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE).minusDays(1));
Review Comment:
Also this one is not correct as it is ignoring any already locked loans (locked by INLINE COB for example)
--
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 #3114: [FINERACT-1678] Exclude loan IDs from execution context parameter name fix
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3114:
URL: https://github.com/apache/fineract/pull/3114#discussion_r1164560200
##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/LoanItemReader.java:
##########
@@ -49,7 +49,7 @@ public void beforeStep(@NotNull StepExecution stepExecution) {
loanIds = Collections.emptyList();
} else {
loanIds = loanRepository.findAllNonClosedLoansBehindOrNullByMinAndMaxLoanId(loanCOBParameter.getMinLoanId(),
- loanCOBParameter.getMaxLoanId(), ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE));
+ loanCOBParameter.getMaxLoanId(), ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE).minusDays(1));
Review Comment:
why -1 day?
--
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 #3114: [FINERACT-1678] Exclude loan IDs from execution context parameter name fix
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3114:
URL: https://github.com/apache/fineract/pull/3114#discussion_r1164559989
##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/LoanCOBPartitioner.java:
##########
@@ -66,34 +69,24 @@ private Map<String, ExecutionContext> getPartitions(int partitionSize, Set<Busin
stopJobExecution();
return Map.of();
}
- int partitionIndex = 1;
- createNewPartition(partitions, partitionIndex, cobBusinessSteps);
if (!Objects.isNull(minAndMaxLoanId)) {
- long remainingLoanIdCount = minAndMaxLoanId.getMaxLoanId() - minAndMaxLoanId.getMinLoanId() + 1;
- long startLoanId = minAndMaxLoanId.getMinLoanId();
- long endLoanId;
- do {
- String key = PARTITION_PREFIX + partitionIndex;
- ExecutionContext executionContext = partitions.get(key);
- if (remainingLoanIdCount > partitionSize) {
- endLoanId = startLoanId + partitionSize - 1;
- partitionIndex++;
- createNewPartition(partitions, partitionIndex, cobBusinessSteps);
- } else {
- endLoanId = minAndMaxLoanId.getMaxLoanId();
- }
- executionContext.put(LoanCOBConstant.LOAN_COB_PARAMETER, new LoanCOBParameter(startLoanId, endLoanId));
- startLoanId = startLoanId + partitionSize;
- remainingLoanIdCount = minAndMaxLoanId.getMaxLoanId() - endLoanId;
- } while (remainingLoanIdCount > 0);
+ List<Long> loanIdsInRange = LongStream.rangeClosed(minAndMaxLoanId.getMinLoanId(), minAndMaxLoanId.getMaxLoanId()).boxed()
+ .toList();
+ List<List<Long>> loanIdPartitions = Lists.partition(loanIdsInRange, partitionSize);
+ for (int i = 0; i < loanIdPartitions.size(); i++) {
+ createNewPartition(partitions, i + 1, cobBusinessSteps, loanIdPartitions.get(i));
+ }
+ } else {
+ createNewPartition(partitions, 1, cobBusinessSteps, List.of(0L));
}
return partitions;
}
private void createNewPartition(Map<String, ExecutionContext> partitions, int partitionIndex,
- Set<BusinessStepNameAndOrder> cobBusinessSteps) {
+ Set<BusinessStepNameAndOrder> cobBusinessSteps, List<Long> loanIds) {
ExecutionContext executionContext = new ExecutionContext();
executionContext.put(LoanCOBConstant.BUSINESS_STEPS, cobBusinessSteps);
+ executionContext.put(LoanCOBConstant.LOAN_COB_PARAMETER, new LoanCOBParameter(loanIds.get(0), loanIds.get(loanIds.size() - 1)));
Review Comment:
why -1 day?
--
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 #3114: [FINERACT-1678] Exclude loan IDs from execution context parameter name fix
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3114:
URL: https://github.com/apache/fineract/pull/3114#discussion_r1164582616
##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/LoanItemReader.java:
##########
@@ -49,7 +49,7 @@ public void beforeStep(@NotNull StepExecution stepExecution) {
loanIds = Collections.emptyList();
} else {
loanIds = loanRepository.findAllNonClosedLoansBehindOrNullByMinAndMaxLoanId(loanCOBParameter.getMinLoanId(),
- loanCOBParameter.getMaxLoanId(), ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE));
+ loanCOBParameter.getMaxLoanId(), ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE).minusDays(1));
Review Comment:
It is ignoring if the loan is already locked and maybe failed...maybe it failed on the very first execution and the closed business date is still NULL, but it failed 2 days ago... it is edge case, but we need to be careful here...(probably not cause any issues in the other hand...)
--
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] taskain7 commented on a diff in pull request #3114: [FINERACT-1678] Exclude loan IDs from execution context parameter name fix
Posted by "taskain7 (via GitHub)" <gi...@apache.org>.
taskain7 commented on code in PR #3114:
URL: https://github.com/apache/fineract/pull/3114#discussion_r1165075146
##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/ApplyLoanLockTasklet.java:
##########
@@ -65,7 +65,7 @@ public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull Chu
loanIds = Collections.emptyList();
} else {
loanIds = new ArrayList<>(loanRepository.findAllNonClosedLoansBehindOrNullByMinAndMaxLoanId(loanCOBParameter.getMinLoanId(),
- loanCOBParameter.getMaxLoanId(), ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE)));
+ loanCOBParameter.getMaxLoanId(), ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE).minusDays(1)));
Review Comment:
moved the `NUMBER_OF_DAYS_BEHIND` to `LoanCOBConstant` and use it here as well
--
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