From df0afcd2285cd25b68e48b5385561b27eb15885e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Thu, 14 Mar 2024 21:16:05 +0100 Subject: [PATCH] Fix activity filter inconsistency in document generation This commit resolves issue 259 where the filtering of activities differed within the document generation and in the list of activities for an accompanying period. This amendment to the Chill Activity Bundle ensures consistent behavior. Additionally, new test methods and query adjustments were applied to the ActivityACLAwareRepository for better functionality. --- .../unreleased/Fixed-20240314-211532.yaml | 7 + .../Repository/ActivityACLAwareRepository.php | 4 +- ...tActivitiesByAccompanyingPeriodContext.php | 20 ++- .../ActivityACLAwareRepositoryTest.php | 28 +++- ...ivitiesByAccompanyingPeriodContextTest.php | 139 ++++++++++++++++++ 5 files changed, 194 insertions(+), 4 deletions(-) create mode 100644 .changes/unreleased/Fixed-20240314-211532.yaml create mode 100644 src/Bundle/ChillActivityBundle/Tests/Service/DocGenerator/ListActivitiesByAccompanyingPeriodContextTest.php diff --git a/.changes/unreleased/Fixed-20240314-211532.yaml b/.changes/unreleased/Fixed-20240314-211532.yaml new file mode 100644 index 000000000..dc3e675cd --- /dev/null +++ b/.changes/unreleased/Fixed-20240314-211532.yaml @@ -0,0 +1,7 @@ +kind: Fixed +body: Keep a consistent behaviour between the filtering of activities within the document + generation (model "accompanying period with activities"), and the same filter in + the list of activities for an accompanying period +time: 2024-03-14T21:15:32.690077164+01:00 +custom: + Issue: "259" diff --git a/src/Bundle/ChillActivityBundle/Repository/ActivityACLAwareRepository.php b/src/Bundle/ChillActivityBundle/Repository/ActivityACLAwareRepository.php index 1f50f7d62..6d9991e6d 100644 --- a/src/Bundle/ChillActivityBundle/Repository/ActivityACLAwareRepository.php +++ b/src/Bundle/ChillActivityBundle/Repository/ActivityACLAwareRepository.php @@ -243,7 +243,8 @@ final readonly class ActivityACLAwareRepository implements ActivityACLAwareRepos thirdparties.thirdpartyids, persons.personids, actions.socialactionids, - issues.socialissueids + issues.socialissueids, + a.user_id FROM activity a LEFT JOIN chill_main_location location ON a.location_id = location.id @@ -283,6 +284,7 @@ final readonly class ActivityACLAwareRepository implements ActivityACLAwareRepos ->addJoinedEntityResult(ActivityPresence::class, 'activityPresence', 'a', 'attendee') ->addFieldResult('activityPresence', 'presence_id', 'id') ->addFieldResult('activityPresence', 'presence_name', 'name') + ->addScalarResult('user_id', 'userId', Types::INTEGER) // results which cannot be mapped into entity ->addScalarResult('comment_comment', 'comment', Types::TEXT) diff --git a/src/Bundle/ChillActivityBundle/Service/DocGenerator/ListActivitiesByAccompanyingPeriodContext.php b/src/Bundle/ChillActivityBundle/Service/DocGenerator/ListActivitiesByAccompanyingPeriodContext.php index 33675754a..baf1c279d 100644 --- a/src/Bundle/ChillActivityBundle/Service/DocGenerator/ListActivitiesByAccompanyingPeriodContext.php +++ b/src/Bundle/ChillActivityBundle/Service/DocGenerator/ListActivitiesByAccompanyingPeriodContext.php @@ -11,6 +11,7 @@ declare(strict_types=1); namespace Chill\ActivityBundle\Service\DocGenerator; +use Chill\ActivityBundle\Entity\Activity; use Chill\ActivityBundle\Entity\ActivityPresence; use Chill\ActivityBundle\Entity\ActivityType; use Chill\ActivityBundle\Repository\ActivityACLAwareRepositoryInterface; @@ -112,7 +113,7 @@ class ListActivitiesByAccompanyingPeriodContext implements } /** - * @return list + * @return list */ private function filterActivitiesByUser(array $activities, User $user): array { @@ -120,6 +121,12 @@ class ListActivitiesByAccompanyingPeriodContext implements array_filter( $activities, function ($activity) use ($user) { + $u = $activity['user']; + + if (null !== $u && $u['username'] === $user->getUsername()) { + return true; + } + $activityUsernames = array_map(static fn ($user) => $user['username'], $activity['users'] ?? []); return \in_array($user->getUsername(), $activityUsernames, true); @@ -129,7 +136,7 @@ class ListActivitiesByAccompanyingPeriodContext implements } /** - * @return list + * @return list */ private function filterWorksByUser(array $works, User $user): array { @@ -216,6 +223,15 @@ class ListActivitiesByAccompanyingPeriodContext implements foreach ($activities as $row) { $activity = $row[0]; + $user = match (null === $row['userId']) { + false => $this->userRepository->find($row['userId']), + true => null, + }; + + $activity['user'] = $this->normalizer->normalize($user, 'docgen', [ + AbstractNormalizer::GROUPS => ['docgen:read'], 'docgen:expects' => User::class, + ]); + $activity['date'] = $this->normalizer->normalize($activity['date'], 'docgen', [ AbstractNormalizer::GROUPS => ['docgen:read'], 'docgen:expects' => \DateTime::class, ]); diff --git a/src/Bundle/ChillActivityBundle/Tests/Repository/ActivityACLAwareRepositoryTest.php b/src/Bundle/ChillActivityBundle/Tests/Repository/ActivityACLAwareRepositoryTest.php index 88eb3e7f8..67b3c1f5d 100644 --- a/src/Bundle/ChillActivityBundle/Tests/Repository/ActivityACLAwareRepositoryTest.php +++ b/src/Bundle/ChillActivityBundle/Tests/Repository/ActivityACLAwareRepositoryTest.php @@ -91,6 +91,29 @@ class ActivityACLAwareRepositoryTest extends KernelTestCase self::assertIsArray($actual); } + /** + * @dataProvider provideDataFindByAccompanyingPeriod + */ + public function testfindByAccompanyingPeriodSimplified(AccompanyingPeriod $period, User $user, string $role, ?int $start = 0, ?int $limit = 1000, array $orderBy = ['date' => 'DESC'], array $filters = []): void + { + $security = $this->prophesize(Security::class); + $security->isGranted($role, $period)->willReturn(true); + $security->getUser()->willReturn($user); + + $repository = new ActivityACLAwareRepository( + $this->authorizationHelperForCurrentUser, + $this->centerResolverManager, + $this->activityRepository, + $this->entityManager, + $security->reveal(), + $this->requestStack + ); + + $actual = $repository->findByAccompanyingPeriodSimplified($period); + + self::assertIsArray($actual); + } + /** * @dataProvider provideDataFindByAccompanyingPeriod */ @@ -301,7 +324,10 @@ class ActivityACLAwareRepositoryTest extends KernelTestCase ->getQuery() ->getResult() ) { - throw new \RuntimeException('no jobs found'); + $job = new UserJob(); + $job->setLabel(['fr' => 'test']); + $this->entityManager->persist($job); + $this->entityManager->flush(); } if (null === $user = $this->entityManager diff --git a/src/Bundle/ChillActivityBundle/Tests/Service/DocGenerator/ListActivitiesByAccompanyingPeriodContextTest.php b/src/Bundle/ChillActivityBundle/Tests/Service/DocGenerator/ListActivitiesByAccompanyingPeriodContextTest.php new file mode 100644 index 000000000..ff9f30c30 --- /dev/null +++ b/src/Bundle/ChillActivityBundle/Tests/Service/DocGenerator/ListActivitiesByAccompanyingPeriodContextTest.php @@ -0,0 +1,139 @@ +listActivitiesByAccompanyingPeriodContext = self::$container->get(ListActivitiesByAccompanyingPeriodContext::class); + $this->accompanyingPeriodRepository = self::$container->get(AccompanyingPeriodRepository::class); + $this->userRepository = self::$container->get(UserRepositoryInterface::class); + } + + /** + * @dataProvider provideAccompanyingPeriod + */ + public function testGetDataWithoutFilteringActivityNorWorks(int $accompanyingPeriodId, int $userId): void + { + $context = $this->getContext(); + $template = new DocGeneratorTemplate(); + $template->setOptions([ + 'mainPerson' => false, + 'person1' => false, + 'person2' => false, + 'thirdParty' => false, + ]); + + $data = $context->getData( + $template, + $this->accompanyingPeriodRepository->find($accompanyingPeriodId), + ['myActivitiesOnly' => false, 'myWorksOnly' => false] + ); + + self::assertIsArray($data); + self::assertArrayHasKey('activities', $data); + self::assertIsArray($data['activities']); + self::assertGreaterThan(0, count($data['activities'])); + self::assertIsArray($data['activities'][0]); + self::assertArrayHasKey('user', $data['activities'][0]); + self::assertIsArray($data['activities'][0]['user']); + } + + /** + * @dataProvider provideAccompanyingPeriod + */ + public function testGetDataWithoutFilteringActivityByUser(int $accompanyingPeriodId, int $userId): void + { + $context = $this->getContext(); + $template = new DocGeneratorTemplate(); + $template->setOptions([ + 'mainPerson' => false, + 'person1' => false, + 'person2' => false, + 'thirdParty' => false, + ]); + + $data = $context->getData( + $template, + $this->accompanyingPeriodRepository->find($accompanyingPeriodId), + ['myActivitiesOnly' => true, 'myWorksOnly' => false, 'creator' => $this->userRepository->find($userId)] + ); + + self::assertIsArray($data); + self::assertArrayHasKey('activities', $data); + self::assertIsArray($data['activities']); + self::assertGreaterThan(0, count($data['activities'])); + self::assertIsArray($data['activities'][0]); + self::assertArrayHasKey('user', $data['activities'][0]); + self::assertIsArray($data['activities'][0]['user']); + } + + public static function provideAccompanyingPeriod(): array + { + self::bootKernel(); + $em = self::$container->get(EntityManagerInterface::class); + + if (null === $period = $em->createQuery('SELECT a FROM '.AccompanyingPeriod::class.' a') + ->setMaxResults(1) + ->getSingleResult()) { + throw new \RuntimeException('no period found'); + } + + if (null === $user = $em->createQuery('SELECT u FROM '.User::class.' u') + ->setMaxResults(1) + ->getSingleResult() + ) { + throw new \RuntimeException('no user found'); + } + + $activity = new Activity(); + $activity + ->setAccompanyingPeriod($period) + ->setUser($user) + ->setDate(new \DateTime()); + + $em->persist($activity); + $em->flush(); + + self::ensureKernelShutdown(); + + return [ + [$period->getId(), $user->getId()], + ]; + } + + private function getContext(): ListActivitiesByAccompanyingPeriodContext + { + return $this->listActivitiesByAccompanyingPeriodContext; + } +}