Merge pull request #138 from City-of-Helsinki/UHF-9171

UHF-9171: Make sure we never attempt to delete default revision
City-of-Helsinki · Oct 18, 2023 · c8ed69e · c8ed69e
2 parents 8f39f1d + f085e55
commit c8ed69e
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 8 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -71,7 +71,7 @@ jobs:
       - name: Start services
         working-directory: ${{ env.DRUPAL_ROOT }}
         run: |
-          vendor/bin/drush runserver $SIMPLETEST_BASE_URL > /dev/null 2>&1 &
+          vendor/bin/drush runserver $SIMPLETEST_BASE_URL --dns > /dev/null 2>&1 &
           chromedriver --port=4444 > /dev/null 2>&1 &
 
       - name: Run PHPUnit tests

diff --git a/src/Entity/Revision/RevisionManager.php b/src/Entity/Revision/RevisionManager.php
@@ -9,6 +9,7 @@
 use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Database\Connection;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
+use Drupal\Core\Entity\TranslatableRevisionableInterface;
 
 /**
  * A class to manage revisions.
@@ -59,7 +60,7 @@ private function assertEntityType(string $entityType) : void {
     try {
       $definition = $this->entityTypeManager->getDefinition($entityType);
 
-      if (!$definition->isRevisionable()) {
+      if (!$definition->isRevisionable() || !$definition->entityClassImplements(TranslatableRevisionableInterface::class)) {
         throw new \InvalidArgumentException('Entity type does not support revisions.');
       }
     }
@@ -149,13 +150,14 @@ public function getRevisionsPerLanguage(
     krsort($revision_ids);
 
     foreach ($revision_ids as $vid) {
-      /** @var \Drupal\Core\Entity\RevisionableInterface $revision */
+      /** @var \Drupal\Core\Entity\TranslatableRevisionableInterface $revision */
       $revision = $storage->loadRevision($vid);
 
       foreach ($revision->getTranslationLanguages() as $langcode => $language) {
-        if ($revision->hasTranslation($langcode) && $revision->getTranslation($langcode)->isRevisionTranslationAffected()) {
-          $revisions[$langcode][] = $revision->getLoadedRevisionId();
+        if (!$this->isValidRevision($langcode, $revision)) {
+          continue;
         }
+        $revisions[$langcode][] = $revision->getLoadedRevisionId();
       }
     }
 
@@ -166,6 +168,31 @@ public function getRevisionsPerLanguage(
     return $revisions;
   }
 
+  /**
+   * Check if given revision is valid for deletion.
+   *
+   * @param string $langcode
+   *   The langcode.
+   * @param \Drupal\Core\Entity\TranslatableRevisionableInterface $revision
+   *   The revision to test.
+   *
+   * @return bool
+   *   TRUE if given revision is valid for deletion.
+   */
+  private function isValidRevision(string $langcode, TranslatableRevisionableInterface $revision) : bool {
+    // Skip default revisions and revision without translation for given
+    // language.
+    if (!$revision->hasTranslation($langcode) || $revision->isDefaultRevision()) {
+      return FALSE;
+    }
+    $revision = $revision->getTranslation($langcode);
+
+    if (!$revision->isRevisionTranslationAffected() || $revision->isLatestTranslationAffectedRevision()) {
+      return FALSE;
+    }
+    return TRUE;
+  }
+
   /**
    * Gets revisions for the given entity type and id.
    *

diff --git a/tests/src/Kernel/Entity/Revision/RevisionManagerTest.php b/tests/src/Kernel/Entity/Revision/RevisionManagerTest.php
@@ -158,16 +158,16 @@ public function testRevision() : void {
       $storage->createRevision($rmt)->save();
     }
     // Make sure items are queued on entity update.
-    $this->assertQueueItems(6);
+    $this->assertQueueItems(5);
 
     $revisions = $this->getSut()->getRevisionsPerLanguage('remote_entity_test', $entity->id());
 
     // We have $i + 1 revisions (11) in total, except sv, which has 10 because
     // we skipped one revision on purpose.
-    foreach (['en' => 6, 'fi' => 6, 'sv' => 5] as $langcode => $expected) {
+    foreach (['en' => 5, 'fi' => 5, 'sv' => 4] as $langcode => $expected) {
       $this->assertCount($expected, $revisions[$langcode]);
     }
-    $this->assertCount(6, $this->getSut()->getRevisions('remote_entity_test', $entity->id()));
+    $this->assertCount(5, $this->getSut()->getRevisions('remote_entity_test', $entity->id()));
 
     // Make sure no revisions are returned when $keep exceeds the number of
     // total revisions.