rymghosn commented on PR #6293: URL: https://github.com/apache/fineract/pull/6293#issuecomment-5992456088
@adamsaghy Thanks, I've added `TaxGroupMixedComponentUpdateTest` and rebased on the latest `develop`. - `updateTaxGroup_addsNewComponentsWhileUpdatingAnExistingOne` sends three new components and one existing one in the same update, and repeats that on fresh groups because the failure depends on iteration order. It checks that all four associations exist and that the existing mapping keeps its id. - `retrieveTaxGroup_exposesBusinessDate` checks the new `businessDate` field. The test exposed one more gap: a component added during an update was never linked to its group, so the insert failed on the NOT NULL `tax_group_id`. `TaxGroup.update()` now sets the owning group on new mappings. I also corrected the commit description: the old `findOneBy()` didn't throw early. Its problem was the NPE on a just-added mapping whose id is still null. To reach this path over REST, the PR also carries the `id`/`endDate` fields on `TaxGroupComponent` from #6291 (an identical hunk, so they merge cleanly in either order). The swagger also documents `businessDate` and declares the `modifiedComponents` dates of the update response as dates. Could you please re-review? Thanks! -- 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]
