From 938a7fa9c7b8e5fa1d4bca11e18abb97260a20ba Mon Sep 17 00:00:00 2001 From: Niklas Natter Date: Fri, 15 May 2020 19:09:19 +0200 Subject: [PATCH 1/2] Set workflow place only on draft dimension --- .../DataMapper/WorkflowDataMapper.php | 32 +++++++++++++++++++ .../ContentWorkflow/ContentWorkflow.php | 2 +- Content/Domain/Model/WorkflowInterface.php | 4 +-- Content/Domain/Model/WorkflowTrait.php | 8 ++--- .../Doctrine/MetadataLoader.php | 2 +- Tests/Unit/Mocks/WorkflowMockWrapperTrait.php | 4 +-- 6 files changed, 42 insertions(+), 10 deletions(-) diff --git a/Content/Application/ContentDataMapper/DataMapper/WorkflowDataMapper.php b/Content/Application/ContentDataMapper/DataMapper/WorkflowDataMapper.php index d6aba756..ff2f3e52 100644 --- a/Content/Application/ContentDataMapper/DataMapper/WorkflowDataMapper.php +++ b/Content/Application/ContentDataMapper/DataMapper/WorkflowDataMapper.php @@ -46,6 +46,38 @@ public function map( */ private function setWorkflowData(WorkflowInterface $object, array $data): void { + $this->setInitialPlaceToDraftDimension($object, $data); + $this->setPublishedToLiveDimension($object, $data); + } + + /** + * @param mixed[] $data + */ + private function setInitialPlaceToDraftDimension(WorkflowInterface $object, array $data): void + { + // we want to set the initial place only to the draft dimension, the live dimension should not have a place + // after the place was set by this mapper initially, the place should only be changed by the ContentWorkflow + // see: https://github.com/sulu/SuluContentBundle/issues/92 + + if (!$object instanceof DimensionContentInterface + || DimensionInterface::STAGE_DRAFT !== $object->getDimension()->getStage()) { + return; + } + + if (!$object->getWorkflowPlace()) { + // TODO: get public workflow registry and set initial place based on $object::getWorkflowName() + $object->setWorkflowPlace(WorkflowInterface::WORKFLOW_PLACE_UNPUBLISHED); + } + } + + /** + * @param mixed[] $data + */ + private function setPublishedToLiveDimension(WorkflowInterface $object, array $data): void + { + // the published property of the draft dimension should only be changed by a ContentWorkflow subscriber + // therefore we only want to copy the published property from the draft to the live dimension + if (!$object instanceof DimensionContentInterface || DimensionInterface::STAGE_LIVE !== $object->getDimension()->getStage()) { return; diff --git a/Content/Application/ContentWorkflow/ContentWorkflow.php b/Content/Application/ContentWorkflow/ContentWorkflow.php index 3cff46ae..30bfcc71 100644 --- a/Content/Application/ContentWorkflow/ContentWorkflow.php +++ b/Content/Application/ContentWorkflow/ContentWorkflow.php @@ -72,7 +72,7 @@ public function __construct( $this->dimensionContentRepository = $dimensionContentRepository; $this->contentMerger = $contentMerger; $this->eventDispatcher = $eventDispatcher ?: new EventDispatcher(); - // TODO get workflow from outside + // TODO: get public workflow registry from outside $this->workflowRegistry = $workflowRegistry ?: new Registry(); $this->workflowRegistry->addWorkflow( $this->getWorkflow(), diff --git a/Content/Domain/Model/WorkflowInterface.php b/Content/Domain/Model/WorkflowInterface.php index 2d50ce7b..6c09f0fd 100644 --- a/Content/Domain/Model/WorkflowInterface.php +++ b/Content/Domain/Model/WorkflowInterface.php @@ -47,9 +47,9 @@ interface WorkflowInterface public static function getWorkflowName(): string; - public function getWorkflowPlace(): string; + public function getWorkflowPlace(): ?string; - public function setWorkflowPlace(string $workflowPlace): void; + public function setWorkflowPlace(?string $workflowPlace): void; public function getWorkflowPublished(): ?\DateTimeImmutable; diff --git a/Content/Domain/Model/WorkflowTrait.php b/Content/Domain/Model/WorkflowTrait.php index b40be988..c6cf9aaa 100644 --- a/Content/Domain/Model/WorkflowTrait.php +++ b/Content/Domain/Model/WorkflowTrait.php @@ -16,9 +16,9 @@ trait WorkflowTrait { /** - * @var string + * @var string|null */ - protected $workflowPlace = WorkflowInterface::WORKFLOW_PLACE_UNPUBLISHED; + protected $workflowPlace; /** * @var \DateTimeImmutable|null @@ -30,12 +30,12 @@ public static function getWorkflowName(): string return WorkflowInterface::WORKFLOW_DEFAULT_NAME; } - public function getWorkflowPlace(): string + public function getWorkflowPlace(): ?string { return $this->workflowPlace; } - public function setWorkflowPlace(string $workflowPlace): void + public function setWorkflowPlace(?string $workflowPlace): void { $this->workflowPlace = $workflowPlace; } diff --git a/Content/Infrastructure/Doctrine/MetadataLoader.php b/Content/Infrastructure/Doctrine/MetadataLoader.php index ba4033a7..31c3864f 100644 --- a/Content/Infrastructure/Doctrine/MetadataLoader.php +++ b/Content/Infrastructure/Doctrine/MetadataLoader.php @@ -100,7 +100,7 @@ public function loadClassMetadata(LoadClassMetadataEventArgs $event): void } if ($reflection->implementsInterface(WorkflowInterface::class)) { - $this->addField($metadata, 'workflowPlace', 'string', ['length' => 32, 'nullable' => false, 'default' => WorkflowInterface::WORKFLOW_PLACE_UNPUBLISHED]); + $this->addField($metadata, 'workflowPlace', 'string', ['length' => 32, 'nullable' => true]); $this->addField($metadata, 'workflowPublished', 'datetime_immutable', ['nullable' => true]); } } diff --git a/Tests/Unit/Mocks/WorkflowMockWrapperTrait.php b/Tests/Unit/Mocks/WorkflowMockWrapperTrait.php index db81d547..a7a6e203 100644 --- a/Tests/Unit/Mocks/WorkflowMockWrapperTrait.php +++ b/Tests/Unit/Mocks/WorkflowMockWrapperTrait.php @@ -25,12 +25,12 @@ public static function getWorkflowName(): string return 'mock-workflow-name'; } - public function getWorkflowPlace(): string + public function getWorkflowPlace(): ?string { return $this->instance->getWorkflowPlace(); } - public function setWorkflowPlace(string $workflowPlace): void + public function setWorkflowPlace(?string $workflowPlace): void { $this->instance->setWorkflowPlace($workflowPlace); } From 77dfe1792fb8f21c61f1661963b661b215259542 Mon Sep 17 00:00:00 2001 From: Niklas Natter Date: Wed, 20 May 2020 10:54:20 +0200 Subject: [PATCH 2/2] Add test cases for WorkflowDataMapper --- .../DataMapper/WorkflowDataMapper.php | 2 +- .../DataMapper/WorkflowDataMapperTest.php | 122 ++++++++++++++---- .../ContentNormalizerTest.php | 8 +- .../Domain/Model/WorkflowTraitTest.php | 2 +- 4 files changed, 104 insertions(+), 30 deletions(-) diff --git a/Content/Application/ContentDataMapper/DataMapper/WorkflowDataMapper.php b/Content/Application/ContentDataMapper/DataMapper/WorkflowDataMapper.php index ff2f3e52..f162960a 100644 --- a/Content/Application/ContentDataMapper/DataMapper/WorkflowDataMapper.php +++ b/Content/Application/ContentDataMapper/DataMapper/WorkflowDataMapper.php @@ -86,7 +86,7 @@ private function setPublishedToLiveDimension(WorkflowInterface $object, array $d $published = $data['published'] ?? null; if (!$published) { - return; + throw new \RuntimeException('Expected "published" to be set in the data array.'); } $object->setWorkflowPublished(new \DateTimeImmutable($published)); diff --git a/Tests/Unit/Content/Application/ContentDataMapper/DataMapper/WorkflowDataMapperTest.php b/Tests/Unit/Content/Application/ContentDataMapper/DataMapper/WorkflowDataMapperTest.php index 03ad61f5..36b733aa 100644 --- a/Tests/Unit/Content/Application/ContentDataMapper/DataMapper/WorkflowDataMapperTest.php +++ b/Tests/Unit/Content/Application/ContentDataMapper/DataMapper/WorkflowDataMapperTest.php @@ -60,7 +60,7 @@ public function testMapLocalizedNoWorkflow(): void $workflowMapper->map($data, $dimensionContent->reveal(), $localizedDimensionContent->reveal()); } - public function testMapUnlocalizedWorkflow(): void + public function testMapUnlocalizedDraft(): void { $workflowMapper = $this->createWorkflowDataMapperInstance(); @@ -71,15 +71,17 @@ public function testMapUnlocalizedWorkflow(): void $dimensionContent = $this->prophesize(DimensionContentInterface::class); $dimensionContent->willImplement(WorkflowInterface::class); $dimension = $this->prophesize(DimensionInterface::class); - $dimension->getStage()->willReturn(DimensionInterface::STAGE_LIVE); + $dimension->getStage()->willReturn(DimensionInterface::STAGE_DRAFT); $dimensionContent->getDimension()->willReturn($dimension->reveal()); + $dimensionContent->getWorkflowPlace()->willReturn(null); - $dimensionContent->setWorkflowPublished(Argument::cetera())->shouldBeCalled(); + $dimensionContent->setWorkflowPlace(WorkflowInterface::WORKFLOW_PLACE_UNPUBLISHED)->shouldBeCalled(); + $dimensionContent->setWorkflowPublished(Argument::cetera())->shouldNotBeCalled(); $workflowMapper->map($data, $dimensionContent->reveal()); } - public function testMapLocalizedWorkflow(): void + public function testMapUnlocalizedDraftPlaceAlreadySet(): void { $workflowMapper = $this->createWorkflowDataMapperInstance(); @@ -89,20 +91,58 @@ public function testMapLocalizedWorkflow(): void $dimensionContent = $this->prophesize(DimensionContentInterface::class); $dimensionContent->willImplement(WorkflowInterface::class); + $dimension = $this->prophesize(DimensionInterface::class); + $dimension->getStage()->willReturn(DimensionInterface::STAGE_DRAFT); + $dimensionContent->getDimension()->willReturn($dimension->reveal()); + $dimensionContent->getWorkflowPlace()->willReturn(WorkflowInterface::WORKFLOW_PLACE_UNPUBLISHED); - $localizedDimensionContent = $this->prophesize(DimensionContentInterface::class); - $localizedDimensionContent->willImplement(WorkflowInterface::class); + $dimensionContent->setWorkflowPlace(Argument::cetera())->shouldNotBeCalled(); + $dimensionContent->setWorkflowPublished(Argument::cetera())->shouldNotBeCalled(); + + $workflowMapper->map($data, $dimensionContent->reveal()); + } + + public function testMapUnlocalizedLive(): void + { + $workflowMapper = $this->createWorkflowDataMapperInstance(); + + $data = [ + 'published' => (new \DateTime())->format('c'), + ]; + + $dimensionContent = $this->prophesize(DimensionContentInterface::class); + $dimensionContent->willImplement(WorkflowInterface::class); $dimension = $this->prophesize(DimensionInterface::class); $dimension->getStage()->willReturn(DimensionInterface::STAGE_LIVE); - $localizedDimensionContent->getDimension()->willReturn($dimension->reveal()); + $dimensionContent->getDimension()->willReturn($dimension->reveal()); - $dimensionContent->setWorkflowPublished(Argument::any())->shouldNotBeCalled(); - $localizedDimensionContent->setWorkflowPublished(Argument::cetera())->shouldBeCalled(); + $dimensionContent->setWorkflowPlace(Argument::cetera())->shouldNotBeCalled(); + $dimensionContent->setWorkflowPublished(Argument::type(\DateTimeInterface::class))->shouldBeCalled(); - $workflowMapper->map($data, $dimensionContent->reveal(), $localizedDimensionContent->reveal()); + $workflowMapper->map($data, $dimensionContent->reveal()); + } + + public function testMapUnlocalizedLivePublishedNotSet(): void + { + $workflowMapper = $this->createWorkflowDataMapperInstance(); + + $data = []; + + $dimensionContent = $this->prophesize(DimensionContentInterface::class); + $dimensionContent->willImplement(WorkflowInterface::class); + $dimension = $this->prophesize(DimensionInterface::class); + $dimension->getStage()->willReturn(DimensionInterface::STAGE_LIVE); + $dimensionContent->getDimension()->willReturn($dimension->reveal()); + + $dimensionContent->setWorkflowPlace(Argument::cetera())->shouldNotBeCalled(); + $dimensionContent->setWorkflowPublished(Argument::cetera())->shouldNotBeCalled(); + + $this->expectException(\RuntimeException::class); + + $workflowMapper->map($data, $dimensionContent->reveal()); } - public function testMapUnlocalizedDraftWorkflow(): void + public function testMapLocalizedDraft(): void { $workflowMapper = $this->createWorkflowDataMapperInstance(); @@ -112,16 +152,24 @@ public function testMapUnlocalizedDraftWorkflow(): void $dimensionContent = $this->prophesize(DimensionContentInterface::class); $dimensionContent->willImplement(WorkflowInterface::class); + + $localizedDimensionContent = $this->prophesize(DimensionContentInterface::class); + $localizedDimensionContent->willImplement(WorkflowInterface::class); $dimension = $this->prophesize(DimensionInterface::class); $dimension->getStage()->willReturn(DimensionInterface::STAGE_DRAFT); - $dimensionContent->getDimension()->willReturn($dimension->reveal()); + $localizedDimensionContent->getDimension()->willReturn($dimension->reveal()); + $localizedDimensionContent->getWorkflowPlace()->willReturn(null); - $dimensionContent->setWorkflowPublished(Argument::any())->shouldNotBeCalled(); + $dimensionContent->setWorkflowPlace(Argument::cetera())->shouldNotBeCalled(); + $dimensionContent->setWorkflowPublished(Argument::cetera())->shouldNotBeCalled(); - $workflowMapper->map($data, $dimensionContent->reveal()); + $localizedDimensionContent->setWorkflowPlace(WorkflowInterface::WORKFLOW_PLACE_UNPUBLISHED)->shouldBeCalled(); + $localizedDimensionContent->setWorkflowPublished(Argument::cetera())->shouldNotBeCalled(); + + $workflowMapper->map($data, $dimensionContent->reveal(), $localizedDimensionContent->reveal()); } - public function testMapLocalizedDraftWorkflow(): void + public function testMapLocalizedDraftPlaceAlreadySet(): void { $workflowMapper = $this->createWorkflowDataMapperInstance(); @@ -137,31 +185,44 @@ public function testMapLocalizedDraftWorkflow(): void $dimension = $this->prophesize(DimensionInterface::class); $dimension->getStage()->willReturn(DimensionInterface::STAGE_DRAFT); $localizedDimensionContent->getDimension()->willReturn($dimension->reveal()); + $localizedDimensionContent->getWorkflowPlace()->willReturn(WorkflowInterface::WORKFLOW_PLACE_UNPUBLISHED); + + $dimensionContent->setWorkflowPlace(Argument::cetera())->shouldNotBeCalled(); + $dimensionContent->setWorkflowPublished(Argument::cetera())->shouldNotBeCalled(); - $dimensionContent->setWorkflowPublished(Argument::any())->shouldNotBeCalled(); - $localizedDimensionContent->setWorkflowPublished(Argument::any())->shouldNotBeCalled(); + $localizedDimensionContent->setWorkflowPlace(Argument::cetera())->shouldNotBeCalled(); + $localizedDimensionContent->setWorkflowPublished(Argument::cetera())->shouldNotBeCalled(); $workflowMapper->map($data, $dimensionContent->reveal(), $localizedDimensionContent->reveal()); } - public function testMapPublishedNotExists(): void + public function testMapLocalizedLive(): void { $workflowMapper = $this->createWorkflowDataMapperInstance(); - $data = []; + $data = [ + 'published' => (new \DateTime())->format('c'), + ]; $dimensionContent = $this->prophesize(DimensionContentInterface::class); $dimensionContent->willImplement(WorkflowInterface::class); + + $localizedDimensionContent = $this->prophesize(DimensionContentInterface::class); + $localizedDimensionContent->willImplement(WorkflowInterface::class); $dimension = $this->prophesize(DimensionInterface::class); $dimension->getStage()->willReturn(DimensionInterface::STAGE_LIVE); - $dimensionContent->getDimension()->willReturn($dimension->reveal()); + $localizedDimensionContent->getDimension()->willReturn($dimension->reveal()); - $dimensionContent->setWorkflowPublished(Argument::any())->shouldNotBeCalled(); + $dimensionContent->setWorkflowPlace(Argument::cetera())->shouldNotBeCalled(); + $dimensionContent->setWorkflowPublished(Argument::cetera())->shouldNotBeCalled(); - $workflowMapper->map($data, $dimensionContent->reveal()); + $localizedDimensionContent->setWorkflowPlace(Argument::cetera())->shouldNotBeCalled(); + $localizedDimensionContent->setWorkflowPublished(Argument::type(\DateTimeInterface::class))->shouldBeCalled(); + + $workflowMapper->map($data, $dimensionContent->reveal(), $localizedDimensionContent->reveal()); } - public function testMapPublishedNull(): void + public function testMapLocalizedLivePublishedNotSet(): void { $workflowMapper = $this->createWorkflowDataMapperInstance(); @@ -169,12 +230,21 @@ public function testMapPublishedNull(): void $dimensionContent = $this->prophesize(DimensionContentInterface::class); $dimensionContent->willImplement(WorkflowInterface::class); + + $localizedDimensionContent = $this->prophesize(DimensionContentInterface::class); + $localizedDimensionContent->willImplement(WorkflowInterface::class); $dimension = $this->prophesize(DimensionInterface::class); $dimension->getStage()->willReturn(DimensionInterface::STAGE_LIVE); - $dimensionContent->getDimension()->willReturn($dimension->reveal()); + $localizedDimensionContent->getDimension()->willReturn($dimension->reveal()); - $dimensionContent->setWorkflowPublished(Argument::any())->shouldNotBeCalled(); + $dimensionContent->setWorkflowPlace(Argument::cetera())->shouldNotBeCalled(); + $dimensionContent->setWorkflowPublished(Argument::cetera())->shouldNotBeCalled(); - $workflowMapper->map($data, $dimensionContent->reveal()); + $localizedDimensionContent->setWorkflowPlace(Argument::cetera())->shouldNotBeCalled(); + $localizedDimensionContent->setWorkflowPublished(Argument::cetera())->shouldNotBeCalled(); + + $this->expectException(\RuntimeException::class); + + $workflowMapper->map($data, $dimensionContent->reveal(), $localizedDimensionContent->reveal()); } } diff --git a/Tests/Unit/Content/Application/ContentNormalizer/ContentNormalizerTest.php b/Tests/Unit/Content/Application/ContentNormalizer/ContentNormalizerTest.php index ffa58d69..8eaae1f0 100644 --- a/Tests/Unit/Content/Application/ContentNormalizer/ContentNormalizerTest.php +++ b/Tests/Unit/Content/Application/ContentNormalizer/ContentNormalizerTest.php @@ -163,6 +163,10 @@ public function getContentRichEntity(): ContentRichEntityInterface $object->setTemplateKey('template-key'); $object->setTemplateData(['someTemplate' => 'data']); + $published = new \DateTimeImmutable('2020-02-02T12:30:00+00:00'); + $object->setWorkflowPlace(WorkflowInterface::WORKFLOW_PLACE_DRAFT); + $object->setWorkflowPublished($published); + $contentNormalizer = $this->createContentNormalizerInstance(); $this->assertSame([ @@ -181,7 +185,7 @@ public function getContentRichEntity(): ContentRichEntityInterface 'excerptTitle' => 'Excerpt Title', 'id' => 5, 'locale' => 'de', - 'published' => null, + 'published' => '2020-02-02T12:30:00+00:00', 'publishedState' => false, 'seoCanonicalUrl' => 'https://caninical.localhost/', 'seoDescription' => 'Seo Description', @@ -193,7 +197,7 @@ public function getContentRichEntity(): ContentRichEntityInterface 'someTemplate' => 'data', 'stage' => 'live', 'template' => 'template-key', - 'workflowPlace' => 'unpublished', + 'workflowPlace' => 'draft', ], $contentNormalizer->normalize($object)); } } diff --git a/Tests/Unit/Content/Domain/Model/WorkflowTraitTest.php b/Tests/Unit/Content/Domain/Model/WorkflowTraitTest.php index f679518d..c3def784 100644 --- a/Tests/Unit/Content/Domain/Model/WorkflowTraitTest.php +++ b/Tests/Unit/Content/Domain/Model/WorkflowTraitTest.php @@ -31,7 +31,7 @@ protected function getWorkflowInstance(): WorkflowInterface public function testGetWorkflowPlace(): void { $workflow = $this->getWorkflowInstance(); - $this->assertSame('unpublished', $workflow->getWorkflowPlace()); + $this->assertNull($workflow->getWorkflowPlace()); } public function testSetWorkflowPlaceReview(): void