Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FINERACT-2081: Enable inline COB execution for locked loans #4171

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

alberto-art3ch
Copy link
Contributor

Description

If the loan is locked, but there is any error attached to the lock, it means the prior execution of COB / Inline COB failed.

For user experience it would be better if we let the inline COB to be triggered again in these situations hence noone is processing the loan at the moment, it just indicates there was some problem during the last processing.

FINERACT-2081

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.

@@ -241,7 +241,7 @@ private boolean isLockOverrulable(LoanAccountLock loanAccountLock) {
if (isBypassUser()) {
return true;
} else {
return StringUtils.isNotBlank(loanAccountLock.getError());
return StringUtils.isBlank(loanAccountLock.getError());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think it is correct to do this. Please undo this change. We can change the lock if we are going with bypass user or there was an error stored on the lock.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Change undone

@@ -75,7 +77,9 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
proceed(filterChain, request, response);
} else {
try {
log.info("calculateRelevantLoanIds ==>");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No info log please! Use debug!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Logs removed

@@ -240,7 +250,8 @@ private Long getTopLevelLoanIdFromBatchRequest(BatchRequest batchRequest) throws

private List<Long> getLoanIdsFromApi(String pathInfo) {
List<Long> loanIds = getLoanIdList(pathInfo);
if (isLoanHardLocked(loanIds)) {
log.info("getLoanIdsFromApi {}", loanIds.size());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No info log please!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Logs removed

@@ -217,7 +227,7 @@ private List<Long> getLoanIdsFromBatchApi(BodyCachingHttpServletRequestWrapper r
// check the body for Loan ID
Long loanId = getTopLevelLoanIdFromBatchRequest(batchRequest);
if (loanId != null) {
if (isLoanHardLocked(loanId)) {
if (isLoanHardLocked(loanId) && isLockOverrulable(loanId)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong condition. If the lock has an error stored, we should allow to run the inline cob!

@@ -240,7 +250,8 @@ private Long getTopLevelLoanIdFromBatchRequest(BatchRequest batchRequest) throws

private List<Long> getLoanIdsFromApi(String pathInfo) {
List<Long> loanIds = getLoanIdList(pathInfo);
if (isLoanHardLocked(loanIds)) {
log.info("getLoanIdsFromApi {}", loanIds.size());
if (isLoanHardLocked(loanIds) && isLockOverrulable(loanIds)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong condition. If the lock has an error stored, we should allow to run the inline cob!

@alberto-art3ch alberto-art3ch force-pushed the enhancement/loan_cob_when_its_locked branch from 8ecb7a9 to 0d60d5d Compare November 18, 2024 15:43
Copy link
Contributor Author

@alberto-art3ch alberto-art3ch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Updated

@@ -241,7 +241,7 @@ private boolean isLockOverrulable(LoanAccountLock loanAccountLock) {
if (isBypassUser()) {
return true;
} else {
return StringUtils.isNotBlank(loanAccountLock.getError());
return StringUtils.isBlank(loanAccountLock.getError());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Change undone

@@ -75,7 +77,9 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
proceed(filterChain, request, response);
} else {
try {
log.info("calculateRelevantLoanIds ==>");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Logs removed

@@ -240,7 +250,8 @@ private Long getTopLevelLoanIdFromBatchRequest(BatchRequest batchRequest) throws

private List<Long> getLoanIdsFromApi(String pathInfo) {
List<Long> loanIds = getLoanIdList(pathInfo);
if (isLoanHardLocked(loanIds)) {
log.info("getLoanIdsFromApi {}", loanIds.size());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Logs removed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants