From 62cefde3dd3544d8423256726ae4f5b9c93b31e7 Mon Sep 17 00:00:00 2001 From: Oliver Kossin Date: Fri, 2 Feb 2024 15:15:41 +0100 Subject: [PATCH] Add MapRequestPayload --- Api/Factory/NewsApiDtoFactory.php | 17 +-- Api/News.php | 24 +++-- Controller/Admin/NewsController.php | 79 +++++--------- Entity/Factory/AbstractFactory.php | 33 ------ Entity/Factory/MediaFactory.php | 2 +- Entity/Factory/NewsFactory.php | 49 +++------ Entity/Factory/NewsFactoryInterface.php | 3 +- Entity/Factory/TagFactory.php | 24 +++-- Entity/News.php | 13 +-- Exception/NewsException.php | 9 ++ Resources/config/services.yaml | 1 - Service/News/NewsService.php | 9 +- Service/News/NewsServiceInterface.php | 5 +- .../Controller/Admin/NewsControllerTest.php | 100 ++++++++++++++++-- Tests/Unit/Entity/Factory/NewsFactoryTest.php | 82 -------------- .../tests/Fixtures/App/var/.gitempty | 0 16 files changed, 200 insertions(+), 250 deletions(-) delete mode 100644 Entity/Factory/AbstractFactory.php create mode 100644 Exception/NewsException.php delete mode 100644 Tests/Unit/Entity/Factory/NewsFactoryTest.php create mode 100644 processTags/doctrine/phpcr-bundle/tests/Fixtures/App/var/.gitempty diff --git a/Api/Factory/NewsApiDtoFactory.php b/Api/Factory/NewsApiDtoFactory.php index cdb0f94..503d40d 100644 --- a/Api/Factory/NewsApiDtoFactory.php +++ b/Api/Factory/NewsApiDtoFactory.php @@ -7,15 +7,15 @@ final class NewsApiDtoFactory { - public function generate(NewsEntity $entity): News + public function generate(NewsEntity $entity, string $locale): News { return new News( - $entity->getId(), - $entity->getTitle(), - $entity->getTeaser(), - $entity->getContent(), - $entity->isEnabled(), - $entity->getPublishedAt(), + id: $entity->getId(), + title: $entity->getTitle(), + teaser: $entity->getTeaser(), + content: $entity->getContent(), + enabled: $entity->isEnabled(), + publishedAt: $entity->getPublishedAt(), route: $entity->getRoute()?->getPath(), tags: $entity->getTagNameArray(), header: [ @@ -25,7 +25,8 @@ public function generate(NewsEntity $entity): News created: $entity->getCreated(), changed: $entity->getChanged(), author: $entity->getCreator()?->getId(), - ext: $entity->getSeo() + ext: $entity->getSeo(), + locale: $locale ); } } diff --git a/Api/News.php b/Api/News.php index 77d59a7..aad6a1c 100644 --- a/Api/News.php +++ b/Api/News.php @@ -13,23 +13,27 @@ namespace TheCadien\Bundle\SuluNewsBundle\Api; +use Symfony\Component\Validator\Constraints as Assert; + final class News { public function __construct( - public int $id, - public ?string $title = null, - public ?string $teaser = null, - public ?array $content = null, - public ?bool $enabled = null, + #[Assert\NotBlank(groups: ['edit'])] + public ?int $id, + public string $title, + public ?string $teaser, + public ?array $content, + public ?bool $enabled, public ?\DateTime $publishedAt = null, - public ?string $route = null, - public ?array $tags = null, - public ?array $header = null, + public ?string $route, + public ?array $tags, + public ?array $header, public ?\DateTime $authored = null, public ?\DateTime $created = null, public ?\DateTime $changed = null, - public ?int $author = null, - public ?string $ext = null + public ?int $author, + public ?string $ext, + public ?string $locale = null ) { } } diff --git a/Controller/Admin/NewsController.php b/Controller/Admin/NewsController.php index 4fe66cb..c6c5772 100644 --- a/Controller/Admin/NewsController.php +++ b/Controller/Admin/NewsController.php @@ -13,25 +13,23 @@ namespace TheCadien\Bundle\SuluNewsBundle\Controller\Admin; -use Sulu\Component\Rest\Exception\EntityNotFoundException; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Attribute\AsController; -use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; +use Symfony\Component\HttpKernel\Attribute\MapRequestPayload; use Symfony\Component\Routing\Annotation\Route; use TheCadien\Bundle\SuluNewsBundle\Api\Factory\NewsApiDtoFactory; use TheCadien\Bundle\SuluNewsBundle\Api\News as NewsApi; use TheCadien\Bundle\SuluNewsBundle\Common\DoctrineListRepresentationFactory; use TheCadien\Bundle\SuluNewsBundle\Entity\News; -use TheCadien\Bundle\SuluNewsBundle\Repository\NewsRepository; +use TheCadien\Bundle\SuluNewsBundle\Exception\NewsException; use TheCadien\Bundle\SuluNewsBundle\Service\News\NewsService; #[AsController] final class NewsController extends AbstractController { public function __construct( - private readonly NewsRepository $repository, private readonly NewsService $newsService, private readonly DoctrineListRepresentationFactory $doctrineListRepresentationFactory, private readonly NewsApiDtoFactory $apiDtoFactory @@ -41,87 +39,66 @@ public function __construct( #[Route('/news', name: 'app.cget_news', methods: ['GET'])] public function cget(Request $request): Response { - $locale = $request->query->get('locale'); $listRepresentation = $this->doctrineListRepresentationFactory->createDoctrineListRepresentation( News::RESOURCE_KEY, [], - ['locale' => $locale] + ['locale' => $request->query->get('locale')] ); return $this->json($listRepresentation->toArray()); } #[Route('/news/{id}', name: 'app.get_news', methods: ['GET'])] - public function get(int $id, Request $request): Response + public function get(News $news, Request $request): Response { - if (!($entity = $this->repository->findById($id)) instanceof \TheCadien\Bundle\SuluNewsBundle\Entity\News) { - throw new NotFoundHttpException(); - } - $apiEntity = $this->generateApiNewsEntity($entity, $request->query->get('locale')); - - return $this->json($apiEntity, 200, []); + return $this->json($this->apiDtoFactory->generate($news, $request->query->get('locale'))); } #[Route('/news', name: 'app.post_news', methods: ['POST'])] - public function post(Request $request): Response + public function post(#[MapRequestPayload(acceptFormat: 'json')] NewsApi $newsApi, Request $request): Response { - $news = $this->newsService->saveNewNews($request->request->all(), $request->query->get('locale')); - - $apiEntity = $this->generateApiNewsEntity($news, $request->query->get('locale')); + try { + $news = $this->newsService->saveNewNews($newsApi, $request->query->get('locale')); + } catch (NewsException) { + throw new NewsException(); + } - return $this->json($apiEntity, 200, [], ['fullNews']); + return $this->json($this->apiDtoFactory->generate($news, $request->query->get('locale')), 200, [], ['fullNews']); } #[Route('/news/{id}', name: 'app.post_news_trigger', methods: ['POST'])] - public function postTrigger(int $id, Request $request): Response + public function postTrigger(News $news, Request $request): Response { - $news = $this->repository->findById($id); - if (!$news instanceof News) { - throw new NotFoundHttpException(); + try { + $news = $this->newsService->updateNewsPublish($news, $request->query->all()); + } catch (NewsException) { + throw new NewsException(); } - $news = $this->newsService->updateNewsPublish($news, $request->query->all()); - - $apiEntity = $this->generateApiNewsEntity($news, $request->query->get( - 'locale' - )); - - return $this->json($apiEntity); + return $this->json($this->apiDtoFactory->generate($news, $request->query->get('locale'))); } #[Route('/news/{id}', name: 'app.put_news', methods: ['PUT'])] - public function put(int $id, Request $request): Response + public function put(News $news, #[MapRequestPayload(acceptFormat: 'json')] NewsApi $newsApi, Request $request): Response { - $entity = $this->repository->findById($id); - if (!$entity instanceof News) { - throw new NotFoundHttpException(); + try { + $updatedEntity = $this->newsService->updateNews($newsApi, $news, $request->query->get('locale')); + } catch (NewsException) { + throw new NewsException(); } - $updatedEntity = $this->newsService->updateNews($request->request->all(), $entity, $request->query->get('locale')); - $apiEntity = $this->generateApiNewsEntity($updatedEntity, $request->query->get('locale')); - - return $this->json($apiEntity); + return $this->json($this->apiDtoFactory->generate($updatedEntity, $request->query->get('locale'))); } #[Route('/news/{id}', name: 'app.delete_news', methods: ['DELETE'])] - public function delete(int $id): Response + public function delete(News $news): Response { try { - $this->newsService->removeNews($id); - } catch (\Exception) { - throw new EntityNotFoundException(News::class, $id); + $this->newsService->removeNews($news->getId()); + } catch (NewsException) { + throw new NewsException(); } return $this->json(null, 204); } - - protected function generateApiNewsEntity(News $entity, string $locale): NewsApi - { - return $this->apiDtoFactory->generate($entity); - } - - public function getPriority(): int - { - return 0; - } } diff --git a/Entity/Factory/AbstractFactory.php b/Entity/Factory/AbstractFactory.php deleted file mode 100644 index 576cb96..0000000 --- a/Entity/Factory/AbstractFactory.php +++ /dev/null @@ -1,33 +0,0 @@ -getProperty($data, 'title')) { - $news->setTitle($this->getProperty($data, 'title')); - } - - if ($this->getProperty($data, 'teaser')) { - $news->setTeaser($this->getProperty($data, 'teaser')); - } - - if ($this->getProperty($data, 'header')) { - $news->setHeader($this->mediaFactory->generateMedia($data['header'])); - } - - if ($this->getProperty($data, 'publishedAt')) { - $news->setPublishedAt(new \DateTime($this->getProperty($data, 'publishedAt'))); - } - - if ($this->getProperty($data, 'content')) { - $news->setContent($this->getProperty($data, 'content')); - } - - if ($this->getProperty($data, 'ext')) { - $news->setSeo($this->getProperty($data['ext'], 'seo')); - } - - if ($tags = $this->getProperty($data, 'tags')) { - $this->tagFactory->processTags($news, $tags); + $news->setTitle($data->title); + $news->setTeaser($data->teaser); + $news->setHeader($data->header); + $news->setPublishedAt($data->publishedAt); + $news->setContent($data->content); + $news->setSeo($data->ext); + + if ($data->tags) { + $this->tagFactory->processTags($news, $data->tags); } if (!$news->getId()) { @@ -75,13 +59,12 @@ public function generateNewsFromRequest(News $news, array $data, string $locale $news->setEnabled($state); } - if ($authored = $this->getProperty($data, 'authored')) { - $news->setCreated(new \DateTime($authored)); + if ($data->authored) { + $news->setCreated($data->authored); } - if ($author = $this->getProperty($data, 'author')) { - // @var Contact $contact - $news->setCreator($this->contactRepository->find($author)); + if ($data->author) { + $news->setCreator($this->contactRepository->find($data->author)); } return $news; diff --git a/Entity/Factory/NewsFactoryInterface.php b/Entity/Factory/NewsFactoryInterface.php index c9ad720..33d2294 100644 --- a/Entity/Factory/NewsFactoryInterface.php +++ b/Entity/Factory/NewsFactoryInterface.php @@ -13,9 +13,10 @@ namespace TheCadien\Bundle\SuluNewsBundle\Entity\Factory; +use TheCadien\Bundle\SuluNewsBundle\Api\News as NewsApi; use TheCadien\Bundle\SuluNewsBundle\Entity\News; interface NewsFactoryInterface { - public function generateNewsFromRequest(News $news, array $data, string $locale): News; + public function generateNewsFromRequest(News $news, NewsApi $data, string $locale): News; } diff --git a/Entity/Factory/TagFactory.php b/Entity/Factory/TagFactory.php index 30dc22f..e848049 100644 --- a/Entity/Factory/TagFactory.php +++ b/Entity/Factory/TagFactory.php @@ -17,7 +17,7 @@ use Sulu\Component\Persistence\RelationTrait; use TheCadien\Bundle\SuluNewsBundle\Entity\News; -class TagFactory extends AbstractFactory implements TagFactoryInterface +class TagFactory implements TagFactoryInterface { use RelationTrait; @@ -35,17 +35,25 @@ public function __construct(private readonly TagManagerInterface $tagManager) */ public function processTags(News $news, $tags) { - $get = fn ($tag) => $tag->getId(); + $get = function ($tag) { + return $tag->getId(); + }; - $delete = fn ($tag) => $news->removeTag($tag); + $delete = function ($tag) use ($news) { + return $news->removeTag($tag); + }; - $update = fn () => true; + $update = function () { + return true; + }; - $add = fn ($tag) => $this->addTag($news, $tag); + $add = function ($tag) use ($news) { + return $this->addTag($news, $tag); + }; $entities = $news->getTags(); - return $this->processSubEntities( + $result = $this->processSubEntities( $entities, $tags, $get, @@ -53,6 +61,10 @@ public function processTags(News $news, $tags) $update, $delete ); + + $this->resetIndexOfSubentites($entities); + + return $result; } /** diff --git a/Entity/News.php b/Entity/News.php index 5928eb1..4db2c5c 100644 --- a/Entity/News.php +++ b/Entity/News.php @@ -102,9 +102,6 @@ public function setEnabled(bool $enabled): void $this->enabled = $enabled; } - /** - * @return string - */ public function getTitle(): ?string { return $this->title; @@ -120,14 +117,11 @@ public function getContent() return $this->content ?: []; } - public function setContent($content): void + public function setContent(mixed $content): void { $this->content = $content; } - /** - * @return \DateTime - */ public function getCreated(): ?\DateTime { return $this->created; @@ -138,9 +132,6 @@ public function setCreated(\DateTime $created): void $this->created = $created; } - /** - * @return \DateTime - */ public function getPublishedAt(): ?\DateTime { return $this->publishedAt; @@ -156,7 +147,7 @@ public function getTeaser(): ?string return $this->teaser; } - public function setTeaser(string $teaser): void + public function setTeaser(?string $teaser): void { $this->teaser = $teaser; } diff --git a/Exception/NewsException.php b/Exception/NewsException.php new file mode 100644 index 0000000..803b31f --- /dev/null +++ b/Exception/NewsException.php @@ -0,0 +1,9 @@ +newsFactory->generateNewsFromRequest($news, $data, $locale); @@ -92,8 +93,8 @@ public function updateNews($data, News $news, string $locale): News $news->setchanger($this->loginUser->getContact()); - if ($news->getRoute()->getPath() !== $data['route']) { - $route = $this->routeFactory->updateNewsRoute($news, $data['route']); + if ($news->getRoute()->getPath() !== $data->route && $data->route) { + $route = $this->routeFactory->updateNewsRoute($news, $data->route); $news->setRoute($route); } diff --git a/Service/News/NewsServiceInterface.php b/Service/News/NewsServiceInterface.php index 3b56014..e8aa1e8 100644 --- a/Service/News/NewsServiceInterface.php +++ b/Service/News/NewsServiceInterface.php @@ -13,11 +13,12 @@ namespace TheCadien\Bundle\SuluNewsBundle\Service\News; +use TheCadien\Bundle\SuluNewsBundle\Api\News as NewsApi; use TheCadien\Bundle\SuluNewsBundle\Entity\News; interface NewsServiceInterface { - public function saveNewNews(array $data, string $locale): News; + public function saveNewNews(NewsApi $data, string $locale): News; - public function updateNews($data, News $article, string $locale): News; + public function updateNews(NewsApi $data, News $article, string $locale): News; } diff --git a/Tests/Functional/Controller/Admin/NewsControllerTest.php b/Tests/Functional/Controller/Admin/NewsControllerTest.php index c512697..ea5b8e1 100644 --- a/Tests/Functional/Controller/Admin/NewsControllerTest.php +++ b/Tests/Functional/Controller/Admin/NewsControllerTest.php @@ -16,22 +16,20 @@ public function testAppGetNews() self::purgeDatabase(); $news = $this->generateNews(); - $newsRepository = $client->getContainer()->get('TheCadien\Bundle\SuluNewsBundle\Repository\NewsRepository'); - $newsRepository->save($news); + $client->getContainer()->get('TheCadien\Bundle\SuluNewsBundle\Repository\NewsRepository')->save($news); $client->request('GET', '/admin/api/news/' . $news->getId() . '?locale=en'); - $response = json_decode($client->getResponse()->getContent()); + $response = \json_decode($client->getResponse()->getContent()); self::assertSame('Test Teaser', $response->teaser); self::assertSame('title', $response->content[0]->type); self::assertSame('Test', $response->content[0]->title); self::assertSame('/test-1', $response->route); self::assertTrue($response->enabled); - self::assertSame([], $response->tags); //todo Test with tags! - self::assertNull($response->author); //todo ! Test Author! - self::assertNull($response->ext); //todo ! Test ext! - + self::assertSame([], $response->tags); // todo Test with tags! + self::assertNull($response->author); // todo ! Test Author! + self::assertNull($response->ext); // todo ! Test ext! } public function testAppGetNewsWithoutData() @@ -43,4 +41,92 @@ public function testAppGetNewsWithoutData() $client->request('GET', '/admin/api/news/1000?locale=en'); self::assertResponseStatusCodeSame(404); } + + public function testPostValidNewsWithOutContent() + { + $client = self::createAuthenticatedClient(); + + self::purgeDatabase(); + + $client->jsonRequest( + 'POST', + '/admin/api/news?locale=en', + [ + 'title' => 'test', + 'teaser' => 'Test Teaser', + 'publishedAt' => '2023-12-23T18:22:54', + ] + ); + + $newsResult = $client->getContainer()->get('TheCadien\Bundle\SuluNewsBundle\Repository\NewsRepository')->findOneBy(['title' => 'test']); + + self::assertSame('test', $newsResult->getTitle()); + self::assertSame('Test Teaser', $newsResult->getTeaser()); + self::assertSame('/news/' . $newsResult->getId(), $newsResult->getRoute()->getPath()); + } + + public function testNewsPostWithInvalidRequest() + { + $client = self::createAuthenticatedClient(); + + self::purgeDatabase(); + + $client->jsonRequest( + 'POST', + '/admin/api/news?locale=en', + [ + 'title' => '', + 'teaser' => '', + 'publishedAt' => '', + ] + ); + + /* Symfony MapRequestPayload 422 status code */ + self::assertResponseStatusCodeSame(422); + } + + public function testNewsPutWithValidRequest() + { + $client = self::createAuthenticatedClient(); + + self::purgeDatabase(); + $news = $this->generateNews(); + + $client->getContainer()->get('TheCadien\Bundle\SuluNewsBundle\Repository\NewsRepository')->save($news); + + $client->jsonRequest( + 'PUT', + '/admin/api/news/' . $news->getId() . '?locale=en', + [ + 'title' => 'New Title', + 'teaser' => 'New Teaser', + 'publishedAt' => '2023-12-23T18:22:54', + 'route' => '/news/fancy-new-route', + 'content' => [ + [ + 'type' => 'editor', + 'text' => '

Test Editor

', + ], + [ + 'type' => 'title', + 'title' => 'Test', + ], + ], + 'tags' => [ + 'Test', + 'Sulu', + 'News', + ], + ] + ); + + $updatetNews = $client->getContainer()->get('TheCadien\Bundle\SuluNewsBundle\Repository\NewsRepository')->findById($news->getId()); + self::assertSame('New Title', $updatetNews->getTitle()); + self::assertSame('New Teaser', $updatetNews->getTeaser()); + self::assertSame('/news/fancy-new-route', $updatetNews->getRoute()->getPath()); + self::assertSame('Max', $updatetNews->getChanger()->getFirstName()); + self::assertSame('Mustermann', $updatetNews->getChanger()->getLastName()); + self::assertSame('title', $updatetNews->getContent()[1]['type']); + self::assertSame('Test', $updatetNews->getContent()[1]['title']); + } } diff --git a/Tests/Unit/Entity/Factory/NewsFactoryTest.php b/Tests/Unit/Entity/Factory/NewsFactoryTest.php deleted file mode 100644 index c0d84ef..0000000 --- a/Tests/Unit/Entity/Factory/NewsFactoryTest.php +++ /dev/null @@ -1,82 +0,0 @@ -prophesize(MediaFactoryInterface::class); - $mediaFactory->generateMedia()->willReturn([]); - - $tagFactory = $this->prophesize(TagFactoryInterface::class); - $tagFactory->processTags()->willReturn([]); - - $contactRepository = $this->prophesize(ContactRepositoryInterface::class); - $contactRepository->find()->willReturn(new Contact()); - - $this->factory = new NewsFactory($mediaFactory->reveal(), $tagFactory->reveal(), $contactRepository->reveal()); - } - - public function testNewNewsFactory(): void - { - $news = $this->factory->generateNewsFromRequest(new news(), $this->generateNewsContentArray()); - static::assertSame('Test Title', $news->getTitle()); - static::assertSame([ - [ - 'type' => 'title', - 'title' => 'Test', - ], - [ - 'type' => 'editor', - 'text' => '

Test Editor

', - ], - ], $news->getContent()); - static::assertSame('Test Teaser', $news->getTeaser()); - // static::assertInstanceOf(Contact::class, $news->getCreator()); - static::assertSame('2031-08-31 00:00:00', $news->getPublishedAt()->format('Y-m-d H:i:s')); - } - - public function testNewNewsFactoryWithEmptyContent(): void - { - $news = $this->factory->generateNewsFromRequest(new news(), $this->generateNewsContentArrayWithoutContent()); - static::assertSame('Test', $news->getTitle()); - static::assertSame([], $news->getContent()); - static::assertSame('Test', $news->getTeaser()); - static::assertSame('2017-08-31 00:00:00', $news->getPublishedAt()->format('Y-m-d H:i:s')); - } -} diff --git a/processTags/doctrine/phpcr-bundle/tests/Fixtures/App/var/.gitempty b/processTags/doctrine/phpcr-bundle/tests/Fixtures/App/var/.gitempty new file mode 100644 index 0000000..e69de29