Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove 'production entities enabled' feature toggle from the Service entity #628

Merged
merged 5 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions assets/js/components/mock/service_create_form.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ <h2>General</h2>
<p class="parsley-errors"></p>
<input type="text" id="dashboard_bundle_service_type_general_teamName"
name="dashboard_bundle_service_type[general][teamName]" required="required"/></div>
<div class="form-row"><label for="dashboard_bundle_service_type_general_productionEntitiesEnabled">Production
entities enabled</label>
<p class="parsley-errors"></p>
<input type="checkbox" id="dashboard_bundle_service_type_general_productionEntitiesEnabled"
name="dashboard_bundle_service_type[general][productionEntitiesEnabled]" value="1"/>
</div>
<div class="form-row"><label for="dashboard_bundle_service_type_general_privacyQuestionsEnabled">Privacy
questions enabled</label>
<p class="parsley-errors"></p>
Expand Down
6 changes: 0 additions & 6 deletions assets/js/components/mock/service_edit_form.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,6 @@ <h2>General</h2>
<input type="text" id="dashboard_bundle_edit_service_type_general_teamName"
name="dashboard_bundle_edit_service_type[general][teamName]" required="required"
value="team-23"/></div>
<div class="form-row"><label for="dashboard_bundle_edit_service_type_general_productionEntitiesEnabled">Production
entities enabled</label>
<p class="parsley-errors"></p>
<input type="checkbox" id="dashboard_bundle_edit_service_type_general_productionEntitiesEnabled"
name="dashboard_bundle_edit_service_type[general][productionEntitiesEnabled]" value="1"/>
</div>
<div class="form-row"><label for="dashboard_bundle_edit_service_type_general_privacyQuestionsEnabled">Privacy
questions enabled</label>
<p class="parsley-errors"></p>
Expand Down
15 changes: 0 additions & 15 deletions ci/qa/phpstan-baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -306,16 +306,6 @@
'count' => 1,
'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Command/PrivacyQuestions/PrivacyQuestionsCommand.php',
];
$ignoreErrors[] = [
'message' => '#^PHPDoc tag @param for parameter \\$privacyQuestionsAnswered with type bool is incompatible with native type string\\|null\\.$#',
'count' => 1,
'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Command/Service/EditServiceCommand.php',
];
$ignoreErrors[] = [
'message' => '#^PHPDoc tag @return with type bool is incompatible with native type string\\|null\\.$#',
'count' => 1,
'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Command/Service/EditServiceCommand.php',
];
$ignoreErrors[] = [
'message' => '#^Call to an undefined method Symfony\\\\Component\\\\HttpFoundation\\\\Session\\\\SessionInterface\\:\\:getFlashBag\\(\\)\\.$#',
'count' => 3,
Expand Down Expand Up @@ -3446,11 +3436,6 @@
'count' => 1,
'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceController.php',
];
$ignoreErrors[] = [
'message' => '#^Parameter \\#12 \\$privacyQuestionsAnswered of class Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Command\\\\Service\\\\EditServiceCommand constructor expects string\\|null, bool given\\.$#',
'count' => 1,
'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceController.php',
];
$ignoreErrors[] = [
'message' => '#^Call to an undefined method Symfony\\\\Component\\\\HttpFoundation\\\\Session\\\\SessionInterface\\:\\:getFlashBag\\(\\)\\.$#',
'count' => 1,
Expand Down
2 changes: 0 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@
"ext-mbstring": "*",
"ext-openssl": "*",
"ext-simplexml": "*",
"ext-soap": "*",
"ext-xml": "*",
"ext-zip": "*",
"cweagans/composer-patches": "^1.7",
"doctrine/doctrine-bundle": "^2.1.2",
"doctrine/doctrine-migrations-bundle": "^3.2",
Expand Down
6 changes: 2 additions & 4 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 31 additions & 0 deletions migrations/Version20240514071702.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

declare(strict_types=1);

namespace Surfnet\ServiceProviderDashboard\Migrations;

use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;

/**
* Drops the `production_entities_enabled` column from the `service` entity
*/
final class Version20240514071702 extends AbstractMigration
{
public function getDescription(): string
{
return 'Drops the `production_entities_enabled` column from the `service` entity';
}

public function up(Schema $schema): void
{
// this up() migration is auto-generated, please modify it to your needs
$this->addSql('ALTER TABLE service DROP production_entities_enabled');
}

public function down(Schema $schema): void
{
// this down() migration is auto-generated, please modify it to your needs
$this->addSql('ALTER TABLE service ADD production_entities_enabled TINYINT(1) NOT NULL');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ class CreateServiceCommand implements Command
*/
private $institutionId;

private bool $productionEntitiesEnabled = false;

private bool $privacyQuestionsEnabled = true;

Expand Down Expand Up @@ -104,11 +103,6 @@ public function setName($name): void
$this->name = $name;
}

public function setProductionEntitiesEnabled(bool $enabled): void
{
$this->productionEntitiesEnabled = $enabled;
}

public function setPrivacyQuestionsEnabled(bool $privacyQuestionsEnabled): void
{
$this->privacyQuestionsEnabled = $privacyQuestionsEnabled;
Expand Down Expand Up @@ -166,11 +160,6 @@ public function getName()
return $this->name;
}

public function isProductionEntitiesEnabled(): bool
{
return $this->productionEntitiesEnabled;
}

public function isPrivacyQuestionsEnabled(): bool
{
return $this->privacyQuestionsEnabled;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ public function __construct(
#[SpDashboardAssert\UrnFormattedTeamName]
#[Assert\NotBlank]
private string $teamName,
private bool $productionEntitiesEnabled,
private bool $privacyQuestionsEnabled,
private bool $clientCredentialClientsEnabled,
#[Assert\NotBlank]
Expand All @@ -51,7 +50,7 @@ public function __construct(
private ?string $intakeStatus,
private ?string $contractSigned,
private ?string $surfconextRepresentativeApproved,
private ?string $privacyQuestionsAnswered,
private bool $privacyQuestionsAnswered,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not nullable anymore?

BTW, I heard a statement, that bool variables "sh|c"ould be named with is in front, like $isPrivacyQuestionAnswered.

Butt hat is probably not for now

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If is is not nullebale, maybe initilize it with false to be sure?

Copy link
Contributor Author

@MKodde MKodde May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I looked it up, and it should always be set. My own testing and the CI functional tests did confirm my assumption. I feel confident to leave it like this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private ?string $institutionId,
#[Assert\NotBlank]
private ?string $organizationNameNl,
Expand All @@ -75,11 +74,6 @@ public function setName(string $name): void
$this->name = $name;
}

public function setProductionEntitiesEnabled(bool $enabled): void
{
$this->productionEntitiesEnabled = $enabled;
}

public function setPrivacyQuestionsEnabled(bool $privacyQuestionsEnabled): void
{
$this->privacyQuestionsEnabled = $privacyQuestionsEnabled;
Expand Down Expand Up @@ -122,10 +116,7 @@ public function setSurfconextRepresentativeApproved(?string $surfconextRepresent
$this->surfconextRepresentativeApproved = $surfconextRepresentativeApproved;
}

/**
* @param bool $privacyQuestionsAnswered
*/
public function setPrivacyQuestionsAnswered(?string $privacyQuestionsAnswered): void
public function setPrivacyQuestionsAnswered(bool $privacyQuestionsAnswered): void
{
$this->privacyQuestionsAnswered = $privacyQuestionsAnswered;
}
Expand Down Expand Up @@ -193,16 +184,11 @@ public function getSurfconextRepresentativeApproved(): ?string
/**
* @return bool
*/
public function isPrivacyQuestionsAnswered(): ?string
public function isPrivacyQuestionsAnswered(): bool
{
return $this->privacyQuestionsAnswered;
}

public function isProductionEntitiesEnabled(): bool
{
return $this->productionEntitiesEnabled;
}

public function isPrivacyQuestionsEnabled(): bool
{
return $this->privacyQuestionsEnabled;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ public function handle(CreateServiceCommand $command): void
$service->setName($name);
$service->setGuid($command->getGuid());
$service->setTeamName($fullTeamName);
$service->setProductionEntitiesEnabled($command->isProductionEntitiesEnabled());
$service->setPrivacyQuestionsEnabled($command->isPrivacyQuestionsEnabled());
$service->setClientCredentialClientsEnabled($command->isClientCredentialClientsEnabled());
$service->setServiceType($command->getServiceType());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ public function handle(EditServiceCommand $command): void

$service->setName($command->getName());
$service->setGuid($command->getGuid());
$service->setProductionEntitiesEnabled($command->isProductionEntitiesEnabled());
$service->setPrivacyQuestionsEnabled($command->isPrivacyQuestionsEnabled());
$service->setClientCredentialClientsEnabled($command->isClientCredentialClientsEnabled());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ public function __construct(
private readonly bool $privacyQuestionsEnabled,
private readonly EntityList $entityList,
private readonly RouterInterface $router,
private readonly bool $productionEntitiesEnabled = false,
) {
}

Expand All @@ -40,7 +39,6 @@ public static function fromService(DomainService $service, EntityList $entityLis
$service->isPrivacyQuestionsEnabled(),
$entityList,
$router,
$service->isProductionEntitiesEnabled()
);
}

Expand Down Expand Up @@ -73,9 +71,4 @@ public function hasTestEntities() : bool
{
return $this->getEntityList()->hasTestEntities();
}

public function isProductionEntitiesEnabled(): bool
{
return $this->productionEntitiesEnabled || $this->hasTestEntities();
}
}
16 changes: 0 additions & 16 deletions src/Surfnet/ServiceProviderDashboard/Domain/Entity/Service.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,9 @@
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Collection;
use Doctrine\ORM\Mapping as ORM;
use Surfnet\ServiceProviderDashboard\Domain\Entity\PrivacyQuestions;
parijke marked this conversation as resolved.
Show resolved Hide resolved
use Surfnet\ServiceProviderDashboard\Infrastructure\DashboardBundle\Repository\ServiceRepository;

/**
* @package Surfnet\ServiceProviderDashboard\Entity
*
* @SuppressWarnings(PHPMD.TooManyFields)
*/
#[ORM\Entity(repositoryClass: ServiceRepository::class)]
Expand Down Expand Up @@ -84,9 +81,6 @@ class Service
#[ORM\Column(length: 255)]
private $teamName;

#[ORM\Column(type: 'boolean')]
private bool $productionEntitiesEnabled = false;

#[ORM\Column(type: 'boolean')]
private bool $privacyQuestionsEnabled = true;

Expand Down Expand Up @@ -190,11 +184,6 @@ public function setName($name): void
$this->name = $name;
}

public function setProductionEntitiesEnabled(bool $enabled): void
{
$this->productionEntitiesEnabled = $enabled;
}

public function setPrivacyQuestionsEnabled(bool $privacyQuestionsEnabled): void
{
$this->privacyQuestionsEnabled = $privacyQuestionsEnabled;
Expand All @@ -221,11 +210,6 @@ public function setPrivacyQuestions(PrivacyQuestions $privacyQuestions): void
$this->privacyQuestions = $privacyQuestions;
}

public function isProductionEntitiesEnabled(): bool
{
return $this->productionEntitiesEnabled;
}

public function isPrivacyQuestionsEnabled(): bool
{
return $this->privacyQuestionsEnabled;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ public function type(
$entityList = $this->entityService
->getEntityListForService($service)
->sortEntitiesByEnvironment();
$isProductionEnabled = $entityList->hasTestEntities() || $service->isProductionEntitiesEnabled();
$form = $this->createForm(CreateNewEntityType::class, $formId);

$form->handleRequest($request);
Expand Down Expand Up @@ -129,7 +128,6 @@ public function type(
'environment' => $targetEnvironment,
'inputId' => $inputId,
'protocols' => $choices,
'productionEnabled' => $isProductionEnabled,
'entities' => $entityList->getEntities(),
'manageId' => $formId,
]
Expand All @@ -156,9 +154,7 @@ public function create(Request $request, $serviceId, $targetEnvironment, $type):
$hasTestEntities = $this->entityService
->getEntityListForService($service)->hasTestEntities();

if (!$service->isProductionEntitiesEnabled() && !$hasTestEntities
&& $targetEnvironment !== Constants::ENVIRONMENT_TEST
) {
if (!$hasTestEntities && $targetEnvironment !== Constants::ENVIRONMENT_TEST) {
throw $this->createAccessDeniedException(
'You do not have access to create entities without publishing to the test environment first'
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ public function edit(Request $request, int $serviceId): RedirectResponse|Respons
$service->getGuid(),
$service->getName(),
$service->getTeamName(),
$service->isProductionEntitiesEnabled(),
$service->isPrivacyQuestionsEnabled(),
$service->isClientCredentialClientsEnabled(),
$service->getServiceType(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,9 @@ class WebTestFixtures extends Fixture
public function load(ObjectManager $manager): void
{
$service = $this->createService('SURFnet', 'urn:collab:group:vm.openconext.org:demo:openconext:org:surf.nl');
$service->setProductionEntitiesEnabled(false);
$manager->persist($service);

$service = $this->createService('Ibuildings B.V.', 'urn:collab:group:vm.openconext.org:demo:openconext:org:ibuildings.nl');
$service->setProductionEntitiesEnabled(true);
$service->setPrivacyQuestionsEnabled(true);
$service->setClientCredentialClientsEnabled(true);
$manager->persist($service);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,6 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
'attr' => ['class' => 'institution-id-container'],
]
)
->add(
'productionEntitiesEnabled',
CheckboxType::class,
[
'required' => false,
]
)
->add(
'privacyQuestionsEnabled',
CheckboxType::class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,6 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
'attr' => ['class' => 'institution-id-container'],
]
)
->add(
'productionEntitiesEnabled',
CheckboxType::class,
[
'required' => false,
]
)
->add(
'privacyQuestionsEnabled',
CheckboxType::class,
Expand Down
2 changes: 1 addition & 1 deletion templates/EntityModal/addEntityModal.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
{% set fieldName = manageId ~ '_environment' %}
<li class="add-entity-field">
{% set fieldId = manageId ~ '_prod' %}
<input class="add-entity-radio" type="radio" name={{ fieldName }} id="{{ fieldId }}" value="production"{% if not productionEnabled %} disabled="disabled"{% endif %}{% if environment is same as('production') and productionEnabled %} checked="checked"{% endif %}>
<input class="add-entity-radio" type="radio" name={{ fieldName }} id="{{ fieldId }}" value="production"{% if environment is same as('production') %} checked="checked"{% endif %}>
<label class="add-entity-label" for="{{ fieldId }}">{{ 'entity.add.environment.production'|trans }}</label>
</li>
<li class="add-entity-field">
Expand Down
Loading
Loading