adamsaghy commented on PR #6196: URL: https://github.com/apache/fineract/pull/6196#issuecomment-5954100674
The gap: removing collateral doesn't return quantity to the client The Javadoc on mergeLoanCollateral says it removes any existing row that isn't in the new set. Here is what happens on modify: The request sends collateral: [], or leaves out an existing row's id. The assembler returns a set without that row. !equals is true, so changes records collateral as changed. mergeLoanCollateral drops the row from the loan, and orphanRemoval deletes it from the database. Nothing adds that row's quantity back to the client. Other paths do return it: the dedicated delete endpoint (LoanCollateralManagementWriteServiceImpl:45) and releaseAttachedCollaterals (LoanApplicationWritePlatformServiceJpaRepositoryImpl:862). Replacing a row is worse. If the request sends a new item without an id in place of an existing one, the new quantity is taken from the client, the old row is deleted, and the old quantity is never returned. Before this PR the loan's collateral never changed on modify, so the PR is what makes this reachable. The fix is to return the quantity to the client for each row mergeLoanCollateral removes, either there or in the assembler. A test that modifies the loan with collateral: [] and checks the client is back at 100 would cover it. Smaller mismatch The PR description still says no tests were added and doesn't mention the Loan.mergeLoanCollateral and Swagger changes. Both came in later commits, and the description should be updated to match. -- 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]
