From 62de2e272658b74793cc9570c69a42f1127ca59e Mon Sep 17 00:00:00 2001 From: Ryan Koch Date: Wed, 1 Jan 2025 10:30:03 -0600 Subject: [PATCH 1/8] Changes module name to include VA.gov prefix. --- .../custom/va_gov_form_builder/va_gov_form_builder.info.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.info.yml b/docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.info.yml index bc3406b916..d3a96562c6 100644 --- a/docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.info.yml +++ b/docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.info.yml @@ -1,4 +1,4 @@ -name: "Form Builder" +name: "VA.gov Form Builder" type: module description: "A module that builds a custom experience for creating and editing Digital Form nodes." core_version_requirement: ^9 || ^10 From 08709b72235dfc414e95dbd119a385211fa2ce47 Mon Sep 17 00:00:00 2001 From: Ryan Koch Date: Wed, 1 Jan 2025 10:31:41 -0600 Subject: [PATCH 2/8] Adds `access form builder` permission. --- .../va_gov_form_builder/va_gov_form_builder.permissions.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.permissions.yml diff --git a/docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.permissions.yml b/docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.permissions.yml new file mode 100644 index 0000000000..85f3d3db04 --- /dev/null +++ b/docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.permissions.yml @@ -0,0 +1,4 @@ +access form builder: + title: "Access Form Builder" + description: "Allows the user to access the VA.gov Form Builder tool." + restrict access: FALSE From 19776cf7d88d3b6bf66cf851f7dab8604015024c Mon Sep 17 00:00:00 2001 From: Ryan Koch Date: Wed, 1 Jan 2025 10:32:07 -0600 Subject: [PATCH 3/8] Adds `Form Builder user` (form_builder_user) role with `access form builder` permission. --- config/sync/user.role.form_builder_user.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 config/sync/user.role.form_builder_user.yml diff --git a/config/sync/user.role.form_builder_user.yml b/config/sync/user.role.form_builder_user.yml new file mode 100644 index 0000000000..1b5dff7c74 --- /dev/null +++ b/config/sync/user.role.form_builder_user.yml @@ -0,0 +1,16 @@ +uuid: 2793e5e4-37aa-49b3-9c29-5e1c7ddc3772 +langcode: en +status: true +dependencies: + module: + - va_gov_backend + - va_gov_form_builder +third_party_settings: + va_gov_backend: + vgb_description: "A user of the VA.gov Form Builder tool." +id: form_builder_user +label: "Form Builder user" +weight: 14 +is_admin: null +permissions: + - "access form builder" From 24c68b10cd10a9b06ddec01aac021b71c4dd4291 Mon Sep 17 00:00:00 2001 From: Ryan Koch Date: Wed, 1 Jan 2025 10:35:13 -0600 Subject: [PATCH 4/8] Adds unit test for permissions.yml --- .../unit/PermissionsTest.php | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 tests/phpunit/va_gov_form_builder/unit/PermissionsTest.php diff --git a/tests/phpunit/va_gov_form_builder/unit/PermissionsTest.php b/tests/phpunit/va_gov_form_builder/unit/PermissionsTest.php new file mode 100644 index 0000000000..d887e24169 --- /dev/null +++ b/tests/phpunit/va_gov_form_builder/unit/PermissionsTest.php @@ -0,0 +1,68 @@ + + */ + private $permissions; + + /** + * Set up the environment for each test. + */ + protected function setUp(): void { + parent::setUp(); + + // Find Drupal root. + $drupalFinder = new DrupalFinder(); + $drupalFinder->locateRoot(__DIR__); + $drupalRoot = $drupalFinder->getDrupalRoot(); + + $yamlFile = $drupalRoot . '/' . $this->modulePath . '/va_gov_form_builder.permissions.yml'; + + $this->permissions = Yaml::parseFile($yamlFile); + } + + /** + * Tests that the `access form builder` permission is present. + */ + public function testPermissionPresent() { + $this->assertArrayHasKey('access form builder', $this->permissions); + + // Assert expected keys are present. + $formBuilderPermission = $this->permissions['access form builder']; + $this->assertArrayHasKey('title', $formBuilderPermission); + $this->assertArrayHasKey('description', $formBuilderPermission); + $this->assertArrayHasKey('restrict access', $formBuilderPermission); + + // Assert `restrict access` is false. + $restrictAccess = $formBuilderPermission['restrict access']; + $this->assertFalse($restrictAccess); + } + +} From 56bd03dfb4f2ae5c792fc96e985de6ca961abd0c Mon Sep 17 00:00:00 2001 From: Ryan Koch Date: Wed, 1 Jan 2025 10:35:40 -0600 Subject: [PATCH 5/8] Updates annotation on existing test to be more accurate. --- tests/phpunit/va_gov_form_builder/unit/LibrariesTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/va_gov_form_builder/unit/LibrariesTest.php b/tests/phpunit/va_gov_form_builder/unit/LibrariesTest.php index d306020496..9e08fc2242 100644 --- a/tests/phpunit/va_gov_form_builder/unit/LibrariesTest.php +++ b/tests/phpunit/va_gov_form_builder/unit/LibrariesTest.php @@ -24,13 +24,13 @@ class LibrariesTest extends VaGovUnitTestBase { /** * The libraries defined in libraries.yml. * - * @var array{ + * @var array * }, * js?: array, * dependencies?: array - * } + * }> */ private $libraries; From 580bf51d78becadd9ed00e82ad4c77b386cc42b1 Mon Sep 17 00:00:00 2001 From: Ryan Koch Date: Wed, 1 Jan 2025 10:36:48 -0600 Subject: [PATCH 6/8] Updates routing: - Use RouteSubscriber rather than repeating same permission on every route. - Update to use new permission. --- .../src/Routing/RouteSubscriber.php | 26 +++++++++++++++++++ .../va_gov_form_builder.routing.yml | 8 ------ .../va_gov_form_builder.services.yml | 5 ++++ 3 files changed, 31 insertions(+), 8 deletions(-) create mode 100644 docroot/modules/custom/va_gov_form_builder/src/Routing/RouteSubscriber.php create mode 100644 docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.services.yml diff --git a/docroot/modules/custom/va_gov_form_builder/src/Routing/RouteSubscriber.php b/docroot/modules/custom/va_gov_form_builder/src/Routing/RouteSubscriber.php new file mode 100644 index 0000000000..14d76da023 --- /dev/null +++ b/docroot/modules/custom/va_gov_form_builder/src/Routing/RouteSubscriber.php @@ -0,0 +1,26 @@ +all() as $route_name => $route) { + if (strpos($route_name, 'va_gov_form_builder.') === 0) { + if (!$route->hasRequirement('_permission', 'access form builder')) { + $route->setRequirement('_permission', 'access form builder'); + } + } + } + } + +} diff --git a/docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.routing.yml b/docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.routing.yml index 346b614ba0..52a9145505 100644 --- a/docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.routing.yml +++ b/docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.routing.yml @@ -2,23 +2,15 @@ va_gov_form_builder.entry: path: "/form-builder" defaults: _controller: '\Drupal\va_gov_form_builder\Controller\VaGovFormBuilderController::entry' - requirements: - _permission: "edit any digital_form content" va_gov_form_builder.intro: path: "/form-builder/intro" defaults: _controller: '\Drupal\va_gov_form_builder\Controller\VaGovFormBuilderController::intro' - requirements: - _permission: "edit any digital_form content" va_gov_form_builder.start_conversion: path: "/form-builder/start-conversion" defaults: _controller: '\Drupal\va_gov_form_builder\Controller\VaGovFormBuilderController::startConversion' - requirements: - _permission: "edit any digital_form content" va_gov_form_builder.name_and_dob: path: "/form-builder/{nid}/name-and-dob" defaults: _controller: '\Drupal\va_gov_form_builder\Controller\VaGovFormBuilderController::nameAndDob' - requirements: - _permission: "edit any digital_form content" diff --git a/docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.services.yml b/docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.services.yml new file mode 100644 index 0000000000..b6af461fe8 --- /dev/null +++ b/docroot/modules/custom/va_gov_form_builder/va_gov_form_builder.services.yml @@ -0,0 +1,5 @@ +services: + va_gov_form_builder.route_subscriber: + class: Drupal\va_gov_form_builder\Routing\RouteSubscriber + tags: + - { name: "event_subscriber" } From 22e185c5dd7f1007c1b331509cea70659453f848 Mon Sep 17 00:00:00 2001 From: Ryan Koch Date: Wed, 1 Jan 2025 10:37:52 -0600 Subject: [PATCH 7/8] Updates existing tests to align with new permission. Note that these existing tests effectively test the RouteSubscriber functionality. --- .../Traits/TestFormLoads.php | 6 ++--- .../functional/Form/IntroTest.php | 22 +++++++++++-------- .../functional/Form/NameAndDobTest.php | 2 +- .../functional/Form/StartConversionTest.php | 2 +- .../templates/FormBuilderPageTemplateTest.php | 2 +- 5 files changed, 19 insertions(+), 15 deletions(-) diff --git a/tests/phpunit/va_gov_form_builder/Traits/TestFormLoads.php b/tests/phpunit/va_gov_form_builder/Traits/TestFormLoads.php index 39b7ca00fd..506f523ad3 100644 --- a/tests/phpunit/va_gov_form_builder/Traits/TestFormLoads.php +++ b/tests/phpunit/va_gov_form_builder/Traits/TestFormLoads.php @@ -10,9 +10,9 @@ trait TestFormLoads { /** * Logs-in a user with appropriate privileges. */ - private function loginDigitalFormUser() { + private function loginFormBuilderUser() { $this->drupalLogin($this->createUser([ - 'edit any digital_form content', + 'access form builder', ])); } @@ -26,7 +26,7 @@ private function loginDigitalFormUser() { */ private function sharedTestFormLoads($url, $expectedText) { // Log in a user with permission. - $this->loginDigitalFormUser(); + $this->loginFormBuilderUser(); // Navigate to page. $this->drupalGet($url); diff --git a/tests/phpunit/va_gov_form_builder/functional/Form/IntroTest.php b/tests/phpunit/va_gov_form_builder/functional/Form/IntroTest.php index 80d6905791..955211e368 100644 --- a/tests/phpunit/va_gov_form_builder/functional/Form/IntroTest.php +++ b/tests/phpunit/va_gov_form_builder/functional/Form/IntroTest.php @@ -2,6 +2,7 @@ namespace tests\phpunit\va_gov_form_builder\functional\Form; +use tests\phpunit\va_gov_form_builder\Traits\TestFormLoads; use Tests\Support\Classes\VaGovExistingSiteBase; /** @@ -13,39 +14,42 @@ * @coversDefaultClass \Drupal\va_gov_form_builder\Form\Intro */ class IntroTest extends VaGovExistingSiteBase { + use TestFormLoads; /** * {@inheritdoc} */ private static $modules = ['va_gov_form_builder']; + /** + * Returns the url for this form (for the given node) + */ + private function getFormPageUrl() { + return '/form-builder/intro'; + } + /** * Set up the environment for each test. */ public function setUp(): void { parent::setUp(); - $this->drupalLogin($this->createUser(['edit any digital_form content'])); - $this->drupalGet('/form-builder/intro'); + $this->loginFormBuilderUser(); + $this->drupalGet($this->getFormPageUrl()); } /** * Test that the form is accessible to a user with the correct privilege. */ public function testFormLoads() { - $this->assertSession()->statusCodeEquals(200); - $this->assertSession()->pageTextContains('Working with the Form Builder'); + $this->sharedTestFormLoads($this->getFormPageUrl(), 'Working with the Form Builder'); } /** * Test that the form is not accessible to a user without privilege. */ public function testFormDoesNotLoad() { - // Log out the good user and log in a user without permission. - $this->drupalLogin($this->createUser([])); - $this->drupalGet('/form-builder/intro'); - - $this->assertSession()->statusCodeNotEquals(200); + $this->sharedTestFormDoesNotLoad($this->getFormPageUrl()); } /** diff --git a/tests/phpunit/va_gov_form_builder/functional/Form/NameAndDobTest.php b/tests/phpunit/va_gov_form_builder/functional/Form/NameAndDobTest.php index 98a963b380..f12f632c1f 100644 --- a/tests/phpunit/va_gov_form_builder/functional/Form/NameAndDobTest.php +++ b/tests/phpunit/va_gov_form_builder/functional/Form/NameAndDobTest.php @@ -53,7 +53,7 @@ private function reloadNode() { public function setUp(): void { parent::setUp(); - $this->loginDigitalFormUser(); + $this->loginFormBuilderUser(); // Create a node that doesn't have any chapters. $this->node = $this->createNode([ diff --git a/tests/phpunit/va_gov_form_builder/functional/Form/StartConversionTest.php b/tests/phpunit/va_gov_form_builder/functional/Form/StartConversionTest.php index cdb1aeae36..53de9e642a 100644 --- a/tests/phpunit/va_gov_form_builder/functional/Form/StartConversionTest.php +++ b/tests/phpunit/va_gov_form_builder/functional/Form/StartConversionTest.php @@ -37,7 +37,7 @@ private function getFormPageUrl() { public function setUp(): void { parent::setUp(); - $this->loginDigitalFormUser(); + $this->loginFormBuilderUser(); $this->drupalGet($this->getFormPageUrl()); } diff --git a/tests/phpunit/va_gov_form_builder/functional/templates/FormBuilderPageTemplateTest.php b/tests/phpunit/va_gov_form_builder/functional/templates/FormBuilderPageTemplateTest.php index 79513d5432..4865211feb 100644 --- a/tests/phpunit/va_gov_form_builder/functional/templates/FormBuilderPageTemplateTest.php +++ b/tests/phpunit/va_gov_form_builder/functional/templates/FormBuilderPageTemplateTest.php @@ -34,7 +34,7 @@ class FormBuilderPageTemplateTest extends VaGovExistingSiteBase { public function setUp(): void { parent::setUp(); - $this->drupalLogin($this->createUser(['edit any digital_form content'])); + $this->drupalLogin($this->createUser(['access form builder'])); // Form Builder entry. $this->drupalGet('/form-builder'); From dcbb5e93f928a800b4bcec92ea2183f9d6298ad5 Mon Sep 17 00:00:00 2001 From: Ryan Koch Date: Fri, 3 Jan 2025 11:03:43 -0600 Subject: [PATCH 8/8] Small fix to `hasRequirement` check. Cannot take a specific permission as a second parameter. We'll set the permission whenever one is not set. --- .../custom/va_gov_form_builder/src/Routing/RouteSubscriber.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docroot/modules/custom/va_gov_form_builder/src/Routing/RouteSubscriber.php b/docroot/modules/custom/va_gov_form_builder/src/Routing/RouteSubscriber.php index 14d76da023..c7876c5878 100644 --- a/docroot/modules/custom/va_gov_form_builder/src/Routing/RouteSubscriber.php +++ b/docroot/modules/custom/va_gov_form_builder/src/Routing/RouteSubscriber.php @@ -16,7 +16,7 @@ class RouteSubscriber extends RouteSubscriberBase { protected function alterRoutes(RouteCollection $collection) { foreach ($collection->all() as $route_name => $route) { if (strpos($route_name, 'va_gov_form_builder.') === 0) { - if (!$route->hasRequirement('_permission', 'access form builder')) { + if (!$route->hasRequirement('_permission')) { $route->setRequirement('_permission', 'access form builder'); } }