From 22d1b2065bdd882899e5121a44a0b96bb614dc4c Mon Sep 17 00:00:00 2001 From: Abrar Ahmad Date: Fri, 4 Nov 2022 19:04:29 +0500 Subject: [PATCH 1/2] [4.x] Add feature to ignore the resource synchronization based on provided condition. (#993) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * wip * add test * readability * remove group * DisabledSync -> ConditionalSync; test both cases with dataset Co-authored-by: Samuel Ć tancl --- src/Contracts/Syncable.php | 2 + src/Database/Concerns/ResourceSyncing.php | 10 ++++- src/Database/Models/TenantPivot.php | 2 +- t | 3 ++ tests/ResourceSyncingTest.php | 53 +++++++++++++++++++++++ 5 files changed, 67 insertions(+), 3 deletions(-) create mode 100755 t diff --git a/src/Contracts/Syncable.php b/src/Contracts/Syncable.php index a481f318..f8e7fd84 100644 --- a/src/Contracts/Syncable.php +++ b/src/Contracts/Syncable.php @@ -18,4 +18,6 @@ interface Syncable /** Get the attributes used for creating the *other* model (i.e. tenant if this is the central one, and central if this is the tenant one). */ public function getSyncedCreationAttributes(): array|null; // todo come up with a better name + + public function shouldSync(): bool; } diff --git a/src/Database/Concerns/ResourceSyncing.php b/src/Database/Concerns/ResourceSyncing.php index fd63738d..ea9f83b4 100644 --- a/src/Database/Concerns/ResourceSyncing.php +++ b/src/Database/Concerns/ResourceSyncing.php @@ -13,8 +13,9 @@ trait ResourceSyncing public static function bootResourceSyncing(): void { static::saved(function (Syncable $model) { - /** @var ResourceSyncing $model */ - $model->triggerSyncEvent(); + if ($model->shouldSync()) { + $model->triggerSyncEvent(); + } }); static::creating(function (self $model) { @@ -37,4 +38,9 @@ trait ResourceSyncing { return null; } + + public function shouldSync(): bool + { + return true; + } } diff --git a/src/Database/Models/TenantPivot.php b/src/Database/Models/TenantPivot.php index 2c7583c1..3cc614a9 100644 --- a/src/Database/Models/TenantPivot.php +++ b/src/Database/Models/TenantPivot.php @@ -14,7 +14,7 @@ class TenantPivot extends Pivot static::saved(function (self $pivot) { $parent = $pivot->pivotParent; - if ($parent instanceof Syncable) { + if ($parent instanceof Syncable && $parent->shouldSync()) { $parent->triggerSyncEvent(); } }); diff --git a/t b/t new file mode 100755 index 00000000..3c74f2e8 --- /dev/null +++ b/t @@ -0,0 +1,3 @@ +#!/bin/bash + +docker-compose exec -T test vendor/bin/pest --no-coverage --filter "$@" diff --git a/tests/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index 430c52ef..e1586bc1 100644 --- a/tests/ResourceSyncingTest.php +++ b/tests/ResourceSyncingTest.php @@ -763,6 +763,43 @@ function creatingResourceInTenantDatabaseCreatesAndMapInCentralDatabase() expect(ResourceUser::first()->role)->toBe('commenter'); } +test('resources are synced only when sync is enabled', function (bool $enabled) { + app()->instance('_tenancy_test_shouldSync', $enabled); + + [$tenant1, $tenant2] = createTenantsAndRunMigrations(); + migrateUsersTableForTenants(); + + tenancy()->initialize($tenant1); + + TenantUserWithConditionalSync::create([ + 'global_id' => 'absd', + 'name' => 'John Doe', + 'email' => 'john@localhost', + 'password' => 'password', + 'role' => 'commenter', + ]); + + tenancy()->end(); + + expect(CentralUserWithConditionalSync::all())->toHaveCount($enabled ? 1 : 0); + expect(CentralUserWithConditionalSync::whereGlobalId('absd')->exists())->toBe($enabled); + + $centralUser = CentralUserWithConditionalSync::create([ + 'global_id' => 'acme', + 'name' => 'John Doe', + 'email' => 'john@localhost', + 'password' => 'password', + 'role' => 'commenter', + ]); + + $centralUser->tenants()->attach('t2'); + + $tenant2->run(function () use ($enabled) { + expect(TenantUserWithConditionalSync::all())->toHaveCount($enabled ? 1 : 0); + expect(TenantUserWithConditionalSync::whereGlobalId('acme')->exists())->toBe($enabled); + }); +})->with([[true], [false]]); + /** * Create two tenants and run migrations for those tenants. */ @@ -979,3 +1016,19 @@ class ResourceUserProvidingMixture extends ResourceUser ]; } } + +class CentralUserWithConditionalSync extends CentralUser +{ + public function shouldSync(): bool + { + return app('_tenancy_test_shouldSync'); + } +} + +class TenantUserWithConditionalSync extends ResourceUser +{ + public function shouldSync(): bool + { + return app('_tenancy_test_shouldSync'); + } +} From b7a6953231146fa92d970edd9dbb16ec74dd2a4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Fri, 4 Nov 2022 15:17:54 +0100 Subject: [PATCH 2/2] mention ./t in CONTRIBUTING.md --- CONTRIBUTING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d76a686a..03aa4ee8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,6 +10,8 @@ Run `composer docker-up` to start the containers. Then run `composer test` to ru If you need to pass additional flags to phpunit, use `./test --foo` instead of `composer test --foo`. Composer scripts unfortunately don't pass CLI arguments. +If you want to run a specific test (or test file), you can also use `./t 'name of the test'`. This is equivalent to `./test --no-coverage --filter 'name of the test'`. + When you're done testing, run `composer docker-down` to shut down the containers. ### Docker on M1