adamsaghy commented on PR #6467: URL: https://github.com/apache/fineract/pull/6467#issuecomment-5733523248
@somasorosdpc Please review the below findings / concerns: **Findings** - WorkingCapitalAdvancedPaymentAllocationsJsonParser.java:67 — CONFIRMED. The template now offers only 5 transaction types, but the write path still parses into the core PaymentAllocationTransactionType and the validator only rejects null. POSTing transactionType: "DOWN_PAYMENT" is accepted and persisted; WorkingCapitalLoanAllocationRequestFactory.getAllocationRule never matches it, so the product carries a dead rule the UI can no longer show or edit. - WorkingCapitalLoanProductApiResourceSwagger.java:195 — CONFIRMED. PostPaymentAllocation.transactionType still documents all 14 core types as allowableValues, directly contradicting the restricted template this PR introduces. WorkingCapitalPaymentAllocationTransactionType.java:61 — PLAUSIBLE. ordinal() + 1 reassigns ids the WC template previously returned (PAYOUT_REFUND 5→3, GOODWILL_CREDIT 6→4, CHARGE_ADJUSTMENT 8→5) and collides with different types in /loanproducts/template (id 3 = DOWN_PAYMENT there). Server round-trips use code, so this only bites a client keying off id. - WorkingCapitalPaymentAllocationTransactionType.java:20 — CONFIRMED. The Apache license header is duplicated (lines 1–18 and 20–37). Compiles and RAT passes, but the second copy should go. Two notes that aren't findings: the new enum's loanTransactionType field and isDefault() are currently unused (all domain/persistence code still uses the core enum), and WorkingCapitalLoanProductCRUDTest only asserts the template list is non-empty — nothing pins the restricted set, so the change has no test coverage. -- 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]
