resolve reviews

.github/workflows/ci.yml

@@ -7,7 +7,7 @@ on:
   push:
     branches: [ master ]
   pull_request:
-    branches: [ master, shift-64622] # todo0 remove shift-64622 before merge
+    branches: [ master, shift-64622] # todo0 remove shift-64622 branch before merge
 
 jobs:
   tests:

tests/AutomaticModeTest.php

@@ -15,27 +15,12 @@ beforeEach(function () {
     Event::listen(TenancyEnded::class, RevertToCentralContext::class);
 });
 
-function contextISSwitchedWhenTenancyInitialized()
-{
-    config(['tenancy.bootstrappers' => [
-        MyBootstrapper::class,
-    ]]);
-
-    $tenant = Tenant::create([
-        'id' => 'acme',
-    ]);
-
-    tenancy()->initialize($tenant);
-
-    expect(app('tenancy_initialized_for_tenant'))->toBe('acme');
-}
-
 test('context is switched when tenancy is initialized', function () {
-    contextISSwitchedWhenTenancyInitialized();
+    contextIsSwitchedWhenTenancyInitialized();
 });
 
 test('context is reverted when tenancy is ended', function () {
-    contextISSwitchedWhenTenancyInitialized();
+    contextIsSwitchedWhenTenancyInitialized();
 
     tenancy()->end();
 
@@ -104,6 +89,21 @@ test('central helper doesnt change tenancy state when called in central context'
     expect(tenant())->toBeNull();
 });
 
+function contextIsSwitchedWhenTenancyInitialized()
+{
+    config(['tenancy.bootstrappers' => [
+        MyBootstrapper::class,
+    ]]);
+
+    $tenant = Tenant::create([
+        'id' => 'acme',
+    ]);
+
+    tenancy()->initialize($tenant);
+
+    expect(app('tenancy_initialized_for_tenant'))->toBe('acme');
+}
+
 class MyBootstrapper implements TenancyBootstrapper
 {
     public function bootstrap(\Stancl\Tenancy\Contracts\Tenant $tenant)

tests/CommandsTest.php

@@ -128,7 +128,7 @@ test('database connection is switched to default when tenancy has been initializ
     databaseConnectionSwitchedToDefault();
 });
 
-test('run commands works', function () {
+test('run command works', function () {
     runCommandWorks();
 });
 

tests/ResourceSyncingTest.php

@@ -127,69 +127,11 @@ test('only the synced columns are updated in the central db', function () {
 });
 
 test('creating the resource in tenant database creates it in central database and creates the mapping', function () {
-    // Assert no user in central DB
+    creatingResourceInTenantDatabaseCreatesAndMapInCentralDatabase();
-    expect(ResourceUser::all())->toHaveCount(0);
-
-    $tenant = ResourceTenant::create();
-    migrateTenantsResource();
-
-    tenancy()->initialize($tenant);
-
-    // Create the same user in tenant DB
-    ResourceUser::create([
-        'global_id' => 'acme',
-        'name' => 'John Doe',
-        'email' => 'john@localhost',
-        'password' => 'secret',
-        'role' => 'commenter', // unsynced
-    ]);
-
-    tenancy()->end();
-
-    // Asset user was created
-    expect(CentralUser::first()->global_id)->toBe('acme');
-    expect(CentralUser::first()->role)->toBe('commenter');
-
-    // Assert mapping was created
-    expect(CentralUser::first()->tenants)->toHaveCount(1);
-
-    // Assert role change doesn't cascade
-    CentralUser::first()->update(['role' => 'central superadmin']);
-    tenancy()->initialize($tenant);
-    expect(ResourceUser::first()->role)->toBe('commenter');
 });
 
 test('trying to update synced resources from central context using tenant models results in an exception', function () {
-    // Assert no user in central DB
+    creatingResourceInTenantDatabaseCreatesAndMapInCentralDatabase();
-    expect(ResourceUser::all())->toHaveCount(0);
-
-    $tenant = ResourceTenant::create();
-    migrateTenantsResource();
-
-    tenancy()->initialize($tenant);
-
-    // Create the same user in tenant DB
-    ResourceUser::create([
-        'global_id' => 'acme',
-        'name' => 'John Doe',
-        'email' => 'john@localhost',
-        'password' => 'secret',
-        'role' => 'commenter', // unsynced
-    ]);
-
-    tenancy()->end();
-
-    // Asset user was created
-    expect(CentralUser::first()->global_id)->toBe('acme');
-    expect(CentralUser::first()->role)->toBe('commenter');
-
-    // Assert mapping was created
-    expect(CentralUser::first()->tenants)->toHaveCount(1);
-
-    // Assert role change doesn't cascade
-    CentralUser::first()->update(['role' => 'central superadmin']);
-    tenancy()->initialize($tenant);
-    expect(ResourceUser::first()->role)->toBe('commenter');
 
     tenancy()->end();
     expect(tenancy()->initialized)->toBeFalse();
@@ -560,6 +502,40 @@ test('an event is fired for all touched resources', function () {
     });
 });
 
+function creatingResourceInTenantDatabaseCreatesAndMapInCentralDatabase()
+{
+    // Assert no user in central DB
+    expect(ResourceUser::all())->toHaveCount(0);
+
+    $tenant = ResourceTenant::create();
+    migrateTenantsResource();
+
+    tenancy()->initialize($tenant);
+
+    // Create the same user in tenant DB
+    ResourceUser::create([
+        'global_id' => 'acme',
+        'name' => 'John Doe',
+        'email' => 'john@localhost',
+        'password' => 'secret',
+        'role' => 'commenter', // unsynced
+    ]);
+
+    tenancy()->end();
+
+    // Asset user was created
+    expect(CentralUser::first()->global_id)->toBe('acme');
+    expect(CentralUser::first()->role)->toBe('commenter');
+
+    // Assert mapping was created
+    expect(CentralUser::first()->tenants)->toHaveCount(1);
+
+    // Assert role change doesn't cascade
+    CentralUser::first()->update(['role' => 'central superadmin']);
+    tenancy()->initialize($tenant);
+    expect(ResourceUser::first()->role)->toBe('commenter');
+}
+
 function migrateTenantsResource()
 {
     test()->artisan('tenants:migrate', [