From b6e03795838d7e272623d49781246bc462cf2279 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Fri, 20 May 2022 15:52:02 +0200 Subject: [PATCH] fix cs and add EntityToIdTransformer --- .../Controller/CalendarController.php | 9 +- .../Controller/CalendarRangeAPIController.php | 25 ++-- .../ChillCalendarBundle/Entity/Calendar.php | 57 +++++---- .../Entity/CalendarRange.php | 1 - .../ChillCalendarBundle/Entity/Invite.php | 3 +- .../ChillCalendarBundle/Form/CalendarType.php | 4 +- .../Menu/AccompanyingCourseMenuBuilder.php | 8 +- .../RemoteCalendarCompilerPass.php | 2 - .../Repository/CalendarRangeRepository.php | 48 +++---- .../Security/Voter/CalendarVoter.php | 31 +++-- .../Tests/Entity/CalendarTest.php | 16 ++- .../IdToEntityDataTransformer.php | 82 ++++++++++++ .../Repository/LocationRepository.php | 1 - .../IdToEntityDataTransformerTest.php | 118 ++++++++++++++++++ 14 files changed, 308 insertions(+), 97 deletions(-) create mode 100644 src/Bundle/ChillMainBundle/Form/DataTransformer/IdToEntityDataTransformer.php create mode 100644 src/Bundle/ChillMainBundle/Tests/Form/DataTransformer/IdToEntityDataTransformerTest.php diff --git a/src/Bundle/ChillCalendarBundle/Controller/CalendarController.php b/src/Bundle/ChillCalendarBundle/Controller/CalendarController.php index ec53cff1b..edc108f5d 100644 --- a/src/Bundle/ChillCalendarBundle/Controller/CalendarController.php +++ b/src/Bundle/ChillCalendarBundle/Controller/CalendarController.php @@ -39,18 +39,18 @@ class CalendarController extends AbstractController { private AuthorizationHelper $authorizationHelper; + private CalendarRepository $calendarRepository; + private EventDispatcherInterface $eventDispatcher; private LoggerInterface $logger; private PaginatorFactory $paginator; - private SerializerInterface $serializer; - - private CalendarRepository $calendarRepository; - private RemoteCalendarConnectorInterface $remoteCalendarConnector; + private SerializerInterface $serializer; + private UserRepository $userRepository; public function __construct( @@ -282,6 +282,7 @@ class CalendarController extends AbstractController // } $entity = new Calendar(); + if ($request->query->has('mainUser')) { $entity->setMainUser($this->userRepository->find($request->query->getInt('mainUser'))); } diff --git a/src/Bundle/ChillCalendarBundle/Controller/CalendarRangeAPIController.php b/src/Bundle/ChillCalendarBundle/Controller/CalendarRangeAPIController.php index 507810dcb..da063ea02 100644 --- a/src/Bundle/ChillCalendarBundle/Controller/CalendarRangeAPIController.php +++ b/src/Bundle/ChillCalendarBundle/Controller/CalendarRangeAPIController.php @@ -15,13 +15,13 @@ use Chill\CalendarBundle\Repository\CalendarRangeRepository; use Chill\MainBundle\CRUD\Controller\ApiController; use Chill\MainBundle\Entity\User; use Chill\MainBundle\Serializer\Model\Collection; +use DateTimeImmutable; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; -use Symfony\Component\Routing\Annotation\Route; -use function count; +use Symfony\Component\Routing\Annotation\Route; class CalendarRangeAPIController extends ApiController { @@ -47,8 +47,10 @@ class CalendarRangeAPIController extends ApiController throw new BadRequestHttpException('You must provide a dateFrom parameter'); } - if (false === $dateFrom = \DateTimeImmutable::createFromFormat(\DateTimeImmutable::ATOM, - $request->query->get('dateFrom'))) { + if (false === $dateFrom = DateTimeImmutable::createFromFormat( + DateTimeImmutable::ATOM, + $request->query->get('dateFrom') + )) { throw new BadRequestHttpException('dateFrom not parsable'); } @@ -56,15 +58,22 @@ class CalendarRangeAPIController extends ApiController throw new BadRequestHttpException('You must provide a dateTo parameter'); } - if (false === $dateTo = \DateTimeImmutable::createFromFormat(\DateTimeImmutable::ATOM, - $request->query->get('dateTo'))) { + if (false === $dateTo = DateTimeImmutable::createFromFormat( + DateTimeImmutable::ATOM, + $request->query->get('dateTo') + )) { throw new BadRequestHttpException('dateTo not parsable'); } $total = $this->calendarRangeRepository->countByAvailableRangesForUser($user, $dateFrom, $dateTo); $paginator = $this->getPaginatorFactory()->create($total); - $ranges = $this->calendarRangeRepository->findByAvailableRangesForUser($user, $dateFrom, $dateTo, - $paginator->getItemsPerPage(), $paginator->getCurrentPageFirstItemNumber()); + $ranges = $this->calendarRangeRepository->findByAvailableRangesForUser( + $user, + $dateFrom, + $dateTo, + $paginator->getItemsPerPage(), + $paginator->getCurrentPageFirstItemNumber() + ); $collection = new Collection($ranges, $paginator); diff --git a/src/Bundle/ChillCalendarBundle/Entity/Calendar.php b/src/Bundle/ChillCalendarBundle/Entity/Calendar.php index 8fc8e9358..1a4fb02b4 100644 --- a/src/Bundle/ChillCalendarBundle/Entity/Calendar.php +++ b/src/Bundle/ChillCalendarBundle/Entity/Calendar.php @@ -27,11 +27,12 @@ use DateTimeImmutable; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping as ORM; +use LogicException; use Symfony\Component\Serializer\Annotation as Serializer; use Symfony\Component\Validator\Constraints\NotBlank; use Symfony\Component\Validator\Constraints\Range; -use Symfony\Component\Validator\Mapping\ClassMetadata; +use Symfony\Component\Validator\Mapping\ClassMetadata; use function in_array; /** @@ -159,8 +160,8 @@ class Calendar implements TrackCreationInterface, TrackUpdateInterface public function addInvite(Invite $invite): self { - if ($invite->getCalendar() instanceof Calendar && $this !== $invite->getCalendar()) { - throw new \LogicException('Not allowed to move an invitation to another Calendar'); + if ($invite->getCalendar() instanceof Calendar && $invite->getCalendar() !== $this) { + throw new LogicException('Not allowed to move an invitation to another Calendar'); } $this->invites[] = $invite; @@ -169,29 +170,6 @@ class Calendar implements TrackCreationInterface, TrackUpdateInterface return $this; } - public function addUser(User $user): self - { - if (!$this->getUsers()->contains($user)) { - $this->addInvite((new Invite())->setUser($user)); - } - - return $this; - } - - public function removeUser(User $user): self - { - if (!$this->getUsers()->contains($user)) { - return $this; - } - - $invite = $this->invites - ->filter(function (Invite $invite) use ($user) { return $invite->getUser() === $user; }) - ->first(); - $this->removeInvite($invite); - - return $this; - } - public function addPerson(Person $person): self { $this->persons[] = $person; @@ -206,6 +184,15 @@ class Calendar implements TrackCreationInterface, TrackUpdateInterface return $this; } + public function addUser(User $user): self + { + if (!$this->getUsers()->contains($user)) { + $this->addInvite((new Invite())->setUser($user)); + } + + return $this; + } + public function getAccompanyingPeriod(): ?AccompanyingPeriod { return $this->accompanyingPeriod; @@ -334,7 +321,7 @@ class Calendar implements TrackCreationInterface, TrackUpdateInterface */ public function getUsers(): Collection { - return $this->getInvites()->map(function (Invite $i) { return $i->getUser(); }); + return $this->getInvites()->map(static function (Invite $i) { return $i->getUser(); }); } public static function loadValidatorMetadata(ClassMetadata $metadata): void @@ -374,6 +361,20 @@ class Calendar implements TrackCreationInterface, TrackUpdateInterface return $this; } + public function removeUser(User $user): self + { + if (!$this->getUsers()->contains($user)) { + return $this; + } + + $invite = $this->invites + ->filter(static function (Invite $invite) use ($user) { return $invite->getUser() === $user; }) + ->first(); + $this->removeInvite($invite); + + return $this; + } + public function setAccompanyingPeriod(?AccompanyingPeriod $accompanyingPeriod): self { $this->accompanyingPeriod = $accompanyingPeriod; @@ -450,6 +451,4 @@ class Calendar implements TrackCreationInterface, TrackUpdateInterface return $this; } - - } diff --git a/src/Bundle/ChillCalendarBundle/Entity/CalendarRange.php b/src/Bundle/ChillCalendarBundle/Entity/CalendarRange.php index 6b0f79c9b..114acf7aa 100644 --- a/src/Bundle/ChillCalendarBundle/Entity/CalendarRange.php +++ b/src/Bundle/ChillCalendarBundle/Entity/CalendarRange.php @@ -11,7 +11,6 @@ declare(strict_types=1); namespace Chill\CalendarBundle\Entity; -use Chill\CalendarBundle\Repository\CalendarRangeRepository; use Chill\MainBundle\Doctrine\Model\TrackCreationInterface; use Chill\MainBundle\Doctrine\Model\TrackCreationTrait; use Chill\MainBundle\Doctrine\Model\TrackUpdateInterface; diff --git a/src/Bundle/ChillCalendarBundle/Entity/Invite.php b/src/Bundle/ChillCalendarBundle/Entity/Invite.php index 96713ed2a..b6a25e412 100644 --- a/src/Bundle/ChillCalendarBundle/Entity/Invite.php +++ b/src/Bundle/ChillCalendarBundle/Entity/Invite.php @@ -18,6 +18,7 @@ use Chill\MainBundle\Doctrine\Model\TrackUpdateInterface; use Chill\MainBundle\Doctrine\Model\TrackUpdateTrait; use Chill\MainBundle\Entity\User; use Doctrine\ORM\Mapping as ORM; +use LogicException; /** * @ORM\Table(name="chill_calendar.invite") @@ -98,7 +99,7 @@ class Invite implements TrackUpdateInterface, TrackCreationInterface public function setUser(?User $user): self { if ($user instanceof User && $this->user instanceof User && $user !== $this->user) { - throw new \LogicException("Not allowed to associate an invite to a different user"); + throw new LogicException('Not allowed to associate an invite to a different user'); } $this->user = $user; diff --git a/src/Bundle/ChillCalendarBundle/Form/CalendarType.php b/src/Bundle/ChillCalendarBundle/Form/CalendarType.php index 35f0b5075..76040d1f4 100644 --- a/src/Bundle/ChillCalendarBundle/Form/CalendarType.php +++ b/src/Bundle/ChillCalendarBundle/Form/CalendarType.php @@ -14,9 +14,7 @@ namespace Chill\CalendarBundle\Form; use Chill\CalendarBundle\Entity\Calendar; use Chill\CalendarBundle\Entity\CalendarRange; use Chill\CalendarBundle\Entity\CancelReason; -use Chill\CalendarBundle\Entity\Invite; use Chill\MainBundle\Entity\Location; -use Chill\MainBundle\Entity\User; use Chill\MainBundle\Form\Type\CommentType; use Chill\MainBundle\Form\Type\PickUserDynamicType; use Chill\MainBundle\Templating\TranslatableStringHelper; @@ -126,6 +124,7 @@ class CalendarType extends AbstractType if (null === $personsAsString) { return []; } + return array_map( fn (string $id): ?Person => $this->om->getRepository(Person::class)->findOneBy(['id' => (int) $id]), explode(',', $personsAsString) @@ -149,6 +148,7 @@ class CalendarType extends AbstractType if (null === $thirdpartyAsString) { return []; } + return array_map( fn (string $id): ?ThirdParty => $this->om->getRepository(ThirdParty::class)->findOneBy(['id' => (int) $id]), explode(',', $thirdpartyAsString) diff --git a/src/Bundle/ChillCalendarBundle/Menu/AccompanyingCourseMenuBuilder.php b/src/Bundle/ChillCalendarBundle/Menu/AccompanyingCourseMenuBuilder.php index 9f5d2c7ca..fdc07d56d 100644 --- a/src/Bundle/ChillCalendarBundle/Menu/AccompanyingCourseMenuBuilder.php +++ b/src/Bundle/ChillCalendarBundle/Menu/AccompanyingCourseMenuBuilder.php @@ -13,20 +13,16 @@ namespace Chill\CalendarBundle\Menu; use Chill\CalendarBundle\Security\Voter\CalendarVoter; use Chill\MainBundle\Routing\LocalMenuBuilderInterface; -use Chill\MainBundle\Security\Authorization\AuthorizationHelper; -use Chill\PersonBundle\Entity\AccompanyingPeriod; use Knp\Menu\MenuItem; -use Symfony\Bundle\SecurityBundle\SecurityBundle; -use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Security; use Symfony\Contracts\Translation\TranslatorInterface; class AccompanyingCourseMenuBuilder implements LocalMenuBuilderInterface { - private Security $security; - protected TranslatorInterface $translator; + private Security $security; + public function __construct( Security $security, TranslatorInterface $translator diff --git a/src/Bundle/ChillCalendarBundle/RemoteCalendar/DependencyInjection/RemoteCalendarCompilerPass.php b/src/Bundle/ChillCalendarBundle/RemoteCalendar/DependencyInjection/RemoteCalendarCompilerPass.php index 7d164a995..852dd6893 100644 --- a/src/Bundle/ChillCalendarBundle/RemoteCalendar/DependencyInjection/RemoteCalendarCompilerPass.php +++ b/src/Bundle/ChillCalendarBundle/RemoteCalendar/DependencyInjection/RemoteCalendarCompilerPass.php @@ -35,7 +35,6 @@ class RemoteCalendarCompilerPass implements CompilerPassInterface if ($config['remote_calendars_sync']['microsoft_graph']['enabled']) { $connector = MSGraphRemoteCalendarConnector::class; - } else { // remove services which cannot be loaded $container->removeDefinition(MapUserCalendarCommand::class); @@ -47,7 +46,6 @@ class RemoteCalendarCompilerPass implements CompilerPassInterface $container->setAlias(Azure::class, 'knpu.oauth2.provider.azure'); } - if (null === $connector) { throw new RuntimeException('Could not configure remote calendar'); } diff --git a/src/Bundle/ChillCalendarBundle/Repository/CalendarRangeRepository.php b/src/Bundle/ChillCalendarBundle/Repository/CalendarRangeRepository.php index 76d62915a..33feb8401 100644 --- a/src/Bundle/ChillCalendarBundle/Repository/CalendarRangeRepository.php +++ b/src/Bundle/ChillCalendarBundle/Repository/CalendarRangeRepository.php @@ -13,14 +13,10 @@ namespace Chill\CalendarBundle\Repository; use Chill\CalendarBundle\Entity\CalendarRange; use Chill\MainBundle\Entity\User; -use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityRepository; use Doctrine\ORM\QueryBuilder; -use Doctrine\Persistence\ManagerRegistry; use Doctrine\Persistence\ObjectRepository; -use Monolog\DateTimeImmutable; -use UnexpectedValueException; class CalendarRangeRepository implements ObjectRepository { @@ -31,6 +27,13 @@ class CalendarRangeRepository implements ObjectRepository $this->repository = $entityManager->getRepository(CalendarRange::class); } + public function countByAvailableRangesForUser(User $user, \DateTimeImmutable $from, \DateTimeImmutable $to): int + { + return $this->buildQueryAvailableRangesForUser($user, $from, $to) + ->select('COUNT(cr)') + ->getQuery()->getSingleScalarResult(); + } + public function find($id): ?CalendarRange { return $this->repository->find($id); @@ -44,7 +47,6 @@ class CalendarRangeRepository implements ObjectRepository return $this->repository->findAll(); } - /** * @return array|CalendarRange[] */ @@ -53,23 +55,6 @@ class CalendarRangeRepository implements ObjectRepository return $this->repository->findBy($criteria, $orderBy, $limit, $offset); } - public function findOneBy(array $criteria): ?CalendarRange - { - return $this->repository->findOneBy($criteria); - } - - public function getClassName(): string - { - return CalendarRange::class; - } - - public function countByAvailableRangesForUser(User $user, \DateTimeImmutable $from, \DateTimeImmutable $to): int - { - return $this->buildQueryAvailableRangesForUser($user, $from, $to) - ->select('COUNT(cr)') - ->getQuery()->getSingleScalarResult(); - } - /** * @return array|CalendarRange[] */ @@ -82,17 +67,27 @@ class CalendarRangeRepository implements ObjectRepository ): array { $qb = $this->buildQueryAvailableRangesForUser($user, $from, $to); - if ($limit !== null) { + if (null !== $limit) { $qb->setMaxResults($limit); } - if ($offset !== null) { + if (null !== $offset) { $qb->setFirstResult($offset); } return $qb->getQuery()->getResult(); } + public function findOneBy(array $criteria): ?CalendarRange + { + return $this->repository->findOneBy($criteria); + } + + public function getClassName(): string + { + return CalendarRange::class; + } + private function buildQueryAvailableRangesForUser(User $user, \DateTimeImmutable $from, \DateTimeImmutable $to): QueryBuilder { $qb = $this->repository->createQueryBuilder('cr'); @@ -110,9 +105,6 @@ class CalendarRangeRepository implements ObjectRepository 'user' => $user, 'startDate' => $from, 'endDate' => $to, - ]) - ; + ]); } - - } diff --git a/src/Bundle/ChillCalendarBundle/Security/Voter/CalendarVoter.php b/src/Bundle/ChillCalendarBundle/Security/Voter/CalendarVoter.php index 5cfaea172..eca4fe437 100644 --- a/src/Bundle/ChillCalendarBundle/Security/Voter/CalendarVoter.php +++ b/src/Bundle/ChillCalendarBundle/Security/Voter/CalendarVoter.php @@ -1,5 +1,14 @@ build(); } - public function getRolesWithHierarchy(): array - { - return ['Calendar' => $this->getRoles()]; - } - public function getRoles(): array { return [ @@ -45,6 +47,11 @@ class CalendarVoter extends AbstractChillVoter implements ProvideRoleHierarchyIn ]; } + public function getRolesWithHierarchy(): array + { + return ['Calendar' => $this->getRoles()]; + } + public function getRolesWithoutScope(): array { return []; @@ -60,20 +67,18 @@ class CalendarVoter extends AbstractChillVoter implements ProvideRoleHierarchyIn if ($subject instanceof AccompanyingPeriod) { switch ($attribute) { case self::SEE: - if ($subject->getStep() === AccompanyingPeriod::STEP_DRAFT) { return false; } // we first check here that the user has read access to the period return $this->security->isGranted(AccompanyingPeriodVoter::SEE, $subject); + default: - throw new \LogicException('subject not implemented'); + throw new LogicException('subject not implemented'); } } - throw new \LogicException('attribute not implemented'); + throw new LogicException('attribute not implemented'); } - - } diff --git a/src/Bundle/ChillCalendarBundle/Tests/Entity/CalendarTest.php b/src/Bundle/ChillCalendarBundle/Tests/Entity/CalendarTest.php index c6d7b73a7..348620faf 100644 --- a/src/Bundle/ChillCalendarBundle/Tests/Entity/CalendarTest.php +++ b/src/Bundle/ChillCalendarBundle/Tests/Entity/CalendarTest.php @@ -1,12 +1,25 @@ assertCount(0, $calendar->getInvites()); $this->assertCount(0, $calendar->getUsers()); } - } diff --git a/src/Bundle/ChillMainBundle/Form/DataTransformer/IdToEntityDataTransformer.php b/src/Bundle/ChillMainBundle/Form/DataTransformer/IdToEntityDataTransformer.php new file mode 100644 index 000000000..0c433aec5 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Form/DataTransformer/IdToEntityDataTransformer.php @@ -0,0 +1,82 @@ +repository = $repository; + $this->multiple = $multiple; + $this->getId = $getId ?? static function (object $o) { return $o->getId(); }; + } + + /** + * @param string $value + * + * @return array|object[]|T[]|T|object + */ + public function reverseTransform($value) + { + if ($this->multiple) { + if (null === $value | '' === $value) { + return []; + } + + return array_map( + fn (string $id): ?object => $this->repository->findOneBy(['id' => (int) $id]), + explode(',', $value) + ); + } + + if (null === $value | '' === $value) { + return null; + } + + return $this->repository->findOneBy(['id' => (int) $value]); + } + + /** + * @param object|T|object[]|T[] $value + */ + public function transform($value): string + { + if ($this->multiple) { + $ids = []; + + foreach ($value as $v) { + $ids[] = call_user_func($this->getId, $v); + } + + return implode(',', $ids); + } + + return call_user_func($this->getId, $value); + } +} diff --git a/src/Bundle/ChillMainBundle/Repository/LocationRepository.php b/src/Bundle/ChillMainBundle/Repository/LocationRepository.php index 845275bd7..253298d49 100644 --- a/src/Bundle/ChillMainBundle/Repository/LocationRepository.php +++ b/src/Bundle/ChillMainBundle/Repository/LocationRepository.php @@ -13,7 +13,6 @@ namespace Chill\MainBundle\Repository; use Chill\MainBundle\Entity\Location; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; -use Doctrine\ORM\Query\ResultSetMappingBuilder; use Doctrine\Persistence\ManagerRegistry; /** diff --git a/src/Bundle/ChillMainBundle/Tests/Form/DataTransformer/IdToEntityDataTransformerTest.php b/src/Bundle/ChillMainBundle/Tests/Form/DataTransformer/IdToEntityDataTransformerTest.php new file mode 100644 index 000000000..ffcd538cd --- /dev/null +++ b/src/Bundle/ChillMainBundle/Tests/Form/DataTransformer/IdToEntityDataTransformerTest.php @@ -0,0 +1,118 @@ +prophesize(ObjectRepository::class); + $repository->findOneBy(Argument::exact(['id' => 1]))->willReturn($o1); + $repository->findOneBy(Argument::exact(['id' => 2]))->willReturn($o2); + + $transformer = new IdToEntityDataTransformer( + $repository->reveal(), + true + ); + + $this->assertEquals([], $transformer->reverseTransform(null)); + $this->assertEquals([], $transformer->reverseTransform('')); + $r = $transformer->reverseTransform('1,2'); + + $this->assertIsArray($r); + $this->assertSame($o1, $r[0]); + $this->assertSame($o2, $r[1]); + } + + public function testReverseTransformSingle() + { + $o = new stdClass(); + + $repository = $this->prophesize(ObjectRepository::class); + $repository->findOneBy(Argument::exact(['id' => 1]))->willReturn($o); + + $transformer = new IdToEntityDataTransformer( + $repository->reveal(), + false + ); + + $this->assertEquals(null, $transformer->reverseTransform(null)); + $this->assertEquals(null, $transformer->reverseTransform('')); + $r = $transformer->reverseTransform('1'); + + $this->assertSame($o, $r); + } + + public function testTransformMulti() + { + $o1 = new class() { + public function getId() + { + return 1; + } + }; + $o2 = new class() { + public function getId() + { + return 2; + } + }; + $repository = $this->prophesize(ObjectRepository::class); + + $transformer = new IdToEntityDataTransformer( + $repository->reveal(), + true + ); + + $this->assertEquals( + '1,2', + $transformer->transform([$o1, $o2]) + ); + } + + public function testTransformSingle() + { + $o = new class() { + public function getId() + { + return 1; + } + }; + $repository = $this->prophesize(ObjectRepository::class); + + $transformer = new IdToEntityDataTransformer( + $repository->reveal(), + false + ); + + $this->assertEquals( + '1', + $transformer->transform($o) + ); + } +}