From b8c27fb827ece62339dadd9f8ca371867ec0977d Mon Sep 17 00:00:00 2001 From: catch <6915-catch@users.noreply.drupalcode.org> Date: Fri, 7 Mar 2025 08:57:56 +0000 Subject: [PATCH] Issue #3499181 by berdir, amateescu, nicxvan, catch, dww: Disallow saving the current default revision as a non-default revision --- .../Drupal/Core/Entity/ContentEntityBase.php | 10 ++--- .../Core/Entity/ContentEntityStorageBase.php | 8 ++++ core/modules/menu_ui/menu_ui.module | 39 +++++++++++++++---- .../MenuUiContentModerationTest.php | 6 +++ .../migrate/destination/EntityRevision.php | 7 +++- .../Unit/destination/EntityRevisionTest.php | 2 +- .../RevisionableContentEntityBaseTest.php | 3 ++ .../Core/Entity/ContentEntityBaseUnitTest.php | 1 + 8 files changed, 61 insertions(+), 15 deletions(-) diff --git a/core/lib/Drupal/Core/Entity/ContentEntityBase.php b/core/lib/Drupal/Core/Entity/ContentEntityBase.php index 28a261697c46..b97bb5d5d1e2 100644 --- a/core/lib/Drupal/Core/Entity/ContentEntityBase.php +++ b/core/lib/Drupal/Core/Entity/ContentEntityBase.php @@ -345,13 +345,13 @@ public function isNewRevision() { * {@inheritdoc} */ public function isDefaultRevision($new_value = NULL) { - $return = $this->isDefaultRevision; - if (isset($new_value)) { - $this->isDefaultRevision = (bool) $new_value; - } + $current_value = $this->isDefaultRevision; // New entities should always ensure at least one default revision exists, // creating an entity without a default revision is an invalid state. - return $this->isNew() || $return; + if (isset($new_value) && (!$this->isNew() || $new_value === TRUE)) { + $this->isDefaultRevision = (bool) $new_value; + } + return (bool) $current_value; } /** diff --git a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php index d082919f11ba..e7b42bc2af74 100644 --- a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php +++ b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php @@ -759,6 +759,14 @@ protected function doPreSave(EntityInterface $entity) { $id = parent::doPreSave($entity); + $previously_default_revision = $entity->wasDefaultRevision(); + $no_longer_default = !$entity->isDefaultRevision(); + $original_same_as_current = $entity->getOriginal()?->getRevisionId() == $entity->getLoadedRevisionId(); + $not_new_revision = !$entity->isNewRevision(); + if ($previously_default_revision && $no_longer_default && $original_same_as_current && $not_new_revision) { + throw new EntityStorageException("An existing default revision of the '{$this->entityTypeId}' entity type can not be changed to a non-default revision."); + } + if (!$entity->isNew()) { // If the ID changed then original can't be loaded, throw an exception // in that case. diff --git a/core/modules/menu_ui/menu_ui.module b/core/modules/menu_ui/menu_ui.module index 2976cad4dc86..f4406bfb1f14 100644 --- a/core/modules/menu_ui/menu_ui.module +++ b/core/modules/menu_ui/menu_ui.module @@ -20,7 +20,7 @@ function _menu_ui_node_save(NodeInterface $node, array $values) { /** @var \Drupal\menu_link_content\MenuLinkContentInterface $entity */ if (!empty($values['entity_id'])) { - $entity = MenuLinkContent::load($values['entity_id']); + $entity = \Drupal::service('entity.repository')->getActive('menu_link_content', $values['entity_id']); if ($entity->isTranslatable() && $node->isTranslatable()) { if (!$entity->hasTranslation($node->language()->getId())) { $entity = $entity->addTranslation($node->language()->getId(), $entity->toArray()); @@ -48,7 +48,26 @@ function _menu_ui_node_save(NodeInterface $node, array $values) { $entity->menu_name->value = $values['menu_name']; $entity->parent->value = $values['parent']; $entity->weight->value = $values['weight'] ?? 0; - $entity->isDefaultRevision($node->isDefaultRevision()); + if ($entity->isNew()) { + // @todo The menu link doesn't need to be changed in a workspace context. + // Fix this in https://www.drupal.org/project/drupal/issues/3511204. + if (!$node->isDefaultRevision() && $node->hasLinkTemplate('latest-version')) { + // If a new menu link is created while saving the node as a pending draft + // (non-default revision), store it as a link to the latest version. + // That ensures that there is a regular, valid link target that is + // only visible to users with permission to view the latest version. + $entity->get('link')->uri = 'internal:/node/' . $node->id() . '/latest'; + } + } + else { + $entity->isDefaultRevision($node->isDefaultRevision()); + if (!$entity->isDefaultRevision()) { + $entity->setNewRevision(TRUE); + } + elseif ($entity->get('link')->uri !== 'entity:node/' . $node->id()) { + $entity->get('link')->uri = 'entity:node/' . $node->id(); + } + } $entity->save(); } @@ -69,12 +88,17 @@ function menu_ui_get_menu_link_defaults(NodeInterface $node) { $defaults = FALSE; if ($node->id()) { $id = FALSE; - // Give priority to the default menu + // Give priority to the default menu. $type_menus = $node_type->getThirdPartySetting('menu_ui', 'available_menus', ['main']); + // An existing menu link either points to the canonical or the latest path, + // in case of a new menu link that was creating while saving as a pending + // draft. + $uri_candidates = ['entity:node/' . $node->id(), 'internal:/node/' . $node->id() . '/latest']; + if (in_array($menu_name, $type_menus)) { $query = \Drupal::entityQuery('menu_link_content') ->accessCheck(TRUE) - ->condition('link.uri', 'entity:node/' . $node->id()) + ->condition('link.uri', $uri_candidates, 'IN') ->condition('menu_name', $menu_name) ->sort('id', 'ASC') ->range(0, 1); @@ -86,7 +110,7 @@ function menu_ui_get_menu_link_defaults(NodeInterface $node) { if (!$id && !empty($type_menus)) { $query = \Drupal::entityQuery('menu_link_content') ->accessCheck(TRUE) - ->condition('link.uri', 'entity:node/' . $node->id()) + ->condition('link.uri', $uri_candidates, 'IN') ->condition('menu_name', array_values($type_menus), 'IN') ->sort('id', 'ASC') ->range(0, 1); @@ -95,8 +119,7 @@ function menu_ui_get_menu_link_defaults(NodeInterface $node) { $id = (!empty($result)) ? reset($result) : FALSE; } if ($id) { - $menu_link = MenuLinkContent::load($id); - $menu_link = \Drupal::service('entity.repository')->getTranslationFromContext($menu_link); + $menu_link = \Drupal::service('entity.repository')->getActive('menu_link_content', $id); $defaults = [ 'entity_id' => $menu_link->id(), 'id' => $menu_link->getPluginId(), @@ -150,7 +173,7 @@ function menu_ui_form_node_form_submit($form, FormStateInterface $form_state) { $values = $form_state->getValue('menu'); if (empty($values['enabled'])) { if ($values['entity_id']) { - $entity = MenuLinkContent::load($values['entity_id']); + $entity = \Drupal::service('entity.repository')->getActive('menu_link_content', $values['entity_id']); $entity->delete(); } } diff --git a/core/modules/menu_ui/tests/src/Functional/MenuUiContentModerationTest.php b/core/modules/menu_ui/tests/src/Functional/MenuUiContentModerationTest.php index 4960b738a5a4..95dc030ba804 100644 --- a/core/modules/menu_ui/tests/src/Functional/MenuUiContentModerationTest.php +++ b/core/modules/menu_ui/tests/src/Functional/MenuUiContentModerationTest.php @@ -177,8 +177,14 @@ public function testMenuUiWithPendingRevisions(): void { $this->drupalGet('node/' . $node->id() . '/edit'); $this->submitForm($edit, 'Save'); $this->assertSession()->pageTextContains("Page {$node->label()} has been updated."); + + // The link is created to the latest page, which the editor is allowed + // see, but an anonymous visitor not. + $this->assertSession()->linkExists('Second test menu link'); + $this->drupalLogout(); $this->assertSession()->linkNotExists('Second test menu link'); + $this->drupalLogin($editor); // Publish the content and ensure the new menu link shows up. $edit = [ 'moderation_state[0][state]' => 'published', diff --git a/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php b/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php index ec18083c9ef7..07595c77bc78 100644 --- a/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php +++ b/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php @@ -148,6 +148,11 @@ protected function getEntity(Row $row, array $old_destination_id_values) { } } if ($entity === NULL) { + // If the entity could not be loaded by revision then the given + // revision does not yet exist. Load the current default revision and + // prepare to save it as a new non-default revision. setNewRevision() + // will unset the current revision ID and the entity is then updated + // with the source revision ID and saved as that. $entity_id = $row->getDestinationProperty($this->getKey('id')); $entity = $this->storage->load($entity_id); @@ -159,11 +164,11 @@ protected function getEntity(Row $row, array $old_destination_id_values) { $entity->enforceIsNew(FALSE); $entity->setNewRevision(TRUE); + $entity->isDefaultRevision(FALSE); } // We need to update the entity, so that the destination row IDs are // correct. $entity = $this->updateEntity($entity, $row); - $entity->isDefaultRevision(FALSE); return $entity; } diff --git a/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php b/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php index 5c2258df3d8a..8417075a075c 100644 --- a/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php +++ b/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php @@ -114,7 +114,7 @@ public function testGetEntityUpdateRevision(): void { ->willReturn($entity->reveal()); // Make sure its set as an update and not the default revision. $entity->setNewRevision(FALSE)->shouldBeCalled(); - $entity->isDefaultRevision(FALSE)->shouldBeCalled(); + $entity->isDefaultRevision()->shouldNotBeCalled(); $row = new Row(['nid' => 1, 'vid' => 2], ['nid' => 1, 'vid' => 2]); $row->setDestinationProperty('vid', 2); diff --git a/core/tests/Drupal/KernelTests/Core/Entity/RevisionableContentEntityBaseTest.php b/core/tests/Drupal/KernelTests/Core/Entity/RevisionableContentEntityBaseTest.php index d560fd3eee64..d6116be9480a 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/RevisionableContentEntityBaseTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/RevisionableContentEntityBaseTest.php @@ -6,6 +6,7 @@ use Drupal\Core\Database\Database; use Drupal\Core\Entity\EntityInterface; +use Drupal\Core\Entity\EntityStorageException; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\entity_test_revlog\Entity\EntityTestMulWithRevisionLog; use Drupal\user\Entity\User; @@ -152,6 +153,8 @@ public function testWasDefaultRevision(): void { $this->assertTrue($entity->wasDefaultRevision()); // Check that the "revision_default" flag cannot be changed once set. + $this->expectException(EntityStorageException::class); + $this->expectExceptionMessage("An existing default revision of the 'entity_test_mul_revlog' entity type can not be changed to a non-default revision."); /** @var \Drupal\entity_test_revlog\Entity\EntityTestMulWithRevisionLog $entity2 */ $entity2 = EntityTestMulWithRevisionLog::create([ 'type' => $entity_type_id, diff --git a/core/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php b/core/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php index ca505d6bd029..2e8125f331d8 100644 --- a/core/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php @@ -281,6 +281,7 @@ public function testIsDefaultRevision(): void { // The last call changed the return value for this call. $this->assertFalse($this->entity->isDefaultRevision()); // The revision for a new entity should always be the default revision. + $this->entity->isDefaultRevision(TRUE); $this->entity->expects($this->any()) ->method('isNew') ->willReturn(TRUE); -- GitLab