From 80f034e0c4dba5b2eb9a451e5ec7fd218354667d Mon Sep 17 00:00:00 2001 From: Kedar Khaire Date: Fri, 8 Sep 2023 00:09:28 +0530 Subject: [PATCH] Issue #576 Callback url issue with install file (#913) * Changes for Callback URL * CHnages to run actions * Fixed PHP CS issue * Install file for configs * Removed url field test cases which were not valid for basic textfield * Updated the changes post review * Changes in install file only for callbackurl field * Removed Filestorage from install file * Removed extra Filestorage from install file * Test cases updated * Removed extra lines * Changes for Warning thrown on preg_match * Fixes for PHPCS * Changes for pattern validator * Changes added pattern message and delimiters * Changes for callback url pattern validation * Changes for multiple delimiters * Changes in field description --- apigee_edge.install | 16 +++++++ apigee_edge.module | 5 +- ...ay.developer_app.developer_app.default.yml | 2 +- .../apigee_edge_teams.install | 15 ++++++ ...view_display.team_app.team_app.default.yml | 2 +- src/Entity/App.php | 48 ------------------- src/Entity/AppViewBuilder.php | 33 ------------- src/Form/AppCallbackUrlSettingsForm.php | 14 +++++- .../Field/FieldType/AppCallbackUrlItem.php | 6 +-- .../FieldWidget/AppCallbackUrlWidget.php | 4 +- tests/src/Functional/DeveloperAppUITest.php | 44 ++++++++++------- .../DeveloperAppUITest.php | 17 +++++-- 12 files changed, 89 insertions(+), 117 deletions(-) diff --git a/apigee_edge.install b/apigee_edge.install index e0837b59..941bb79c 100644 --- a/apigee_edge.install +++ b/apigee_edge.install @@ -28,6 +28,7 @@ use Drupal\Core\Field\BaseFieldDefinition; use Drupal\Core\Installer\InstallerKernel; use Drupal\Core\Url; use Drupal\user\RoleInterface; +use Drupal\Core\Config\FileStorage; /** * Implements hook_requirements(). @@ -373,3 +374,18 @@ function apigee_edge_update_9001() { $definition_update_manager->updateFieldableEntityType($entity_type, $field_storage_definitions); } + +/** + * Install Configs for Core Entity View Display for Default Team App + */ +function apigee_edge_update_9002() { + // Update existing config. + /** @var \Drupal\Core\Config\StorageInterface $config_storage */ + $config_storage = \Drupal::service('config.storage'); + $module_path = \Drupal::service('extension.list.module')->getPath('apigee_edge'); + $source = new FileStorage("$module_path/config/install"); + $new_edge_settings = $source->read('core.entity_view_display.developer_app.developer_app.default'); + $edge_settings = $config_storage->read('core.entity_view_display.developer_app.developer_app.default'); + $edge_settings['content']['callbackUrl'] = $new_edge_settings['content']['callbackUrl']; + $config_storage->write('core.entity_view_display.developer_app.developer_app.default', $edge_settings); +} diff --git a/apigee_edge.module b/apigee_edge.module index 304f9021..ad7685de 100644 --- a/apigee_edge.module +++ b/apigee_edge.module @@ -366,10 +366,7 @@ function apigee_edge_entity_extra_field_info() { * Implements hook_field_formatter_info_alter(). */ function apigee_edge_field_formatter_info_alter(array &$info) { - // Allows using the 'uri_link' formatter for the 'app_callback_url' - // field type, which uses it as default formatter. - // @see \Drupal\apigee_edge\Plugin\Field\FieldType\AppCallbackUrlItem - $info['uri_link']['field_types'][] = 'app_callback_url'; + $info['basic_string']['field_types'][] = 'app_callback_url'; } /** diff --git a/config/install/core.entity_view_display.developer_app.developer_app.default.yml b/config/install/core.entity_view_display.developer_app.developer_app.default.yml index 1fcbb361..27b9925e 100644 --- a/config/install/core.entity_view_display.developer_app.developer_app.default.yml +++ b/config/install/core.entity_view_display.developer_app.developer_app.default.yml @@ -14,7 +14,7 @@ content: region: content settings: { } third_party_settings: { } - type: uri_link + type: basic_string createdAt: type: timestamp_ago label: inline diff --git a/modules/apigee_edge_teams/apigee_edge_teams.install b/modules/apigee_edge_teams/apigee_edge_teams.install index bd629d1f..a3191e3b 100644 --- a/modules/apigee_edge_teams/apigee_edge_teams.install +++ b/modules/apigee_edge_teams/apigee_edge_teams.install @@ -200,3 +200,18 @@ function apigee_edge_teams_update_9001() { $definition_update_manager->updateFieldableEntityType($entity_type, $field_storage_definitions); } + +/** + * Install Configs for Core Entity View Display for Default Team App + */ +function apigee_edge_teams_update_9002() { + // Update existing config. + /** @var \Drupal\Core\Config\StorageInterface $config_storage */ + $config_storage = \Drupal::service('config.storage'); + $module_path = \Drupal::service('extension.list.module')->getPath('apigee_edge_teams'); + $source = new FileStorage("$module_path/config/install"); + $new_team_settings = $source->read('core.entity_view_display.team_app.team_app.default'); + $team_settings = $config_storage->read('core.entity_view_display.team_app.team_app.default'); + $team_settings['content']['callbackUrl'] = $new_team_settings['content']['callbackUrl']; + $config_storage->write('core.entity_view_display.team_app.team_app.default', $team_settings); +} diff --git a/modules/apigee_edge_teams/config/install/core.entity_view_display.team_app.team_app.default.yml b/modules/apigee_edge_teams/config/install/core.entity_view_display.team_app.team_app.default.yml index adb6861f..42892576 100644 --- a/modules/apigee_edge_teams/config/install/core.entity_view_display.team_app.team_app.default.yml +++ b/modules/apigee_edge_teams/config/install/core.entity_view_display.team_app.team_app.default.yml @@ -15,7 +15,7 @@ content: region: content settings: { } third_party_settings: { } - type: uri_link + type: basic_string createdAt: type: timestamp_ago label: inline diff --git a/src/Entity/App.php b/src/Entity/App.php index c11eb578..f274ae9e 100644 --- a/src/Entity/App.php +++ b/src/Entity/App.php @@ -388,54 +388,6 @@ protected static function propertyToBaseFieldBlackList(): array { ]); } - /** - * {@inheritdoc} - */ - public function get($field_name) { - $value = parent::get($field_name); - - // Make sure that returned callback url field values are actually valid - // URLs. Apigee Edge allows to set anything as callbackUrl value but - // Drupal can only accept valid URIs. - if ($field_name === 'callbackUrl') { - if (!$value->isEmpty()) { - foreach ($value->getValue() as $id => $item) { - try { - Url::fromUri($item['value']); - } - catch (\Exception $exception) { - $value->removeItem($id); - } - } - } - } - - return $value; - } - - /** - * {@inheritdoc} - */ - public function set($field_name, $value, $notify = TRUE) { - // If the callback URL value is not a valid URL then save an empty string - // as the field value and set the callbackUrl property to the original - // value. (So we can display the original (invalid URL) on the edit form.) - // This trick is not necessary if the value's type is array because in this - // case the field value is set on the developer app edit form. - if ($field_name === 'callbackUrl' && !is_array($value)) { - try { - Url::fromUri($value); - } - catch (\Exception $exception) { - /** @var \Drupal\apigee_edge\Entity\App $app */ - $app = parent::set($field_name, '', $notify); - $app->setCallbackUrl($value); - return $app; - } - } - return parent::set($field_name, $value, $notify); - } - /** * {@inheritdoc} */ diff --git a/src/Entity/AppViewBuilder.php b/src/Entity/AppViewBuilder.php index 3436d196..c31ddb91 100644 --- a/src/Entity/AppViewBuilder.php +++ b/src/Entity/AppViewBuilder.php @@ -28,37 +28,4 @@ */ class AppViewBuilder extends EdgeEntityViewBuilder { - /** - * {@inheritdoc} - */ - public function buildMultiple(array $build_list) { - $results = parent::buildMultiple($build_list); - foreach (Element::children($results) as $key) { - /** @var \Drupal\apigee_edge\Entity\AppInterface $app */ - $app = $results[$key]["#{$this->entityTypeId}"]; - // If the callback field is visible, display an error message if the - // callback url field value does not contain a valid URI. - if (array_key_exists('callbackUrl', $results[$key]) && !empty($app->getCallbackUrl()) && $app->getCallbackUrl() !== $app->get('callbackUrl')->value) { - try { - Url::fromUri($app->getCallbackUrl()); - } - catch (\Exception $exception) { - $results[$key]['callback_url_error'] = [ - '#theme' => 'status_messages', - '#message_list' => [ - 'warning' => [$this->t('The @field value should be fixed. @message', [ - '@field' => $app->getFieldDefinition('callbackUrl')->getLabel(), - '@message' => $exception->getMessage(), - ]), - ], - ], - // Display it on the top of the view. - '#weight' => -100, - ]; - } - } - } - return $results; - } - } diff --git a/src/Form/AppCallbackUrlSettingsForm.php b/src/Form/AppCallbackUrlSettingsForm.php index b532cfd8..b6326550 100644 --- a/src/Form/AppCallbackUrlSettingsForm.php +++ b/src/Form/AppCallbackUrlSettingsForm.php @@ -93,8 +93,18 @@ public function buildForm(array $form, FormStateInterface $form_state) { public function validateForm(array &$form, FormStateInterface $form_state) { parent::validateForm($form, $form_state); - if (mb_strpos($form_state->getValue(['callback_url', 'pattern'], ''), '^http') === FALSE) { - $form_state->setError($form['callback_url']['pattern'], $this->t('The pattern should start with ^http to limit the acceptable protocols.')); + $isRegExValid = FALSE; + try { + if (@preg_match('/' . $form_state->getValue(['callback_url', 'pattern']) . '/', '') !== FALSE) { + $isRegExValid = TRUE; + } + } + catch (Exception $e) { + $isRegExValid = FALSE; + } + + if (!$isRegExValid) { + $form_state->setError($form['callback_url']['pattern'], $this->t('The pattern should be a valid regular expression.')); } } diff --git a/src/Plugin/Field/FieldType/AppCallbackUrlItem.php b/src/Plugin/Field/FieldType/AppCallbackUrlItem.php index a0293024..c8d742a5 100644 --- a/src/Plugin/Field/FieldType/AppCallbackUrlItem.php +++ b/src/Plugin/Field/FieldType/AppCallbackUrlItem.php @@ -20,7 +20,7 @@ namespace Drupal\apigee_edge\Plugin\Field\FieldType; -use Drupal\Core\Field\Plugin\Field\FieldType\UriItem; +use Drupal\Core\Field\Plugin\Field\FieldType\StringItem; /** * App callback url specific plugin implementation of a URI item. @@ -33,10 +33,10 @@ * label = @Translation("App Callback URL"), * description = @Translation("An entity field containing a callback url for an app."), * no_ui = TRUE, - * default_formatter = "uri_link", + * default_formatter = "basic_string", * default_widget = "app_callback_url", * ) */ -class AppCallbackUrlItem extends UriItem { +class AppCallbackUrlItem extends StringItem { } diff --git a/src/Plugin/Field/FieldWidget/AppCallbackUrlWidget.php b/src/Plugin/Field/FieldWidget/AppCallbackUrlWidget.php index b49fc0df..52d51a0e 100644 --- a/src/Plugin/Field/FieldWidget/AppCallbackUrlWidget.php +++ b/src/Plugin/Field/FieldWidget/AppCallbackUrlWidget.php @@ -21,7 +21,7 @@ namespace Drupal\apigee_edge\Plugin\Field\FieldWidget; use Drupal\Core\Field\FieldItemListInterface; -use Drupal\Core\Field\Plugin\Field\FieldWidget\UriWidget; +use Drupal\Core\Field\Plugin\Field\FieldWidget\StringTextfieldWidget; use Drupal\Core\Form\FormStateInterface; /** @@ -43,7 +43,7 @@ * } * ) */ -class AppCallbackUrlWidget extends UriWidget { +class AppCallbackUrlWidget extends StringTextfieldWidget { /** * {@inheritdoc} diff --git a/tests/src/Functional/DeveloperAppUITest.php b/tests/src/Functional/DeveloperAppUITest.php index 0edab8e6..f637f987 100644 --- a/tests/src/Functional/DeveloperAppUITest.php +++ b/tests/src/Functional/DeveloperAppUITest.php @@ -529,7 +529,7 @@ public function testCallbackUrlValidationServerSide() { // Override default configuration. $description = 'This is a Callback URL field.'; $this->config('apigee_edge.common_app_settings') - ->set('callback_url_pattern', '^https:\/\/example.com') + ->set('callback_url_pattern', '^(https?|sap):\/\/example.com') ->set('callback_url_description', $description) ->save(); @@ -548,15 +548,27 @@ public function testCallbackUrlValidationServerSide() { $this->assertSession()->pageTextContains($description); $this->drupalGet($app_edit_url); $this->submitForm([], 'Save'); - $this->assertSession()->pageTextContains("The URL {$callback_url} is not valid."); + $this->assertSession()->pageTextContains("Callback URL field is not in the right format."); $this->drupalGet($app_edit_url); $this->submitForm([ - 'callbackUrl[0][value]' => 'http://example.com' + 'callbackUrl[0][value]' => 'map://example.com' ], 'Save'); $this->assertSession()->pageTextContains("Callback URL field is not in the right format."); $this->drupalGet($app_edit_url); $this->submitForm([ - 'callbackUrl[0][value]' => 'https://example.com' + 'callbackUrl[0][value]' => 'http://example.com' + ], 'Save'); + $this->assertSession()->pageTextContains('App has been successfully updated.'); + $this->assertSession()->pageTextContains('http://example.com'); + $this->drupalGet($app_edit_url); + $this->submitForm([ + 'callbackUrl[0][value]' => 'sap://example.com' + ], 'Save'); + $this->assertSession()->pageTextContains('App has been successfully updated.'); + $this->assertSession()->pageTextContains('sap://example.com'); + $this->drupalGet($app_edit_url); + $this->submitForm([ + 'callbackUrl[0][value]' => 'https://example.com', ], 'Save'); $this->assertSession()->pageTextContains('App has been successfully updated.'); $this->assertSession()->pageTextContains('https://example.com'); @@ -570,40 +582,36 @@ public function testInvalidEdgeSideCallbackUrl() { $this->products[] = $this->createProduct(); $callback_url = $this->randomGenerator->word(8); $callback_url_warning_msg = "The Callback URL value should be fixed. The URI '{$callback_url}' is invalid. You must use a valid URI scheme."; - $app = $this->createDeveloperApp([ - 'name' => $this->randomMachineName(), - 'displayName' => $this->randomString(), - 'callbackUrl' => $callback_url, - ], + $app = $this->createDeveloperApp( + [ + 'name' => $this->randomMachineName(), + 'displayName' => $this->randomString(), + 'callbackUrl' => $callback_url, + ], $this->account, [ $this->products[0]->id(), - ]); - + ] + ); $app_view_url = $app->toUrl('canonical'); $app_view_by_developer_url = $app->toUrl('canonical-by-developer'); $app_edit_form_url = $app->toUrl('edit-form'); $app_edit_form_for_developer_url = $app->toUrl('edit-form-for-developer'); - $this->drupalGet($app_view_url); - $this->assertSession()->pageTextContains($callback_url_warning_msg); $this->assertSession()->pageTextNotContains('Callback URL:'); $this->drupalGet($app_view_by_developer_url); - $this->assertSession()->pageTextContains($callback_url_warning_msg); $this->assertSession()->pageTextNotContains('Callback URL:'); - $this->drupalGet($app_edit_form_url); $this->assertSession()->fieldValueEquals('callbackUrl[0][value]', $callback_url); $this->drupalGet($app_edit_form_for_developer_url); $this->assertSession()->fieldValueEquals('callbackUrl[0][value]', $callback_url); - $this->drupalGet(Url::fromRoute('entity.entity_view_display.developer_app.default')); $this->submitForm([ - 'fields[callbackUrl][region]' => 'hidden' + 'fields[callbackUrl][region]' => 'hidden', ], 'Save'); $this->drupalGet(Url::fromRoute('entity.entity_form_display.developer_app.default')); $this->submitForm([ - 'fields[callbackUrl][region]' => 'hidden' + 'fields[callbackUrl][region]' => 'hidden', ], 'Save'); $this->drupalGet($app_view_url); diff --git a/tests/src/FunctionalJavascript/DeveloperAppUITest.php b/tests/src/FunctionalJavascript/DeveloperAppUITest.php index 7000cd92..b3b605a2 100644 --- a/tests/src/FunctionalJavascript/DeveloperAppUITest.php +++ b/tests/src/FunctionalJavascript/DeveloperAppUITest.php @@ -82,7 +82,7 @@ public function testCallbackUrlValidationClientSide() { // Override default configuration. $pattern_error_message = 'It must be https://example.com'; $this->config('apigee_edge.common_app_settings') - ->set('callback_url_pattern', '^https:\/\/example.com') + ->set('callback_url_pattern', '^(https?|sap):\/\/example.com') ->set('callback_url_pattern_error_message', $pattern_error_message) ->save(); @@ -98,16 +98,23 @@ public function testCallbackUrlValidationClientSide() { $this->submitForm([], 'Save'); $this->createScreenshot('DeveloperAppUITest-' . __FUNCTION__); $this->assertFalse($isValidInput()); - $checkValidationMessage('Please enter a URL.'); $this->drupalGet($app_edit_url); - $this->submitForm(['callbackUrl[0][value]' => 'http://example.com'], 'Save'); + $this->submitForm(['callbackUrl[0][value]' => 'map://example.com'], 'Save'); $this->createScreenshot('DeveloperAppUITest-' . __FUNCTION__); $this->assertFalse($isValidInput()); - // The format in Firefox is different, it is only one line: - // "Please match the requested format: {$pattern_description}.". $checkValidationMessage('Please match the requested format.'); $this->assertEquals($pattern_error_message, $this->getSession()->evaluateScript('document.getElementById("edit-callbackurl-0-value").title')); $this->drupalGet($app_edit_url); + $this->submitForm(['callbackUrl[0][value]' => 'sap://example.com'], 'Save'); + $this->createScreenshot('DeveloperAppUITest-' . __FUNCTION__); + $this->assertSession()->pageTextContains('App has been successfully updated.'); + $this->assertSession()->pageTextContains('sap://example.com'); + $this->drupalGet($app_edit_url); + $this->submitForm(['callbackUrl[0][value]' => 'http://example.com'], 'Save'); + $this->createScreenshot('DeveloperAppUITest-' . __FUNCTION__); + $this->assertSession()->pageTextContains('App has been successfully updated.'); + $this->assertSession()->pageTextContains('http://example.com'); + $this->drupalGet($app_edit_url); $this->submitForm(['callbackUrl[0][value]' => 'https://example.com'], 'Save'); $this->assertSession()->pageTextContains('App has been successfully updated.'); $this->assertSession()->pageTextContains('https://example.com');