Add shadow functionality to publishing process #243

alexander-schranz · 2023-03-23T19:04:43Z

This implements the shadowing behaviour like discussed with @chirimoya. The data of the draft page stays as it is and when publishing a Shadow page the content from the ShadowLocale will be copied to the published page.

TODO Functional Tests

DE (Published: Not Shadow, Unpublished: Shadow to EN) EN Publish should change nothing
DE (Published: Nothing, Unpublished: Shadow to EN) EN Publish should change nothing
DE (Published: Shadow to EN, Unpublished: Not Shadow) EN Publish should update Live Shadow
DE (Published: Shadow to EN, Unpublished: Shadow to EN) DE Unpublish EN Publish should publish nothing for DE

TODO Unit Tests

Update PublishTransitionSubscriberTest for new cases
Update ContentCopierTests for new Params
Update UnublishTransitionSubscriberTest for new cases
Update ShadowDataMapperTests for new cases

coveralls · 2023-03-23T20:09:10Z

Pull Request Test Coverage Report for Build 4525650416

96 of 96 (100.0%) changed or added relevant lines in 10 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 100.0%

Totals
Change from base Build 4501819618:	0.0%
Covered Lines:	2503
Relevant Lines:	2503

💛 - Coveralls

alexander-schranz · 2023-03-23T20:17:50Z

Content/Infrastructure/Doctrine/MetadataLoader.php

@@ -60,6 +60,18 @@ public function loadClassMetadata(LoadClassMetadataEventArgs $event): void
            $this->addIndex($metadata, 'idx_stage', ['stage']);
        }

+        if ($reflection->implementsInterface(ShadowInterface::class)) {


Just moved this to change the ordering of the columns as I think it make sense to have this columns visible first in the database table view before seo, excerpt, ...

alexander-schranz · 2023-03-23T20:28:01Z

Content/Infrastructure/Sulu/Admin/ContentViewBuilderFactory.php

+            foreach ($views as $view) {
+                if ($view instanceof PreviewFormViewBuilderInterface) {
+                    $view->setPreviewCondition('shadowOn != true');
+                }
+            }


Quickfix as the preview currently can not handle shadow feature.

alexander-schranz · 2023-03-23T23:43:35Z

Tests/Functional/Integration/ExampleControllerAvailableAndShadowLocalesTest.php

As it is a very individual test case with different tests depending on each other I thought it most make sense to have it in an own class

alexander-schranz · 2023-03-24T09:14:04Z

Content/Application/ContentCopier/ContentCopierInterface.php

+        array $data = [],
+        array $ignoredAttributes = []


While looking now again on this maybe put this together in an array $options so this interface don't is so messed up and more future proof. What do you think @Prokyonn @wachterjohannes

$contentCopier->copy( $contentRichEntity, $sourceDimensionAttributes, $contentRichEntity, $targetDimensionAttributes, options: [ 'data': [ 'shadowOn' => true, 'shadowLocale' => 'de', 'url' => '/test-de', ], 'ignoredAttributes': [ // ... ], ] )

sounds okfor me 👍

I would also prefer the options parameter, the interface would be cleaner and with correct php doctypes the developer gets the same information

wachterjohannes

For me the code Looks good 👍 if you want to refactor the additional arguments with options - do it! else it would be also OK for me like it is now

wachterjohannes · 2023-03-24T14:13:14Z

Content/Application/ContentCopier/ContentCopierInterface.php

+        array $data = [],
+        array $ignoredAttributes = []


sounds okfor me 👍

Prokyonn · 2023-03-23T23:20:20Z

Content/Application/ContentCopier/ContentCopierInterface.php

@@ -26,14 +26,18 @@ interface ContentCopierInterface
     * @param mixed[] $sourceDimensionAttributes
     * @param ContentRichEntityInterface<T> $targetContentRichEntity
     * @param mixed[] $targetDimensionAttributes
+     * @param mixed[] $data This data is merged with the data of the source content before set on the target content
+     * @param string[] $ignoredAttributes This attributes stayed untouched


Suggested change

* @param string[] $ignoredAttributes This attributes stayed untouched

* @param string[] $ignoredAttributes These attributes stay untouched

moved this to a new $options parameter as written above do you want to recheck the comment there?

Prokyonn · 2023-03-23T23:20:41Z

Content/Application/ContentCopier/ContentCopierInterface.php

@@ -42,13 +46,17 @@ public function copy(
     * @param DimensionContentCollectionInterface<T> $dimensionContentCollection
     * @param ContentRichEntityInterface<T> $targetContentRichEntity
     * @param mixed[] $targetDimensionAttributes
+     * @param mixed[] $data This data is merged with the data of the source content before set on the target content
+     * @param string[] $ignoredAttributes This attributes stayed untouched


Suggested change

* @param string[] $ignoredAttributes This attributes stayed untouched

* @param string[] $ignoredAttributes These attributes stay untouched

Prokyonn · 2023-03-23T23:20:55Z

Content/Application/ContentCopier/ContentCopierInterface.php

@@ -57,12 +65,16 @@ public function copyFromDimensionContentCollection(
     * @param T $dimensionContent
     * @param ContentRichEntityInterface<T> $targetContentRichEntity
     * @param mixed[] $targetDimensionAttributes
+     * @param mixed[] $data This data is merged with the data of the source content before set on the target content
+     * @param string[] $ignoredAttributes This attributes stayed untouched


Suggested change

* @param string[] $ignoredAttributes This attributes stayed untouched

* @param string[] $ignoredAttributes These attributes stay untouched

Prokyonn · 2023-03-23T23:32:35Z

Content/Application/ContentDataMapper/DataMapper/TemplateDataMapper.php

            $name = $property->getName();

            // Float are converted to ints in php array as key so we need convert it to string
            if (\is_float($name)) {
                $name = (string) $name;
            }

-            if (\array_key_exists($name, $data)) {
+            $value = $property->isLocalized() ? $defaultLocalizedData[$name] ?? null : $defaultLocalizedData[$name] ?? null;
+            if (\array_key_exists($name, $data)) { // values not explicitly given need to stay untouched for e.g. for shadow pages urls


Suggested change

if (\array_key_exists($name, $data)) { // values not explicitly given need to stay untouched for e.g. for shadow pages urls

if (\array_key_exists($name, $data)) { // values not explicitly given need to stay untouched for e.g. shadow pages urls

Prokyonn · 2023-03-24T16:16:27Z

Content/Application/ContentCopier/ContentCopierInterface.php

+        array $data = [],
+        array $ignoredAttributes = []


I would also prefer the options parameter, the interface would be cleaner and with correct php doctypes the developer gets the same information

…parameters

alexander-schranz · 2023-03-26T16:56:03Z

composer.json

@@ -65,6 +65,7 @@
        "thecodingmachine/phpstan-strict-rules": "^1.0"
    },
    "conflict": {
+        "coduo/php-matcher": "6.0.12",


conflict because of: coduo/php-matcher#463

alexander-schranz requested a review from Prokyonn March 23, 2023 19:04

alexander-schranz added the Feature New functionality not yet included in Bundle label Mar 23, 2023

alexander-schranz force-pushed the feature/shadow-handling branch 5 times, most recently from e8a891f to 9412ac3 Compare March 23, 2023 20:07

alexander-schranz force-pushed the feature/shadow-handling branch from 9412ac3 to 869cabb Compare March 23, 2023 20:09

alexander-schranz commented Mar 23, 2023

View reviewed changes

alexander-schranz force-pushed the feature/shadow-handling branch 12 times, most recently from 60e7c7f to 7f12e26 Compare March 23, 2023 22:32

Add shadow functionality to publishing process

02a164e

alexander-schranz force-pushed the feature/shadow-handling branch 2 times, most recently from 02a164e to 5d114ba Compare March 23, 2023 23:25

alexander-schranz marked this pull request as ready for review March 23, 2023 23:26

alexander-schranz force-pushed the feature/shadow-handling branch 3 times, most recently from 5a8882c to f6eefd9 Compare March 23, 2023 23:29

Add functional test for shadow and available locale handling

7a32a03

alexander-schranz force-pushed the feature/shadow-handling branch from f6eefd9 to 7a32a03 Compare March 23, 2023 23:38

alexander-schranz commented Mar 23, 2023

View reviewed changes

alexander-schranz commented Mar 24, 2023

View reviewed changes

wachterjohannes approved these changes Mar 24, 2023

View reviewed changes

Prokyonn reviewed Mar 24, 2023

View reviewed changes

Prokyonn approved these changes Mar 24, 2023

View reviewed changes

Change ContentCopierInterface to use options instead of two seperate …

fad544f

…parameters

alexander-schranz force-pushed the feature/shadow-handling branch from c39c0fa to fad544f Compare March 24, 2023 17:23

alexander-schranz added 2 commits March 25, 2023 14:48

Fix php-cs

b7dec0c

Conflict incompatible coduo/php-matcher 6.0.12 version

a430fbe

alexander-schranz force-pushed the feature/shadow-handling branch from d88eec0 to a430fbe Compare March 26, 2023 16:55

alexander-schranz commented Mar 26, 2023

View reviewed changes

alexander-schranz merged commit 70f3f85 into sulu:0.x Mar 27, 2023

alexander-schranz deleted the feature/shadow-handling branch March 27, 2023 09:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add shadow functionality to publishing process #243

Add shadow functionality to publishing process #243

alexander-schranz commented Mar 23, 2023 •

edited

Loading

coveralls commented Mar 23, 2023 •

edited

Loading

alexander-schranz Mar 23, 2023 •

edited

Loading

alexander-schranz Mar 23, 2023

alexander-schranz Mar 23, 2023

alexander-schranz Mar 24, 2023 •

edited

Loading

wachterjohannes Mar 24, 2023

Prokyonn Mar 24, 2023

wachterjohannes left a comment

wachterjohannes Mar 24, 2023

Prokyonn Mar 23, 2023

alexander-schranz Mar 24, 2023

Prokyonn Mar 23, 2023

Prokyonn Mar 23, 2023

Prokyonn Mar 23, 2023

Prokyonn Mar 24, 2023

alexander-schranz Mar 26, 2023

	* @param string[] $ignoredAttributes This attributes stayed untouched
	* @param string[] $ignoredAttributes These attributes stay untouched

	if (\array_key_exists($name, $data)) { // values not explicitly given need to stay untouched for e.g. for shadow pages urls
	if (\array_key_exists($name, $data)) { // values not explicitly given need to stay untouched for e.g. shadow pages urls

Add shadow functionality to publishing process #243

Add shadow functionality to publishing process #243

Conversation

alexander-schranz commented Mar 23, 2023 • edited Loading

TODO Functional Tests

TODO Unit Tests

coveralls commented Mar 23, 2023 • edited Loading

Pull Request Test Coverage Report for Build 4525650416

💛 - Coveralls

alexander-schranz Mar 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexander-schranz Mar 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wachterjohannes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexander-schranz commented Mar 23, 2023 •

edited

Loading

coveralls commented Mar 23, 2023 •

edited

Loading

alexander-schranz Mar 23, 2023 •

edited

Loading

alexander-schranz Mar 24, 2023 •

edited

Loading