diff --git a/src/app/Console/Command.php b/src/app/Console/Command.php --- a/src/app/Console/Command.php +++ b/src/app/Console/Command.php @@ -43,6 +43,19 @@ } /** + * Find a group. + * + * @param string $group Group ID or email + * @param bool $withDeleted Include deleted + * + * @return \App\Group|null + */ + public function getGroup($group, $withDeleted = false) + { + return $this->getObject(\App\Group::class, $group, 'email', $withDeleted); + } + + /** * Find an object. * * @param string $objectClass The name of the class diff --git a/src/app/Console/Commands/DB/PingCommand.php b/src/app/Console/Commands/DB/PingCommand.php --- a/src/app/Console/Commands/DB/PingCommand.php +++ b/src/app/Console/Commands/DB/PingCommand.php @@ -22,16 +22,6 @@ protected $description = 'Ping the database [and wait for it to respond]'; /** - * Create a new command instance. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - } - - /** * Execute the console command. * * @return mixed diff --git a/src/app/Console/Commands/DB/VerifyTimezoneCommand.php b/src/app/Console/Commands/DB/VerifyTimezoneCommand.php --- a/src/app/Console/Commands/DB/VerifyTimezoneCommand.php +++ b/src/app/Console/Commands/DB/VerifyTimezoneCommand.php @@ -22,16 +22,6 @@ protected $description = "Verify the application's timezone compared to the DB timezone"; /** - * Create a new command instance. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - } - - /** * Execute the console command. * * @return mixed diff --git a/src/app/Console/Commands/Data/Import/IP4NetsCommand.php b/src/app/Console/Commands/Data/Import/IP4NetsCommand.php --- a/src/app/Console/Commands/Data/Import/IP4NetsCommand.php +++ b/src/app/Console/Commands/Data/Import/IP4NetsCommand.php @@ -22,16 +22,6 @@ protected $description = 'Update IP4 Networks'; /** - * Create a new command instance. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - } - - /** * Execute the console command. * * @return int diff --git a/src/app/Console/Commands/Data/Import/IP6NetsCommand.php b/src/app/Console/Commands/Data/Import/IP6NetsCommand.php --- a/src/app/Console/Commands/Data/Import/IP6NetsCommand.php +++ b/src/app/Console/Commands/Data/Import/IP6NetsCommand.php @@ -22,16 +22,6 @@ protected $description = 'Import IP6 Networks.'; /** - * Create a new command instance. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - } - - /** * Execute the console command. * * @return mixed diff --git a/src/app/Console/Commands/Data/ImportCommand.php b/src/app/Console/Commands/Data/ImportCommand.php --- a/src/app/Console/Commands/Data/ImportCommand.php +++ b/src/app/Console/Commands/Data/ImportCommand.php @@ -21,16 +21,6 @@ protected $description = 'Command description'; /** - * Create a new command instance. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - } - - /** * Execute the console command. * * @return int diff --git a/src/app/Console/Commands/Group/AddMemberCommand.php b/src/app/Console/Commands/Group/AddMemberCommand.php --- a/src/app/Console/Commands/Group/AddMemberCommand.php +++ b/src/app/Console/Commands/Group/AddMemberCommand.php @@ -30,7 +30,7 @@ { $input = $this->argument('group'); $member = \strtolower($this->argument('member')); - $group = $this->getObject(\App\Group::class, $input, 'email'); + $group = $this->getGroup($input); if (empty($group)) { $this->error("Group {$input} does not exist."); diff --git a/src/app/Console/Commands/Group/DeleteCommand.php b/src/app/Console/Commands/Group/DeleteCommand.php --- a/src/app/Console/Commands/Group/DeleteCommand.php +++ b/src/app/Console/Commands/Group/DeleteCommand.php @@ -28,7 +28,7 @@ public function handle() { $input = $this->argument('group'); - $group = $this->getObject(\App\Group::class, $input, 'email'); + $group = $this->getGroup($input); if (empty($group)) { $this->error("Group {$input} does not exist."); diff --git a/src/app/Console/Commands/Group/ForceDeleteCommand.php b/src/app/Console/Commands/Group/ForceDeleteCommand.php new file mode 100644 --- /dev/null +++ b/src/app/Console/Commands/Group/ForceDeleteCommand.php @@ -0,0 +1,47 @@ +getGroup($this->argument('group'), true); + + if (!$group) { + $this->error("Group not found."); + return 1; + } + + if (!$group->trashed()) { + $this->error("The group is not yet deleted."); + return 1; + } + + DB::beginTransaction(); + $group->forceDelete(); + DB::commit(); + } +} diff --git a/src/app/Console/Commands/Group/InfoCommand.php b/src/app/Console/Commands/Group/InfoCommand.php --- a/src/app/Console/Commands/Group/InfoCommand.php +++ b/src/app/Console/Commands/Group/InfoCommand.php @@ -28,7 +28,7 @@ public function handle() { $input = $this->argument('group'); - $group = $this->getObject(\App\Group::class, $input, 'email'); + $group = $this->getGroup($input); if (empty($group)) { $this->error("Group {$input} does not exist."); diff --git a/src/app/Console/Commands/Group/RemoveMemberCommand.php b/src/app/Console/Commands/Group/RemoveMemberCommand.php --- a/src/app/Console/Commands/Group/RemoveMemberCommand.php +++ b/src/app/Console/Commands/Group/RemoveMemberCommand.php @@ -29,8 +29,7 @@ { $input = $this->argument('group'); $member = \strtolower($this->argument('member')); - - $group = $this->getObject(\App\Group::class, $input, 'email'); + $group = $this->getGroup($input); if (empty($group)) { $this->error("Group {$input} does not exist."); diff --git a/src/app/Console/Commands/Group/RestoreCommand.php b/src/app/Console/Commands/Group/RestoreCommand.php new file mode 100644 --- /dev/null +++ b/src/app/Console/Commands/Group/RestoreCommand.php @@ -0,0 +1,47 @@ +getGroup($this->argument('group'), true); + + if (!$group) { + $this->error("Group not found."); + return 1; + } + + if (!$group->trashed()) { + $this->error("The group is not deleted."); + return 1; + } + + DB::beginTransaction(); + $group->restore(); + DB::commit(); + } +} diff --git a/src/app/Console/Commands/PlanPackages.php b/src/app/Console/Commands/PlanPackages.php --- a/src/app/Console/Commands/PlanPackages.php +++ b/src/app/Console/Commands/PlanPackages.php @@ -22,16 +22,6 @@ protected $description = "List packages for plans."; /** - * Create a new command instance. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - } - - /** * Execute the console command. * * @return mixed diff --git a/src/app/Console/Commands/PowerDNS/Domain/CreateCommand.php b/src/app/Console/Commands/PowerDNS/Domain/CreateCommand.php --- a/src/app/Console/Commands/PowerDNS/Domain/CreateCommand.php +++ b/src/app/Console/Commands/PowerDNS/Domain/CreateCommand.php @@ -21,16 +21,6 @@ protected $description = 'Create a domain in PowerDNS'; /** - * Create a new command instance. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - } - - /** * Execute the console command. * * @return mixed diff --git a/src/app/Console/Commands/PowerDNS/Domain/DeleteCommand.php b/src/app/Console/Commands/PowerDNS/Domain/DeleteCommand.php --- a/src/app/Console/Commands/PowerDNS/Domain/DeleteCommand.php +++ b/src/app/Console/Commands/PowerDNS/Domain/DeleteCommand.php @@ -21,16 +21,6 @@ protected $description = 'Delete a PowerDNS domain'; /** - * Create a new command instance. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - } - - /** * Execute the console command. * * @return mixed diff --git a/src/app/Console/Commands/PowerDNS/Domain/ReadCommand.php b/src/app/Console/Commands/PowerDNS/Domain/ReadCommand.php --- a/src/app/Console/Commands/PowerDNS/Domain/ReadCommand.php +++ b/src/app/Console/Commands/PowerDNS/Domain/ReadCommand.php @@ -21,16 +21,6 @@ protected $description = 'Read a PowerDNS domain'; /** - * Create a new command instance. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - } - - /** * Execute the console command. * * @return mixed diff --git a/src/app/Console/Development/DomainStatus.php b/src/app/Console/Development/DomainStatus.php --- a/src/app/Console/Development/DomainStatus.php +++ b/src/app/Console/Development/DomainStatus.php @@ -22,16 +22,6 @@ protected $description = "Set/get a domain's status."; /** - * Create a new command instance. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - } - - /** * Execute the console command. * * @return mixed diff --git a/src/app/Entitlement.php b/src/app/Entitlement.php --- a/src/app/Entitlement.php +++ b/src/app/Entitlement.php @@ -146,6 +146,32 @@ } /** + * Restore object entitlements. + * + * @param \App\User|\App\Domain|\App\Group $object The user|domain|group object + */ + public static function restoreEntitlementsFor($object): void + { + // We'll restore only these that were deleted last. So, first we get + // the maximum deleted_at timestamp and then use it to select + // entitlements for restore + $deleted_at = $object->entitlements()->withTrashed()->max('deleted_at'); + + if ($deleted_at) { + $threshold = (new \Carbon\Carbon($deleted_at))->subMinute(); + + // Restore object entitlements + $object->entitlements()->withTrashed() + ->where('deleted_at', '>=', $threshold) + ->update(['updated_at' => now(), 'deleted_at' => null]); + + // Note: We're assuming that cost of entitlements was correct + // on deletion, so we don't have to re-calculate it again. + // TODO: We should probably re-calculate the cost + } + } + + /** * The SKU concerned. * * @return \Illuminate\Database\Eloquent\Relations\BelongsTo diff --git a/src/app/Observers/DomainObserver.php b/src/app/Observers/DomainObserver.php --- a/src/app/Observers/DomainObserver.php +++ b/src/app/Observers/DomainObserver.php @@ -115,21 +115,7 @@ public function restored(Domain $domain) { // Restore domain entitlements - // We'll restore only these that were deleted last. So, first we get - // the maximum deleted_at timestamp and then use it to select - // domain entitlements for restore - $deleted_at = \App\Entitlement::withTrashed() - ->where('entitleable_id', $domain->id) - ->where('entitleable_type', Domain::class) - ->max('deleted_at'); - - if ($deleted_at) { - \App\Entitlement::withTrashed() - ->where('entitleable_id', $domain->id) - ->where('entitleable_type', Domain::class) - ->where('deleted_at', '>=', (new \Carbon\Carbon($deleted_at))->subMinute()) - ->update(['updated_at' => now(), 'deleted_at' => null]); - } + \App\Entitlement::restoreEntitlementsFor($domain); // Create the domain in LDAP again \App\Jobs\Domain\CreateJob::dispatch($domain->id); diff --git a/src/app/Observers/GroupObserver.php b/src/app/Observers/GroupObserver.php --- a/src/app/Observers/GroupObserver.php +++ b/src/app/Observers/GroupObserver.php @@ -76,6 +76,31 @@ } /** + * Handle the group "restoring" event. + * + * @param \App\Group $group The group + * + * @return void + */ + public function restoring(Group $group) + { + // Make sure it's not DELETED/LDAP_READY/SUSPENDED anymore + if ($group->isDeleted()) { + $group->status ^= Group::STATUS_DELETED; + } + if ($group->isLdapReady()) { + $group->status ^= Group::STATUS_LDAP_READY; + } + if ($group->isSuspended()) { + $group->status ^= Group::STATUS_SUSPENDED; + } + + $group->status |= Group::STATUS_ACTIVE; + + // Note: $group->save() is invoked between 'restoring' and 'restored' events + } + + /** * Handle the group "restored" event. * * @param \App\Group $group The group @@ -84,7 +109,10 @@ */ public function restored(Group $group) { - // + // Restore group entitlements + \App\Entitlement::restoreEntitlementsFor($group); + + \App\Jobs\Group\CreateJob::dispatch($group->id); } /** diff --git a/src/app/Observers/UserObserver.php b/src/app/Observers/UserObserver.php --- a/src/app/Observers/UserObserver.php +++ b/src/app/Observers/UserObserver.php @@ -299,37 +299,15 @@ */ public function restored(User $user) { - $wallets = $user->wallets()->pluck('id')->all(); - // Restore user entitlements - // We'll restore only these that were deleted last. So, first we get - // the maximum deleted_at timestamp and then use it to select - // entitlements for restore - $deleted_at = \App\Entitlement::withTrashed() - ->where('entitleable_id', $user->id) - ->where('entitleable_type', User::class) - ->max('deleted_at'); - - if ($deleted_at) { - $threshold = (new \Carbon\Carbon($deleted_at))->subMinute(); - - // We need at least the user domain so it can be created in ldap. - // FIXME: What if the domain is owned by someone else? - $domain = $user->domain(); - if ($domain->trashed() && !$domain->isPublic()) { - // Note: Domain entitlements will be restored by the DomainObserver - $domain->restore(); - } - - // Restore user entitlements - \App\Entitlement::withTrashed() - ->where('entitleable_id', $user->id) - ->where('entitleable_type', User::class) - ->where('deleted_at', '>=', $threshold) - ->update(['updated_at' => now(), 'deleted_at' => null]); - - // Note: We're assuming that cost of entitlements was correct - // on user deletion, so we don't have to re-calculate it again. + \App\Entitlement::restoreEntitlementsFor($user); + + // We need at least the user domain so it can be created in ldap. + // FIXME: What if the domain is owned by someone else? + $domain = $user->domain(); + if ($domain->trashed() && !$domain->isPublic()) { + // Note: Domain entitlements will be restored by the DomainObserver + $domain->restore(); } // FIXME: Should we reset user aliases? or re-validate them in any way? diff --git a/src/tests/Feature/Console/Group/ForceDeleteTest.php b/src/tests/Feature/Console/Group/ForceDeleteTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Feature/Console/Group/ForceDeleteTest.php @@ -0,0 +1,71 @@ +deleteTestGroup('group-test@kolabnow.com'); + } + + /** + * {@inheritDoc} + */ + public function tearDown(): void + { + $this->deleteTestGroup('group-test@kolabnow.com'); + + parent::tearDown(); + } + + /** + * Test command runs + */ + public function testHandle(): void + { + Queue::fake(); + + // Warning: We're not using artisan() here, as this will not + // allow us to test "empty output" cases + + $group = $this->getTestGroup('group-test@kolabnow.com'); + + // Non-existing group + $code = \Artisan::call("group:force-delete test@group.com"); + $output = trim(\Artisan::output()); + + $this->assertSame(1, $code); + $this->assertSame("Group not found.", $output); + + // Non-deleted group + $code = \Artisan::call("group:force-delete {$group->email}"); + $output = trim(\Artisan::output()); + + $this->assertSame(1, $code); + $this->assertSame("The group is not yet deleted.", $output); + + $group->delete(); + $this->assertTrue($group->trashed()); + + // Existing and deleted group + $code = \Artisan::call("group:force-delete {$group->email}"); + $output = trim(\Artisan::output()); + + $this->assertSame(0, $code); + $this->assertSame('', $output); + $this->assertCount( + 0, + Group::withTrashed()->where('email', $group->email)->get() + ); + } +} diff --git a/src/tests/Feature/Console/Group/RestoreTest.php b/src/tests/Feature/Console/Group/RestoreTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Feature/Console/Group/RestoreTest.php @@ -0,0 +1,68 @@ +deleteTestGroup('group-test@kolabnow.com'); + } + + /** + * {@inheritDoc} + */ + public function tearDown(): void + { + $this->deleteTestGroup('group-test@kolabnow.com'); + + parent::tearDown(); + } + + /** + * Test command runs + */ + public function testHandle(): void + { + Queue::fake(); + + // Warning: We're not using artisan() here, as this will not + // allow us to test "empty output" cases + + $group = $this->getTestGroup('group-test@kolabnow.com'); + + // Non-existing group + $code = \Artisan::call("group:restore test@group.com"); + $output = trim(\Artisan::output()); + + $this->assertSame(1, $code); + $this->assertSame("Group not found.", $output); + + // Non-deleted group + $code = \Artisan::call("group:restore {$group->email}"); + $output = trim(\Artisan::output()); + + $this->assertSame(1, $code); + $this->assertSame("The group is not deleted.", $output); + + $group->delete(); + $this->assertTrue($group->trashed()); + + // Existing and deleted group + $code = \Artisan::call("group:restore {$group->email}"); + $output = trim(\Artisan::output()); + + $this->assertSame(0, $code); + $this->assertSame('', $output); + $this->assertFalse($group->fresh()->trashed()); + } +} diff --git a/src/tests/Feature/GroupTest.php b/src/tests/Feature/GroupTest.php --- a/src/tests/Feature/GroupTest.php +++ b/src/tests/Feature/GroupTest.php @@ -213,6 +213,56 @@ $this->assertSame($result->id, $group->id); } + /* + * Test group restoring + */ + public function testRestore(): void + { + Queue::fake(); + + $user = $this->getTestUser('user-test@kolabnow.com'); + $group = $this->getTestGroup('group-test@kolabnow.com'); + $group->assignToWallet($user->wallets->first()); + + $entitlements = \App\Entitlement::where('entitleable_id', $group->id); + + $this->assertSame(1, $entitlements->count()); + + $group->delete(); + + $this->assertTrue($group->fresh()->trashed()); + $this->assertSame(0, $entitlements->count()); + $this->assertSame(1, $entitlements->withTrashed()->count()); + + Queue::fake(); + + $group->restore(); + $group->refresh(); + + $this->assertFalse($group->trashed()); + $this->assertFalse($group->isDeleted()); + $this->assertFalse($group->isSuspended()); + $this->assertFalse($group->isLdapReady()); + $this->assertTrue($group->isActive()); + + $this->assertSame(1, $entitlements->count()); + $entitlements->get()->each(function ($ent) { + $this->assertTrue($ent->updated_at->greaterThan(\Carbon\Carbon::now()->subSeconds(5))); + }); + + Queue::assertPushed(\App\Jobs\Group\CreateJob::class, 1); + Queue::assertPushed( + \App\Jobs\Group\CreateJob::class, + function ($job) use ($group) { + $groupEmail = TestCase::getObjectProperty($job, 'groupEmail'); + $groupId = TestCase::getObjectProperty($job, 'groupId'); + + return $groupEmail === $group->email + && $groupId === $group->id; + } + ); + } + /** * Tests for GroupSettingsTrait functionality and GroupSettingObserver */