From bd3919efcb11f314071525bdc97979a3c9fdb4a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Sun, 26 Dec 2021 01:00:50 +0100 Subject: [PATCH] notification: store users which are unread instead of read, and in dedicated table The query "which are the unread notification" is much more frequent than the read one. We then store the unread items in a dedicated table. --- composer.json | 11 +- .../Controller/NotificationController.php | 58 ++++++++-- .../ChillMainBundle/Entity/Notification.php | 57 +++++++--- .../Repository/NotificationRepository.php | 102 +++++++++++++++--- .../views/Notification/list.html.twig | 86 +++++++++------ .../migrations/Version20211225231532.php | 39 +++++++ .../translations/messages.fr.yml | 6 +- .../Controller/HouseholdMemberController.php | 2 +- .../config/services/notification.yaml | 1 + 9 files changed, 283 insertions(+), 79 deletions(-) create mode 100644 src/Bundle/ChillMainBundle/migrations/Version20211225231532.php diff --git a/composer.json b/composer.json index a46446531..312cd1cee 100644 --- a/composer.json +++ b/composer.json @@ -11,7 +11,6 @@ "php": "^7.4", "champs-libres/async-uploader-bundle": "dev-sf4#d57134aee8e504a83c902ff0cf9f8d36ac418290", "champs-libres/wopi-bundle": "dev-master#59b468503b9413f8d588ef9e626e7675560db3d8", - "ocramius/package-versions": "^1.10", "doctrine/doctrine-bundle": "^2.1", "doctrine/doctrine-migrations-bundle": "^3.0", "doctrine/orm": "^2.7", @@ -22,6 +21,7 @@ "knplabs/knp-time-bundle": "^1.12", "league/csv": "^9.7.1", "nyholm/psr7": "^1.4", + "ocramius/package-versions": "^1.10", "phpoffice/phpspreadsheet": "^1.16", "ramsey/uuid-doctrine": "^1.7", "sensio/framework-extra-bundle": "^5.5", @@ -47,6 +47,7 @@ "twig/extra-bundle": "^3.0", "twig/intl-extra": "^3.0", "twig/markdown-extra": "^3.3", + "twig/string-extra": "^3.3", "twig/twig": "^3.0" }, "conflict": { @@ -72,7 +73,13 @@ "bin-dir": "bin", "optimize-autoloader": true, "sort-packages": true, - "vendor-dir": "tests/app/vendor" + "vendor-dir": "tests/app/vendor", + "allow-plugins": { + "composer/package-versions-deprecated": true, + "phpstan/extension-installer": true, + "ergebnis/composer-normalize": true, + "phpro/grumphp": true + } }, "autoload": { "psr-4": { diff --git a/src/Bundle/ChillMainBundle/Controller/NotificationController.php b/src/Bundle/ChillMainBundle/Controller/NotificationController.php index 19d5142ca..c3ec0e549 100644 --- a/src/Bundle/ChillMainBundle/Controller/NotificationController.php +++ b/src/Bundle/ChillMainBundle/Controller/NotificationController.php @@ -118,9 +118,9 @@ class NotificationController extends AbstractController } /** - * @Route("/my", name="chill_main_notification_my") + * @Route("/inbox", name="chill_main_notification_my") */ - public function myAction(): Response + public function inboxAction(): Response { $this->denyAccessUnlessGranted('IS_AUTHENTICATED_REMEMBERED'); $currentUser = $this->security->getUser(); @@ -134,21 +134,61 @@ class NotificationController extends AbstractController $offset = $paginator->getCurrentPage()->getFirstItemNumber() ); + return $this->render('@ChillMain/Notification/list.html.twig', [ + 'datas' => $this->itemsForTemplate($notifications), + 'notifications' => $notifications, + 'paginator' => $paginator, + 'step' => 'inbox', + 'unreads' => $this->countUnread(), + ]); + } + + /** + * @Route("/sent", name="chill_main_notification_sent") + */ + public function sentAction(): Response + { + $this->denyAccessUnlessGranted('IS_AUTHENTICATED_REMEMBERED'); + $currentUser = $this->security->getUser(); + + $notificationsNbr = $this->notificationRepository->countAllForSender($currentUser); + $paginator = $this->paginatorFactory->create($notificationsNbr); + + $notifications = $this->notificationRepository->findAllForSender( + $currentUser, + $limit = $paginator->getItemsPerPage(), + $offset = $paginator->getCurrentPage()->getFirstItemNumber() + ); + + return $this->render('@ChillMain/Notification/list.html.twig', [ + 'datas' => $this->itemsForTemplate($notifications), + 'notifications' => $notifications, + 'paginator' => $paginator, + 'step' => 'sent', + 'unreads' => $this->countUnread(), + ]); + } + + private function countUnread(): array + { + return [ + 'sent' => $this->notificationRepository->countUnreadByUserWhereSender($this->security->getUser()), + 'inbox' => $this->notificationRepository->countUnreadByUserWhereAddressee($this->security->getUser()), + ]; + } + + private function itemsForTemplate(array $notifications): array + { $templateData = []; foreach ($notifications as $notification) { - $data = [ + $templateData[] = [ 'template' => $this->notificationHandlerManager->getTemplate($notification), 'template_data' => $this->notificationHandlerManager->getTemplateData($notification), 'notification' => $notification, ]; - $templateData[] = $data; } - return $this->render('@ChillMain/Notification/show.html.twig', [ - 'datas' => $templateData, - 'notifications' => $notifications, - 'paginator' => $paginator, - ]); + return $templateData; } } diff --git a/src/Bundle/ChillMainBundle/Entity/Notification.php b/src/Bundle/ChillMainBundle/Entity/Notification.php index a9b576f1f..c484f9826 100644 --- a/src/Bundle/ChillMainBundle/Entity/Notification.php +++ b/src/Bundle/ChillMainBundle/Entity/Notification.php @@ -50,11 +50,6 @@ class Notification */ private string $message = ''; - /** - * @ORM\Column(type="json") - */ - private array $read = []; - /** * @ORM\Column(type="string", length=255) */ @@ -71,9 +66,16 @@ class Notification */ private User $sender; + /** + * @ORM\ManyToMany(targetEntity=User::class) + * @ORM\JoinTable(name="chill_main_notification_addresses_unread") + */ + private Collection $unreadBy; + public function __construct() { $this->addressees = new ArrayCollection(); + $this->unreadBy = new ArrayCollection(); $this->setDate(new DateTimeImmutable()); } @@ -81,6 +83,16 @@ class Notification { if (!$this->addressees->contains($addressee)) { $this->addressees[] = $addressee; + $this->addUnreadBy($addressee); + } + + return $this; + } + + public function addUnreadBy(User $user): self + { + if (!$this->unreadBy->contains($user)) { + $this->unreadBy->add($user); } return $this; @@ -109,11 +121,6 @@ class Notification return $this->message; } - public function getRead(): array - { - return $this->read; - } - public function getRelatedEntityClass(): ?string { return $this->relatedEntityClass; @@ -129,9 +136,32 @@ class Notification return $this->sender; } + public function isReadBy(User $user): bool + { + return !$this->unreadBy->contains($user); + } + + public function markAsReadBy(User $user): self + { + return $this->removeUnreadBy($user); + } + + public function markAsUnreadBy(User $user): self + { + return $this->addUnreadBy($user); + } + public function removeAddressee(User $addressee): self { $this->addressees->removeElement($addressee); + $this->unreadBy->removeElement($addressee); + + return $this; + } + + public function removeUnreadBy(User $user): self + { + $this->unreadBy->removeElement($user); return $this; } @@ -150,13 +180,6 @@ class Notification return $this; } - public function setRead(array $read): self - { - $this->read = $read; - - return $this; - } - public function setRelatedEntityClass(string $relatedEntityClass): self { $this->relatedEntityClass = $relatedEntityClass; diff --git a/src/Bundle/ChillMainBundle/Repository/NotificationRepository.php b/src/Bundle/ChillMainBundle/Repository/NotificationRepository.php index 4af19550c..eea2d41e1 100644 --- a/src/Bundle/ChillMainBundle/Repository/NotificationRepository.php +++ b/src/Bundle/ChillMainBundle/Repository/NotificationRepository.php @@ -13,25 +13,75 @@ namespace Chill\MainBundle\Repository; use Chill\MainBundle\Entity\Notification; use Chill\MainBundle\Entity\User; +use Doctrine\DBAL\Types\Types; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityRepository; use Doctrine\ORM\Query; +use Doctrine\ORM\QueryBuilder; use Doctrine\Persistence\ObjectRepository; final class NotificationRepository implements ObjectRepository { + private EntityManagerInterface $em; + private EntityRepository $repository; public function __construct(EntityManagerInterface $entityManager) { + $this->em = $entityManager; $this->repository = $entityManager->getRepository(Notification::class); } - public function countAllForAttendee(User $addressee): int // TODO passer à attendees avec S + public function countAllForAttendee(User $addressee): int { - $query = $this->queryAllForAttendee($addressee, $countQuery = true); + return $this->queryByAddressee($addressee) + ->select('count(n)') + ->getQuery() + ->getSingleScalarResult(); + } - return $query->getSingleScalarResult(); + public function countAllForSender(User $sender): int + { + return $this->queryBySender($sender) + ->select('count(n)') + ->getQuery() + ->getSingleScalarResult(); + } + + public function countUnreadByUser(User $user): int + { + $sql = 'SELECT count(*) AS c FROM chill_main_notification_addresses_unread WHERE user_id = ?'; + + $rsm = new Query\ResultSetMapping(); + $rsm->addScalarResult('c', 'c', Types::INTEGER); + + $nq = $this->em->createNativeQuery($sql, $rsm); + + return $nq->getSingleScalarResult(); + } + + public function countUnreadByUserWhereAddressee(User $user): int + { + $qb = $this->repository->createQueryBuilder('n'); + $qb + ->select('count(n)') + ->where($qb->expr()->isMemberOf(':user', 'n.addressees')) + ->andWhere($qb->expr()->isMemberOf(':user', 'n.unreadBy')) + ->setParameter('user', $user); + + return $qb->getQuery()->getSingleScalarResult(); + } + + public function countUnreadByUserWhereSender(User $user): int + { + $qb = $this->repository->createQueryBuilder('n'); + $qb + ->select('count(n)') + ->where($qb->expr()->eq('n.sender', ':user')) + ->andWhere($qb->expr()->isMemberOf(':user', 'n.unreadBy')) + ->setParameter('user', $user); + + return $qb->getQuery()->getSingleScalarResult(); } public function find($id, $lockMode = null, $lockVersion = null): ?Notification @@ -53,9 +103,9 @@ final class NotificationRepository implements ObjectRepository * * @return Notification[] */ - public function findAllForAttendee(User $addressee, $limit = null, $offset = null): array // TODO passer à attendees avec S + public function findAllForAttendee(User $addressee, $limit = null, $offset = null): array { - $query = $this->queryAllForAttendee($addressee); + $query = $this->queryByAddressee($addressee)->select('n'); if ($limit) { $query = $query->setMaxResults($limit); @@ -65,7 +115,22 @@ final class NotificationRepository implements ObjectRepository $query = $query->setFirstResult($offset); } - return $query->getResult(); + return $query->getQuery()->getResult(); + } + + public function findAllForSender(User $sender, $limit = null, $offset = null): array + { + $query = $this->queryBySender($sender)->select('n'); + + if ($limit) { + $query = $query->setMaxResults($limit); + } + + if ($offset) { + $query = $query->setFirstResult($offset); + } + + return $query->getQuery()->getResult(); } /** @@ -89,22 +154,25 @@ final class NotificationRepository implements ObjectRepository return Notification::class; } - private function queryAllForAttendee(User $addressee, bool $countQuery = false): Query + private function queryByAddressee(User $addressee, bool $countQuery = false): QueryBuilder { $qb = $this->repository->createQueryBuilder('n'); - $select = 'n'; - - if ($countQuery) { - $select = 'count(n)'; - } - $qb - ->select($select) - ->join('n.addressees', 'a') - ->where('a = :addressee') + ->where($qb->expr()->isMemberOf(':addressee', 'n.addressees')) ->setParameter('addressee', $addressee); - return $qb->getQuery(); + return $qb; + } + + private function queryBySender(User $sender): QueryBuilder + { + $qb = $this->repository->createQueryBuilder('n'); + + $qb + ->where($qb->expr()->eq('n.sender', ':sender')) + ->setParameter('sender', $sender); + + return $qb; } } diff --git a/src/Bundle/ChillMainBundle/Resources/views/Notification/list.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Notification/list.html.twig index 4f4f81ba2..4a1f504f4 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Notification/list.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Notification/list.html.twig @@ -1,43 +1,65 @@ {% extends "@ChillMain/layout.html.twig" %} +{% block title 'notification.List'|trans %} + {% block content %}
-
-

{{ "Notifications list" | trans }}

+
+
+

{{ block('title') }}

- {%for data in datas %} - {% set notification = data.notification %} + -
-
{{ 'Message'|trans }}
-
{{ notification.message }}
-
+ {% if datas|length == 0 %} + {% if step == 'inbox' %} +

{{ 'notification.Any notification received'|trans }}

+ {% else %} +

{{ 'notification.Any notification sent'|trans }}

+ {% endif %} + {% else %} +
+ {% for data in datas %} + {% set notification = data.notification %} +
+ {% include data.template with data.template_data %} +
+
+ {% if step == 'inbox' %} +
{{ 'notification.from'|trans }}: {{ notification.sender|chill_entity_render_string }}
+ {% endif %} +
{{ 'notification.adressees'|trans }}{% for a in notification.addressees %}{{ a|chill_entity_render_string }}{% if not loop.last %}, {% endif %}{% endfor %}
+
{{ notification.date|format_datetime('long', 'short') }}
+
+
+ {{ notification.message|u.truncate(250, '…', false)|chill_markdown_to_html }} +
+
+
+ {% endfor %} -
-
{{ 'Date'|trans }}
-
{{ notification.date | date('long') }}
-
+
+ {% endif %} +
- -
-
{{ 'Sender'|trans }}
-
{{ notification.sender }}
-
- -
-
{{ 'Addressees'|trans }}
-
{{ notification.addressees |join(', ') }}
-
- -
-
{{ 'Entity'|trans }}
-
- {% include data.template with data.template_data %} -
-
- {% else %} -

{{ notification.Any notification received }}

- {% endfor %}
{% endblock content %} diff --git a/src/Bundle/ChillMainBundle/migrations/Version20211225231532.php b/src/Bundle/ChillMainBundle/migrations/Version20211225231532.php new file mode 100644 index 000000000..eabcce641 --- /dev/null +++ b/src/Bundle/ChillMainBundle/migrations/Version20211225231532.php @@ -0,0 +1,39 @@ +addSql('DROP TABLE chill_main_notification_addresses_unread'); + $this->addSql('ALTER TABLE chill_main_notification ADD read JSONB DEFAULT \'[]\''); + } + + public function getDescription(): string + { + return 'Store notification readed by user in a specific table'; + } + + public function up(Schema $schema): void + { + $this->addSql('CREATE TABLE chill_main_notification_addresses_unread (notification_id INT NOT NULL, user_id INT NOT NULL, PRIMARY KEY(notification_id, user_id))'); + $this->addSql('CREATE INDEX IDX_154A075FEF1A9D84 ON chill_main_notification_addresses_unread (notification_id)'); + $this->addSql('CREATE INDEX IDX_154A075FA76ED395 ON chill_main_notification_addresses_unread (user_id)'); + $this->addSql('ALTER TABLE chill_main_notification_addresses_unread ADD CONSTRAINT FK_154A075FEF1A9D84 FOREIGN KEY (notification_id) REFERENCES chill_main_notification (id) ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE'); + $this->addSql('ALTER TABLE chill_main_notification_addresses_unread ADD CONSTRAINT FK_154A075FA76ED395 FOREIGN KEY (user_id) REFERENCES users (id) ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE'); + $this->addSql('ALTER TABLE chill_main_notification DROP read'); + } +} diff --git a/src/Bundle/ChillMainBundle/translations/messages.fr.yml b/src/Bundle/ChillMainBundle/translations/messages.fr.yml index 1d6fa70e0..9b810a837 100644 --- a/src/Bundle/ChillMainBundle/translations/messages.fr.yml +++ b/src/Bundle/ChillMainBundle/translations/messages.fr.yml @@ -354,4 +354,8 @@ Created by: Créé par notification: Notify: Notifier - Notification create: Notification envoyée + Notification created: Notification envoyée + Any notification received: Aucune notification reçue + Any notification sent: Aucune notification envoyée + Notifications received: Notifications reçues + Notifications sent: Notification envoyées diff --git a/src/Bundle/ChillPersonBundle/Controller/HouseholdMemberController.php b/src/Bundle/ChillPersonBundle/Controller/HouseholdMemberController.php index ddf5d9683..78234d114 100644 --- a/src/Bundle/ChillPersonBundle/Controller/HouseholdMemberController.php +++ b/src/Bundle/ChillPersonBundle/Controller/HouseholdMemberController.php @@ -187,7 +187,7 @@ class HouseholdMemberController extends ApiController $_format, ['groups' => ['read']] ); - } catch (Exception\InvalidArgumentException | Exception\UnexpectedValueException $e) { + } catch (Exception\InvalidArgumentException|Exception\UnexpectedValueException $e) { throw new BadRequestException("Deserialization error: {$e->getMessage()}", 45896, $e); } diff --git a/src/Bundle/ChillPersonBundle/config/services/notification.yaml b/src/Bundle/ChillPersonBundle/config/services/notification.yaml index 58187defb..c5d04f983 100644 --- a/src/Bundle/ChillPersonBundle/config/services/notification.yaml +++ b/src/Bundle/ChillPersonBundle/config/services/notification.yaml @@ -1,3 +1,4 @@ services: Chill\PersonBundle\Notification\AccompanyingPeriodNotificationRenderer: autowire: true + autoconfigure: true