From d9ddd1bb8cf4f52d9c73b8ef888255da189b13ed Mon Sep 17 00:00:00 2001 From: Toby Twigger Date: Mon, 30 Mar 2020 18:45:37 +0100 Subject: [PATCH] More optimisation --- .../ParticipantApi/RoleController.php | 21 +-- app/Http/Middleware/BindRoleRepository.php | 39 ++++++ app/Rules/PositionCanBeUsed.php | 3 + app/Rules/UserIsAvailableToBeAssigned.php | 6 +- app/Support/LogicRoleRepository.php | 121 ++++++++++++++++++ app/Support/LogicUserGroupRepository.php | 96 ++++++++++++++ routes/admin/web.php | 4 +- routes/participant/api.php | 2 +- routes/participant/web.php | 4 +- .../ParticipantApi/RoleControllerTest.php | 12 +- 10 files changed, 274 insertions(+), 34 deletions(-) create mode 100644 app/Http/Middleware/BindRoleRepository.php create mode 100644 app/Support/LogicRoleRepository.php create mode 100644 app/Support/LogicUserGroupRepository.php diff --git a/app/Http/Controllers/ParticipantApi/RoleController.php b/app/Http/Controllers/ParticipantApi/RoleController.php index d63f52a..5d3ea71 100644 --- a/app/Http/Controllers/ParticipantApi/RoleController.php +++ b/app/Http/Controllers/ParticipantApi/RoleController.php @@ -11,36 +11,17 @@ use BristolSU\Module\AssignRoles\Http\Requests\ParticipantApi\RoleController\UpdateRequest; use BristolSU\Support\Activity\Activity; use BristolSU\Support\Authentication\Contracts\Authentication; -use BristolSU\Support\Logic\Contracts\LogicRepository; -use BristolSU\Support\Logic\Contracts\LogicTester; use BristolSU\Support\ModuleInstance\ModuleInstance; class RoleController extends Controller { - /** - * @var LogicTester - */ - private $logicTester; - - public function __construct(LogicTester $logicTester) - { - $this->logicTester = $logicTester; - } - - public function index(Activity $activity, ModuleInstance $moduleInstance, Role $role, Authentication $authentication, LogicRepository $logicRepository) + public function index(Activity $activity, ModuleInstance $moduleInstance, Role $role, Authentication $authentication) { $this->authorize('role.index'); $roles = $role->allThroughGroup( $authentication->getGroup() ); - if(settings('logic_group', null) !== null) { - $logic = $logicRepository->getById(settings('logic_group')); - - $roles = $roles->filter(function(\BristolSU\ControlDB\Contracts\Models\Role $role) use ($logic) { - return $this->logicTester->evaluate($logic, null, $role->group(), $role); - })->values(); - } return $roles->map(function(\BristolSU\ControlDB\Contracts\Models\Role $role) { $roleArray = $role->toArray(); diff --git a/app/Http/Middleware/BindRoleRepository.php b/app/Http/Middleware/BindRoleRepository.php new file mode 100644 index 0000000..9815c1e --- /dev/null +++ b/app/Http/Middleware/BindRoleRepository.php @@ -0,0 +1,39 @@ +logicTester = $logicTester; + } + + public function handle(Request $request, \Closure $next) + { + app()->extend(Role::class, function($roleRepository, $app) { + return new LogicRoleRepository($roleRepository, $this->logicTester); + }); + + app()->extend(UserRole::class, function($userRole, $app) { + return new LogicUserGroupRepository($userRole, $this->logicTester); + }); + + return $next($request); + } + +} \ No newline at end of file diff --git a/app/Rules/PositionCanBeUsed.php b/app/Rules/PositionCanBeUsed.php index bcef277..0d70c74 100644 --- a/app/Rules/PositionCanBeUsed.php +++ b/app/Rules/PositionCanBeUsed.php @@ -5,6 +5,8 @@ use BristolSU\ControlDB\Contracts\Repositories\Role; use BristolSU\Module\AssignRoles\Support\PositionSettingRetrieval; use BristolSU\Support\Authentication\Contracts\Authentication; +use BristolSU\Support\Logic\Contracts\LogicRepository; +use BristolSU\Support\Logic\Facade\LogicTester; use Illuminate\Contracts\Validation\Rule; class PositionCanBeUsed implements Rule @@ -42,6 +44,7 @@ public function passes($attribute, $value) } if(in_array($value, $settings['only_one_role'])) { $roles = $this->roleRepository->allThroughGroup($this->group()); + return $roles->filter(function(\BristolSU\ControlDB\Contracts\Models\Role $role) use ($value) { return $role->positionId() === (int) $value; })->count() === 0; diff --git a/app/Rules/UserIsAvailableToBeAssigned.php b/app/Rules/UserIsAvailableToBeAssigned.php index 1017734..8e07740 100644 --- a/app/Rules/UserIsAvailableToBeAssigned.php +++ b/app/Rules/UserIsAvailableToBeAssigned.php @@ -49,9 +49,9 @@ public function passes($attribute, $value) return false; } $user = $this->userRepository->getById($value); - $logic = app(LogicRepository::class)->getById($settings['logic_id']); - return $user->roles()->filter(function(Role $role) use ($settings, $logic, $user) { - return LogicTester::evaluate($logic, $user, $role->group(), $role) && in_array($role->id(), $settings['user_only_has_one_role']); + + return $user->roles()->filter(function(Role $role) use ($settings, $user) { + return in_array($role->id(), $settings['user_only_has_one_role']); })->count() === 0; } diff --git a/app/Support/LogicRoleRepository.php b/app/Support/LogicRoleRepository.php new file mode 100644 index 0000000..cc3a81e --- /dev/null +++ b/app/Support/LogicRoleRepository.php @@ -0,0 +1,121 @@ +roleRepository = $roleRepository; + $this->logicTester = $logicTester; + } + + /** + * @inheritDoc + */ + public function getById(int $id): RoleModel + { + return $this->roleRepository->getById($id); + } + + /** + * @inheritDoc + */ + public function all(): Collection + { + return $this->filter($this->roleRepository->all()); + } + + /** + * @inheritDoc + */ + public function getByDataProviderId(int $dataProviderId): RoleModel + { + return $this->roleRepository->getByDataProviderId($dataProviderId); + } + + /** + * @inheritDoc + */ + public function create(int $positionId, int $groupId, int $dataProviderId): RoleModel + { + return $this->roleRepository->create($positionId, $groupId, $dataProviderId); + } + + /** + * @inheritDoc + */ + public function delete(int $id): void + { + $this->roleRepository->delete($id); + } + + /** + * @inheritDoc + */ + public function allThroughGroup(\BristolSU\ControlDB\Contracts\Models\Group $group): Collection + { + return $this->filter($this->roleRepository->allThroughGroup($group)); + + } + + /** + * @inheritDoc + */ + public function allThroughPosition(\BristolSU\ControlDB\Contracts\Models\Position $position): Collection + { + return $this->filter($this->roleRepository->allThroughPosition($position)); + } + + private function hasLogicGroup() + { + return $this->logicGroupId() !== null; + } + + private function logicGroupId() + { + $id = settings('logic_group', null); + if($id === null) { + return null; + } + return (int) $id; + } + + private function logicGroup() + { + return app(LogicRepository::class)->getById( + $this->logicGroupId() + ); + } + + private function isInLogicGroup(\BristolSU\ControlDB\Contracts\Models\Role $role) + { + return $this->logicTester->evaluate($this->logicGroup(), null, $role->group(), $role); + } + + private function filter(Collection $roles) + { + if($this->hasLogicGroup()) { + return $roles->filter(function(\BristolSU\ControlDB\Contracts\Models\Role $role) { + return $this->isInLogicGroup($role); + })->values(); + } + return $roles; + } +} \ No newline at end of file diff --git a/app/Support/LogicUserGroupRepository.php b/app/Support/LogicUserGroupRepository.php new file mode 100644 index 0000000..a79bda7 --- /dev/null +++ b/app/Support/LogicUserGroupRepository.php @@ -0,0 +1,96 @@ +userRoleRepository = $userRoleRepository; + $this->logicTester = $logicTester; + } + + /** + * @inheritDoc + */ + public function getUsersThroughRole(Role $role): Collection + { + return $this->userRoleRepository->getUsersThroughRole($role); + } + + /** + * @inheritDoc + */ + public function getRolesThroughUser(User $user): Collection + { + return $this->filter($this->userRoleRepository->getRolesThroughUser($user)); + } + + /** + * @inheritDoc + */ + public function addUserToRole(User $user, Role $role): void + { + $this->userRoleRepository->addUserToRole($user, $role); + } + + /** + * @inheritDoc + */ + public function removeUserFromRole(User $user, Role $role): void + { + $this->userRoleRepository->removeUserFromRole($user, $role); + } + + private function hasLogicGroup() + { + return $this->logicGroupId() !== null; + } + + private function logicGroupId() + { + $id = settings('logic_group', null); + if($id === null) { + return null; + } + return (int) $id; + } + + private function logicGroup() + { + return app(LogicRepository::class)->getById( + $this->logicGroupId() + ); + } + + private function isInLogicGroup(\BristolSU\ControlDB\Contracts\Models\Role $role) + { + return $this->logicTester->evaluate($this->logicGroup(), null, $role->group(), $role); + } + + private function filter(Collection $roles) + { + if($this->hasLogicGroup()) { + return $roles->filter(function(\BristolSU\ControlDB\Contracts\Models\Role $role) { + return $this->isInLogicGroup($role); + })->values(); + } + return $roles; + } +} \ No newline at end of file diff --git a/routes/admin/web.php b/routes/admin/web.php index 44c4af6..2fcc381 100644 --- a/routes/admin/web.php +++ b/routes/admin/web.php @@ -13,4 +13,6 @@ use Illuminate\Support\Facades\Route; -Route::get('/', 'AdminPageController@index'); \ No newline at end of file +Route::middleware([\BristolSU\Module\AssignRoles\Http\Middleware\BindRoleRepository::class])->group(function() { + Route::get('/', 'AdminPageController@index'); +}); diff --git a/routes/participant/api.php b/routes/participant/api.php index 6a4a450..a405568 100644 --- a/routes/participant/api.php +++ b/routes/participant/api.php @@ -2,7 +2,7 @@ use Illuminate\Support\Facades\Route; -Route::namespace('ParticipantApi')->group(function() { +Route::namespace('ParticipantApi')->middleware(\BristolSU\Module\AssignRoles\Http\Middleware\BindRoleRepository::class)->group(function() { Route::resource('role', 'RoleController')->only('index', 'store', 'destroy', 'update'); Route::prefix('role/{role}')->group(function() { diff --git a/routes/participant/web.php b/routes/participant/web.php index 22d241d..72b33a4 100644 --- a/routes/participant/web.php +++ b/routes/participant/web.php @@ -13,4 +13,6 @@ use Illuminate\Support\Facades\Route; -Route::get('/', 'ParticipantPageController@index'); \ No newline at end of file +Route::middleware([\BristolSU\Module\AssignRoles\Http\Middleware\BindRoleRepository::class])->group(function() { + Route::get('/', 'ParticipantPageController@index'); +}); diff --git a/tests/Http/Controllers/ParticipantApi/RoleControllerTest.php b/tests/Http/Controllers/ParticipantApi/RoleControllerTest.php index bb4fa6b..3acc63d 100644 --- a/tests/Http/Controllers/ParticipantApi/RoleControllerTest.php +++ b/tests/Http/Controllers/ParticipantApi/RoleControllerTest.php @@ -8,6 +8,8 @@ use BristolSU\ControlDB\Models\Position; use BristolSU\ControlDB\Models\Role; use BristolSU\Module\AssignRoles\Http\Controllers\ParticipantApi\RoleController; +use BristolSU\Module\AssignRoles\Http\Middleware\BindRoleRepository; +use BristolSU\Module\AssignRoles\Support\LogicRoleRepository; use BristolSU\Module\AssignRoles\Support\PositionSettingRetrieval; use BristolSU\Module\Tests\AssignRoles\TestCase; use BristolSU\Support\Logic\Contracts\Audience\LogicAudience; @@ -127,19 +129,13 @@ public function index_returns_all_roles_through_a_group_that_are_also_in_the_log ->fail(null, $rolesNotInGroup[4]->group(), $rolesNotInGroup[4]) ->otherwise(true); - $this->app->when(RoleController::class) + + $this->app->when(BindRoleRepository::class) ->needs(\BristolSU\Support\Logic\Contracts\LogicTester::class) ->give(function() { return $this->logicTester(); }); -// $logicAudience = $this->prophesize(LogicAudience::class); -// $logicAudience->roleAudience(Argument::that(function($arg) use ($logicGroup) { -// return $arg instanceof Logic && $arg->is($logicGroup); -// }))->shouldBeCalled()->willReturn(collect($rolesInLogicGroup)); -// $this->instance(LogicAudience::class, $logicAudience->reveal()); -// - $response = $this->getJson($this->userApiUrl('/role')); $response->assertStatus(200);