From ce43147f7474c51b53b75a8ec0596989cfc9aa3a Mon Sep 17 00:00:00 2001 From: Ian Lindsay <6673081+ilindsay@users.noreply.github.com> Date: Wed, 11 Dec 2024 17:35:12 +0000 Subject: [PATCH] fix: added check for organisations with no op admin VOL-5955 (#503) --- .../Organisation/Organisation.php | 12 +---- .../src/Entity/Organisation/Organisation.php | 14 ++++++ .../Organisation/OrganisationTest.php | 20 +++------ .../Organisation/OrganisationEntityTest.php | 44 +++++++++++++++++++ 4 files changed, 66 insertions(+), 24 deletions(-) diff --git a/app/api/module/Api/src/Domain/QueryHandler/Organisation/Organisation.php b/app/api/module/Api/src/Domain/QueryHandler/Organisation/Organisation.php index 264174436b..e84a5be688 100644 --- a/app/api/module/Api/src/Domain/QueryHandler/Organisation/Organisation.php +++ b/app/api/module/Api/src/Domain/QueryHandler/Organisation/Organisation.php @@ -1,21 +1,10 @@ <?php -/** - * Organisation - * - * @author Rob Caiger <rob@clocal.co.uk> - */ - namespace Dvsa\Olcs\Api\Domain\QueryHandler\Organisation; use Dvsa\Olcs\Api\Domain\QueryHandler\AbstractQueryHandler; use Dvsa\Olcs\Transfer\Query\QueryInterface; -/** - * Organisation - * - * @author Rob Caiger <rob@clocal.co.uk> - */ class Organisation extends AbstractQueryHandler { protected $repoServiceName = 'Organisation'; @@ -37,6 +26,7 @@ public function handleQuery(QueryInterface $query) 'isDisqualified' => $organisation->getDisqualifications()->count() > 0, 'taValueOptions' => $this->getTrafficAreaValueOptions($allowedOperatorLocation), 'allowedOperatorLocation' => $allowedOperatorLocation, + 'hasOperatorAdmin' => $organisation->hasOperatorAdmin() ] ); } diff --git a/app/api/module/Api/src/Entity/Organisation/Organisation.php b/app/api/module/Api/src/Entity/Organisation/Organisation.php index 7032b765ef..0dfbc58bfb 100644 --- a/app/api/module/Api/src/Entity/Organisation/Organisation.php +++ b/app/api/module/Api/src/Entity/Organisation/Organisation.php @@ -529,6 +529,20 @@ public function getAdministratorUsers() return $this->organisationUsers->matching($criteria); } + public function hasOperatorAdmin(): bool + { + $administratorUsers = $this->getAdministratorUsers(); + + /** @var OrganisationUserEntity $orgUser */ + foreach ($administratorUsers as $orgUser) { + if ($orgUser->getUser()->isOperatorAdministrator()) { + return true; + } + } + + return false; + } + /** * can only delete operator admin if more than one exists */ diff --git a/app/api/test/module/Api/src/Domain/QueryHandler/Organisation/OrganisationTest.php b/app/api/test/module/Api/src/Domain/QueryHandler/Organisation/OrganisationTest.php index 407b63aba1..fda35d7279 100644 --- a/app/api/test/module/Api/src/Domain/QueryHandler/Organisation/OrganisationTest.php +++ b/app/api/test/module/Api/src/Domain/QueryHandler/Organisation/OrganisationTest.php @@ -1,10 +1,6 @@ <?php -/** - * Organisation Test - * - * @author Rob Caiger <rob@clocal.co.uk> - */ +declare(strict_types=1); namespace Dvsa\OlcsTest\Api\Domain\QueryHandler\Organisation; @@ -14,13 +10,7 @@ use Dvsa\Olcs\Api\Domain\Repository\TrafficArea as TrafficAreaRepo; use Dvsa\Olcs\Transfer\Query\Organisation\Organisation as Qry; use Mockery as m; -use SAML2\Utilities\ArrayCollection; -/** - * Organisation Test - * - * @author Rob Caiger <rob@clocal.co.uk> - */ class OrganisationTest extends QueryHandlerTestCase { public function setUp(): void @@ -32,7 +22,7 @@ public function setUp(): void parent::setUp(); } - public function testHandleQueryDisqualified() + public function testHandleQueryDisqualified(): void { $query = Qry::create(['id' => 111]); @@ -40,6 +30,7 @@ public function testHandleQueryDisqualified() $mockOrganisation->shouldReceive('serialize')->andReturn(['foo' => 'bar']); $mockOrganisation->shouldReceive('getDisqualifications->count')->andReturn(2); $mockOrganisation->shouldReceive('getAllowedOperatorLocation')->andReturn('GB')->once(); + $mockOrganisation->expects('hasOperatorAdmin')->withNoArgs()->andReturnTrue(); $mockTa = m::mock() ->shouldReceive('getId') @@ -64,13 +55,14 @@ public function testHandleQueryDisqualified() 'foo' => 'bar', 'isDisqualified' => true, 'allowedOperatorLocation' => 'GB', + 'hasOperatorAdmin' => true, 'taValueOptions' => [1 => 'foo'], ]; $this->assertEquals($expected, $this->sut->handleQuery($query)->serialize()); } - public function testHandleQueryNotDisqualified() + public function testHandleQueryNotDisqualified(): void { $query = Qry::create(['id' => 111]); @@ -78,6 +70,7 @@ public function testHandleQueryNotDisqualified() $mockOrganisation->shouldReceive('serialize')->andReturn(['foo' => 'bar']); $mockOrganisation->shouldReceive('getDisqualifications->count')->andReturn(0); $mockOrganisation->shouldReceive('getAllowedOperatorLocation')->andReturn('GB')->once(); + $mockOrganisation->expects('hasOperatorAdmin')->withNoArgs()->andReturnFalse(); $mockTa = m::mock() ->shouldReceive('getId') @@ -102,6 +95,7 @@ public function testHandleQueryNotDisqualified() 'foo' => 'bar', 'isDisqualified' => false, 'allowedOperatorLocation' => 'GB', + 'hasOperatorAdmin' => false, 'taValueOptions' => [1 => 'foo'], ]; diff --git a/app/api/test/module/Api/src/Entity/Organisation/OrganisationEntityTest.php b/app/api/test/module/Api/src/Entity/Organisation/OrganisationEntityTest.php index 7daca0f9ea..85379a1211 100644 --- a/app/api/test/module/Api/src/Entity/Organisation/OrganisationEntityTest.php +++ b/app/api/test/module/Api/src/Entity/Organisation/OrganisationEntityTest.php @@ -689,6 +689,50 @@ public function testCanDeleteOperatorAdminFalse(): void $this->assertFalse($entity->canDeleteOperatorAdmin()); } + /** + * first user not op admin, 2nd user is op admin, 3rd user doesn't need to be checked + */ + public function testHasOperatorAdminTrue(): void + { + $entity = new Entity(); + + $user1 = m::mock(OrganisationUser::class)->makePartial(); + $user1->setIsAdministrator('Y'); + $user1->expects('getUser->isOperatorAdministrator')->withNoArgs()->andReturnFalse(); + + $user2 = m::mock(OrganisationUser::class)->makePartial(); + $user2->setIsAdministrator('Y'); + $user2->expects('getUser->isOperatorAdministrator')->withNoArgs()->andReturnTrue(); + + $user3 = m::mock(OrganisationUser::class)->makePartial(); + $user3->setIsAdministrator('Y'); + $user3->expects('getUser->isOperatorAdministrator')->never(); + + $entity->setOrganisationUsers(new ArrayCollection([$user1, $user2, $user3])); + + $this->assertTrue($entity->hasOperatorAdmin()); + } + + /** + * no operator admins are found + */ + public function testHasOperatorAdminFalse(): void + { + $entity = new Entity(); + + $user1 = m::mock(OrganisationUser::class)->makePartial(); + $user1->setIsAdministrator('Y'); + $user1->expects('getUser->isOperatorAdministrator')->withNoArgs()->andReturnFalse(); + + $user2 = m::mock(OrganisationUser::class)->makePartial(); + $user2->setIsAdministrator('Y'); + $user2->expects('getUser->isOperatorAdministrator')->withNoArgs()->andReturnFalse(); + + $entity->setOrganisationUsers(new ArrayCollection([$user1, $user2])); + + $this->assertFalse($entity->hasOperatorAdmin()); + } + /** * test if org user not found due to soft delete */