mirror of
https://github.com/archtechx/tenancy.git
synced 2026-08-06 06:54:03 +00:00
Fix duplicate entry error when updating TenantPivot columns (#1469)
`TriggerSyncingEvents` registered the pivot attach listener on `saved`, which fires on both inserts and updates. So updating pivot columns on a `TenantPivot` (e.g. via `updateExistingPivot()`) re-ran the attach flow and tried to create the tenant resource again, causing a duplicate entry error. Switched the listener to `created` so we only attach when the pivot record is first created (detaching already uses `deleting`, so using `created` makes things a bit more consistent). Added a regression test before the fix (https://github.com/archtechx/tenancy/pull/1469/commits/5e3eb4322cc487d540796317e813ac2ffe076646). The fix (https://github.com/archtechx/tenancy/pull/1469/commits/d2fb4bc0d524cd98ec680e973b791f475afc3548) then made the test pass. Also improved the `saving` listener's comment a bit so that it's clear where the "central resource not available" exception actually comes from, since that wasn't obvious (also added the `@throws` annotation to `getCentralResourceAndTenant()`). Closes #1467
This commit is contained in:
parent
3156c87ee3
commit
869ad78454
2 changed files with 35 additions and 4 deletions
|
|
@ -22,13 +22,14 @@ trait TriggerSyncingEvents
|
||||||
public static function bootTriggerSyncingEvents(): void
|
public static function bootTriggerSyncingEvents(): void
|
||||||
{
|
{
|
||||||
static::saving(static function (self $pivot) {
|
static::saving(static function (self $pivot) {
|
||||||
// Try getting the central resource to see if it is available
|
// Try getting the central resource to see if it is available.
|
||||||
// If it is not available, throw an exception to interrupt the saving process
|
// If it is not, getCentralResourceAndTenant() throws (indirectly, via findCentralResource() -> getResourceClass()),
|
||||||
// And prevent creating a pivot record without a central resource
|
// interrupting the save, preventing the creation of a pivot record without a central resource.
|
||||||
$pivot->getCentralResourceAndTenant();
|
$pivot->getCentralResourceAndTenant();
|
||||||
});
|
});
|
||||||
|
|
||||||
static::saved(static function (self $pivot) {
|
// Only attach when the pivot is created
|
||||||
|
static::created(static function (self $pivot) {
|
||||||
/**
|
/**
|
||||||
* @var static&Pivot $pivot
|
* @var static&Pivot $pivot
|
||||||
* @var SyncMaster|null $centralResource
|
* @var SyncMaster|null $centralResource
|
||||||
|
|
@ -55,6 +56,10 @@ trait TriggerSyncingEvents
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @throws CentralResourceNotAvailableInPivotException Throws when the tenant is the pivot parent
|
||||||
|
* but the central resource class cannot be resolved (thrown indirectly via findCentralResource() -> getResourceClass())
|
||||||
|
*/
|
||||||
public function getCentralResourceAndTenant(): array
|
public function getCentralResourceAndTenant(): array
|
||||||
{
|
{
|
||||||
/** @var $this&Pivot $this */
|
/** @var $this&Pivot $this */
|
||||||
|
|
|
||||||
|
|
@ -274,6 +274,32 @@ test('attaching central resources to tenants or vice versa creates synced tenant
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('updating pivot column does not re-create the synced tenant resource', function () {
|
||||||
|
// Add an extra pivot column so we can update it
|
||||||
|
Schema::table('tenant_users', fn (Blueprint $table) => $table->string('note')->nullable());
|
||||||
|
|
||||||
|
$centralUser = CentralUser::create([
|
||||||
|
'global_id' => 'acme',
|
||||||
|
'name' => 'John Doe',
|
||||||
|
'email' => 'john@localhost',
|
||||||
|
'password' => 'secret',
|
||||||
|
'role' => 'commenter',
|
||||||
|
]);
|
||||||
|
|
||||||
|
$tenant = Tenant::create();
|
||||||
|
migrateUsersTableForTenants();
|
||||||
|
|
||||||
|
// Attaching creates the tenant resource
|
||||||
|
$centralUser->tenants()->attach($tenant);
|
||||||
|
$tenant->run(fn () => expect(TenantUser::count())->toBe(1));
|
||||||
|
|
||||||
|
// Updating a pivot column does not re-attach and create the resource again
|
||||||
|
// (which would throw a duplicate entry error -- regression test for #1467)
|
||||||
|
$centralUser->tenants()->updateExistingPivot($tenant->getTenantKey(), ['note' => 'foo']);
|
||||||
|
|
||||||
|
$tenant->run(fn () => expect(TenantUser::count())->toBe(1));
|
||||||
|
});
|
||||||
|
|
||||||
test('detaching central users from tenants or vice versa force deletes the synced tenant resource', function (bool $attachUserToTenant) {
|
test('detaching central users from tenants or vice versa force deletes the synced tenant resource', function (bool $attachUserToTenant) {
|
||||||
$centralUser = CentralUser::create([
|
$centralUser = CentralUser::create([
|
||||||
'global_id' => 'acme',
|
'global_id' => 'acme',
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue