From 097bf6a2723807c127ac0c51d30b6161114f8eae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20P?= Date: Mon, 11 Nov 2024 17:21:36 +0100 Subject: [PATCH] Implement voting for editor permissions (draft) --- src/Controller/BibliographyController.php | 16 ++++++--- src/Controller/CollectionController.php | 11 ++++-- src/Controller/CollectorController.php | 41 +++++++++++++++++++++++ src/Entity/Bibliography.php | 21 ++++++++++-- src/Entity/Collection.php | 14 ++++++++ src/Entity/Collector.php | 14 ++++++++ src/Repository/BibliographyRepository.php | 23 +++++++++++++ src/Repository/CollectionRepository.php | 11 ++++++ src/Repository/CollectorRepository.php | 11 ++++++ src/Security/Voter/RecordVoter.php | 37 ++++++++++++++++++-- templates/bibliography/index.html.twig | 14 +++++--- templates/collection/index.html.twig | 8 ++--- templates/collector/index.html.twig | 22 ++++++++---- templates/home/index.html.twig | 16 +++------ 14 files changed, 220 insertions(+), 39 deletions(-) diff --git a/src/Controller/BibliographyController.php b/src/Controller/BibliographyController.php index b0cd588..9bbb1a2 100644 --- a/src/Controller/BibliographyController.php +++ b/src/Controller/BibliographyController.php @@ -28,6 +28,11 @@ class BibliographyController extends AbstractController $bibliography->setCollections($collections); $bibliography->setCollectors($collectors); + $repo = $em->getRepository(Bibliography::class); + $isEditable = $repo->hasCreatorEditor($bibliography->getCreator()); + + $bibliography->setEditableStatus($isEditable); + return $this->render('bibliography/index.html.twig', [ 'controller_name' => 'BibliographyController', 'record' => $bibliography, @@ -85,10 +90,11 @@ class BibliographyController extends AbstractController return $this->redirectToRoute('app_home'); } - $em->remove($bibliography); - $em->flush(); - - $this->addFlash('notice', 'Record deleted successfully'); + if ($bibliography) { + $em->remove($bibliography); + $em->flush(); + $this->addFlash('notice', 'Record deleted successfully'); + } return $this->redirectToRoute('app_bibliography_landing'); } @@ -100,7 +106,7 @@ class BibliographyController extends AbstractController public function copy(Bibliography $bibliography, EntityManagerInterface $em): Response { try { - $this->denyAccessUnlessGranted(RecordVoter::EDIT, $bibliography); + $this->denyAccessUnlessGranted(RecordVoter::COPY, $bibliography); } catch (AccessDeniedException) { $this->addFlash('warning', 'You are not authorized to copy this record'); diff --git a/src/Controller/CollectionController.php b/src/Controller/CollectionController.php index 5aeb132..bf73644 100644 --- a/src/Controller/CollectionController.php +++ b/src/Controller/CollectionController.php @@ -20,9 +20,12 @@ class CollectionController extends AbstractController { $repo = $em->getRepository(Bibliography::class); $bibliographies = $repo->findAllByCollection($collection->getId()); - $collection->setBibliographies($bibliographies); + $repo = $em->getRepository(Collection::class); + $isEditable = $repo->hasCreatorEditor($collection->getCreator()); + $collection->setEditableStatus($isEditable); + return $this->render('collection/index.html.twig', [ 'controller_name' => 'CollectionController', 'record' => $collection, @@ -56,8 +59,10 @@ class CollectionController extends AbstractController return $this->redirectToRoute('app_home'); } - $em->remove($collection); - $em->flush(); + if ($collection) { + $em->remove($collection); + $em->flush(); + } $this->addFlash('notice', 'Record deleted successfully'); diff --git a/src/Controller/CollectorController.php b/src/Controller/CollectorController.php index ebaacc5..1dd677e 100644 --- a/src/Controller/CollectorController.php +++ b/src/Controller/CollectorController.php @@ -5,6 +5,7 @@ namespace App\Controller; use App\Entity\Collector; use App\Entity\Collection; use App\Entity\Bibliography; +use App\RecordStatus; use App\Security\Voter\RecordVoter; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; @@ -21,6 +22,10 @@ class CollectorController extends AbstractController $bibliographies = $repo->findAllByCollector($collector->getId()); $collector->setBibliographies($bibliographies); + $repo = $em->getRepository(Collector::class); + $isEditable = $repo->hasCreatorEditor($collector->getCreator()); + $collector->setEditableStatus($isEditable); + return $this->render('collector/index.html.twig', [ 'controller_name' => 'CollectorController', 'record' => $collector, @@ -61,4 +66,40 @@ class CollectorController extends AbstractController return $this->redirectToRoute('app_collector_landing'); } + /** + * @todo Move clone logic to __clone() in Entity or Repository + */ + #[Route('/collector/copy/{id<\d+>}', name: 'app_collector_copy')] + public function copy(Collector $collector, EntityManagerInterface $em): Response + { + try { + $this->denyAccessUnlessGranted(RecordVoter::EDIT, $collector); + } + catch (AccessDeniedException) { + $this->addFlash('warning', 'You are not authorized to copy this record'); + return $this->redirectToRoute('app_home'); + } + + $user = $this->getUser(); + $editor = "{$user->getFirstname()} {$user->getLastName()}"; + + $copy = clone $collector; + $copy->setEditor($editor); + $copy->setOwner($editor); + $copy->setCreator($user->getUsername()); + $repo = $em->getRepository(Bibliography::class); + $copy->setBibliographies( + $repo->findAllByCollector($collector->getId()) + ); + $copy->setName("{$collector->getName()} - Copy"); + $copy->setModifiedAt(new \DateTimeImmutable()); + $copy->setStatus(RecordStatus::Draft->value); + + $em->persist($copy); + $em->flush(); + + $this->addFlash('notice', 'Record copied successfully'); + + return $this->redirectToRoute('app_collector', ['id' => $copy->getId()]); + } } diff --git a/src/Entity/Bibliography.php b/src/Entity/Bibliography.php index cd089c1..e2f84e9 100644 --- a/src/Entity/Bibliography.php +++ b/src/Entity/Bibliography.php @@ -4,14 +4,14 @@ namespace App\Entity; use App\RecordInterface; use App\Repository\BibliographyRepository; -use App\Repository\CollectionRepository; -use App\Repository\CollectorRepository; use DateTimeImmutable; -use Doctrine\ORM\EntityManager; use Doctrine\ORM\Mapping as ORM; use Doctrine\DBAL\Types\Types; use App\RecordStatus; +use ContainerBNNizmi\getBibliographyRepositoryService; use Doctrine\Common\Collections\Collection as DoctrineCollection; +use Doctrine\DBAL\Query\QueryBuilder as QueryQueryBuilder; +use Doctrine\ORM\QueryBuilder; #[ORM\Entity(repositoryClass: BibliographyRepository::class)] #[ORM\Table(name: 'bibliography')] @@ -64,6 +64,9 @@ class Bibliography implements RecordInterface private DoctrineCollection $sites; + // Checks if the record can be edited by an 'editor' user + private bool $isEditable = false; + public function getId(): ?int { return $this->id; @@ -189,4 +192,16 @@ class Bibliography implements RecordInterface return $this; } + + public function getEditableStatus(): bool + { + return $this->isEditable; + } + + public function setEditableStatus(bool $status): static + { + $this->isEditable = $status; + + return $this; + } } diff --git a/src/Entity/Collection.php b/src/Entity/Collection.php index 41106a9..6aec74b 100644 --- a/src/Entity/Collection.php +++ b/src/Entity/Collection.php @@ -83,6 +83,8 @@ class Collection implements RecordInterface #[ORM\ManyToMany(targetEntity: Bibliography::class, inversedBy: 'collection')] private DoctrineCollection $bibliographies; + private bool $isEditable = false; + public function getId(): ?int { return $this->id; @@ -292,4 +294,16 @@ class Collection implements RecordInterface return $this; } + + public function getEditableStatus(): bool + { + return $this->isEditable; + } + + public function setEditableStatus(bool $status): static + { + $this->isEditable = $status; + + return $this; + } } diff --git a/src/Entity/Collector.php b/src/Entity/Collector.php index ae2c316..ee5ebac 100644 --- a/src/Entity/Collector.php +++ b/src/Entity/Collector.php @@ -83,6 +83,8 @@ class Collector implements RecordInterface #[ORM\ManyToMany(targetEntity: Bibliography::class, inversedBy: 'collector')] private DoctrineCollection $bibliographies; + private bool $isEditable = false; + public function getId(): ?int { return $this->id; @@ -292,4 +294,16 @@ class Collector implements RecordInterface return $this; } + + public function getEditableStatus(): bool + { + return $this->isEditable; + } + + public function setEditableStatus(bool $status): static + { + $this->isEditable = $status; + + return $this; + } } diff --git a/src/Repository/BibliographyRepository.php b/src/Repository/BibliographyRepository.php index 2a665bd..01b34c6 100644 --- a/src/Repository/BibliographyRepository.php +++ b/src/Repository/BibliographyRepository.php @@ -3,6 +3,7 @@ namespace App\Repository; use App\Entity\Bibliography; +use App\Entity\User; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; use Doctrine\Persistence\ManagerRegistry; use Doctrine\Common\Collections\ArrayCollection; @@ -18,6 +19,28 @@ class BibliographyRepository extends ServiceEntityRepository parent::__construct($registry, Bibliography::class); } + public function hasCreatorEditor(string $creator): bool + { + $em = $this->getEntityManager(); + $repo = $em->getRepository(User::class); + + $creator = $repo->findOneBy(['username' => $creator]); + + return in_array('ROLE_EDITOR', $creator->getRoles()); + } + + /* + public function delete(int $id): void + { + $qb = $this->createQueryBuilder('b') + ->delete(Bibliography::class, 'bib') + ->where('bib.id = :id') + ->setParameter('id', $id); + + $qb->getQuery()->execute(); + } + */ + public function findAllByCollection(int $collectionId): ?ArrayCollection { $rsm = new ResultSetMappingBuilder($this->getEntityManager()); diff --git a/src/Repository/CollectionRepository.php b/src/Repository/CollectionRepository.php index 34cf587..58bdefe 100644 --- a/src/Repository/CollectionRepository.php +++ b/src/Repository/CollectionRepository.php @@ -3,6 +3,7 @@ namespace App\Repository; use App\Entity\Collection; +use App\Entity\User; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; use Doctrine\Persistence\ManagerRegistry; use Doctrine\Common\Collections\ArrayCollection; @@ -18,6 +19,16 @@ class CollectionRepository extends ServiceEntityRepository parent::__construct($registry, Collection::class); } + public function hasCreatorEditor(string $creator): bool + { + $em = $this->getEntityManager(); + $repo = $em->getRepository(User::class); + + $creator = $repo->findOneBy(['username' => $creator]); + + return in_array('ROLE_EDITOR', $creator->getRoles()); + } + public function findAllByBibliography(int $biblioId): ?ArrayCollection { $rsm = new ResultSetMappingBuilder($this->getEntityManager()); diff --git a/src/Repository/CollectorRepository.php b/src/Repository/CollectorRepository.php index 841823d..2474766 100644 --- a/src/Repository/CollectorRepository.php +++ b/src/Repository/CollectorRepository.php @@ -3,6 +3,7 @@ namespace App\Repository; use App\Entity\Collector; +use App\Entity\User; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; use Doctrine\Persistence\ManagerRegistry; use Doctrine\Common\Collections\ArrayCollection; @@ -18,6 +19,16 @@ class CollectorRepository extends ServiceEntityRepository parent::__construct($registry, Collector::class); } + public function hasCreatorEditor(string $creator): bool + { + $em = $this->getEntityManager(); + $repo = $em->getRepository(User::class); + + $creator = $repo->findOneBy(['username' => $creator]); + + return in_array('ROLE_EDITOR', $creator->getRoles()); + } + public function findAllByBibliography(int $biblioId): ?ArrayCollection { $rsm = new ResultSetMappingBuilder($this->getEntityManager()); diff --git a/src/Security/Voter/RecordVoter.php b/src/Security/Voter/RecordVoter.php index 461e499..e9c4b73 100644 --- a/src/Security/Voter/RecordVoter.php +++ b/src/Security/Voter/RecordVoter.php @@ -5,16 +5,21 @@ namespace App\Security\Voter; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\Voter\Voter; use Symfony\Component\Security\Core\User\UserInterface; +use App\Entity\User; +use App\RecordStatus; +use App\Repository\UserRepository; +use Doctrine\ORM\EntityManager; final class RecordVoter extends Voter { public const EDIT = 'RECORD_EDIT'; public const DELETE = 'RECORD_DELETE'; + public const COPY = 'RECORD_COPY'; public const VIEW = 'RECORD_VIEW'; protected function supports(string $attribute, mixed $subject): bool { - return in_array($attribute, [self::EDIT, self::VIEW, self::DELETE]) + return in_array($attribute, [self::EDIT, self::VIEW, self::DELETE, self::COPY]) && $subject instanceof \App\RecordInterface; } @@ -32,11 +37,37 @@ final class RecordVoter extends Voter // TODO: Better way to check roles? switch ($attribute) { case self::EDIT: - case self::DELETE: + $editorAllowed = true; + if (in_array('ROLE_EDITOR', $roles)) { + $creator = $subject->getCreator(); + $isCreator = $creator === $user->getUsername(); + $creatorIsEditor = $subject->getEditableStatus(); + + $editorAllowed &= $isCreator || $creatorIsEditor; + } + return in_array('ROLE_ADMIN', $roles) - || in_array('ROLE_REVISOR', $roles); + || in_array('ROLE_REVISOR', $roles) + || $editorAllowed; + case self::DELETE: + $editorAllowed = true; + if (in_array('ROLE_EDITOR', $roles)) { + $isCreator = $subject->getCreator() === $user->getUsername(); + $isValidStatus = match($subject->getStatus()) { + RecordStatus::Draft => true, + RecordStatus::Complete => true, + default => false + }; + + $editorAllowed &= $isCreator && $isValidStatus; + } + + return in_array('ROLE_ADMIN', $roles) + || in_array('ROLE_REVISOR', $roles) + || $editorAllowed; break; + case self::COPY: case self::VIEW: return ! in_array('ROLE_READER', $roles); break; diff --git a/templates/bibliography/index.html.twig b/templates/bibliography/index.html.twig index e349e8a..4f8cda6 100644 --- a/templates/bibliography/index.html.twig +++ b/templates/bibliography/index.html.twig @@ -27,16 +27,20 @@
- {% if is_granted('ROLE_ADMIN') or is_granted('ROLE_REVISOR') %} + {% if not is_granted('ROLE_READER') %}
+ {% if is_granted('ROLE_REVISOR') or + is_granted('ROLE_ADMIN') or + record.editableStatus %} + {% endif %} Copy @@ -44,14 +48,16 @@ - + data-delete-record-target="path" data-action="click->delete-record#warn"> Delete - + + {% endif %}
{% endif %} diff --git a/templates/collection/index.html.twig b/templates/collection/index.html.twig index a8cfdae..9b919be 100644 --- a/templates/collection/index.html.twig +++ b/templates/collection/index.html.twig @@ -43,14 +43,14 @@ - + data-delete-record-target="path" data-action="click->delete-record#warn"> Delete - +
{% endif %} @@ -74,7 +74,7 @@ External identifier(s){{ record.externalIdentifier }} External link(s){{ record.link }} Subject headings{{ record.subjectHeadings }} - ArCOA URI{{ record.uri }} + ArCOA URIarcoa.cnr.it/collection/{{ record.id }} Editorial notes{{ record.notes }} diff --git a/templates/collector/index.html.twig b/templates/collector/index.html.twig index d1a1f7e..b68fd44 100644 --- a/templates/collector/index.html.twig +++ b/templates/collector/index.html.twig @@ -7,6 +7,15 @@

Collector

{{ record.name }}

+ {% for message in app.flashes('notice') %} +
+ + {{ message }} +
+ {% endfor %} +
{% endif %} @@ -64,7 +74,7 @@ External identifier(s){{ record.externalIdentifier }} External link(s){{ record.link }} Subject headings{{ record.subjectHeadings }} - ArCOA URI{{ record.uri }} + ArCOA URIarcoa.cnr.it/collector/{{ record.id }} Editorial notes{{ record.notes }} diff --git a/templates/home/index.html.twig b/templates/home/index.html.twig index ec95002..a6fe78d 100644 --- a/templates/home/index.html.twig +++ b/templates/home/index.html.twig @@ -4,10 +4,13 @@ {% block rightpanel %} {% for message in app.flashes('warning') %} -
+

Warning

- +
{{ message }}
@@ -40,13 +43,4 @@ - {% endblock %}