Cocoa-Puffs commented on PR #6385:
URL: https://github.com/apache/fineract/pull/6385#issuecomment-5604829728
@galovics I have addressed the concerns!
> The template amount is just balance.getTotalOutstanding() as it stands
right now, so for a WC loan where penalties and discount fee accrue daily and
backdated transactions are allowed, a payoff quote for any date other than
today is going to be wrong with no way for the caller to tell.
Working Capital loans employ what is essentially a cash-like accounting,
meaning that fee accrued is only dependent on how much money was repaid, not
the time elapsed. Every outstanding bucket is `due − paid − writtenOff`, all
plain columns; there is no time term anywhere. `applyDisbursement` folds the
whole discount into principal at disbursement, and
`realizedIncomeFromDiscountFee` only moves income recognition, it never touches
`getTotalOutstanding()`. Only a payment or a charge moves the number, and both
are explicit commands. A date-parameterised recomputation would return the same
value for every date. I have added multiple scenarios into
`WorkingCapitalLoanPrepayment.feature`
For the same reason we only echo transactionDate. I have added a validation
so it cannot be future dated, because the repayment validator also rejects
future dated transactions I would like to highlight that as far as I can see
this is unprecedented when it comes to transaction templates, no other template
does similar validations, leaving it to the various transaction processing
validators to reject the transactions if they are sent with the wrong value.
> repository.getReferenceById(loanId) returns a lazy proxy - an unknown
loanId won't fail here, it'll throw EntityNotFoundException on first property
access (getCurrency()), which has no exception mapper and surfaces as a 500
instead of a 404. retrieveOne in the same class does this correctly via
findByIdWithFullDetails(...).orElseThrow(...) - worth reusing that pattern
here, especially since getCurrency() and the balance can both be null in
general.
The method now throws `WorkingCapitalLoanNotFoundException` if a loan is not
found with the given id, mirroring other methods.
> The new Swagger response DTO
(WorkingCapitalLoanTransactionTemplateResponse) doesn't match what the endpoint
actually returns - it declares id (which the data class doesn't have) and omits
currency (which it does). The generated client will always get null for a field
that never exists and won't expose the one that does.
I have modified the swagger response DTO to match what the endpoint actually
returns.
> Smaller things: the permission check happens after loan resolution rather
than before (every other endpoint in this resource does it the other way), and
this also introduces a second, parallel "template" mechanism on the same
resource ({loanId}/transactions/template?command=X vs. the existing
{loanId}/template?templateType=X) with different query param names and
different backing services for the same concept - worth confirming that's
intentional rather than folding prepayLoan into the existing one.
Moved the permission check to before the loan resolution. As for the
template mechanism, I believe it mirrors core behaviour.
Term loans already have two template types split between lifecycle vs
transaction operations.
|endpoint | core supports | WC supports|
-- | -- | --
{loanId}/template?templateType= | approval — one value | approve, disburse,
repayment, goodwillCredit, creditBalanceRefund, recoveryPayment, discountFee,
discountFeeAdjustment, chargeOff — nine
{loanId}/transactions/template?command= | repayment, prepayLoan, chargeOff,
writeOff, foreclosure, payoutRefund, goodwillCredit, … | prepayLoan
The problem is more the opposite. WC currently crams in 7 transaction
templates into the lifecycle endpoint. That is why the split looks arbitrary
today. I have migrated the 7 transaction templates to the proper transaction
template endpoint to mirror core behaviour.
|endpoint | commands|
-- | --
{loanId}/template?templateType= | approve, disburse
{loanId}/transactions/template?command= | repayment, goodwillCredit,
creditBalanceRefund, recoveryPayment, discountFee, discountFeeAdjustment,
chargeOff, prepayLoan
--
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]
