FIX Unpublising parent pages should include chid pages

Issue [#88](#88) Uses `SiteTree::enforce_strict_hierarchy` config in getting the dependent documents for indexing child pages.
silverstripe · Sep 28, 2023 · 9c67c32 · 9c67c32
1 parent 6081ac9
commit 9c67c32
Show file tree

Hide file tree

Showing 5 changed files with 159 additions and 9 deletions.
diff --git a/src/DataObject/DataObjectDocument.php b/src/DataObject/DataObjectDocument.php
@@ -5,6 +5,7 @@
 use Exception;
 use InvalidArgumentException;
 use LogicException;
+use SilverStripe\CMS\Model\SiteTree;
 use SilverStripe\Core\Config\Configurable;
 use SilverStripe\Core\Extensible;
 use SilverStripe\Core\Injector\Injectable;
@@ -455,6 +456,10 @@ public function getDependentDocuments(): array
             }
         }
 
+        if ($ownedDataObject instanceof SiteTree && SiteTree::config()->get('enforce_strict_hierarchy')) {
+            $docs = array_merge($docs, $this->getChildDocuments($ownedDataObject));
+        }
+
         $dependentDocs = array_values($docs);
         $this->getDataObject()->invokeWithExtensions('updateSearchDependentDocuments', $dependentDocs);
 
@@ -636,4 +641,17 @@ public function onRemoveFromSearchIndexes(string $event): void
         }
     }
 
+    public function getChildDocuments(SiteTree $page): array
+    {
+        $docs = [];
+
+        foreach ($page->AllChildren() as $record) {
+            $document = DataObjectDocument::create($record);
+            $docs[$document->getIdentifier()] = $document;
+            $docs = array_merge($docs, $this->getChildDocuments($record));
+        }
+
+        return $docs;
+    }
+
 }
diff --git a/src/Jobs/RemoveDataObjectJob.php b/src/Jobs/RemoveDataObjectJob.php
@@ -58,14 +58,22 @@ public function setup(): void
             // refetch everything on the live stage
             Versioned::set_stage(Versioned::LIVE);
 
-            return array_map(function (DataObjectDocument $doc) {
-                return DataObjectDocument::create(
-                    DataObject::get_by_id(
-                        $doc->getSourceClass(),
-                        $doc->getDataObject()->ID
-                    )
-                );
-            }, $dependentDocs);
+            return array_reduce(
+                $dependentDocs,
+                function (array $carry, DataObjectDocument $doc) {
+                    $record = DataObject::get_by_id($doc->getSourceClass(), $doc->getDataObject()->ID);
+
+                    if (!$record) {
+                        return $carry;
+                    }
+
+                    $document = DataObjectDocument::create($record);
+                    $carry[$document->getIdentifier()] = $document;
+
+                    return $carry;
+                },
+                []
+            );
         });
 
         $this->setDocuments($documents);

diff --git a/tests/DataObject/DataObjectDocumentTest.php b/tests/DataObject/DataObjectDocumentTest.php
@@ -3,6 +3,7 @@
 namespace SilverStripe\SearchService\Tests\DataObject;
 
 use Page;
+use SilverStripe\ORM\ArrayList;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\ORM\RelationList;
@@ -531,6 +532,7 @@ public function testGetDependentDocuments(): void
         $config->set('getSearchableClasses', [
             DataObjectFake::class,
             ImageFake::class,
+            Page::class,
         ]);
         $config->set('getFieldsForClass', [
             DataObjectFake::class => [
@@ -543,6 +545,10 @@ public function testGetDependentDocuments(): void
             ImageFake::class => [
                 new Field('tagtitles', 'Tags.Title'),
             ],
+            Page::class => [
+                new Field('title'),
+                new Field('content'),
+            ],
         ]);
 
         $dataobject = $this->objFromFixture(TagFake::class, 'one');
@@ -576,6 +582,33 @@ public function testGetDependentDocuments(): void
         }
 
         $this->assertEqualsCanonicalizing($expectedDocuments, $resultDocuments);
+
+        $pageOne = $this->objFromFixture(Page::class, 'page1');
+        $pageDoc = DataObjectDocument::create($pageOne);
+        /** @var DataObjectDocument[] $dependentPages */
+        $dependentPages = $pageDoc->getDependentDocuments();
+
+        // Grab all the expected pages
+        $pageTwo = $this->objFromFixture(Page::class, 'page2');
+        $pageThree = $this->objFromFixture(Page::class, 'page3');
+        $pageSeven = $this->objFromFixture(Page::class, 'page7');
+        $pageEight = $this->objFromFixture(Page::class, 'page8');
+
+        $expectedPages = [
+            sprintf('%s-%s', Page::class, $pageTwo->ID),
+            sprintf('%s-%s', Page::class, $pageThree->ID),
+            sprintf('%s-%s', Page::class, $pageSeven->ID),
+            sprintf('%s-%s', Page::class, $pageEight->ID),
+        ];
+
+        $resultPages = [];
+
+        // Now let's check that each Document represents the Pages that we expected
+        foreach ($dependentPages as $document) {
+            $resultPages[] = sprintf('%s-%s', $document->getSourceClass(), $document->getDataObject()?->ID);
+        }
+
+        $this->assertEqualsCanonicalizing($expectedPages, $resultPages);
     }
 
     public function testExtensionRequired(): void
@@ -626,4 +659,28 @@ public function testDeletedDataObject(): void
         unserialize(serialize($doc));
     }
 
+    public function testGetChildDocuments(): void
+    {
+        $pageOne = $this->objFromFixture(Page::class, 'page1');
+        $pageTwo = $this->objFromFixture(Page::class, 'page2');
+        $pageThree = $this->objFromFixture(Page::class, 'page3');
+        $pageSeven = $this->objFromFixture(Page::class, 'page7');
+        $pageEight = $this->objFromFixture(Page::class, 'page8');
+
+        $parentDocument = DataObjectDocument::create($pageOne);
+        $identifierPrefix = preg_replace('/\d$/', '', $parentDocument->getIdentifier());
+        $childDocs = $parentDocument->getChildDocuments($pageOne);
+
+        $expectedIdentifiers = [
+            $identifierPrefix . $pageTwo->ID,
+            $identifierPrefix . $pageThree->ID,
+            $identifierPrefix . $pageSeven->ID,
+            $identifierPrefix . $pageEight->ID,
+        ];
+
+        $resultIdentifiers = ArrayList::create($childDocs)->column('getIdentifier');
+
+        $this->assertEqualsCanonicalizing($expectedIdentifiers, $resultIdentifiers);
+    }
+
 }
diff --git a/tests/Jobs/RemoveDataObjectJobTest.php b/tests/Jobs/RemoveDataObjectJobTest.php
@@ -2,6 +2,7 @@
 
 namespace SilverStripe\SearchService\Tests\Jobs;
 
+use Page;
 use SilverStripe\SearchService\DataObject\DataObjectDocument;
 use SilverStripe\SearchService\Jobs\RemoveDataObjectJob;
 use SilverStripe\SearchService\Schema\Field;
@@ -16,7 +17,10 @@
 class RemoveDataObjectJobTest extends SearchServiceTest
 {
 
-    protected static $fixture_file = '../fixtures.yml'; // phpcs:ignore
+    protected static $fixture_file = [ // @phpcs:ignore
+        '../fixtures.yml',
+        '../pages.yml',
+    ];
 
     /**
      * @phpcsSuppress SlevomatCodingStandard.TypeHints.PropertyTypeHint.MissingNativeTypeHint
@@ -83,4 +87,59 @@ public function testJob(): void
         $this->assertEqualsCanonicalizing($expectedTitles, $resultTitles);
     }
 
+    public function testUnpublishedParentPage(): void
+    {
+        $config = $this->mockConfig();
+
+        $config->set(
+            'getSearchableClasses',
+            [
+                Page::class,
+            ]
+        );
+
+        $config->set(
+            'getFieldsForClass',
+            [
+                Page::class => [
+                    new Field('title'),
+                    new Field('content'),
+                ],
+            ]
+        );
+
+        // Publish all pages in fixtures since the internal dependency checks looks for live version
+        for ($i=1; $i<=8; $i++) {
+            $this->objFromFixture(Page::class, 'page' . $i)->publishRecursive();
+        }
+
+        // Queue up a job to remove a page with child pages are added as related documents
+        $pageOne = $this->objFromFixture(Page::class, 'page1');
+        $pageDoc = DataObjectDocument::create($pageOne);
+        $job = RemoveDataObjectJob::create($pageDoc);
+        $job->setup();
+
+        // Grab what Documents the Job determined it needed to action
+        /** @var DataObjectDocument[] $documents */
+        $documents = $job->getDocuments();
+
+        // There should be two Pages with this Tag assigned
+        $this->assertCount(4, $documents);
+
+        $expectedTitles = [
+            'Child Page 1',
+            'Child Page 2',
+            'Grandchild Page 1',
+            'Grandchild Page 2',
+        ];
+
+        $resultTitles = [];
+
+        foreach ($documents as $document) {
+            $resultTitles[] = $document->getDataObject()?->Title;
+        }
+
+        $this->assertEqualsCanonicalizing($expectedTitles, $resultTitles);
+    }
+
 }
diff --git a/tests/pages.yml b/tests/pages.yml
@@ -21,3 +21,11 @@ Page:
     Title: Subsite Page 1
     Subsite: =>SilverStripe\Subsites\Model\Subsite.subsite1
     ShowInSearch: 1
+  page7:
+    Title: Grandchild Page 1
+    Parent: =>Page.page2
+    ShowInSearch: 1
+  page8:
+    Title: Grandchild Page 2
+    Parent: =>Page.page2
+    ShowInSearch: 1