diff --git a/src/Bundle/ChillPersonBundle/Controller/ReassignAccompanyingPeriodController.php b/src/Bundle/ChillPersonBundle/Controller/ReassignAccompanyingPeriodController.php index fde9746d3..3e5b59c2a 100644 --- a/src/Bundle/ChillPersonBundle/Controller/ReassignAccompanyingPeriodController.php +++ b/src/Bundle/ChillPersonBundle/Controller/ReassignAccompanyingPeriodController.php @@ -98,7 +98,7 @@ class ReassignAccompanyingPeriodController extends AbstractController $userFrom = $form['user']->getData(); $postalCodes = $form['postal_code']->getData() instanceof PostalCode ? [$form['postal_code']->getData()] : []; - $total = $this->accompanyingPeriodACLAwareRepository->countByUserOpenedAccompanyingPeriod($userFrom); + $total = $this->accompanyingPeriodACLAwareRepository->countByUserAndPostalCodesOpenedAccompanyingPeriod($userFrom, $postalCodes); $paginator = $this->paginatorFactory->create($total); $paginator->setItemsPerPage(50); $periods = $this->accompanyingPeriodACLAwareRepository diff --git a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php index e62e2b3ac..79a7710ff 100644 --- a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php +++ b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php @@ -14,6 +14,7 @@ namespace Chill\PersonBundle\Repository; use Chill\MainBundle\Entity\Address; use Chill\MainBundle\Entity\Center; use Chill\MainBundle\Entity\Location; +use Chill\MainBundle\Entity\PostalCode; use Chill\MainBundle\Entity\Scope; use Chill\MainBundle\Entity\User; use Chill\MainBundle\Entity\UserJob; @@ -21,12 +22,8 @@ use Chill\MainBundle\Security\Authorization\AuthorizationHelperForCurrentUserInt use Chill\MainBundle\Security\Resolver\CenterResolverManagerInterface; use Chill\PersonBundle\Entity\AccompanyingPeriod; use Chill\PersonBundle\Entity\AccompanyingPeriodParticipation; -use Chill\PersonBundle\Entity\Household\PersonHouseholdAddress; use Chill\PersonBundle\Entity\Person; use Chill\PersonBundle\Security\Authorization\AccompanyingPeriodVoter; -use DateTime; -use DateTimeImmutable; -use Doctrine\DBAL\Types\Types; use Doctrine\ORM\NonUniqueResultException; use Doctrine\ORM\NoResultException; use Doctrine\ORM\Query\Expr\Join; @@ -60,51 +57,33 @@ final readonly class AccompanyingPeriodACLAwareRepository implements Accompanyin $this->centerResolver = $centerResolverDispatcher; } - public function buildQueryOpenedAccompanyingCourseByUser(?User $user, array $postalCodes = []): QueryBuilder + public function buildQueryOpenedAccompanyingCourseByUserAndPostalCodes(?User $user, array $postalCodes = []): QueryBuilder { $qb = $this->accompanyingPeriodRepository->createQueryBuilder('ap'); $qb->where($qb->expr()->eq('ap.user', ':user')) ->andWhere( $qb->expr()->neq('ap.step', ':draft'), - $qb->expr()->orX( - $qb->expr()->isNull('ap.closingDate'), - $qb->expr()->gt('ap.closingDate', ':now') - ) + $qb->expr()->neq('ap.step', ':closed'), ) ->setParameter('user', $user) - ->setParameter('now', new DateTime('now')) - ->setParameter('draft', AccompanyingPeriod::STEP_DRAFT); + ->setParameter('draft', AccompanyingPeriod::STEP_DRAFT) + ->setParameter('closed', AccompanyingPeriod::STEP_CLOSED); if ([] !== $postalCodes) { - $qb->join('ap.locationHistories', 'location_history') - ->leftJoin(PersonHouseholdAddress::class, 'person_address', Join::WITH, 'IDENTITY(location_history.personLocation) = IDENTITY(person_address.person)') + $qb->join('ap.locationHistories', 'location_history', Join::WITH, 'location_history.endDate IS NULL') + ->leftJoin(Person\PersonCurrentAddress::class, 'person_address', Join::WITH, 'IDENTITY(location_history.personLocation) = IDENTITY(person_address.person)') ->join( Address::class, 'address', Join::WITH, - 'COALESCE(IDENTITY(location_history.addressLocation), IDENTITY(person_address.address)) = address.id' + 'COALESCE(IDENTITY(person_address.address), IDENTITY(location_history.addressLocation)) = address.id' ) + ->join('address.postcode', 'postcode') ->andWhere( - $qb->expr()->orX( - $qb->expr()->isNull('person_address'), - $qb->expr()->andX( - $qb->expr()->lte('person_address.validFrom', ':now'), - $qb->expr()->orX( - $qb->expr()->isNull('person_address.validTo'), - $qb->expr()->lt('person_address.validTo', ':now') - ) - ) - ) + $qb->expr()->in('postcode.code', ':postal_codes') ) - ->andWhere( - $qb->expr()->isNull('location_history.endDate') - ) - ->andWhere( - $qb->expr()->in('address.postcode', ':postal_codes') - ) - ->setParameter('now', new DateTimeImmutable('now'), Types::DATE_IMMUTABLE) - ->setParameter('postal_codes', $postalCodes); + ->setParameter('postal_codes', array_map(fn (PostalCode $postalCode) => $postalCode->getCode(), $postalCodes)); } return $qb; @@ -116,7 +95,7 @@ final readonly class AccompanyingPeriodACLAwareRepository implements Accompanyin */ public function countByUnDispatched(array $jobs, array $services, array $administrativeLocations): int { - $qb = $this->addACLByUnDispatched( + $qb = $this->addACLMultiCenterOnQuery( $this->buildQueryUnDispatched($jobs, $services, $administrativeLocations), $this->buildCenterOnScope() ); @@ -132,22 +111,12 @@ final readonly class AccompanyingPeriodACLAwareRepository implements Accompanyin return 0; } - return $this->buildQueryOpenedAccompanyingCourseByUser($user, $postalCodes) - ->select('COUNT(ap)') - ->getQuery() - ->getSingleScalarResult(); - } + $qb = $this->buildQueryOpenedAccompanyingCourseByUserAndPostalCodes($user, $postalCodes); + $qb = $this->addACLMultiCenterOnQuery($qb, $this->buildCenterOnScope(), false); - public function countByUserOpenedAccompanyingPeriod(?User $user): int - { - if (null === $user) { - return 0; - } + $qb->select('COUNT(DISTINCT ap)'); - return $this->buildQueryOpenedAccompanyingCourseByUser($user) - ->select('COUNT(ap)') - ->getQuery() - ->getSingleScalarResult(); + return $qb->getQuery()->getSingleScalarResult(); } public function findByPerson( @@ -283,7 +252,7 @@ final readonly class AccompanyingPeriodACLAwareRepository implements Accompanyin $qb = $this->buildQueryUnDispatched($jobs, $services, $administrativeAdministrativeLocations); $qb->select('ap'); - $qb = $this->addACLByUnDispatched($qb, $this->buildCenterOnScope(), false); + $qb = $this->addACLMultiCenterOnQuery($qb, $this->buildCenterOnScope(), false); $qb = $this->addOrderLimitClauses($qb, $orderBy, $limit, $offset); return $qb->getQuery()->getResult(); @@ -295,32 +264,9 @@ final readonly class AccompanyingPeriodACLAwareRepository implements Accompanyin return []; } - $qb = $this->buildQueryOpenedAccompanyingCourseByUser($user); - - $qb->setFirstResult($offset) - ->setMaxResults($limit); - - foreach ($orderBy as $field => $direction) { - $qb->addOrderBy('ap.' . $field, $direction); - } - - return $qb->getQuery()->getResult(); - } - - public function findByUserOpenedAccompanyingPeriod(?User $user, array $orderBy = [], int $limit = 0, int $offset = 50): array - { - if (null === $user) { - return []; - } - - $qb = $this->buildQueryOpenedAccompanyingCourseByUser($user); - - $qb->setFirstResult($offset) - ->setMaxResults($limit); - - foreach ($orderBy as $field => $direction) { - $qb->addOrderBy('ap.' . $field, $direction); - } + $qb = $this->buildQueryOpenedAccompanyingCourseByUserAndPostalCodes($user, $postalCodes); + $qb = $this->addACLMultiCenterOnQuery($qb, $this->buildCenterOnScope(), false); + $qb = $this->addOrderLimitClauses($qb, $orderBy, $limit, $offset); return $qb->getQuery()->getResult(); } @@ -331,7 +277,7 @@ final readonly class AccompanyingPeriodACLAwareRepository implements Accompanyin * @param bool $allowNoCenter if true, will allow to see the periods linked to person which does not have any center. Very few edge case when some Person are not associated to a center. * @return QueryBuilder */ - public function addACLByUnDispatched(QueryBuilder $qb, array $centerScopes, bool $allowNoCenter = false): QueryBuilder + public function addACLMultiCenterOnQuery(QueryBuilder $qb, array $centerScopes, bool $allowNoCenter = false): QueryBuilder { $user = $this->security->getUser(); @@ -366,9 +312,9 @@ final readonly class AccompanyingPeriodACLAwareRepository implements Accompanyin // - and the parcours is not confidential OR the user is the referrer OR the user can see the confidential course $orOnScope = $qb->expr()->orX( $qb->expr()->isMemberOf(':scope_' . $idx, 'ap.scopes'), - $qb->expr()->eq('ap.user', ':user') + $qb->expr()->eq('ap.user', ':user_executing') ); - $qb->setParameter('user', $user); + $qb->setParameter('user_executing', $user); if (in_array($scope, $scopesCanSeeConfidential, true)) { $orScopeInsideCenter->add($orOnScope); @@ -378,7 +324,7 @@ final readonly class AccompanyingPeriodACLAwareRepository implements Accompanyin $orOnScope, $qb->expr()->orX( 'ap.confidential = FALSE', - $qb->expr()->eq('ap.user', ':user') + $qb->expr()->eq('ap.user', ':user_executing') ) ); $orScopeInsideCenter->add($andXOnScope); diff --git a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepositoryInterface.php b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepositoryInterface.php index 61b631904..7b31887b9 100644 --- a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepositoryInterface.php +++ b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepositoryInterface.php @@ -31,8 +31,6 @@ interface AccompanyingPeriodACLAwareRepositoryInterface */ public function countByUserAndPostalCodesOpenedAccompanyingPeriod(?User $user, array $postalCodes): int; - public function countByUserOpenedAccompanyingPeriod(?User $user): int; - /** * @return array */ @@ -40,8 +38,8 @@ interface AccompanyingPeriodACLAwareRepositoryInterface Person $person, string $role, ?array $orderBy = [], - ?int $limit = null, - ?int $offset = null + ?int $limit = null, + ?int $offset = null ): array; /** @@ -57,10 +55,4 @@ interface AccompanyingPeriodACLAwareRepositoryInterface * @return list */ public function findByUserAndPostalCodesOpenedAccompanyingPeriod(?User $user, array $postalCodes, array $orderBy = [], int $limit = 0, int $offset = 50): array; - - /** - * @deprecated - * @return list - */ - public function findByUserOpenedAccompanyingPeriod(?User $user, array $orderBy = [], int $limit = 0, int $offset = 50): array; } diff --git a/src/Bundle/ChillPersonBundle/Tests/Repository/AccompanyingPeriodACLAwareRepositoryTest.php b/src/Bundle/ChillPersonBundle/Tests/Repository/AccompanyingPeriodACLAwareRepositoryTest.php index 90e0d2384..cb8d56ba3 100644 --- a/src/Bundle/ChillPersonBundle/Tests/Repository/AccompanyingPeriodACLAwareRepositoryTest.php +++ b/src/Bundle/ChillPersonBundle/Tests/Repository/AccompanyingPeriodACLAwareRepositoryTest.php @@ -80,7 +80,178 @@ class AccompanyingPeriodACLAwareRepositoryTest extends KernelTestCase $em->remove($period); } - $em->flush(); + //$em->flush(); + } + + /** + * @dataProvider provideDataFindByUserAndPostalCodesOpenedAccompanyingPeriod + * @param list, scopeCanSeeConfidential: list}> $centerScopes + * @param list $expectedContains + * @param list $expectedNotContains + */ + public function testFindByUserAndPostalCodesOpenedAccompanyingPeriod(User $user, User $searched, array $centerScopes, array $expectedContains, array $expectedNotContains, string $message): void + { + $security = $this->prophesize(Security::class); + $security->getUser()->willReturn($user); + + $authorizationHelper = $this->prophesize(AuthorizationHelperForCurrentUserInterface::class); + $centers = []; + + foreach ($centerScopes as ['center' => $center, 'scopeOnRole' => $scopes, 'scopeCanSeeConfidential' => $scopesCanSeeConfidential]) { + $centers[spl_object_hash($center)] = $center; + $authorizationHelper->getReachableScopes(AccompanyingPeriodVoter::SEE, $center) + ->willReturn($scopes); + $authorizationHelper->getReachableScopes(AccompanyingPeriodVoter::SEE_CONFIDENTIAL_ALL, $center) + ->willReturn($scopesCanSeeConfidential); + } + $authorizationHelper->getReachableCenters(AccompanyingPeriodVoter::SEE)->willReturn(array_values($centers)); + + $repository = new AccompanyingPeriodACLAwareRepository( + $this->accompanyingPeriodRepository, + $security->reveal(), + $authorizationHelper->reveal(), + $this->centerResolverManager + ); + + $actual = array_map( + fn (AccompanyingPeriod $period) => $period->getId(), + $repository->findByUserAndPostalCodesOpenedAccompanyingPeriod($searched, [], ['id' => 'DESC'], 20, 0) + ); + + foreach ($expectedContains as $expected) { + self::assertContains($expected->getId(), $actual, $message); + } + foreach ($expectedNotContains as $expected) { + self::assertNotContains($expected->getId(), $actual, $message); + } + } + + public function provideDataFindByUserAndPostalCodesOpenedAccompanyingPeriod(): iterable + { + $this->setUp(); + + if (null === $user = $this->entityManager->createQuery("SELECT u FROM " . User::class . " u")->setMaxResults(1)->getSingleResult()) { + throw new \RuntimeException("no user found"); + } + + if (null === $anotherUser = $this->entityManager->createQuery("SELECT u FROM " . User::class . " u WHERE u.id != :uid")->setParameter('uid', $user->getId()) + ->setMaxResults(1)->getSingleResult()) { + throw new \RuntimeException("no user found"); + } + + /** @var Person $person */ + [$person, $anotherPerson, $person2, $person3] = $this->entityManager + ->createQuery("SELECT p FROM " . Person::class . " p JOIN p.centerCurrent current_center") + ->setMaxResults(4) + ->getResult(); + + if (null === $person || null === $anotherPerson || null === $person2 || null === $person3) { + throw new \RuntimeException("no person found"); + } + + $scopes = $this->scopeRepository->findAll(); + + if (3 > count($scopes)) { + throw new \RuntimeException("not enough scopes for this test"); + } + $scopesCanSee = [ $scopes[0] ]; + $scopesGroup2 = [ $scopes[1] ]; + + $centers = $this->centerRepository->findActive(); + $aCenterNotAssociatedToPerson = array_values(array_filter($centers, fn (Center $c) => $c !== $person->getCenter()))[0]; + + if (2 > count($centers)) { + throw new \RuntimeException("not enough centers for this test"); + } + + $period = $this->buildPeriod($person, $scopesCanSee, $user, true); + $period->setUser($user); + + yield [ + $anotherUser, + $user, + [ + [ + 'center' => $person->getCenter(), + 'scopeOnRole' => $scopesCanSee, + 'scopeCanSeeConfidential' => [], + ], + ], + [$period], + [], + "period should be visible with expected scopes", + ]; + + yield [ + $anotherUser, + $user, + [ + [ + 'center' => $person->getCenter(), + 'scopeOnRole' => $scopesGroup2, + 'scopeCanSeeConfidential' => [], + ], + ], + [], + [$period], + "period should not be visible without expected scopes", + ]; + + yield [ + $anotherUser, + $user, + [ + [ + 'center' => $person->getCenter(), + 'scopeOnRole' => $scopesGroup2, + 'scopeCanSeeConfidential' => [], + ], + [ + 'center' => $aCenterNotAssociatedToPerson, + 'scopeOnRole' => $scopesCanSee, + 'scopeCanSeeConfidential' => [], + ], + ], + [], + [$period], + "period should not be visible for user having right in another scope (with multiple centers)" + ]; + + $period = $this->buildPeriod($person, $scopesCanSee, $user, true); + $period->setUser($user); + $period->setConfidential(true); + + yield [ + $anotherUser, + $user, + [ + [ + 'center' => $person->getCenter(), + 'scopeOnRole' => $scopesCanSee, + 'scopeCanSeeConfidential' => [], + ], + ], + [], + [$period], + "period confidential should not be visible", + ]; + + yield [ + $anotherUser, + $user, + [ + [ + 'center' => $person->getCenter(), + 'scopeOnRole' => $scopesCanSee, + 'scopeCanSeeConfidential' => $scopesCanSee, + ], + ], + [$period], + [], + "period confidential be visible if user has required scopes", + ]; + + $this->entityManager->flush(); } /** @@ -165,6 +336,7 @@ class AccompanyingPeriodACLAwareRepositoryTest extends KernelTestCase $period = $this->buildPeriod($person, $scopesCanSee, $user, true); + // expected scope: can see the period yield [ $anotherUser,