From 52adbcfa1c7a5b3c6c8f66eefb67d6f258b02a4f Mon Sep 17 00:00:00 2001 From: Lee Rowlands <lee.rowlands@previousnext.com.au> Date: Fri, 22 Dec 2023 09:17:04 +1000 Subject: [PATCH] Issue #3379091 by phenaproxima, Wim Leers: Make NodeType config entities fully validatable --- .../book/config/optional/node.type.book.yml | 2 +- .../config/optional/node.type.file_test.yml | 2 +- .../forum/config/optional/node.type.forum.yml | 2 +- .../config/install/node.type.page.yml | 4 +- .../config/install/node.type.basic_page.yml | 4 +- .../install/node.type.sql_import_node.yml | 4 +- .../node/config/schema/node.schema.yml | 15 ++++++ core/modules/node/node.post_update.php | 21 ++++++++ core/modules/node/src/Entity/NodeType.php | 12 ++--- core/modules/node/src/NodeTypeForm.php | 17 +++++++ .../migrate/destination/EntityNodeType.php | 17 +++++++ ...-description-from-article-content-type.php | 24 ++++++++++ .../config/install/node.type.default.yml | 2 +- .../sync/node.type.import.yml | 2 +- .../Update/NullHelpAndDescriptionTest.php | 45 +++++++++++++++++ .../src/Kernel/NodeTypeValidationTest.php | 48 +++++++++++++++++++ .../config/install/node.type.article.yml | 2 +- .../config/install/node.type.article.yml | 2 +- .../config/install/node.type.page.yml | 2 +- .../config/install/node.type.recipe.yml | 2 +- .../config/install/node.type.article.yml | 2 +- .../config/install/node.type.page.yml | 2 +- .../config/install/node.type.article.yml | 2 +- .../config/install/node.type.page.yml | 2 +- 24 files changed, 212 insertions(+), 25 deletions(-) create mode 100644 core/modules/node/tests/fixtures/update/remove-description-from-article-content-type.php create mode 100644 core/modules/node/tests/src/Functional/Update/NullHelpAndDescriptionTest.php diff --git a/core/modules/book/config/optional/node.type.book.yml b/core/modules/book/config/optional/node.type.book.yml index 0c07a79e8f9d..9722037d8282 100644 --- a/core/modules/book/config/optional/node.type.book.yml +++ b/core/modules/book/config/optional/node.type.book.yml @@ -7,7 +7,7 @@ dependencies: name: 'Book page' type: book description: '<em>Books</em> have a built-in hierarchical navigation. Use for handbooks or tutorials.' -help: '' +help: null new_revision: true preview_mode: 1 display_submitted: true diff --git a/core/modules/file/tests/modules/file_test_views/config/optional/node.type.file_test.yml b/core/modules/file/tests/modules/file_test_views/config/optional/node.type.file_test.yml index d5efeed2110c..1e062ec83c90 100644 --- a/core/modules/file/tests/modules/file_test_views/config/optional/node.type.file_test.yml +++ b/core/modules/file/tests/modules/file_test_views/config/optional/node.type.file_test.yml @@ -1,7 +1,7 @@ type: file_test name: File test description: 'Bundle for testing file relationships.' -help: '' +help: null new_revision: true display_submitted: true preview_mode: 1 diff --git a/core/modules/forum/config/optional/node.type.forum.yml b/core/modules/forum/config/optional/node.type.forum.yml index 8ed965df3fb6..2bb8ed367c41 100644 --- a/core/modules/forum/config/optional/node.type.forum.yml +++ b/core/modules/forum/config/optional/node.type.forum.yml @@ -7,7 +7,7 @@ dependencies: name: 'Forum topic' type: forum description: 'A <em>forum topic</em> starts a new discussion thread within a forum.' -help: '' +help: null new_revision: false preview_mode: 1 display_submitted: true diff --git a/core/modules/language/tests/language_entity_field_access_test/config/install/node.type.page.yml b/core/modules/language/tests/language_entity_field_access_test/config/install/node.type.page.yml index 6ebd149e49be..5b22929e2f4d 100644 --- a/core/modules/language/tests/language_entity_field_access_test/config/install/node.type.page.yml +++ b/core/modules/language/tests/language_entity_field_access_test/config/install/node.type.page.yml @@ -3,8 +3,8 @@ status: true dependencies: { } name: page type: page -description: '' -help: '' +description: null +help: null new_revision: false preview_mode: 1 display_submitted: false diff --git a/core/modules/media_library/tests/modules/media_library_test/config/install/node.type.basic_page.yml b/core/modules/media_library/tests/modules/media_library_test/config/install/node.type.basic_page.yml index 23abb54599c6..d89941abbd0a 100644 --- a/core/modules/media_library/tests/modules/media_library_test/config/install/node.type.basic_page.yml +++ b/core/modules/media_library/tests/modules/media_library_test/config/install/node.type.basic_page.yml @@ -2,8 +2,8 @@ langcode: en status: true name: 'Basic Page' type: basic_page -description: '' -help: '' +description: null +help: null new_revision: true preview_mode: 1 display_submitted: true diff --git a/core/modules/migrate/tests/modules/migrate_high_water_test/config/install/node.type.sql_import_node.yml b/core/modules/migrate/tests/modules/migrate_high_water_test/config/install/node.type.sql_import_node.yml index 55c0888e6cf4..b024f89598a1 100644 --- a/core/modules/migrate/tests/modules/migrate_high_water_test/config/install/node.type.sql_import_node.yml +++ b/core/modules/migrate/tests/modules/migrate_high_water_test/config/install/node.type.sql_import_node.yml @@ -2,8 +2,8 @@ langcode: en status: true name: High Water import node type: high_water_import_node -description: '' -help: '' +description: null +help: null new_revision: false preview_mode: 1 display_submitted: true diff --git a/core/modules/node/config/schema/node.schema.yml b/core/modules/node/config/schema/node.schema.yml index 7ce661e745aa..87621d6e252b 100644 --- a/core/modules/node/config/schema/node.schema.yml +++ b/core/modules/node/config/schema/node.schema.yml @@ -11,6 +11,8 @@ node.settings: node.type.*: type: config_entity label: 'Content type' + constraints: + FullyValidatable: ~ mapping: name: type: required_label @@ -26,15 +28,28 @@ node.type.*: description: type: text label: 'Description' + nullable: true + constraints: + NotBlank: + allowNull: true help: type: text label: 'Explanation or submission guidelines' + nullable: true + constraints: + NotBlank: + allowNull: true new_revision: type: boolean label: 'Whether a new revision should be created by default' preview_mode: type: integer label: 'Preview before submitting' + constraints: + # These are the values of the DRUPAL_DISABLED, DRUPAL_OPTIONAL, and + # DRUPAL_REQUIRED constants. + # @see \Drupal\node\NodeTypeForm::form() + Choice: [0, 1, 2] display_submitted: type: boolean label: 'Display setting for author and date Submitted by post information' diff --git a/core/modules/node/node.post_update.php b/core/modules/node/node.post_update.php index 0e76fd8ed51c..c02da56a16a0 100644 --- a/core/modules/node/node.post_update.php +++ b/core/modules/node/node.post_update.php @@ -5,6 +5,27 @@ * Post update functions for Node. */ +use Drupal\Core\Config\Entity\ConfigEntityUpdater; +use Drupal\node\NodeTypeInterface; + +/** + * Converts empty `description` and `help` in content types to NULL. + */ +function node_post_update_set_node_type_description_and_help_to_null(array &$sandbox): void { + \Drupal::classResolver(ConfigEntityUpdater::class) + ->update($sandbox, 'node_type', function (NodeTypeInterface $node_type): bool { + // Content types' `help` and `description` fields must be stored as NULL + // at the config level if they are empty. + if (trim($node_type->getDescription()) === '') { + $node_type->set('description', NULL); + } + if (trim($node_type->getHelp()) === '') { + $node_type->set('help', NULL); + } + return TRUE; + }); +} + /** * Implements hook_removed_post_updates(). */ diff --git a/core/modules/node/src/Entity/NodeType.php b/core/modules/node/src/Entity/NodeType.php index 8662408aa8f3..ff2cb28e3595 100644 --- a/core/modules/node/src/Entity/NodeType.php +++ b/core/modules/node/src/Entity/NodeType.php @@ -78,16 +78,16 @@ class NodeType extends ConfigEntityBundleBase implements NodeTypeInterface { /** * A brief description of this node type. * - * @var string + * @var string|null */ - protected $description; + protected $description = NULL; /** * Help information shown to the user when creating a Node of this type. * - * @var string + * @var string|null */ - protected $help; + protected $help = NULL; /** * Default value of the 'Create new revision' checkbox of this node type. @@ -164,14 +164,14 @@ public function setPreviewMode($preview_mode) { * {@inheritdoc} */ public function getHelp() { - return $this->help; + return $this->help ?? ''; } /** * {@inheritdoc} */ public function getDescription() { - return $this->description; + return $this->description ?? ''; } /** diff --git a/core/modules/node/src/NodeTypeForm.php b/core/modules/node/src/NodeTypeForm.php index 00f829af35ba..445f567c9009 100644 --- a/core/modules/node/src/NodeTypeForm.php +++ b/core/modules/node/src/NodeTypeForm.php @@ -201,6 +201,23 @@ public function validateForm(array &$form, FormStateInterface $form_state) { } } + /** + * {@inheritdoc} + */ + public function buildEntity(array $form, FormStateInterface $form_state) { + /** @var \Drupal\node\NodeTypeInterface $entity */ + $entity = parent::buildEntity($form, $form_state); + + // The description and help text cannot be empty strings. + if (trim($form_state->getValue('description')) === '') { + $entity->set('description', NULL); + } + if (trim($form_state->getValue('help')) === '') { + $entity->set('help', NULL); + } + return $entity; + } + /** * {@inheritdoc} */ diff --git a/core/modules/node/src/Plugin/migrate/destination/EntityNodeType.php b/core/modules/node/src/Plugin/migrate/destination/EntityNodeType.php index 152b9e46fc7b..cdee1f5fcfa8 100644 --- a/core/modules/node/src/Plugin/migrate/destination/EntityNodeType.php +++ b/core/modules/node/src/Plugin/migrate/destination/EntityNodeType.php @@ -12,6 +12,23 @@ */ class EntityNodeType extends EntityConfigBase { + /** + * {@inheritdoc} + */ + public function getEntity(Row $row, array $old_destination_id_values) { + /** @var \Drupal\node\NodeTypeInterface $node_type */ + $node_type = parent::getEntity($row, $old_destination_id_values); + + // Config schema does not allow description or help text to be empty. + if ($node_type->getDescription() === '') { + $node_type->set('description', NULL); + } + if ($node_type->getHelp() === '') { + $node_type->set('help', NULL); + } + return $node_type; + } + /** * {@inheritdoc} */ diff --git a/core/modules/node/tests/fixtures/update/remove-description-from-article-content-type.php b/core/modules/node/tests/fixtures/update/remove-description-from-article-content-type.php new file mode 100644 index 000000000000..acbc1c95b21d --- /dev/null +++ b/core/modules/node/tests/fixtures/update/remove-description-from-article-content-type.php @@ -0,0 +1,24 @@ +<?php + +/** + * @file + * Empties the description of the `article` content type. + */ + +use Drupal\Core\Database\Database; + +$connection = Database::getConnection(); + +$data = $connection->select('config') + ->condition('name', 'node.type.article') + ->fields('config', ['data']) + ->execute() + ->fetchField(); +$data = unserialize($data); +$data['description'] = "\n"; +$connection->update('config') + ->condition('name', 'node.type.article') + ->fields([ + 'data' => serialize($data), + ]) + ->execute(); diff --git a/core/modules/node/tests/modules/node_test_config/config/install/node.type.default.yml b/core/modules/node/tests/modules/node_test_config/config/install/node.type.default.yml index cfb29c6d8cbf..c5c17fff30e0 100644 --- a/core/modules/node/tests/modules/node_test_config/config/install/node.type.default.yml +++ b/core/modules/node/tests/modules/node_test_config/config/install/node.type.default.yml @@ -3,7 +3,7 @@ status: true name: Default type: default description: 'Default description.' -help: '' +help: null new_revision: true preview_mode: 1 display_submitted: true diff --git a/core/modules/node/tests/modules/node_test_config/sync/node.type.import.yml b/core/modules/node/tests/modules/node_test_config/sync/node.type.import.yml index f1bafe46d224..8b1b7cb5fc48 100644 --- a/core/modules/node/tests/modules/node_test_config/sync/node.type.import.yml +++ b/core/modules/node/tests/modules/node_test_config/sync/node.type.import.yml @@ -1,7 +1,7 @@ type: import name: Import description: 'Import description.' -help: '' +help: null new_revision: false display_submitted: true preview_mode: 1 diff --git a/core/modules/node/tests/src/Functional/Update/NullHelpAndDescriptionTest.php b/core/modules/node/tests/src/Functional/Update/NullHelpAndDescriptionTest.php new file mode 100644 index 000000000000..92811bc136cb --- /dev/null +++ b/core/modules/node/tests/src/Functional/Update/NullHelpAndDescriptionTest.php @@ -0,0 +1,45 @@ +<?php + +namespace Drupal\Tests\node\Functional\Update; + +use Drupal\FunctionalTests\Update\UpdatePathTestBase; +use Drupal\node\Entity\NodeType; + +/** + * Tests the upgrade path for making content types' help and description NULL. + * + * @group node + */ +class NullHelpAndDescriptionTest extends UpdatePathTestBase { + + /** + * {@inheritdoc} + */ + protected function setDatabaseDumpFiles() { + $this->databaseDumpFiles = [ + __DIR__ . '/../../../../../system/tests/fixtures/update/drupal-9.4.0.filled.standard.php.gz', + __DIR__ . '/../../../fixtures/update/remove-description-from-article-content-type.php', + ]; + } + + /** + * Tests the upgrade path for updating empty help and description to NULL. + */ + public function testRunUpdates() { + $node_type = NodeType::load('article'); + $this->assertInstanceOf(NodeType::class, $node_type); + + $this->assertSame('', $node_type->get('help')); + $this->assertSame("\n", $node_type->get('description')); + $this->runUpdates(); + + $node_type = NodeType::load('article'); + $this->assertInstanceOf(NodeType::class, $node_type); + + $this->assertNull($node_type->get('help')); + $this->assertNull($node_type->get('description')); + $this->assertSame('', $node_type->getHelp()); + $this->assertSame('', $node_type->getDescription()); + } + +} diff --git a/core/modules/node/tests/src/Kernel/NodeTypeValidationTest.php b/core/modules/node/tests/src/Kernel/NodeTypeValidationTest.php index c5bcc47355e6..e3c40bff49be 100644 --- a/core/modules/node/tests/src/Kernel/NodeTypeValidationTest.php +++ b/core/modules/node/tests/src/Kernel/NodeTypeValidationTest.php @@ -19,6 +19,14 @@ class NodeTypeValidationTest extends ConfigEntityValidationTestBase { */ protected static $modules = ['field', 'node', 'text', 'user']; + /** + * {@inheritdoc} + */ + protected static array $propertiesWithOptionalValues = [ + 'description', + 'help', + ]; + /** * {@inheritdoc} */ @@ -28,4 +36,44 @@ protected function setUp(): void { $this->entity = $this->createContentType(); } + /** + * Tests that a node type's preview mode is constrained to certain values. + */ + public function testPreviewModeValidation(): void { + $this->entity->setPreviewMode(38); + $this->assertValidationErrors(['preview_mode' => 'The value you selected is not a valid choice.']); + + $this->entity->setPreviewMode(-1); + $this->assertValidationErrors(['preview_mode' => 'The value you selected is not a valid choice.']); + + $allowed_values = [ + DRUPAL_DISABLED, + DRUPAL_OPTIONAL, + DRUPAL_REQUIRED, + ]; + foreach ($allowed_values as $allowed_value) { + $this->entity->setPreviewMode($allowed_value); + $this->assertValidationErrors([]); + } + } + + /** + * Tests that description and help text can be NULL, but not empty strings. + */ + public function testDescriptionAndHelpCannotBeEmpty(): void { + $this->entity->set('description', NULL)->set('help', NULL); + // The entity's getters should cast NULL values to empty strings. + $this->assertSame('', $this->entity->getDescription()); + $this->assertSame('', $this->entity->getHelp()); + // But NULL values should be valid at the config level. + $this->assertValidationErrors([]); + + // But they cannot be empty strings, because that doesn't make sense. + $this->entity->set('description', '')->set('help', ''); + $this->assertValidationErrors([ + 'description' => 'This value should not be blank.', + 'help' => 'This value should not be blank.', + ]); + } + } diff --git a/core/modules/system/tests/modules/olivero_test/config/install/node.type.article.yml b/core/modules/system/tests/modules/olivero_test/config/install/node.type.article.yml index 1fd439ce71bf..ae8e9d12580b 100644 --- a/core/modules/system/tests/modules/olivero_test/config/install/node.type.article.yml +++ b/core/modules/system/tests/modules/olivero_test/config/install/node.type.article.yml @@ -4,7 +4,7 @@ dependencies: { } name: Article type: article description: 'Use <em>articles</em> for time-sensitive content like news, press releases or blog posts.' -help: '' +help: null new_revision: true preview_mode: 1 display_submitted: true diff --git a/core/profiles/demo_umami/config/install/node.type.article.yml b/core/profiles/demo_umami/config/install/node.type.article.yml index 1fd439ce71bf..ae8e9d12580b 100644 --- a/core/profiles/demo_umami/config/install/node.type.article.yml +++ b/core/profiles/demo_umami/config/install/node.type.article.yml @@ -4,7 +4,7 @@ dependencies: { } name: Article type: article description: 'Use <em>articles</em> for time-sensitive content like news, press releases or blog posts.' -help: '' +help: null new_revision: true preview_mode: 1 display_submitted: true diff --git a/core/profiles/demo_umami/config/install/node.type.page.yml b/core/profiles/demo_umami/config/install/node.type.page.yml index 57dcc0c9926f..176315b58244 100644 --- a/core/profiles/demo_umami/config/install/node.type.page.yml +++ b/core/profiles/demo_umami/config/install/node.type.page.yml @@ -4,7 +4,7 @@ dependencies: { } name: 'Basic page' type: page description: 'Use <em>basic pages</em> for your static content, such as an ''About us'' page.' -help: '' +help: null new_revision: true preview_mode: 1 display_submitted: false diff --git a/core/profiles/demo_umami/config/install/node.type.recipe.yml b/core/profiles/demo_umami/config/install/node.type.recipe.yml index 89ed3215c74e..1810e77bcf29 100644 --- a/core/profiles/demo_umami/config/install/node.type.recipe.yml +++ b/core/profiles/demo_umami/config/install/node.type.recipe.yml @@ -10,7 +10,7 @@ third_party_settings: name: Recipe type: recipe description: 'Add a new recipe to the site.' -help: '' +help: null new_revision: true preview_mode: 1 display_submitted: false diff --git a/core/profiles/nightwatch_a11y_testing/config/install/node.type.article.yml b/core/profiles/nightwatch_a11y_testing/config/install/node.type.article.yml index 1fd439ce71bf..ae8e9d12580b 100644 --- a/core/profiles/nightwatch_a11y_testing/config/install/node.type.article.yml +++ b/core/profiles/nightwatch_a11y_testing/config/install/node.type.article.yml @@ -4,7 +4,7 @@ dependencies: { } name: Article type: article description: 'Use <em>articles</em> for time-sensitive content like news, press releases or blog posts.' -help: '' +help: null new_revision: true preview_mode: 1 display_submitted: true diff --git a/core/profiles/nightwatch_a11y_testing/config/install/node.type.page.yml b/core/profiles/nightwatch_a11y_testing/config/install/node.type.page.yml index 57dcc0c9926f..176315b58244 100644 --- a/core/profiles/nightwatch_a11y_testing/config/install/node.type.page.yml +++ b/core/profiles/nightwatch_a11y_testing/config/install/node.type.page.yml @@ -4,7 +4,7 @@ dependencies: { } name: 'Basic page' type: page description: 'Use <em>basic pages</em> for your static content, such as an ''About us'' page.' -help: '' +help: null new_revision: true preview_mode: 1 display_submitted: false diff --git a/core/profiles/standard/config/install/node.type.article.yml b/core/profiles/standard/config/install/node.type.article.yml index 1fd439ce71bf..ae8e9d12580b 100644 --- a/core/profiles/standard/config/install/node.type.article.yml +++ b/core/profiles/standard/config/install/node.type.article.yml @@ -4,7 +4,7 @@ dependencies: { } name: Article type: article description: 'Use <em>articles</em> for time-sensitive content like news, press releases or blog posts.' -help: '' +help: null new_revision: true preview_mode: 1 display_submitted: true diff --git a/core/profiles/standard/config/install/node.type.page.yml b/core/profiles/standard/config/install/node.type.page.yml index 57dcc0c9926f..176315b58244 100644 --- a/core/profiles/standard/config/install/node.type.page.yml +++ b/core/profiles/standard/config/install/node.type.page.yml @@ -4,7 +4,7 @@ dependencies: { } name: 'Basic page' type: page description: 'Use <em>basic pages</em> for your static content, such as an ''About us'' page.' -help: '' +help: null new_revision: true preview_mode: 1 display_submitted: false -- GitLab