From 3347b0a2eaf53faaf4755b7db2e29aa38e5596a6 Mon Sep 17 00:00:00 2001 From: Johan Kromhout Date: Thu, 23 Jan 2025 16:28:37 +0100 Subject: [PATCH] Make hidden Type of Service field editable again Prior to this change, the Type of Service field was hidden and not editable because users could be confused by certain internal SURF types, such as SURF or Recommended. This change hides those types from the UI and ensures they cannot be modified from SP Dashboard. Fixes https://github.com/SURFnet/sp-dashboard/issues/1322 --- assets/scss/pages/base.scss | 4 - assets/type_of_service.json | 6 +- .../Entity/SaveOidcngEntityCommand.php | 7 +- .../Command/Entity/SaveSamlEntityCommand.php | 9 +- .../EntityChangeRequestCommandHandler.php | 9 ++ .../PublishEntityProductionCommandHandler.php | 6 + .../PublishEntityTestCommandHandler.php | 6 + .../Domain/Entity/Entity/Coin.php | 10 ++ .../TypeOfServiceRepositoryFromConfig.php | 5 +- .../Domain/Service/TypeOfServiceService.php | 76 +++++++++++ .../Domain/ValueObject/TypeOfService.php | 1 + .../ValueObject/TypeOfServiceCollection.php | 8 ++ .../Form/Entity/OidcngEntityType.php | 3 +- .../Form/Entity/SamlEntityType.php | 3 +- .../Resources/config/services.yml | 2 + .../EntityChangeRequestCommandHandlerTest.php | 50 ++++++- ...lishEntityProductionCommandHandlerTest.php | 122 ++++++++++++++++++ .../PublishEntityTestCommandHandlerTest.php | 114 +++++++++++++++- .../Service/TypeOfServiceServiceTest.php | 101 +++++++++++++++ .../TypeOfServiceCollectionTest.php | 45 +++++++ tests/webtests/EntityEditTest.php | 2 +- 21 files changed, 565 insertions(+), 24 deletions(-) create mode 100644 src/Surfnet/ServiceProviderDashboard/Domain/Service/TypeOfServiceService.php create mode 100644 tests/unit/Domain/Service/TypeOfServiceServiceTest.php create mode 100644 tests/unit/Domain/ValueObject/TypeOfServiceCollectionTest.php diff --git a/assets/scss/pages/base.scss b/assets/scss/pages/base.scss index 6f3d31ae1..1617a1ce6 100644 --- a/assets/scss/pages/base.scss +++ b/assets/scss/pages/base.scss @@ -63,7 +63,3 @@ .hidden { display: none; } - -div.form-row:has(#dashboard_bundle_entity_type_metadata_typeOfService) { - display: none; -} \ No newline at end of file diff --git a/assets/type_of_service.json b/assets/type_of_service.json index 084c40cc9..1edb52eed 100644 --- a/assets/type_of_service.json +++ b/assets/type_of_service.json @@ -1,7 +1,8 @@ [ { "typeEn" : "SURF", - "typeNl" : "SURF" + "typeNl" : "SURF", + "typeHidden": true }, { "typeEn" : "Education", @@ -25,7 +26,8 @@ }, { "typeEn" : "Recommended", - "typeNl" : "Aangeraden" + "typeNl" : "Aangeraden", + "typeHidden": true }, { "typeEn" : "Productivity", diff --git a/src/Surfnet/ServiceProviderDashboard/Application/Command/Entity/SaveOidcngEntityCommand.php b/src/Surfnet/ServiceProviderDashboard/Application/Command/Entity/SaveOidcngEntityCommand.php index 664d143dc..169adc834 100644 --- a/src/Surfnet/ServiceProviderDashboard/Application/Command/Entity/SaveOidcngEntityCommand.php +++ b/src/Surfnet/ServiceProviderDashboard/Application/Command/Entity/SaveOidcngEntityCommand.php @@ -156,16 +156,15 @@ class SaveOidcngEntityCommand implements SaveEntityCommandInterface #[Assert\Length(max: 300)] private $applicationUrl; - /** - * // phpcs:ignore - * `new Assert\NotBlank(),` TODO enable in https://github.com/SURFnet/sp-dashboard/issues/1322 - */ /** @var TypeOfService[] */ #[Assert\All([ + new Assert\NotBlank(), new Assert\Type(type: TypeOfService::class), ])] #[Assert\Count( + min: 1, max: 3, + minMessage: 'validator.type-of-service.min', maxMessage: 'validator.type-of-service.max', )] private array $typeOfService; diff --git a/src/Surfnet/ServiceProviderDashboard/Application/Command/Entity/SaveSamlEntityCommand.php b/src/Surfnet/ServiceProviderDashboard/Application/Command/Entity/SaveSamlEntityCommand.php index e53480a11..b417cf8a4 100644 --- a/src/Surfnet/ServiceProviderDashboard/Application/Command/Entity/SaveSamlEntityCommand.php +++ b/src/Surfnet/ServiceProviderDashboard/Application/Command/Entity/SaveSamlEntityCommand.php @@ -110,16 +110,15 @@ class SaveSamlEntityCommand implements SaveEntityCommandInterface #[Assert\Length(max: 300)] private ?string $applicationUrl = null; - /** - * // phpcs:ignore - * `new Assert\NotBlank(),` TODO enable in https://github.com/SURFnet/sp-dashboard/issues/1322 - * @var TypeOfService[] - */ + /** @var TypeOfService[] */ #[Assert\All([ + new Assert\NotBlank(), new Assert\Type(type: TypeOfService::class), ])] #[Assert\Count( + min: 1, max: 3, + minMessage: 'validator.type-of-service.min', maxMessage: 'validator.type-of-service.max', )] private array $typeOfService = []; diff --git a/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/EntityChangeRequestCommandHandler.php b/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/EntityChangeRequestCommandHandler.php index a7bd36ad3..b057c3ab6 100644 --- a/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/EntityChangeRequestCommandHandler.php +++ b/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/EntityChangeRequestCommandHandler.php @@ -31,9 +31,13 @@ use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\JiraTicketNumber; use Surfnet\ServiceProviderDashboard\Domain\Repository\EntityChangeRequestRepository; use Surfnet\ServiceProviderDashboard\Domain\Service\ContractualBaseService; +use Surfnet\ServiceProviderDashboard\Domain\Service\TypeOfServiceService; use Surfnet\ServiceProviderDashboard\Infrastructure\HttpClient\Exceptions\RuntimeException\PublishMetadataException; use Symfony\Component\HttpFoundation\RequestStack; +/** + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ class EntityChangeRequestCommandHandler implements CommandHandler { private readonly string $summaryTranslationKey; @@ -43,6 +47,7 @@ class EntityChangeRequestCommandHandler implements CommandHandler public function __construct( private readonly EntityChangeRequestRepository $repository, private readonly ContractualBaseService $contractualBaseHelper, + private readonly TypeOfServiceService $typeOfServiceService, private readonly EntityServiceInterface $entityService, private readonly TicketService $ticketService, private readonly RequestStack $requestStack, @@ -67,6 +72,7 @@ public function handle(PublishProductionCommandInterface $command): void throw new EntityNotFoundException('Unable to request changes to a unkown entity in Manage'); } $pristineEntity = $this->entityService->getPristineManageEntityById($entity->getId(), $entity->getEnvironment()); + try { $this->logger->info( sprintf( @@ -74,7 +80,10 @@ public function handle(PublishProductionCommandInterface $command): void $entity->getMetaData()->getNameEn() ) ); + $this->contractualBaseHelper->writeContractualBase($entity); + $entity->getMetaData()?->getCoin()->setTypeOfService($this->typeOfServiceService->restoreHiddenTypes($entity, $pristineEntity)); + // Create the Jira ticket (we need the ticket id for the revision notes in manage later on) $ticket = $this->ticketService->createJiraTicket( $entity, diff --git a/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/PublishEntityProductionCommandHandler.php b/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/PublishEntityProductionCommandHandler.php index 1d48cb7e5..29ab9bbd8 100644 --- a/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/PublishEntityProductionCommandHandler.php +++ b/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/PublishEntityProductionCommandHandler.php @@ -30,6 +30,7 @@ use Surfnet\ServiceProviderDashboard\Domain\Entity\ManageEntity; use Surfnet\ServiceProviderDashboard\Domain\Repository\PublishEntityRepository; use Surfnet\ServiceProviderDashboard\Domain\Service\ContractualBaseService; +use Surfnet\ServiceProviderDashboard\Domain\Service\TypeOfServiceService; use Surfnet\ServiceProviderDashboard\Infrastructure\HttpClient\Exceptions\RuntimeException\PublishMetadataException; use Symfony\Component\HttpFoundation\RequestStack; @@ -45,6 +46,7 @@ class PublishEntityProductionCommandHandler implements CommandHandler public function __construct( private readonly PublishEntityRepository $publishClient, private readonly ContractualBaseService $contractualBaseHelper, + private readonly TypeOfServiceService $typeOfServiceService, private readonly EntityServiceInterface $entityService, private readonly TicketService $ticketService, private readonly RequestStack $requestStack, @@ -77,6 +79,7 @@ public function handle(PublishProductionCommandInterface $command): void // The entity as it is now known in Manage $pristineEntity = $this->entityService->getPristineManageEntityById($entity->getId(), $entity->getEnvironment()); } + try { $this->logger->info( sprintf( @@ -84,7 +87,10 @@ public function handle(PublishProductionCommandInterface $command): void $entity->getMetaData()->getNameEn() ) ); + $this->contractualBaseHelper->writeContractualBase($entity); + $entity->getMetaData()?->getCoin()->setTypeOfService($this->typeOfServiceService->restoreHiddenTypes($entity, $pristineEntity)); + $publishResponse = $this->publishClient->publish( $entity, $pristineEntity, diff --git a/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/PublishEntityTestCommandHandler.php b/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/PublishEntityTestCommandHandler.php index 24cd77613..96c469953 100644 --- a/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/PublishEntityTestCommandHandler.php +++ b/src/Surfnet/ServiceProviderDashboard/Application/CommandHandler/Entity/PublishEntityTestCommandHandler.php @@ -27,6 +27,7 @@ use Surfnet\ServiceProviderDashboard\Domain\Entity\ManageEntity; use Surfnet\ServiceProviderDashboard\Domain\Repository\PublishEntityRepository; use Surfnet\ServiceProviderDashboard\Domain\Service\ContractualBaseService; +use Surfnet\ServiceProviderDashboard\Domain\Service\TypeOfServiceService; use Surfnet\ServiceProviderDashboard\Infrastructure\HttpClient\Exceptions\RuntimeException\PublishMetadataException; use Symfony\Component\HttpFoundation\RequestStack; use function array_key_exists; @@ -36,6 +37,7 @@ class PublishEntityTestCommandHandler implements CommandHandler public function __construct( private readonly PublishEntityRepository $publishClient, private readonly ContractualBaseService $contractualBaseHelper, + private readonly TypeOfServiceService $typeOfServiceService, private readonly EntityServiceInterface $entityService, private readonly LoggerInterface $logger, private readonly RequestStack $requestStack, @@ -54,6 +56,7 @@ public function handle(PublishEntityTestCommand $command): void $pristineEntity = $this->entityService->getPristineManageEntityById($entity->getId(), $entity->getEnvironment()); $this->contractualBaseHelper->writeContractualBaseForTestEntity($entity, $pristineEntity); } + try { $this->logger->info( sprintf( @@ -61,6 +64,9 @@ public function handle(PublishEntityTestCommand $command): void $entity->getMetaData()->getNameEn() ) ); + + $entity->getMetaData()?->getCoin()->setTypeOfService($this->typeOfServiceService->restoreHiddenTypes($entity, $pristineEntity)); + $publishResponse = $this->publishClient->publish( $entity, $pristineEntity, diff --git a/src/Surfnet/ServiceProviderDashboard/Domain/Entity/Entity/Coin.php b/src/Surfnet/ServiceProviderDashboard/Domain/Entity/Entity/Coin.php index bdc02e721..1d033d0bb 100644 --- a/src/Surfnet/ServiceProviderDashboard/Domain/Entity/Entity/Coin.php +++ b/src/Surfnet/ServiceProviderDashboard/Domain/Entity/Entity/Coin.php @@ -126,6 +126,11 @@ public function getTypeOfService(): TypeOfServiceCollection return $this->typeOfService; } + public function hasTypeOfService(): bool + { + return $this->typeOfService !== null; + } + public function getContractualBase(): ?string { return $this->contractualBase; @@ -176,4 +181,9 @@ public function asArray(): array 'metaDataFields.coin:ss:idp_visible_only' => $this->isIdpVisibleOnly(), ]; } + + public function setTypeOfService(TypeOfServiceCollection $typeOfService): void + { + $this->typeOfService = $typeOfService; + } } diff --git a/src/Surfnet/ServiceProviderDashboard/Domain/Repository/TypeOfServiceRepositoryFromConfig.php b/src/Surfnet/ServiceProviderDashboard/Domain/Repository/TypeOfServiceRepositoryFromConfig.php index 78024412c..91b0329fd 100644 --- a/src/Surfnet/ServiceProviderDashboard/Domain/Repository/TypeOfServiceRepositoryFromConfig.php +++ b/src/Surfnet/ServiceProviderDashboard/Domain/Repository/TypeOfServiceRepositoryFromConfig.php @@ -59,7 +59,8 @@ private function load(): void } $this->collection = new TypeOfServiceCollection(); foreach ($data as $entry) { - $typeOfService = new TypeOfService($entry->typeEn, $entry->typeNl); + $typeHidden = $entry?->typeHidden ?? false; + $typeOfService = new TypeOfService($entry->typeEn, $entry->typeNl, $typeHidden); $this->collection->add($typeOfService); } } @@ -69,7 +70,7 @@ private function load(): void */ public function getTypesOfServiceChoices(): array { - return $this->collection->getArray(); + return $this->collection->filterHidden()->getArray(); } public function findByEnglishTypeOfService(string $enTos): ?TypeOfService diff --git a/src/Surfnet/ServiceProviderDashboard/Domain/Service/TypeOfServiceService.php b/src/Surfnet/ServiceProviderDashboard/Domain/Service/TypeOfServiceService.php new file mode 100644 index 000000000..efe4ca0e4 --- /dev/null +++ b/src/Surfnet/ServiceProviderDashboard/Domain/Service/TypeOfServiceService.php @@ -0,0 +1,76 @@ +getMetaData()?->getCoin()->hasTypeOfService() === true) { + $typeOfService = $entity->getMetaData()->getCoin()->getTypeOfService(); + } + + if ($pristineEntity && $pristineEntity->getMetaData()?->getCoin()->hasTypeOfService()) { + $pristineTypeOfService = $pristineEntity->getMetaData()->getCoin()->getTypeOfService(); + } + + return $this->mergeCollections($typeOfService, $pristineTypeOfService ?? null); + } + + public function mergeCollections( + TypeOfServiceCollection $types, + ?TypeOfServiceCollection $pristineTypes + ): TypeOfServiceCollection { + $result = new TypeOfServiceCollection(); + + $this->addNonHiddenTypes($types, $result); + + if ($pristineTypes === null) { + return $result; + } + + $this->restoreHiddenPristineTypes($pristineTypes, $result); + + return $result; + } + + private function addNonHiddenTypes(TypeOfServiceCollection $types, TypeOfServiceCollection $result): void + { + foreach ($types->getArray() as $type) { + if (!$type->typeHidden && !$result->has($type->typeEn)) { + $result->add($type); + } + } + } + + private function restoreHiddenPristineTypes(TypeOfServiceCollection $pristineTypes, TypeOfServiceCollection $result): void + { + foreach ($pristineTypes->getArray() as $pristineType) { + if ($pristineType->typeHidden && !$result->has($pristineType->typeEn)) { + $result->add($pristineType); + } + } + } +} diff --git a/src/Surfnet/ServiceProviderDashboard/Domain/ValueObject/TypeOfService.php b/src/Surfnet/ServiceProviderDashboard/Domain/ValueObject/TypeOfService.php index 97514d4c7..a6cf5a74f 100644 --- a/src/Surfnet/ServiceProviderDashboard/Domain/ValueObject/TypeOfService.php +++ b/src/Surfnet/ServiceProviderDashboard/Domain/ValueObject/TypeOfService.php @@ -25,6 +25,7 @@ public function __construct( public string $typeEn, public string $typeNl = '', + public bool $typeHidden = false ) { } } diff --git a/src/Surfnet/ServiceProviderDashboard/Domain/ValueObject/TypeOfServiceCollection.php b/src/Surfnet/ServiceProviderDashboard/Domain/ValueObject/TypeOfServiceCollection.php index caa1c3f54..f97c6e70c 100644 --- a/src/Surfnet/ServiceProviderDashboard/Domain/ValueObject/TypeOfServiceCollection.php +++ b/src/Surfnet/ServiceProviderDashboard/Domain/ValueObject/TypeOfServiceCollection.php @@ -116,4 +116,12 @@ public function getServicesAsArray(): array } return $services; } + + public function filterHidden(): self + { + $filteredCollection = new self(); + $filteredCollection->types = array_values(array_filter($this->types, static fn ($type) => $type->typeHidden !== true)); + + return $filteredCollection; + } } diff --git a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Form/Entity/OidcngEntityType.php b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Form/Entity/OidcngEntityType.php index 3b7a12f0b..bc148ddb2 100644 --- a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Form/Entity/OidcngEntityType.php +++ b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Form/Entity/OidcngEntityType.php @@ -263,12 +263,13 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'typeOfService', ChoiceType::class, [ - 'required' => false, + 'required' => true, 'choices' => $this->typeOfServiceProvider->getTypesOfServiceChoices(), 'choice_value' => fn(TypeOfService $tos): string => $tos->typeEn, 'choice_label' => fn(TypeOfService $tos): string => $tos->typeEn, 'choice_attr' => fn(): array => [ 'class' => 'decorated', + 'data-parsley-mincheck' => 1, 'data-parsley-maxcheck' => 3, ], 'autocomplete' => true, // Enables the UX-Autocomplete diff --git a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Form/Entity/SamlEntityType.php b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Form/Entity/SamlEntityType.php index 27784b25d..7255df766 100644 --- a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Form/Entity/SamlEntityType.php +++ b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Form/Entity/SamlEntityType.php @@ -245,12 +245,13 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'typeOfService', ChoiceType::class, [ - 'required' => false, + 'required' => true, 'choices' => $this->typeOfServiceProvider->getTypesOfServiceChoices(), 'choice_value' => fn(TypeOfService $tos): string => $tos->typeEn, 'choice_label' => fn(TypeOfService $tos): string => $tos->typeEn, 'choice_attr' => fn(): array => [ 'class' => 'decorated', + 'data-parsley-mincheck' => 1, 'data-parsley-maxcheck' => 3, ], 'autocomplete' => true, // Enables the UX-Autocomplete diff --git a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Resources/config/services.yml b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Resources/config/services.yml index cb793b1b9..a7dc56301 100644 --- a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Resources/config/services.yml +++ b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Resources/config/services.yml @@ -127,6 +127,7 @@ services: arguments: - '@surfnet.manage.client.publish_client.prod_environment' - '@Surfnet\ServiceProviderDashboard\Domain\Service\ContractualBaseService' + - '@Surfnet\ServiceProviderDashboard\Domain\Service\TypeOfServiceService' - '@Surfnet\ServiceProviderDashboard\Application\Service\EntityService' - '@Surfnet\ServiceProviderDashboard\Application\Service\TicketService' - '@request_stack' @@ -596,6 +597,7 @@ services: '@Surfnet\ServiceProviderDashboard\Infrastructure\DashboardBundle\Repository\ServiceRepository' Surfnet\ServiceProviderDashboard\Domain\Service\ContractualBaseService: + Surfnet\ServiceProviderDashboard\Domain\Service\TypeOfServiceService: Surfnet\ServiceProviderDashboard\Application\Service\AttributeServiceInterface: '@Surfnet\ServiceProviderDashboard\Application\Service\AttributeService' diff --git a/tests/integration/Application/CommandHandler/Entity/EntityChangeRequestCommandHandlerTest.php b/tests/integration/Application/CommandHandler/Entity/EntityChangeRequestCommandHandlerTest.php index d572f409a..e46bab9b5 100644 --- a/tests/integration/Application/CommandHandler/Entity/EntityChangeRequestCommandHandlerTest.php +++ b/tests/integration/Application/CommandHandler/Entity/EntityChangeRequestCommandHandlerTest.php @@ -42,7 +42,9 @@ use Surfnet\ServiceProviderDashboard\Domain\Entity\Service; use Surfnet\ServiceProviderDashboard\Domain\Repository\EntityChangeRequestRepository; use Surfnet\ServiceProviderDashboard\Domain\Service\ContractualBaseService; +use Surfnet\ServiceProviderDashboard\Domain\Service\TypeOfServiceService; use Surfnet\ServiceProviderDashboard\Domain\ValueObject\Issue; +use Surfnet\ServiceProviderDashboard\Domain\ValueObject\TypeOfService; use Surfnet\ServiceProviderDashboard\Domain\ValueObject\TypeOfServiceCollection; use Symfony\Component\HttpFoundation\RequestStack; @@ -74,6 +76,7 @@ public function setUp(): void $this->commandHandler = new EntityChangeRequestCommandHandler( $this->entityChangeRequestRepository, new ContractualBaseService(), + new TypeOfServiceService(), $this->entityService, $this->ticketService, $this->requestStack, @@ -131,9 +134,6 @@ public function test_it_should_update_contractual_base_on_change_request(?string $this->commandHandler->handle($command); } - /** - * @return ManageEntity - */ private function createEntity(): ManageEntity { $coin = new Coin( @@ -176,4 +176,48 @@ private function createEntity(): ManageEntity return $manageEntity; } + public function test_it_should_ensure_hidden_type_of_service_selections_are_preserved() + { + $manageEntity = $this->createEntity(); + + $applicant = new Contact('john:doe', 'john@example.com', 'John Doe'); + + /** @var ManageEntity $pristineEntity */ + $pristineEntity = unserialize(serialize($manageEntity)); + $pristineEntity->getMetaData()->getCoin()->getTypeOfService()->add(new TypeOfService('SURF', 'SURF', true)); + $pristineEntity->getMetaData()->getCoin()->getTypeOfService()->add(new TypeOfService('Education', 'Onderwijs', false)); + $manageEntity->getMetaData()->getCoin()->getTypeOfService()->add(new TypeOfService('Productivity', 'Productiviteit', false)); + + $manageEntity->getMetaData()->getCoin()->getTypeOfService()->add(new TypeOfService('Privacy/security', 'Privacy/beveiliging', false)); + $manageEntity->getMetaData()->getCoin()->getTypeOfService()->add(new TypeOfService('Recommended', 'Aangeraden', true)); + $manageEntity->getMetaData()->getCoin()->getTypeOfService()->add(new TypeOfService('Productivity', 'Productiviteit', false)); + + $this->entityService->shouldReceive('getPristineManageEntityById') + ->once() + ->with($manageEntity->getId(), $manageEntity->getEnvironment()) + ->andReturn($pristineEntity); + + $this->ticketService->shouldReceive('createJiraTicket') + ->once() + ->andReturn(new Issue('ISSUE-123', 'foo', 'bar')); + + $expectedDiff = [ + 'metaDataFields.coin:ss:type_of_service:en' => 'Productivity,Privacy/security,SURF', + 'metaDataFields.coin:ss:type_of_service:nl' => 'Productiviteit,Privacy/beveiliging,SURF', + 'metaDataFields.coin:contractual_base' => 'IX', + ]; + + $this->entityChangeRequestRepository->shouldReceive('openChangeRequest') + ->once() + ->withArgs(function($entity, $pristineEntity) use ($expectedDiff){ + if($pristineEntity->diff($entity)->getDiff() !== $expectedDiff){ + $this->fail(var_export($pristineEntity->diff($entity)->getDiff(), true) . ' is not equal to ' . var_export($expectedDiff, true)); + } + return $pristineEntity->diff($entity)->getDiff() === $expectedDiff; + }) + ->andReturn(['id' => 1]); + + $command = new EntityChangeRequestCommand($manageEntity, $applicant); + $this->commandHandler->handle($command); + } } diff --git a/tests/integration/Application/CommandHandler/Entity/PublishEntityProductionCommandHandlerTest.php b/tests/integration/Application/CommandHandler/Entity/PublishEntityProductionCommandHandlerTest.php index e1a38e309..10b33b2ba 100644 --- a/tests/integration/Application/CommandHandler/Entity/PublishEntityProductionCommandHandlerTest.php +++ b/tests/integration/Application/CommandHandler/Entity/PublishEntityProductionCommandHandlerTest.php @@ -26,17 +26,28 @@ use Psr\Log\LoggerInterface; use Surfnet\SamlBundle\Security\Authentication\Token\SamlToken; use Surfnet\ServiceProviderDashboard\Application\Command\Entity\PublishEntityProductionCommand; +use Surfnet\ServiceProviderDashboard\Application\Command\Entity\PublishEntityTestCommand; use Surfnet\ServiceProviderDashboard\Application\CommandHandler\Entity\PublishEntityProductionCommandHandler; use Surfnet\ServiceProviderDashboard\Application\Service\EntityServiceInterface; use Surfnet\ServiceProviderDashboard\Application\Service\MailService; use Surfnet\ServiceProviderDashboard\Application\Service\TicketService; use Surfnet\ServiceProviderDashboard\Domain\Entity\Constants; use Surfnet\ServiceProviderDashboard\Domain\Entity\Contact; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\AllowedIdentityProviders; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\AttributeList; use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\Coin; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\ContactList; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\Logo; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\MetaData; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\Organization; use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\Protocol; use Surfnet\ServiceProviderDashboard\Domain\Entity\ManageEntity; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Service; use Surfnet\ServiceProviderDashboard\Domain\Repository\PublishEntityRepository; use Surfnet\ServiceProviderDashboard\Domain\Service\ContractualBaseService; +use Surfnet\ServiceProviderDashboard\Domain\Service\TypeOfServiceService; +use Surfnet\ServiceProviderDashboard\Domain\ValueObject\TypeOfService; +use Surfnet\ServiceProviderDashboard\Domain\ValueObject\TypeOfServiceCollection; use Surfnet\ServiceProviderDashboard\Infrastructure\DashboardSamlBundle\Security\Identity; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface; @@ -92,6 +103,7 @@ public function setUp(): void $this->commandHandler = new PublishEntityProductionCommandHandler( $this->publishEntityClient, new ContractualBaseService(), + new TypeOfServiceService(), $this->entityService, $this->ticketService, $this->requestStack, @@ -115,6 +127,9 @@ public function test_it_can_publish() $coin = m::mock(Coin::class); $coin->shouldReceive('setContractualBase')->with(Constants::CONTRACTUAL_BASE_IX); + $coin->shouldReceive('hasTypeOfService')->andReturn(false); + $coin->shouldReceive('getTypeOfService')->andReturn(new TypeOfServiceCollection()); + $coin->shouldReceive('setTypeOfService'); $manageEntity->shouldReceive('getMetaData->getCoin')->andReturn($coin); $protocol = m::mock(Protocol::class); @@ -191,6 +206,8 @@ public function test_it_can_republish() $coin = m::mock(Coin::class); $coin->shouldReceive('setContractualBase')->with(Constants::CONTRACTUAL_BASE_IX); + $coin->shouldReceive('hasTypeOfService')->andReturn(false); + $coin->shouldReceive('setTypeOfService'); $manageEntity->shouldReceive('getMetaData->getCoin')->andReturn($coin); $protocol = m::mock(Protocol::class); @@ -301,6 +318,8 @@ public function test_does_not_create_ticket_when_client_resetting() $coin = m::mock(Coin::class); $coin->shouldReceive('setContractualBase')->with(Constants::CONTRACTUAL_BASE_IX); + $coin->shouldReceive('hasTypeOfService')->andReturn(false); + $coin->shouldReceive('setTypeOfService'); $manageEntity->shouldReceive('getMetaData->getCoin')->andReturn($coin); $protocol = m::mock(Protocol::class); @@ -347,4 +366,107 @@ public function test_does_not_create_ticket_when_client_resetting() $command->markPublishClientReset(); $this->commandHandler->handle($command); } + + + public function test_it_should_ensure_hidden_type_of_service_selections_are_preserved() + { + $manageEntity = $this->createEntity(); + + $applicant = new Contact('john:doe', 'john@example.com', 'John Doe'); + + /** @var ManageEntity $pristineEntity */ + $pristineEntity = unserialize(serialize($manageEntity)); + $pristineEntityCoin = $pristineEntity->getMetaData()->getCoin()->getTypeOfService(); + $manageEntityCoin = $manageEntity->getMetaData()->getCoin()->getTypeOfService(); + + $pristineEntityCoin->add(new TypeOfService('SURF', 'SURF', true)); + $pristineEntityCoin->add(new TypeOfService('Education', 'Onderwijs', false)); + + $manageEntityCoin->add(new TypeOfService('Productivity', 'Productiviteit', false)); + $manageEntityCoin->add(new TypeOfService('Privacy/security', 'Privacy/beveiliging', false)); + $manageEntityCoin->add(new TypeOfService('Recommended', 'Aangeraden', true)); + $manageEntityCoin->add(new TypeOfService('Productivity', 'Productiviteit', false)); + + $this->logger + ->shouldReceive('info') + ->times(2); + + $this->entityService->shouldReceive('getPristineManageEntityById') + ->once() + ->with($manageEntity->getId(), $manageEntity->getEnvironment()) + ->andReturn($pristineEntity); + + $this->ticketService->shouldReceive('createJiraTicket') + ->once() + ->andReturn(new \Surfnet\ServiceProviderDashboard\Domain\ValueObject\Issue('ISSUE-123', 'foo', 'bar')); + + $expectedTypeOfService = new TypeOfServiceCollection(); + $expectedTypeOfService->add(new TypeOfService('Productivity', 'Productiviteit', false)); + $expectedTypeOfService->add(new TypeOfService('Privacy/security', 'Privacy/beveiliging', false)); + $expectedTypeOfService->add(new TypeOfService('SURF', 'SURF', true)); + + $command = new PublishEntityProductionCommand($manageEntity, $applicant); + + $this->publishEntityClient + ->shouldReceive('publish') + ->once() + ->withArgs(function (ManageEntity $entity, $pristineEntity, $applicant) use ($expectedTypeOfService){ + if ($expectedTypeOfService->getArray() != $entity->getMetaData()->getCoin()->getTypeOfService()->getArray()) { + $this->fail( + var_export($entity->getMetaData()->getCoin()->getTypeOfService()->getArray(), true) . ' is not equal to ' . var_export( + $expectedTypeOfService->getArray(), + true + ) + ); + } + return true; + }) + ->andReturn([ + 'id' => '123', + ]); + + $this->commandHandler->handle($command); + } + + private function createEntity(): ManageEntity + { + $coin = new Coin( + null, + null, + null, + null, + null, + new TypeOfServiceCollection(), + null, + null, + null, + null + ); + + $manageEntity = new ManageEntity( + 1, + new AttributeList(), + new MetaData( + 'https://test.entity.id', + 'https://test.metadata.url', + null, + null, + 'Test Description', + null, + 'Test Entity', + null, + new ContactList(), + new Organization('Test org', "Test organisation", null, null, null, null), + $coin, + new Logo(null, null, null) + ), + new AllowedIdentityProviders([], true), new Protocol(Constants::TYPE_SAML), + null, + new Service() + ); + $manageEntity->setId('f1e394b2-815b-4018-b780-898976322016'); + $manageEntity->setEnvironment('production'); + $manageEntity->setStatus('published'); + return $manageEntity; + } } diff --git a/tests/integration/Application/CommandHandler/Entity/PublishEntityTestCommandHandlerTest.php b/tests/integration/Application/CommandHandler/Entity/PublishEntityTestCommandHandlerTest.php index d07d8bdd7..2216ddad9 100644 --- a/tests/integration/Application/CommandHandler/Entity/PublishEntityTestCommandHandlerTest.php +++ b/tests/integration/Application/CommandHandler/Entity/PublishEntityTestCommandHandlerTest.php @@ -20,9 +20,19 @@ use Surfnet\ServiceProviderDashboard\Application\Service\EntityServiceInterface; use Surfnet\ServiceProviderDashboard\Domain\Entity\Contact; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\AllowedIdentityProviders; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\AttributeList; use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\Coin; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\ContactList; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\Logo; use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\MetaData; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\Organization; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\Protocol; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Service; use Surfnet\ServiceProviderDashboard\Domain\Service\ContractualBaseService; +use Surfnet\ServiceProviderDashboard\Domain\Service\TypeOfServiceService; +use Surfnet\ServiceProviderDashboard\Domain\ValueObject\TypeOfService; +use Surfnet\ServiceProviderDashboard\Domain\ValueObject\TypeOfServiceCollection; use Surfnet\ServiceProviderDashboard\Infrastructure\Manage\Client\PublishEntityClient; use Mockery as m; use Mockery\Adapter\Phpunit\MockeryTestCase; @@ -34,7 +44,6 @@ use Surfnet\ServiceProviderDashboard\Domain\Entity\ManageEntity; use Surfnet\ServiceProviderDashboard\Infrastructure\HttpClient\Exceptions\RuntimeException\PublishMetadataException; use Symfony\Component\HttpFoundation\RequestStack; -use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface; class PublishEntityTestCommandHandlerTest extends MockeryTestCase { @@ -59,6 +68,7 @@ public function setUp(): void $this->commandHandler = new PublishEntityTestCommandHandler( $this->client, new ContractualBaseService(), + new TypeOfServiceService(), $this->entityService, $this->logger, $this->requestStack @@ -88,6 +98,8 @@ public function test_it_can_publish_to_manage() $coin ->shouldReceive('getContractualBase') + ->shouldReceive('hasTypeOfService')->andReturn(false) + ->shouldReceive('setTypeOfService') ->andReturn('some_contractual_base_value'); $this->logger @@ -148,6 +160,8 @@ public function test_it_handles_failing_publish() $coin ->shouldReceive('getContractualBase') + ->shouldReceive('hasTypeOfService')->andReturn(false) + ->shouldReceive('setTypeOfService') ->andReturn('some_contractual_base_value'); $coin @@ -194,4 +208,102 @@ public function test_it_handles_failing_publish() $this->commandHandler->handle($command); } + + public function test_it_should_ensure_hidden_type_of_service_selections_are_preserved() + { + $manageEntity = $this->createEntity(); + + $applicant = new Contact('john:doe', 'john@example.com', 'John Doe'); + + /** @var ManageEntity $pristineEntity */ + $pristineEntity = unserialize(serialize($manageEntity)); + + $pristineEntityCoin = $pristineEntity->getMetaData()->getCoin()->getTypeOfService(); + $manageEntityCoin = $manageEntity->getMetaData()->getCoin()->getTypeOfService(); + + $pristineEntityCoin->add(new TypeOfService('SURF', 'SURF', true)); + $pristineEntityCoin->add(new TypeOfService('Education', 'Onderwijs', false)); + + $manageEntityCoin->add(new TypeOfService('Productivity', 'Productiviteit', false)); + $manageEntityCoin->add(new TypeOfService('Privacy/security', 'Privacy/beveiliging', false)); + $manageEntityCoin->add(new TypeOfService('Recommended', 'Aangeraden', true)); + $manageEntityCoin->add(new TypeOfService('Productivity', 'Productiviteit', false)); + $this->logger + ->shouldReceive('info') + ->times(1); + + $this->entityService->shouldReceive('getPristineManageEntityById') + ->once() + ->with($manageEntity->getId(), $manageEntity->getEnvironment()) + ->andReturn($pristineEntity); + + $expectedTypeOfService = new TypeOfServiceCollection(); + $expectedTypeOfService->add(new TypeOfService('Productivity', 'Productiviteit', false)); + $expectedTypeOfService->add(new TypeOfService('Privacy/security', 'Privacy/beveiliging', false)); + $expectedTypeOfService->add(new TypeOfService('SURF', 'SURF', true)); + + $command = new PublishEntityTestCommand($manageEntity, $applicant); + + $this->client + ->shouldReceive('publish') + ->once() + ->withArgs(function (ManageEntity $entity, $pristineEntity, $applicant) use ($expectedTypeOfService){ + if ($expectedTypeOfService->getArray() != $entity->getMetaData()->getCoin()->getTypeOfService()->getArray()) { + $this->fail( + var_export($entity->getMetaData()->getCoin()->getTypeOfService()->getArray(), true) . ' is not equal to ' . var_export( + $expectedTypeOfService->getArray(), + true + ) + ); + } + return true; + }) + ->andReturn([ + 'id' => '123', + ]); + + $this->commandHandler->handle($command); + } + + private function createEntity(): ManageEntity + { + $coin = new Coin( + null, + null, + null, + null, + null, + new TypeOfServiceCollection(), + null, + null, + null, + null + ); + + $manageEntity = new ManageEntity( + 1, + new AttributeList(), + new MetaData( + 'https://test.entity.id', + 'https://test.metadata.url', + null, + null, + 'Test Description', + null, + 'Test Entity', + null, + new ContactList(), + new Organization('Test org', "Test organisation", null, null, null, null), + $coin, + new Logo(null, null, null) + ), + new AllowedIdentityProviders([], true), new Protocol(Constants::TYPE_SAML), + null, + new Service() + ); + $manageEntity->setId('f1e394b2-815b-4018-b780-898976322016'); + $manageEntity->setEnvironment('production'); + $manageEntity->setStatus('published'); + return $manageEntity; + } } diff --git a/tests/unit/Domain/Service/TypeOfServiceServiceTest.php b/tests/unit/Domain/Service/TypeOfServiceServiceTest.php new file mode 100644 index 000000000..4de96fcbc --- /dev/null +++ b/tests/unit/Domain/Service/TypeOfServiceServiceTest.php @@ -0,0 +1,101 @@ +add($type1); + $types->add($type2); + + $result = $service->mergeCollections($types, null); + + $this->assertCount(1, $result->getArray()); + $this->assertSame('type1', $result->getArray()[0]->typeEn); + } + + public function testApplyHiddenTypesWithPristineTypes(): void + { + $service = new TypeOfServiceService(); + + $type1 = new TypeOfService('type1', '', false); + $types = new TypeOfServiceCollection(); + $types->add($type1); + + $pristineType1 = new TypeOfService('type2', '', true); + $pristineTypes = new TypeOfServiceCollection(); + $pristineTypes->add($pristineType1); + + $result = $service->mergeCollections($types, $pristineTypes); + + $this->assertCount(2, $result->getArray()); + $this->assertSame('type1', $result->getArray()[0]->typeEn); + $this->assertSame('type2', $result->getArray()[1]->typeEn); + } + + public function testNoDuplicatesAddedBasedOnTypeEn(): void + { + $service = new TypeOfServiceService(); + + $type1 = new TypeOfService('type1', '', false); + $type2 = new TypeOfService('type1', '', true); // Duplicate typeEn + $types = new TypeOfServiceCollection(); + $types->add($type1); + $types->add($type2); + + $result = $service->mergeCollections($types, null); + + $this->assertCount(1, $result->getArray()); + $this->assertSame('type1', $result->getArray()[0]->typeEn); + } + + public function testNoDuplicatesAddedBasedOnTypeEnForPristineTypes(): void + { + $service = new TypeOfServiceService(); + + $type1 = new TypeOfService('type1', '', false); + $types = new TypeOfServiceCollection(); + $types->add($type1); + + $pristineType1 = new TypeOfService('type2', '', true); + $pristineType2 = new TypeOfService('type2', '', true); // Duplicate typeEn + $pristineTypes = new TypeOfServiceCollection(); + $pristineTypes->add($pristineType1); + $pristineTypes->add($pristineType2); + + $result = $service->mergeCollections($types, $pristineTypes); + + $this->assertCount(2, $result->getArray()); + $this->assertSame('type1', $result->getArray()[0]->typeEn); + $this->assertSame('type2', $result->getArray()[1]->typeEn); + } +} \ No newline at end of file diff --git a/tests/unit/Domain/ValueObject/TypeOfServiceCollectionTest.php b/tests/unit/Domain/ValueObject/TypeOfServiceCollectionTest.php new file mode 100644 index 000000000..509871829 --- /dev/null +++ b/tests/unit/Domain/ValueObject/TypeOfServiceCollectionTest.php @@ -0,0 +1,45 @@ +add(new TypeOfService('#1', '', true)); + $typeOfServiceCollection->add(new TypeOfService('#2', '', false)); + $typeOfServiceCollection->add(new TypeOfService('#3', '', true)); + $typeOfServiceCollection->add(new TypeOfService('#4', '', false)); + + $expectedCollection = new TypeOfServiceCollection(); + $expectedCollection->add(new TypeOfService('#2', '', false)); + $expectedCollection->add(new TypeOfService('#4', '', false)); + + $this->assertEquals($expectedCollection, $typeOfServiceCollection->filterHidden()); + } + +} diff --git a/tests/webtests/EntityEditTest.php b/tests/webtests/EntityEditTest.php index 33cc7f045..ea8add2ed 100644 --- a/tests/webtests/EntityEditTest.php +++ b/tests/webtests/EntityEditTest.php @@ -184,7 +184,7 @@ public function test_it_loads_xml_from_textarea() self::fillFormField($formElement, '#dashboard_bundle_entity_type_attributes_organizationAttribute_motivation', 'foo'); self::fillFormField($formElement, '#dashboard_bundle_entity_type_attributes_personalCodeAttribute_motivation', 'foo'); // Also check a type of service (as they are mandatory) -// self::findBy('option.decorated:nth-child(4)')->click(); + self::findBy('option.decorated:nth-child(4)')->click(); self::findBy('#dashboard_bundle_entity_type_publishButton')->click(); $pageTitle = self::$pantherClient->refreshCrawler()->filter('h1')->first()->text();