mariiaKraievska commented on PR #6398:
URL: https://github.com/apache/fineract/pull/6398#issuecomment-5794250057
> Would you please review whether the below is valid?
>
> ANNUAL_EIR product invariants are only enforced when the strategy is in
the request — WorkingCapitalLoanProductDataValidator.java:834
>
> The discount > 0 and annualEir non-null checks sit behind
requirePairedFields, which is now false for any partial update that omits
paymentAmountCalculationStrategy — the common case. Two requests slip through
on an existing ANNUAL_EIR product:
>
> * PUT {"discount": 0, "locale":"en"} — validateTermFields only applies
zeroOrPositiveAmount(), the strategy block is skipped, and updateRelatedDetail
sets discount = 0.
> * PUT {"annualEir": null, "locale":"en"} — parameterExists is true for an
explicit JSON null, extractBigDecimalNamed returns null, so neither the notNull
nor the positiveAmount branch fires; isChangeInBigDecimalParameterNamed sees
43.7562 → null as a change and nulls the column. ({"annualEir": 0} is correctly
rejected.)
>
> Neither crashes, but the product is left in a state validateForCreate
would reject and that can no longer originate loans: the next application hits
must.be.greater.than.zero.for.annual.eir.strategy or annualEir is mandatory
from WorkingCapitalLoanApplicationDataValidator, so the operator sees the
failure one API call away from the request that caused it. Suggest checking the
effective post-update values (request value falling back to the persisted one)
rather than gating on requirePairedFields.
Good catch, you’re right.
Those partial PUTs could leave an ANNUAL_EIR product in a state create
would’ve rejected. I switched the checks to effective values (request falling
back to what’s already stored) and added tests for discount: 0 and annualEir:
null.
--
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]
