Prevent deletion of options that have children attached to them.
closes #562
This commit is contained in:
parent
d5bf8734ef
commit
1216f950e2
4 changed files with 52 additions and 6 deletions
|
@ -0,0 +1,16 @@
|
||||||
|
<?php
|
||||||
|
/*
|
||||||
|
* Pterodactyl - Panel
|
||||||
|
* Copyright (c) 2015 - 2017 Dane Everitt <dane@daneeveritt.com>.
|
||||||
|
*
|
||||||
|
* This software is licensed under the terms of the MIT license.
|
||||||
|
* https://opensource.org/licenses/MIT
|
||||||
|
*/
|
||||||
|
|
||||||
|
namespace Pterodactyl\Exceptions\Service\ServiceOption;
|
||||||
|
|
||||||
|
use Pterodactyl\Exceptions\DisplayException;
|
||||||
|
|
||||||
|
class HasChildrenException extends DisplayException
|
||||||
|
{
|
||||||
|
}
|
|
@ -9,9 +9,11 @@
|
||||||
|
|
||||||
namespace Pterodactyl\Services\Services\Options;
|
namespace Pterodactyl\Services\Services\Options;
|
||||||
|
|
||||||
|
use Webmozart\Assert\Assert;
|
||||||
use Pterodactyl\Exceptions\Service\HasActiveServersException;
|
use Pterodactyl\Exceptions\Service\HasActiveServersException;
|
||||||
use Pterodactyl\Contracts\Repository\ServerRepositoryInterface;
|
use Pterodactyl\Contracts\Repository\ServerRepositoryInterface;
|
||||||
use Pterodactyl\Contracts\Repository\ServiceOptionRepositoryInterface;
|
use Pterodactyl\Contracts\Repository\ServiceOptionRepositoryInterface;
|
||||||
|
use Pterodactyl\Exceptions\Service\ServiceOption\HasChildrenException;
|
||||||
|
|
||||||
class OptionDeletionService
|
class OptionDeletionService
|
||||||
{
|
{
|
||||||
|
@ -46,17 +48,22 @@ class OptionDeletionService
|
||||||
* @return int
|
* @return int
|
||||||
*
|
*
|
||||||
* @throws \Pterodactyl\Exceptions\Service\HasActiveServersException
|
* @throws \Pterodactyl\Exceptions\Service\HasActiveServersException
|
||||||
|
* @throws \Pterodactyl\Exceptions\Service\ServiceOption\HasChildrenException
|
||||||
*/
|
*/
|
||||||
public function handle($option)
|
public function handle($option)
|
||||||
{
|
{
|
||||||
$servers = $this->serverRepository->findCountWhere([
|
Assert::integerish($option, 'First argument passed to handle must be integer, received %s.');
|
||||||
['option_id', '=', $option],
|
|
||||||
]);
|
|
||||||
|
|
||||||
|
$servers = $this->serverRepository->findCountWhere([['option_id', '=', $option]]);
|
||||||
if ($servers > 0) {
|
if ($servers > 0) {
|
||||||
throw new HasActiveServersException(trans('exceptions.service.options.delete_has_servers'));
|
throw new HasActiveServersException(trans('exceptions.service.options.delete_has_servers'));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$children = $this->repository->findCountWhere([['config_from', '=', $option]]);
|
||||||
|
if ($children > 0) {
|
||||||
|
throw new HasChildrenException(trans('exceptions.service.options.has_children'));
|
||||||
|
}
|
||||||
|
|
||||||
return $this->repository->delete($option);
|
return $this->repository->delete($option);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -24,6 +24,7 @@ return [
|
||||||
'delete_has_servers' => 'A service option with active servers attached to it cannot be deleted from the Panel.',
|
'delete_has_servers' => 'A service option with active servers attached to it cannot be deleted from the Panel.',
|
||||||
'invalid_copy_id' => 'The service option selected for copying a script from either does not exist, or is copying a script itself.',
|
'invalid_copy_id' => 'The service option selected for copying a script from either does not exist, or is copying a script itself.',
|
||||||
'must_be_child' => 'The "Copy Settings From" directive for this option must be a child option for the selected service.',
|
'must_be_child' => 'The "Copy Settings From" directive for this option must be a child option for the selected service.',
|
||||||
|
'has_children' => 'This service option is a parent to one or more other options. Please delete those options before deleting this option.',
|
||||||
],
|
],
|
||||||
'variables' => [
|
'variables' => [
|
||||||
'env_not_unique' => 'The environment variable :name must be unique to this service option.',
|
'env_not_unique' => 'The environment variable :name must be unique to this service option.',
|
||||||
|
|
|
@ -11,20 +11,22 @@ namespace Tests\Unit\Services\Services\Options;
|
||||||
|
|
||||||
use Mockery as m;
|
use Mockery as m;
|
||||||
use Tests\TestCase;
|
use Tests\TestCase;
|
||||||
|
use Pterodactyl\Exceptions\DisplayException;
|
||||||
use Pterodactyl\Exceptions\Service\HasActiveServersException;
|
use Pterodactyl\Exceptions\Service\HasActiveServersException;
|
||||||
use Pterodactyl\Contracts\Repository\ServerRepositoryInterface;
|
use Pterodactyl\Contracts\Repository\ServerRepositoryInterface;
|
||||||
use Pterodactyl\Services\Services\Options\OptionDeletionService;
|
use Pterodactyl\Services\Services\Options\OptionDeletionService;
|
||||||
use Pterodactyl\Contracts\Repository\ServiceOptionRepositoryInterface;
|
use Pterodactyl\Contracts\Repository\ServiceOptionRepositoryInterface;
|
||||||
|
use Pterodactyl\Exceptions\Service\ServiceOption\HasChildrenException;
|
||||||
|
|
||||||
class OptionDeletionServiceTest extends TestCase
|
class OptionDeletionServiceTest extends TestCase
|
||||||
{
|
{
|
||||||
/**
|
/**
|
||||||
* @var \Pterodactyl\Contracts\Repository\ServiceOptionRepositoryInterface
|
* @var \Pterodactyl\Contracts\Repository\ServiceOptionRepositoryInterface|\Mockery\Mock
|
||||||
*/
|
*/
|
||||||
protected $repository;
|
protected $repository;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @var \Pterodactyl\Contracts\Repository\ServerRepositoryInterface
|
* @var \Pterodactyl\Contracts\Repository\ServerRepositoryInterface|\Mockery\Mock
|
||||||
*/
|
*/
|
||||||
protected $serverRepository;
|
protected $serverRepository;
|
||||||
|
|
||||||
|
@ -33,6 +35,9 @@ class OptionDeletionServiceTest extends TestCase
|
||||||
*/
|
*/
|
||||||
protected $service;
|
protected $service;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Setup tests.
|
||||||
|
*/
|
||||||
public function setUp()
|
public function setUp()
|
||||||
{
|
{
|
||||||
parent::setUp();
|
parent::setUp();
|
||||||
|
@ -49,6 +54,7 @@ class OptionDeletionServiceTest extends TestCase
|
||||||
public function testOptionIsDeletedIfNoServersAreFound()
|
public function testOptionIsDeletedIfNoServersAreFound()
|
||||||
{
|
{
|
||||||
$this->serverRepository->shouldReceive('findCountWhere')->with([['option_id', '=', 1]])->once()->andReturn(0);
|
$this->serverRepository->shouldReceive('findCountWhere')->with([['option_id', '=', 1]])->once()->andReturn(0);
|
||||||
|
$this->repository->shouldReceive('findCountWhere')->with([['config_from', '=', 1]])->once()->andReturn(0);
|
||||||
$this->repository->shouldReceive('delete')->with(1)->once()->andReturn(1);
|
$this->repository->shouldReceive('delete')->with(1)->once()->andReturn(1);
|
||||||
|
|
||||||
$this->assertEquals(1, $this->service->handle(1));
|
$this->assertEquals(1, $this->service->handle(1));
|
||||||
|
@ -63,9 +69,25 @@ class OptionDeletionServiceTest extends TestCase
|
||||||
|
|
||||||
try {
|
try {
|
||||||
$this->service->handle(1);
|
$this->service->handle(1);
|
||||||
} catch (\Exception $exception) {
|
} catch (DisplayException $exception) {
|
||||||
$this->assertInstanceOf(HasActiveServersException::class, $exception);
|
$this->assertInstanceOf(HasActiveServersException::class, $exception);
|
||||||
$this->assertEquals(trans('exceptions.service.options.delete_has_servers'), $exception->getMessage());
|
$this->assertEquals(trans('exceptions.service.options.delete_has_servers'), $exception->getMessage());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Test that an exception is thrown if children options exist.
|
||||||
|
*/
|
||||||
|
public function testExceptionIsThrownIfChildrenArePresent()
|
||||||
|
{
|
||||||
|
$this->serverRepository->shouldReceive('findCountWhere')->with([['option_id', '=', 1]])->once()->andReturn(0);
|
||||||
|
$this->repository->shouldReceive('findCountWhere')->with([['config_from', '=', 1]])->once()->andReturn(1);
|
||||||
|
|
||||||
|
try {
|
||||||
|
$this->service->handle(1);
|
||||||
|
} catch (DisplayException $exception) {
|
||||||
|
$this->assertInstanceOf(HasChildrenException::class, $exception);
|
||||||
|
$this->assertEquals(trans('exceptions.service.options.has_children'), $exception->getMessage());
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue