[IP-481]: merge duplicate clients#580
Draft
nielsdrost7 wants to merge 1 commit into
Draft
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1e74eaa to
d043201
Compare
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A Merge clients bulk action on the company-panel clients list. Select exactly two clients, pick which one to keep, and every record of the duplicate is moved onto the primary before the duplicate is soft-deleted.
RelationMergeService(Modules/Clients/Services)merge(Relation $primary, Relation $duplicate): Relation, inside a DB transaction:InvalidArgumentException(translated) when the two records are the same or belong to different companies.invoices.customer_id,quotes.prospect_id,payments.customer_idexpenses.vendor_idandexpenses.customer_id(both FKrelations)tasks.customer_id,projects.customer_id,contacts.relation_idaddresses.addressable,communications.communicationable,notes.notablewithoutGlobalScopes()so trashed invoices/quotes and records outside the current tenant scope move too (the same-company guard has already run).trading_name,unique_name,id_number,coc_number,vat_number,currency_code,language,primary_contact_idare copied from the duplicate only where the primary's value is empty.Soft deletes on
relationsNew migration
2026_07_04_000001_add_soft_deletes_to_relations_tableadds nullabledeleted_at;Relationnow uses theSoftDeletestrait. Schema check: therelationstable has no unique indexes (unique_nameis just a nullable column), so soft-deleted rows cannot collide with new inserts. One behavioural note:RelationService::generateRelationNumber()derives the next number from the latest non-trashed relation, so a number belonging to a merged-away duplicate could in principle be reissued — there is no DB unique constraint onrelation_number, so nothing breaks, but worth knowing.Bulk action (RelationsTable)
Added only inside the existing
->toolbarActions()BulkActionGroupto keep the conflict surface with #221 minimal (row-action group untouched). Gated onPermission::EDIT_RELATIONS. Validates that exactly two records are selected (danger notification otherwise), shows a radio to choose the primary, calls the service, and notifies success/failure.Closes
Closes #481 — merge duplicate clients (bulk action + service re-parenting all dependent records + soft delete)
Follow-up issues (not resolved here — reopen if auto-closed on merge)
Closes #589 — Field-by-field conflict picker for client merge (out of scope for #481's initial cut; currently the primary always wins on conflicts)
Tests
Modules/Clients/Tests/Feature/RelationMergeTest.php— 8 PHPUnit tests: re-parenting of every dependent record type (incl. morphs and notes), relations visible under the primary after merge, gap-fill without overwrite,assertSoftDeletedon the duplicate, both guard clauses, the Livewire bulk action happy path, and the not-exactly-two-selected rejection.CI note
Upstream CI is not running; validated locally (PHP 8.3, sqlite in-memory) against the
origin/developbaseline: