From 35c7d55b8c9d45640254296af34ff6c817724fd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 13 Apr 2022 23:17:16 +0200 Subject: [PATCH] fix cs --- .../Controller/NotificationController.php | 91 ++++++----- .../ChillMainBundle/Entity/Notification.php | 154 +++++++++--------- .../ChillMainBundle/Form/NotificationType.php | 2 +- .../Tests/Entity/NotificationTest.php | 36 ++-- .../migrations/Version20220413154743.php | 19 ++- 5 files changed, 153 insertions(+), 149 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Controller/NotificationController.php b/src/Bundle/ChillMainBundle/Controller/NotificationController.php index 5c1508321..f40c85ffb 100644 --- a/src/Bundle/ChillMainBundle/Controller/NotificationController.php +++ b/src/Bundle/ChillMainBundle/Controller/NotificationController.php @@ -32,16 +32,17 @@ use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\Routing\Annotation\Route; use Symfony\Component\Security\Core\Security; use Symfony\Contracts\Translation\TranslatorInterface; +use function in_array; /** * @Route("/{_locale}/notification") */ class NotificationController extends AbstractController { - private EntityManagerInterface $em; - private LoggerInterface $chillLogger; + private EntityManagerInterface $em; + private LoggerInterface $logger; private NotificationHandlerManager $notificationHandlerManager; @@ -74,49 +75,6 @@ class NotificationController extends AbstractController $this->translator = $translator; } - /** - * @Route("/{id}/access_key", name="chill_main_notification_grant_access_by_access_key") - */ - public function getAccessByAccessKey(Notification $notification, Request $request): Response - { - $this->denyAccessUnlessGranted('IS_AUTHENTICATED_REMEMBERED'); - - if (!$this->security->getUser() instanceof User) { - throw new AccessDeniedHttpException('You must be authenticated and a user to create a notification'); - } - - foreach (['accessKey', 'email'] as $param) { - if (!$request->query->has($param)) { - throw new BadRequestHttpException("Missing $param parameter"); - } - } - - if ($notification->getAccessKey() !== $request->query->getAlnum('accessKey')) { - throw new AccessDeniedHttpException('access key is invalid'); - } - - if (!in_array($request->query->get('email'), $notification->getAddressesEmails())) { - return (new Response('The email address is no more associated with this notification')) - ->setStatusCode(Response::HTTP_FORBIDDEN); - } - - $notification->addAddressee($this->security->getUser()); - - $this->getDoctrine()->getManager()->flush(); - - $logMsg = '[Notification] a user is granted access to notification trough an access key'; - $context = [ - 'notificationId' => $notification->getId(), - 'email' => $request->query->get('email'), - 'user' => $this->security->getUser()->getId(), - ]; - - $this->logger->info($logMsg, $context); - $this->chillLogger->info($logMsg, $context); - - return $this->redirectToRoute('chill_main_notification_show', ['id' => $notification->getId()]); - } - /** * @Route("/create", name="chill_main_notification_create") */ @@ -202,6 +160,49 @@ class NotificationController extends AbstractController ]); } + /** + * @Route("/{id}/access_key", name="chill_main_notification_grant_access_by_access_key") + */ + public function getAccessByAccessKey(Notification $notification, Request $request): Response + { + $this->denyAccessUnlessGranted('IS_AUTHENTICATED_REMEMBERED'); + + if (!$this->security->getUser() instanceof User) { + throw new AccessDeniedHttpException('You must be authenticated and a user to create a notification'); + } + + foreach (['accessKey', 'email'] as $param) { + if (!$request->query->has($param)) { + throw new BadRequestHttpException("Missing {$param} parameter"); + } + } + + if ($notification->getAccessKey() !== $request->query->getAlnum('accessKey')) { + throw new AccessDeniedHttpException('access key is invalid'); + } + + if (!in_array($request->query->get('email'), $notification->getAddressesEmails(), true)) { + return (new Response('The email address is no more associated with this notification')) + ->setStatusCode(Response::HTTP_FORBIDDEN); + } + + $notification->addAddressee($this->security->getUser()); + + $this->getDoctrine()->getManager()->flush(); + + $logMsg = '[Notification] a user is granted access to notification trough an access key'; + $context = [ + 'notificationId' => $notification->getId(), + 'email' => $request->query->get('email'), + 'user' => $this->security->getUser()->getId(), + ]; + + $this->logger->info($logMsg, $context); + $this->chillLogger->info($logMsg, $context); + + return $this->redirectToRoute('chill_main_notification_show', ['id' => $notification->getId()]); + } + /** * @Route("/inbox", name="chill_main_notification_my") */ diff --git a/src/Bundle/ChillMainBundle/Entity/Notification.php b/src/Bundle/ChillMainBundle/Entity/Notification.php index d3d4dd0eb..8dad39d12 100644 --- a/src/Bundle/ChillMainBundle/Entity/Notification.php +++ b/src/Bundle/ChillMainBundle/Entity/Notification.php @@ -19,6 +19,8 @@ use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Validator\Constraints as Assert; use Symfony\Component\Validator\Context\ExecutionContextInterface; +use function count; +use function in_array; /** * @ORM\Entity @@ -32,6 +34,11 @@ use Symfony\Component\Validator\Context\ExecutionContextInterface; */ class Notification implements TrackUpdateInterface { + /** + * @ORM\Column(type="text", nullable=false) + */ + private string $accessKey; + private array $addedAddresses = []; /** @@ -40,6 +47,21 @@ class Notification implements TrackUpdateInterface */ private Collection $addressees; + /** + * a list of destinee which will receive notifications. + * + * @var array|string[] + * @ORM\Column(type="json") + */ + private array $addressesEmails = []; + + /** + * a list of emails adresses which were added to the notification. + * + * @var array|string[] + */ + private array $addressesEmailsAdded = []; + private ?ArrayCollection $addressesOnLoad = null; /** @@ -105,25 +127,6 @@ class Notification implements TrackUpdateInterface */ private ?User $updatedBy; - /** - * a list of destinee which will receive notifications - * @var array|string[] - * @ORM\Column(type="json") - */ - private array $addressesEmails = []; - - /** - * a list of emails adresses which were added to the notification - * - * @var array|string[] - */ - private array $addressesEmailsAdded = []; - - /** - * @ORM\Column(type="text", nullable=false) - */ - private string $accessKey; - public function __construct() { $this->addressees = new ArrayCollection(); @@ -133,46 +136,6 @@ class Notification implements TrackUpdateInterface $this->accessKey = bin2hex(openssl_random_pseudo_bytes(24)); } - /** - * @return array|string[] - */ - public function getAddressesEmails(): array - { - return $this->addressesEmails; - } - - /** - * @return array|string[] - */ - public function getAddressesEmailsAdded(): array - { - return $this->addressesEmailsAdded; - } - - /** - * @return string - */ - public function getAccessKey(): string - { - return $this->accessKey; - } - - public function addAddressesEmail(string $email) - { - if (!in_array($email, $this->addressesEmails, true)) { - $this->addressesEmails[] = $email; - $this->addressesEmailsAdded[] = $email; - } - } - - public function removeAddressesEmail(string $email) - { - if (in_array($email, $this->addressesEmails, true)) { - $this->addressesEmails = array_filter($this->addressesEmails, fn ($e) => $e !== $email); - $this->addressesEmailsAdded = array_filter($this->addressesEmailsAdded, fn ($e) => $e !== $email); - } - } - public function addAddressee(User $addressee): self { if (!$this->addressees->contains($addressee)) { @@ -183,12 +146,12 @@ class Notification implements TrackUpdateInterface return $this; } - /** - * @return array - */ - public function getAddedAddresses(): array + public function addAddressesEmail(string $email) { - return $this->addedAddresses; + if (!in_array($email, $this->addressesEmails, true)) { + $this->addressesEmails[] = $email; + $this->addressesEmailsAdded[] = $email; + } } public function addComment(NotificationComment $comment): self @@ -210,6 +173,30 @@ class Notification implements TrackUpdateInterface return $this; } + /** + * @Assert\Callback + * + * @param array $payload + */ + public function assertCountAddresses(ExecutionContextInterface $context, $payload): void + { + if (0 === (count($this->getAddressesEmails()) + count($this->getAddressees()))) { + $context->buildViolation('notification.At least one addressee') + ->atPath('addressees') + ->addViolation(); + } + } + + public function getAccessKey(): string + { + return $this->accessKey; + } + + public function getAddedAddresses(): array + { + return $this->addedAddresses; + } + /** * @return Collection|User[] */ @@ -223,6 +210,22 @@ class Notification implements TrackUpdateInterface return $this->addressees; } + /** + * @return array|string[] + */ + public function getAddressesEmails(): array + { + return $this->addressesEmails; + } + + /** + * @return array|string[] + */ + public function getAddressesEmailsAdded(): array + { + return $this->addressesEmailsAdded; + } + public function getComments(): Collection { return $this->comments; @@ -339,6 +342,14 @@ class Notification implements TrackUpdateInterface return $this; } + public function removeAddressesEmail(string $email) + { + if (in_array($email, $this->addressesEmails, true)) { + $this->addressesEmails = array_filter($this->addressesEmails, static fn ($e) => $e !== $email); + $this->addressesEmailsAdded = array_filter($this->addressesEmailsAdded, static fn ($e) => $e !== $email); + } + } + public function removeComment(NotificationComment $comment): self { $this->comments->removeElement($comment); @@ -408,19 +419,4 @@ class Notification implements TrackUpdateInterface return $this; } - - /** - * @Assert\Callback() - * @param ExecutionContextInterface $context - * @param array $payload - * @return void - */ - public function assertCountAddresses(ExecutionContextInterface $context, $payload): void - { - if (0 === (count($this->getAddressesEmails()) + count($this->getAddressees()))) { - $context->buildViolation('notification.At least one addressee') - ->atPath('addressees') - ->addViolation(); - } - } } diff --git a/src/Bundle/ChillMainBundle/Form/NotificationType.php b/src/Bundle/ChillMainBundle/Form/NotificationType.php index d0c8bc2cd..22fd19baf 100644 --- a/src/Bundle/ChillMainBundle/Form/NotificationType.php +++ b/src/Bundle/ChillMainBundle/Form/NotificationType.php @@ -40,7 +40,7 @@ class NotificationType extends AbstractType ->add('message', ChillTextareaType::class, [ 'required' => false, ]) - ->add('addressesEmails', ChillCollectionType::class, [ + ->add('addressesEmails', ChillCollectionType::class, [ 'label' => 'notification.dest by email', 'help' => 'notification.dest by email help', 'by_reference' => false, diff --git a/src/Bundle/ChillMainBundle/Tests/Entity/NotificationTest.php b/src/Bundle/ChillMainBundle/Tests/Entity/NotificationTest.php index 9fc0d6cb2..0575fa52a 100644 --- a/src/Bundle/ChillMainBundle/Tests/Entity/NotificationTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Entity/NotificationTest.php @@ -88,6 +88,24 @@ final class NotificationTest extends KernelTestCase $this->assertNotContains($user1, $notification->getUnreadBy()->toArray()); } + public function testAddressesEmail(): void + { + $notification = new Notification(); + + $notification->addAddressesEmail('test'); + $notification->addAddressesEmail('other'); + + $this->assertContains('test', $notification->getAddressesEmails()); + $this->assertContains('other', $notification->getAddressesEmails()); + $this->assertContains('test', $notification->getAddressesEmailsAdded()); + $this->assertContains('other', $notification->getAddressesEmailsAdded()); + + $notification->removeAddressesEmail('other'); + + $this->assertNotContains('other', $notification->getAddressesEmails()); + $this->assertNotContains('other', $notification->getAddressesEmailsAdded()); + } + /** * @dataProvider generateNotificationData */ @@ -122,22 +140,4 @@ final class NotificationTest extends KernelTestCase $this->assertContains($addresseeId, $unreadIds); } } - - public function testAddressesEmail(): void - { - $notification = new Notification(); - - $notification->addAddressesEmail('test'); - $notification->addAddressesEmail('other'); - - $this->assertContains('test', $notification->getAddressesEmails()); - $this->assertContains('other', $notification->getAddressesEmails()); - $this->assertContains('test', $notification->getAddressesEmailsAdded()); - $this->assertContains('other', $notification->getAddressesEmailsAdded()); - - $notification->removeAddressesEmail('other'); - - $this->assertNotContains('other', $notification->getAddressesEmails()); - $this->assertNotContains('other', $notification->getAddressesEmailsAdded()); - } } diff --git a/src/Bundle/ChillMainBundle/migrations/Version20220413154743.php b/src/Bundle/ChillMainBundle/migrations/Version20220413154743.php index 87da6a7f8..174419cda 100644 --- a/src/Bundle/ChillMainBundle/migrations/Version20220413154743.php +++ b/src/Bundle/ChillMainBundle/migrations/Version20220413154743.php @@ -1,5 +1,12 @@ addSql('ALTER TABLE chill_main_notification DROP adressesEmails'); + $this->addSql('ALTER TABLE chill_main_notification DROP accessKey'); + } + public function getDescription(): string { return 'add access keys and emails dest to notifications'; @@ -30,10 +43,4 @@ final class Version20220413154743 extends AbstractMigration $this->addSql('ALTER TABLE chill_main_notification ALTER accessKey DROP DEFAULT'); $this->addSql('ALTER TABLE chill_main_notification ALTER accessKey SET NOT NULL'); } - - public function down(Schema $schema): void - { - $this->addSql('ALTER TABLE chill_main_notification DROP adressesEmails'); - $this->addSql('ALTER TABLE chill_main_notification DROP accessKey'); - } }