Skip to content

Commit

Permalink
fix: vol 5955 transport consultant bug (#507)
Browse files Browse the repository at this point in the history
* fix: amend form inputs

* fix: added check for organisations with no op admin VOL-5955 (#503)

* feat: partially complete fetchOperatorDetials

Signed-off-by: Gabriel Guimaraes <80750139+gabrielg2020@users.noreply.github.com>

* fix: amend the form for conditional licence field

* fix: amended routing

* fix: updated to use new query

* feat: add a check for existing licence plus whether it has operator admin (#505)

* chore: olcs-common 7.16.0 olcs-transfer 7.9.0 (#506)

* chore: olcs-common 7.16.0 olcs-transfer 7.9.0

* fix: add sec ignore for xml

* fix: move snyk ignore

---------

Co-authored-by: Shaun Hare <shaun.hare@dvsa.gov.uk>

* feat: added files for ERRU version 3.4 VOL-5800 (#504)

* chore: wip needs queryhandler setting

* fix: temp push to check

* fix: wrong import for query

* fix: extra whitespace

---------

Signed-off-by: Gabriel Guimaraes <80750139+gabrielg2020@users.noreply.github.com>
Co-authored-by: Ian Lindsay <6673081+ilindsay@users.noreply.github.com>
Co-authored-by: Gabriel Guimaraes <80750139+gabrielg2020@users.noreply.github.com>
  • Loading branch information
3 people authored Dec 13, 2024
1 parent 13f85f8 commit 04461a2
Show file tree
Hide file tree
Showing 13 changed files with 317 additions and 46 deletions.
1 change: 1 addition & 0 deletions app/api/module/Api/config/query-map.config.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@

// Licence
TransferQuery\Licence\BusinessDetails::class => QueryHandler\Licence\BusinessDetails::class,
TransferQuery\Licence\ExistsWithOperatorAdmin::class => QueryHandler\Licence\ExistsWithOperatorAdmin::class,
TransferQuery\Licence\Licence::class => QueryHandler\Licence\Licence::class,
TransferQuery\Licence\LicenceWithCorrespondenceCd::class => QueryHandler\Licence\LicenceWithCorrespondenceCd::class,
TransferQuery\Licence\LicenceByNumber::class => QueryHandler\Licence\LicenceByNumber::class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
QueryHandler\Licence\Addresses::class => Misc\CanAccessLicenceWithId::class,
QueryHandler\Licence\BusinessDetails::class => Misc\CanAccessLicenceWithId::class,
QueryHandler\Licence\ConditionUndertaking::class => Misc\CanAccessLicenceWithId::class,
QueryHandler\Licence\ExistsWithOperatorAdmin::class => Misc\NoValidationRequired::class,
QueryHandler\Licence\Licence::class => Misc\CanAccessLicenceWithId::class,
QueryHandler\Licence\LicenceWithCorrespondenceCd::class => Misc\CanAccessLicenceWithId::class,
QueryHandler\Licence\LicenceByNumber::class => Misc\CanAccessLicenceWithLicNo::class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php

declare(strict_types=1);

namespace Dvsa\Olcs\Api\Domain\QueryHandler\Licence;

use Dvsa\Olcs\Api\Domain\Exception\NotFoundException;
use Dvsa\Olcs\Api\Domain\QueryHandler\AbstractQueryHandler;
use Dvsa\Olcs\Transfer\Query\QueryInterface;
use Dvsa\Olcs\Api\Domain\Repository\Licence as LicenceRepo;
use Dvsa\Olcs\Transfer\Query\Licence\ExistsWithOperatorAdmin as ExistsWithOperatorAdminQry;

class ExistsWithOperatorAdmin extends AbstractQueryHandler
{
protected $repoServiceName = 'Licence';

public function handleQuery(QueryInterface $query)
{
/**
* @var LicenceRepo $repo
* @var ExistsWithOperatorAdminQry $query
*/
$repo = $this->getRepo();

try {
$licence = $repo->fetchByLicNoWithoutAdditionalData($query->getLicNo());
$licenceExists = true;
$hasOperatorAdmin = $licence->getOrganisation()->hasOperatorAdmin();
} catch (NotFoundException) {
$licenceExists = false;
$hasOperatorAdmin = false;
}

return [
'licenceExists' => $licenceExists,
'hasOperatorAdmin' => $hasOperatorAdmin,
];
}
}
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -37,6 +26,7 @@ public function handleQuery(QueryInterface $query)
'isDisqualified' => $organisation->getDisqualifications()->count() > 0,
'taValueOptions' => $this->getTrafficAreaValueOptions($allowedOperatorLocation),
'allowedOperatorLocation' => $allowedOperatorLocation,
'hasOperatorAdmin' => $organisation->hasOperatorAdmin()
]
);
}
Expand Down
14 changes: 14 additions & 0 deletions app/api/module/Api/src/Entity/Organisation/Organisation.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<?php

declare(strict_types=1);

namespace Dvsa\OlcsTest\Api\Domain\QueryHandler\Licence;

use Dvsa\Olcs\Api\Domain\Exception\NotFoundException;
use Dvsa\Olcs\Api\Domain\QueryHandler\Licence\ExistsWithOperatorAdmin as QueryHandler;
use Dvsa\Olcs\Api\Domain\Repository\Licence as Repo;
use Dvsa\Olcs\Api\Entity\Licence\Licence as LicenceEntity;
use Dvsa\Olcs\Api\Entity\Organisation\Organisation;
use Dvsa\Olcs\Transfer\Query\Licence\ExistsWithOperatorAdmin as Query;
use Dvsa\OlcsTest\Api\Domain\QueryHandler\QueryHandlerTestCase;
use Mockery as m;

class ExistsWithOperatorAdminTest extends QueryHandlerTestCase
{
public function setUp(): void
{
$this->sut = new QueryHandler();
$this->mockRepo('Licence', Repo::class);

parent::setUp();
}

public function testHandleQueryNotFound(): void
{
$licNo = 'PB2141421';
$query = Query::create(['licNo' => $licNo]);

$this->repoMap['Licence']->expects('fetchByLicNoWithoutAdditionalData')->with($licNo)->andThrow(NotFoundException::class);

$expectedResult = [
'licenceExists' => false,
'hasOperatorAdmin' => false,
];

$this->assertEquals($expectedResult, $this->sut->handleQuery($query));
}

/**
* @dataProvider dpHandleQuery
*/
public function testHandleQuery(bool $isOperatorAdmin): void
{
$licNo = 'PB2141421';
$query = Query::create(['licNo' => $licNo]);

$organisation = m::mock(Organisation::class);
$organisation->expects('hasOperatorAdmin')->andReturn($isOperatorAdmin);

$licence = m::mock(LicenceEntity::class);
$licence->expects('getOrganisation')->andReturn($organisation);


$this->repoMap['Licence']->expects('fetchByLicNoWithoutAdditionalData')->with($licNo)->andReturn($licence);

$expectedResult = [
'licenceExists' => true,
'hasOperatorAdmin' => $isOperatorAdmin,
];

$this->assertEquals($expectedResult, $this->sut->handleQuery($query));
}

public function dpHandleQuery(): array
{
return [
[true],
[false],
];
}
}
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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
Expand All @@ -32,14 +22,15 @@ public function setUp(): void
parent::setUp();
}

public function testHandleQueryDisqualified()
public function testHandleQueryDisqualified(): void
{
$query = Qry::create(['id' => 111]);

$mockOrganisation = m::mock(\Dvsa\Olcs\Api\Entity\Organisation\Organisation::class)->makePartial();
$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')
Expand All @@ -64,20 +55,22 @@ 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]);

$mockOrganisation = m::mock(\Dvsa\Olcs\Api\Entity\Organisation\Organisation::class)->makePartial();
$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')
Expand All @@ -102,6 +95,7 @@ public function testHandleQueryNotDisqualified()
'foo' => 'bar',
'isDisqualified' => false,
'allowedOperatorLocation' => 'GB',
'hasOperatorAdmin' => false,
'taValueOptions' => [1 => 'foo'],
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Common\Service\Helper\TranslationHelperService;
use Common\Service\Helper\UrlHelperService;
use Common\Service\Script\ScriptFactory;
use Dvsa\Olcs\Transfer\Query\Licence\ExistsWithOperatorAdmin;
use Dvsa\Olcs\Transfer\Command\User\RegisterConsultantAndOperator;
use Dvsa\Olcs\Utils\Translation\NiTextTranslation;
use Laminas\Http\Response;
Expand All @@ -26,16 +27,17 @@
class ConsultantRegistrationController extends AbstractController
{
public function __construct(
NiTextTranslation $niTextTranslationUtil,
AuthorizationService $authService,
protected FormHelperService $formHelper,
protected ScriptFactory $scriptFactory,
protected TranslationHelperService $translationHelper,
protected UrlHelperService $urlHelper,
NiTextTranslation $niTextTranslationUtil,
AuthorizationService $authService,
protected FormHelperService $formHelper,
protected ScriptFactory $scriptFactory,
protected TranslationHelperService $translationHelper,
protected UrlHelperService $urlHelper,
protected FlashMessengerHelperService $flashMessengerHelper,
protected ConsultantRegistration $consultantRegistrationSession,
protected CreateAccountMapper $formatDataMapper
) {
protected ConsultantRegistration $consultantRegistrationSession,
protected CreateAccountMapper $formatDataMapper
)
{
parent::__construct($niTextTranslationUtil, $authService);
}

Expand All @@ -58,8 +60,17 @@ public function addAction()

if ($form->isValid()) {
$formData = $form->getData();
if(($formData['fields']['existingOperatorLicence'] ?? null) === 'Y') {
$this->redirect()->toRoute('user-registration/contact-your-administrator');
if (($formData['fields']['existingOperatorLicence'] ?? null) === 'Y') {
$licenceNumber = $formData['fields']['licenceContent']['licenceNumber'];
$checks = $this->licenseHasAdmin($licenceNumber);

if (!$checks['licenceExists'] ?? false) {
$form->setMessages(['fields' => ['licenceContent' => ['licenceNumber' => ['record-not-found']]]]);
} elseif (!$checks['hasOperatorAdmin'] ?? false) {
$this->redirect()->toRoute('user-registration/operator');
} else {
$this->redirect()->toRoute('user-registration/contact-your-administrator');
}
} elseif (($formData['fields']['existingOperatorLicence'] ?? null) === 'N') {
$this->redirect()->toRoute('user-registration/operator-representation');
}
Expand All @@ -72,6 +83,15 @@ public function addAction()
]);
}

private function licenseHasAdmin(string $licenceNumber): array
{
$response = $this->handleQuery(ExistsWithOperatorAdmin::create(['licNo' => $licenceNumber]));
if ($response->isOk()) {
return $response->getResult();
}
return [];
}

/**
* @return Response|ViewModel
*/
Expand Down Expand Up @@ -197,6 +217,7 @@ private function registerConsultantAndOperator($consultantFormData)
$this->redirect()->toRoute('user-registration');
}


private function alterForm($form)
{
// inject link into terms agreed label
Expand Down
Loading

0 comments on commit 04461a2

Please sign in to comment.