rymghosn commented on PR #6286: URL: https://github.com/apache/fineract/pull/6286#issuecomment-5992456560
@adamsaghy Agreed, and done. I removed the code-level name checks and added unique constraints instead (`uq_m_tax_component_name` on `m_tax_component.name`, `uq_m_tax_group_name` on `m_tax_group.name`), in a new changeset `0258_add_unique_constraint_tax_component_and_group_name.xml`. The entities declare the same constraints. - **Existing data:** a tenant that already has duplicate names couldn't otherwise get the constraint. Before adding it, the changeset keeps the name on the oldest row and appends the id to the later duplicates, e.g. `VAT (12)`. - **Errors:** a violation is mapped the same way charges handle a duplicate name. It's a `PlatformDataIntegrityException` with `error.msg.tax.component.duplicate.name` / `error.msg.tax.group.duplicate.name` (HTTP 403). Other integrity problems still go through `ErrorHandler.getMappable`. - **Case sensitivity:** comparison now follows the database collation (case-sensitive on PostgreSQL), like the other unique names in Fineract. - **Scope:** I also dropped the duplicate-component-in-group check from this PR. It's unrelated to names, and the existing overlap validation already rejects real duplicates. - **Tests:** `TaxNameUniquenessTest` covers create and rename for both components and groups. It fails on `develop` and passes with this PR, and the existing `TaxesTest` still passes. Rebased on the latest `develop`. 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]
