From 9044bce2866eceaa78a2c73498bd31f358d7e9e8 Mon Sep 17 00:00:00 2001 From: AaronGilMartinez Date: Wed, 4 Dec 2024 10:19:39 +0100 Subject: [PATCH 1/8] Set validation for configuration and add test. --- src/Form/ConfigForm.php | 44 ++++++++++----- tests/src/Functional/SettingsTest.php | 80 +++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 15 deletions(-) create mode 100644 tests/src/Functional/SettingsTest.php diff --git a/src/Form/ConfigForm.php b/src/Form/ConfigForm.php index 284e54d9..9ea820bf 100644 --- a/src/Form/ConfigForm.php +++ b/src/Form/ConfigForm.php @@ -1,5 +1,7 @@ config(static::SETTINGS); + $cool_settings = $config->get('cool'); $form['server'] = [ - '#type' => 'textfield', + '#type' => 'url', '#title' => $this->t('Collabora Online server URL'), - '#default_value' => $config->get('cool')['server'], + '#default_value' => $cool_settings['server'] ?? '', '#required' => TRUE, ]; $form['wopi_base'] = [ - '#type' => 'textfield', - '#title' => $this->t('WOPI host URL. Likely https://<drupal_server>'), - '#default_value' => $config->get('cool')['wopi_base'], + '#type' => 'url', + '#title' => $this->t('WOPI host URL'), + '#description' => $this->t('Likely https://<drupal_server>'), + '#default_value' => $cool_settings['wopi_base'] ?? '', '#required' => TRUE, ]; $form['key_id'] = [ '#type' => 'textfield', '#title' => $this->t('JWT private key ID'), - '#default_value' => $config->get('cool')['key_id'], + '#default_value' => $cool_settings['key_id'] ?? '', '#required' => TRUE, ]; $form['access_token_ttl'] = [ - '#type' => 'textfield', + '#type' => 'number', '#title' => $this->t('Access Token Expiration (in seconds)'), - '#default_value' => $config->get('cool')['access_token_ttl'], + '#default_value' => $cool_settings['access_token_ttl'] ?? 0, + '#min' => 0, '#required' => TRUE, ]; $form['disable_cert_check'] = [ '#type' => 'checkbox', '#title' => $this->t('Disable TLS certificate check for COOL.'), - '#default_value' => $config->get('cool')['disable_cert_check'], + '#default_value' => $cool_settings['disable_cert_check'] ?? FALSE, ]; $form['allowfullscreen'] = [ '#type' => 'checkbox', '#title' => $this->t('Allow COOL to use fullscreen mode.'), - '#default_value' => $config->get('cool')['allowfullscreen'] ?? FALSE, + '#default_value' => $cool_settings['allowfullscreen'] ?? FALSE, ]; return parent::buildForm($form, $form_state); @@ -90,12 +95,21 @@ public function buildForm(array $form, FormStateInterface $form_state) { /** * {@inheritdoc} */ - public function submitForm(array &$form, FormStateInterface $form_state) { + public function validateForm(array &$form, FormStateInterface $form_state) { + // Remove slashes at the end of wopi_base URL. $wopi_base = rtrim($form_state->getValue('wopi_base'), '/'); + $form_state->setValueForElement($form['wopi_base'], $wopi_base); + parent::validateForm($form, $form_state); + } + + /** + * {@inheritdoc} + */ + public function submitForm(array &$form, FormStateInterface $form_state): void { $this->config(static::SETTINGS) ->set('cool.server', $form_state->getValue('server')) - ->set('cool.wopi_base', $wopi_base) + ->set('cool.wopi_base', $form_state->getValue('wopi_base')) ->set('cool.key_id', $form_state->getValue('key_id')) ->set('cool.access_token_ttl', $form_state->getValue('access_token_ttl')) ->set('cool.disable_cert_check', $form_state->getValue('disable_cert_check')) diff --git a/tests/src/Functional/SettingsTest.php b/tests/src/Functional/SettingsTest.php new file mode 100644 index 00000000..ed83c8dd --- /dev/null +++ b/tests/src/Functional/SettingsTest.php @@ -0,0 +1,80 @@ +assertSession(); + + // User without permission can't access the configuration page. + $this->drupalGet(Url::fromRoute('collabora-online.settings')); + $assert_session->pageTextContains('You are not authorized to access this page.'); + $assert_session->statusCodeEquals(403); + + // User with permission can access the configuration page. + $user = $this->createUser(['administer site configuration']); + $this->drupalLogin($user); + $this->drupalGet(Url::fromRoute('collabora-online.settings')); + $assert_session->statusCodeEquals(200); + + // Check fields and default values. + $this->drupalGet(Url::fromRoute('collabora-online.settings')); + $assert_session->fieldValueEquals('Collabora Online server URL', 'https://localhost:9980/'); + $assert_session->fieldValueEquals('WOPI host URL', 'https://localhost/'); + $assert_session->fieldValueEquals('JWT private key ID', 'cool'); + $assert_session->fieldValueEquals('Access Token Expiration (in seconds)', '86400'); + $assert_session->fieldValueEquals('Disable TLS certificate check for COOL.', ''); + $assert_session->fieldValueEquals('Allow COOL to use fullscreen mode.', '1'); + + // Check that fields values are saved. + $assert_session->fieldExists('Collabora Online server URL') + ->setValue('http://collaboraserver.com/'); + $assert_session->fieldExists('WOPI host URL') + ->setValue('http://wopihost.com/'); + $assert_session->fieldExists('JWT private key ID') + ->setValue('a_random_token'); + $assert_session->fieldExists('Access Token Expiration (in seconds)') + ->setValue('3600'); + $assert_session->fieldExists('Disable TLS certificate check for COOL.') + ->check(); + // Since default is checked we disable the full screen option. + $assert_session->fieldExists('Allow COOL to use fullscreen mode.') + ->uncheck(); + $assert_session->buttonExists('Save configuration')->press(); + $assert_session->statusMessageContains('The configuration options have been saved.', 'status'); + + // Check the values in fields. + $this->drupalGet(Url::fromRoute('collabora-online.settings')); + $assert_session->fieldValueEquals('Collabora Online server URL', 'http://collaboraserver.com/'); + $assert_session->fieldValueEquals('WOPI host URL', 'http://wopihost.com'); + $assert_session->fieldValueEquals('JWT private key ID', 'a_random_token'); + $assert_session->fieldValueEquals('Access Token Expiration (in seconds)', '3600'); + $assert_session->fieldValueEquals('Disable TLS certificate check for COOL.', '1'); + $assert_session->fieldValueEquals('Allow COOL to use fullscreen mode.', ''); + } + +} From a5a83cab6176142a0339e8d28df7bb868ef3789a Mon Sep 17 00:00:00 2001 From: AaronGilMartinez Date: Tue, 10 Dec 2024 12:57:20 +0100 Subject: [PATCH 2/8] Add required fields and validation checks. --- tests/src/Functional/SettingsTest.php | 28 +++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/src/Functional/SettingsTest.php b/tests/src/Functional/SettingsTest.php index ed83c8dd..6958b28c 100644 --- a/tests/src/Functional/SettingsTest.php +++ b/tests/src/Functional/SettingsTest.php @@ -70,11 +70,39 @@ public function testSettingsForm(): void { // Check the values in fields. $this->drupalGet(Url::fromRoute('collabora-online.settings')); $assert_session->fieldValueEquals('Collabora Online server URL', 'http://collaboraserver.com/'); + // Slash is removed at the end of Wopi URL. $assert_session->fieldValueEquals('WOPI host URL', 'http://wopihost.com'); $assert_session->fieldValueEquals('JWT private key ID', 'a_random_token'); $assert_session->fieldValueEquals('Access Token Expiration (in seconds)', '3600'); $assert_session->fieldValueEquals('Disable TLS certificate check for COOL.', '1'); $assert_session->fieldValueEquals('Allow COOL to use fullscreen mode.', ''); + + // Check required fields, set empty values and check errors displayed. + $this->drupalGet(Url::fromRoute('collabora-online.settings')); + $assert_session->fieldExists('Collabora Online server URL')->setValue(''); + $assert_session->fieldExists('WOPI host URL')->setValue(''); + $assert_session->fieldExists('JWT private key ID')->setValue(''); + $assert_session->fieldExists('Access Token Expiration (in seconds)')->setValue(''); + $assert_session->fieldExists('Disable TLS certificate check for COOL.')->uncheck(); + $assert_session->fieldExists('Allow COOL to use fullscreen mode.')->uncheck(); + $assert_session->buttonExists('Save configuration')->press(); + $assert_session->statusMessageContains('Collabora Online server URL field is required.', 'error'); + $assert_session->statusMessageContains('WOPI host URL field is required.', 'error'); + $assert_session->statusMessageContains('JWT private key ID field is required.', 'error'); + $assert_session->statusMessageContains('Access Token Expiration (in seconds) field is required.', 'error'); + $assert_session->statusMessageNotContains('The configuration options have been saved.', 'status'); + + // Check validation for fields. + $this->drupalGet(Url::fromRoute('collabora-online.settings')); + // Set invalid value for URL fields. + $assert_session->fieldExists('Collabora Online server URL')->setValue('/internal'); + $assert_session->fieldExists('WOPI host URL')->setValue('any-other-value'); + // Set invalid values for numeric field. + $assert_session->fieldExists('Access Token Expiration (in seconds)')->setValue('text'); + $assert_session->buttonExists('Save configuration')->press(); + $assert_session->statusMessageContains('The URL /internal is not valid.', 'error'); + $assert_session->statusMessageContains('The URL any-other-value is not valid.', 'error'); + $assert_session->statusMessageNotContains('Access Token Expiration (in seconds) must be a number.', 'status'); } } From ac74dd24d83bbc2523a2c39f26f5b0c5dec1dba6 Mon Sep 17 00:00:00 2001 From: AaronGilMartinez Date: Wed, 11 Dec 2024 11:52:09 +0100 Subject: [PATCH 3/8] Add license to settings test. --- tests/src/Functional/SettingsTest.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/src/Functional/SettingsTest.php b/tests/src/Functional/SettingsTest.php index 6958b28c..dd50c6c9 100644 --- a/tests/src/Functional/SettingsTest.php +++ b/tests/src/Functional/SettingsTest.php @@ -1,5 +1,15 @@ Date: Wed, 11 Dec 2024 13:13:12 +0100 Subject: [PATCH 4/8] Improve documentation on settings test. --- tests/src/Functional/SettingsTest.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/src/Functional/SettingsTest.php b/tests/src/Functional/SettingsTest.php index dd50c6c9..4a677fbb 100644 --- a/tests/src/Functional/SettingsTest.php +++ b/tests/src/Functional/SettingsTest.php @@ -51,7 +51,7 @@ public function testSettingsForm(): void { $this->drupalGet(Url::fromRoute('collabora-online.settings')); $assert_session->statusCodeEquals(200); - // Check fields and default values. + // The form contains default values from module install. $this->drupalGet(Url::fromRoute('collabora-online.settings')); $assert_session->fieldValueEquals('Collabora Online server URL', 'https://localhost:9980/'); $assert_session->fieldValueEquals('WOPI host URL', 'https://localhost/'); @@ -60,13 +60,13 @@ public function testSettingsForm(): void { $assert_session->fieldValueEquals('Disable TLS certificate check for COOL.', ''); $assert_session->fieldValueEquals('Allow COOL to use fullscreen mode.', '1'); - // Check that fields values are saved. + // Change the form values, then submit the form. $assert_session->fieldExists('Collabora Online server URL') ->setValue('http://collaboraserver.com/'); $assert_session->fieldExists('WOPI host URL') ->setValue('http://wopihost.com/'); $assert_session->fieldExists('JWT private key ID') - ->setValue('a_random_token'); + ->setValue('name_of_a_key'); $assert_session->fieldExists('Access Token Expiration (in seconds)') ->setValue('3600'); $assert_session->fieldExists('Disable TLS certificate check for COOL.') @@ -77,17 +77,17 @@ public function testSettingsForm(): void { $assert_session->buttonExists('Save configuration')->press(); $assert_session->statusMessageContains('The configuration options have been saved.', 'status'); - // Check the values in fields. + // The settings have been updated. $this->drupalGet(Url::fromRoute('collabora-online.settings')); $assert_session->fieldValueEquals('Collabora Online server URL', 'http://collaboraserver.com/'); // Slash is removed at the end of Wopi URL. $assert_session->fieldValueEquals('WOPI host URL', 'http://wopihost.com'); - $assert_session->fieldValueEquals('JWT private key ID', 'a_random_token'); + $assert_session->fieldValueEquals('JWT private key ID', 'name_of_a_key'); $assert_session->fieldValueEquals('Access Token Expiration (in seconds)', '3600'); $assert_session->fieldValueEquals('Disable TLS certificate check for COOL.', '1'); $assert_session->fieldValueEquals('Allow COOL to use fullscreen mode.', ''); - // Check required fields, set empty values and check errors displayed. + // Test validation of required fields. $this->drupalGet(Url::fromRoute('collabora-online.settings')); $assert_session->fieldExists('Collabora Online server URL')->setValue(''); $assert_session->fieldExists('WOPI host URL')->setValue(''); @@ -102,7 +102,7 @@ public function testSettingsForm(): void { $assert_session->statusMessageContains('Access Token Expiration (in seconds) field is required.', 'error'); $assert_session->statusMessageNotContains('The configuration options have been saved.', 'status'); - // Check validation for fields. + // Test validation of bad form values. $this->drupalGet(Url::fromRoute('collabora-online.settings')); // Set invalid value for URL fields. $assert_session->fieldExists('Collabora Online server URL')->setValue('/internal'); From abf16c13e8efd70a03b6d0ded2bdd04d7946a38b Mon Sep 17 00:00:00 2001 From: AaronGilMartinez Date: Wed, 11 Dec 2024 11:54:45 +0100 Subject: [PATCH 5/8] Remove negative assertion. --- tests/src/Functional/SettingsTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/src/Functional/SettingsTest.php b/tests/src/Functional/SettingsTest.php index 4a677fbb..94ecdbe1 100644 --- a/tests/src/Functional/SettingsTest.php +++ b/tests/src/Functional/SettingsTest.php @@ -100,7 +100,6 @@ public function testSettingsForm(): void { $assert_session->statusMessageContains('WOPI host URL field is required.', 'error'); $assert_session->statusMessageContains('JWT private key ID field is required.', 'error'); $assert_session->statusMessageContains('Access Token Expiration (in seconds) field is required.', 'error'); - $assert_session->statusMessageNotContains('The configuration options have been saved.', 'status'); // Test validation of bad form values. $this->drupalGet(Url::fromRoute('collabora-online.settings')); From e207fa66d055e4028928f6402d02718ee841284c Mon Sep 17 00:00:00 2001 From: AaronGilMartinez Date: Wed, 11 Dec 2024 13:07:58 +0100 Subject: [PATCH 6/8] Add check for empty configuration. --- tests/src/Functional/SettingsTest.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/src/Functional/SettingsTest.php b/tests/src/Functional/SettingsTest.php index 94ecdbe1..64a6a9a1 100644 --- a/tests/src/Functional/SettingsTest.php +++ b/tests/src/Functional/SettingsTest.php @@ -112,6 +112,17 @@ public function testSettingsForm(): void { $assert_session->statusMessageContains('The URL /internal is not valid.', 'error'); $assert_session->statusMessageContains('The URL any-other-value is not valid.', 'error'); $assert_session->statusMessageNotContains('Access Token Expiration (in seconds) must be a number.', 'status'); + + // Test form with no configuration. + \Drupal::configFactory()->getEditable('collabora_online.settings')->setData([])->save(); + $this->drupalGet(Url::fromRoute('collabora-online.settings')); + $assert_session->fieldValueEquals('Collabora Online server URL', ''); + $assert_session->fieldValueEquals('WOPI host URL', ''); + $assert_session->fieldValueEquals('JWT private key ID', ''); + $assert_session->fieldValueEquals('Access Token Expiration (in seconds)', '0'); + $assert_session->fieldValueEquals('Disable TLS certificate check for COOL.', ''); + $assert_session->fieldValueEquals('Allow COOL to use fullscreen mode.', ''); + $assert_session->buttonExists('Save configuration'); } } From 89e36f5b24d17f10c4c4c294730f9a882a3447a8 Mon Sep 17 00:00:00 2001 From: AaronGilMartinez Date: Wed, 11 Dec 2024 14:03:21 +0100 Subject: [PATCH 7/8] Rename test. --- tests/src/Functional/{SettingsTest.php => ConfigFormTest.php} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename tests/src/Functional/{SettingsTest.php => ConfigFormTest.php} (98%) diff --git a/tests/src/Functional/SettingsTest.php b/tests/src/Functional/ConfigFormTest.php similarity index 98% rename from tests/src/Functional/SettingsTest.php rename to tests/src/Functional/ConfigFormTest.php index 64a6a9a1..bbf7329c 100644 --- a/tests/src/Functional/SettingsTest.php +++ b/tests/src/Functional/ConfigFormTest.php @@ -20,7 +20,7 @@ /** * Tests the Collabora configuration. */ -class SettingsTest extends BrowserTestBase { +class ConfigFormTest extends BrowserTestBase { /** * {@inheritdoc} @@ -37,7 +37,7 @@ class SettingsTest extends BrowserTestBase { /** * Tests the configuration for the Collabora settings form. */ - public function testSettingsForm(): void { + public function testConfigForm(): void { $assert_session = $this->assertSession(); // User without permission can't access the configuration page. From dc7cfaad8ee58d47bad9c0779402ed7de0f4f8a5 Mon Sep 17 00:00:00 2001 From: AaronGilMartinez Date: Wed, 11 Dec 2024 15:36:34 +0100 Subject: [PATCH 8/8] Add missing licenses. --- src/Form/ConfigForm.php | 4 ++-- tests/src/Kernel/MediaHelperTest.php | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Form/ConfigForm.php b/src/Form/ConfigForm.php index 9ea820bf..87a32058 100644 --- a/src/Form/ConfigForm.php +++ b/src/Form/ConfigForm.php @@ -1,7 +1,5 @@