FINERACT-1981: Fix progressive loan undo previous repayment #4090
Conversation
| @@ -2665,12 +2665,6 @@ public ChangedTransactionDetail adjustExistingTransaction(final LoanTransaction | |||
| final LoanLifecycleStateMachine loanLifecycleStateMachine, final LoanTransaction transactionForAdjustment, | |||
| final List<Long> existingTransactionIds, final List<Long> existingReversedTransactionIds, | |||
| final ScheduleGeneratorDTO scheduleGeneratorDTO, final ExternalId reversalExternalId) { | |||
| HolidayDetailDTO holidayDetailDTO = scheduleGeneratorDTO.getHolidayDetailDTO(); | |||
There was a problem hiding this comment.
I am assuming here you just moved out since these validations already part of the LoanTransactionValidator, right?
There was a problem hiding this comment.
are these validations triggered in any other place where the LoanTransactioNValidator is not in the call hierarchy? have you had the chance to check that?
There was a problem hiding this comment.
this method is only called from LoanWritePlatformServiceJpaRepositoryImpl.adjustLoanTransaction(), where i have moved the validations
magyari-adam
force-pushed
the
feature/FINERACT-1981
branch
from
1bec7ec to
e017796
Compare
| @@ -2665,12 +2665,6 @@ public ChangedTransactionDetail adjustExistingTransaction(final LoanTransaction | |||
| final LoanLifecycleStateMachine loanLifecycleStateMachine, final LoanTransaction transactionForAdjustment, | |||
| final List<Long> existingTransactionIds, final List<Long> existingReversedTransactionIds, | |||
| final ScheduleGeneratorDTO scheduleGeneratorDTO, final ExternalId reversalExternalId) { | |||
| HolidayDetailDTO holidayDetailDTO = scheduleGeneratorDTO.getHolidayDetailDTO(); | |||
There was a problem hiding this comment.
are these validations triggered in any other place where the LoanTransactioNValidator is not in the call hierarchy? have you had the chance to check that?
| HolidayDetailDTO holidayDetailDTO = scheduleGeneratorDTO.getHolidayDetailDTO(); | ||
| if (loan.getLoanRepaymentScheduleDetail().getLoanScheduleType().equals(LoanScheduleType.CUMULATIVE)) { | ||
| // validate cumulative | ||
| loanTransactionValidator.validateActivityNotBeforeLastTransactionDate(loan, transactionToAdjust.getTransactionDate(), | ||
| LoanEvent.LOAN_REPAYMENT_OR_WAIVER); | ||
| } | ||
| // common validations | ||
| loanTransactionValidator.validateRepaymentDateIsOnHoliday(newTransactionDetail.getTransactionDate(), | ||
| holidayDetailDTO.isAllowTransactionsOnHoliday(), holidayDetailDTO.getHolidays()); | ||
| loanTransactionValidator.validateRepaymentDateIsOnNonWorkingDay(newTransactionDetail.getTransactionDate(), | ||
| holidayDetailDTO.getWorkingDays(), holidayDetailDTO.isAllowTransactionsOnNonWorkingDay()); |
There was a problem hiding this comment.
I don't see any test changes related to the validations. Is this covered somewhere?
There was a problem hiding this comment.
i have added a cucumber test for the new intended progressive loan behaviour, cumulative is unchanged
There was a problem hiding this comment.
but wait, what about triggering the validation errors? I understand the positive case passes but do we have coverage for the validations triggering errors?
There was a problem hiding this comment.
i have only moved these from loan, existing tests should cover it
Description
Should be able to undo earlier (not latest) transactions on progressive loans.
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.