EP/myep
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: relax filtering constraints for non-admin groups price users

Browse Source
This commit is contained in:
Björn Fromme
2026-08-24 14:32:45 +02:00
parent 178df9b787
commit 35826088cb
8 changed files with 50 additions and 233 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Controller/Admin/AccommodationBooking/FilterController.php
+2 -9
View File
@@ -6,7 +6,6 @@ namespace App\Controller\Admin\AccommodationBooking;
use App\Controller\Traits\ListFilterTrait;
use App\Entity\User;
use App\Form\Admin\Filter\AccommodationBookingFilterOptionsProvider;
use App\Form\Admin\Filter\AccommodationBookingFilterType;
use App\Form\Model\Filter\AccommodationBookingFilterDto;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
@@ -26,25 +25,19 @@ class FilterController extends AbstractController
{
use ListFilterTrait;
public function __construct(
private readonly AccommodationBookingFilterOptionsProvider $filterOptions,
) {
}
#[Route('/admin/accommodation-booking/filter', name: 'app_admin_accommodationbooking_filter', methods: ['GET'])]
public function filter(Request $request): Response
{
// ROLE_GROUPS_ADMIN is granted explicitly since 6c1073e4 removed it from ROLE_ADMIN's
// role_hierarchy — an administrator who needs this has to hold it in their own right.
$seesAllBookings = $this->isGranted('ROLE_GROUPS_ADMIN');
$isGroupsAdmin = $this->isGranted('ROLE_GROUPS_ADMIN');
$user = $this->getUser();
$filterView = $this->createListFilterView(
$request,
AccommodationBookingFilterType::class,
AccommodationBookingFilterDto::defaults($seesAllBookings, $user instanceof User ? $user : null),
AccommodationBookingFilterDto::defaults($isGroupsAdmin, $user instanceof User ? $user : null),
'app_admin_accommodationbooking',
$this->filterOptions->formOptions($seesAllBookings),
);
return $this->render('admin/accommodation_booking/modal_filter.html.twig', [
src/Controller/Admin/AccommodationBooking/IndexController.php
+2 -13
View File
@@ -6,7 +6,6 @@ namespace App\Controller\Admin\AccommodationBooking;
use App\Controller\Traits\ListFilterTrait;
use App\Entity\User;
use App\Form\Admin\Filter\AccommodationBookingFilterOptionsProvider;
use App\Form\Admin\Filter\AccommodationBookingFilterType;
use App\Form\Model\Filter\AccommodationBookingFilterDto;
use App\Repository\Groups\AccommodationBookingRepository;
@@ -24,7 +23,6 @@ class IndexController extends AbstractController
public function __construct(
private readonly AccommodationBookingRepository $bookingRepository,
private readonly AccommodationBookingFilterOptionsProvider $filterOptions,
private readonly PaginatorInterface $paginator,
) {
}
@@ -34,30 +32,21 @@ class IndexController extends AbstractController
{
// ROLE_GROUPS_ADMIN is granted explicitly since 6c1073e4 removed it from ROLE_ADMIN's
// role_hierarchy — an administrator who needs this has to hold it in their own right.
$seesAllBookings = $this->isGranted('ROLE_GROUPS_ADMIN');
$isGroupsAdmin = $this->isGranted('ROLE_GROUPS_ADMIN');
$user = $this->getUser();
$user = $user instanceof User ? $user : null;
// The form writes into $filter, so after this call it holds either the defaults or
// whatever the query string asked for.
$filter = AccommodationBookingFilterDto::defaults($seesAllBookings, $user);
$filter = AccommodationBookingFilterDto::defaults($isGroupsAdmin, $user);
$filterView = $this->createListFilterView(
$request,
AccommodationBookingFilterType::class,
$filter,
'app_admin_accommodationbooking',
$this->filterOptions->formOptions($seesAllBookings),
);
// The manager fields are not part of the form for this role, but the scope is pinned
// here as well so that a hand-written query parameter cannot widen it either.
if (!$seesAllBookings) {
$filter->managedBy = $user;
$filter->unassigned = false;
$filter->managedByLocked = true;
}
$pagination = $this->paginator->paginate(
$this->bookingRepository->createFilteredQueryBuilder($filter),
$request->query->getInt('page', 1),
src/Form/Admin/Filter/AccommodationBookingFilterOptionsProvider.php
-64
View File
@@ -1,64 +0,0 @@
<?php
declare(strict_types=1);
namespace App\Form\Admin\Filter;
use App\Entity\User;
use App\Repository\Groups\AccommodationBookingRepository;
use App\Repository\Groups\AccommodationRepository;
use App\Repository\UserRepository;
/**
* The choice lists for the accommodation booking filter form.
*
* The list view and the filter modal have to configure the form identically: the modal writes
* the query string, the list hydrates the very same form from it, and a field missing on either
* side would silently drop that filter.
*/
final readonly class AccommodationBookingFilterOptionsProvider
{
public function __construct(
private UserRepository $userRepository,
private AccommodationBookingRepository $bookingRepository,
private AccommodationRepository $accommodationRepository,
) {
}
/**
* @param bool $seesAllBookings whether the current user may look past their own bookings
*
* @return array<string, mixed>
*/
public function formOptions(bool $seesAllBookings): array
{
return [
'can_filter_by_manager' => $seesAllBookings,
'managers' => $seesAllBookings ? $this->managers() : [],
'accommodations' => $this->accommodationRepository->findBy([], ['name' => 'ASC']),
];
}
/**
* Everyone worth filtering by: the staff a booking can be assigned to, plus whoever is
* currently assigned to one.
*
* The second half matters because the two lists drift apart — somebody assigned last season
* may have lost the groups role since, and their bookings would otherwise be impossible to
* find. The edit form keeps the current value in its choices for the same reason.
*
* @return User[]
*/
private function managers(): array
{
$managers = [];
foreach ([...$this->userRepository->findGroupsStaff(), ...$this->bookingRepository->findAssignedManagers()] as $manager) {
$managers[(int) $manager->getId()] = $manager;
}
uasort($managers, static fn (User $a, User $b) => strcasecmp($a->getDisplayName(), $b->getDisplayName()));
return array_values($managers);
}
}
src/Form/Admin/Filter/AccommodationBookingFilterType.php
+12 -24
View File
@@ -9,6 +9,8 @@ use App\Entity\User;
use App\Enum\Groups\AccommodationBookingOrigin;
use App\Enum\Groups\AccommodationBookingStatus;
use App\Form\Model\Filter\AccommodationBookingFilterDto;
use App\Repository\Groups\AccommodationRepository;
use App\Repository\UserRepository;
use Symfony\Bridge\Doctrine\Form\Type\EntityType;
use Symfony\Component\Form\Extension\Core\Type\CheckboxType;
use Symfony\Component\Form\Extension\Core\Type\EnumType;
@@ -21,7 +23,7 @@ class AccommodationBookingFilterType extends AbstractListFilterType
{
protected function filterFields(array $options): array
{
$fields = [
return [
'status' => [EnumType::class, [
'label' => 'Status',
'class' => AccommodationBookingStatus::class,
@@ -41,36 +43,28 @@ class AccommodationBookingFilterType extends AbstractListFilterType
'accommodation' => [EntityType::class, [
'label' => 'Gruppenhaus',
'class' => Accommodation::class,
'choices' => $options['accommodations'],
'query_builder' => static fn (AccommodationRepository $er) => $er
->createQueryBuilder('a')
->orderBy('a.name', 'ASC'),
'choice_label' => 'name',
'placeholder' => 'alle',
'required' => false,
]],
];
// Mirrors the edit form: only group admins get to look at other people's bookings, so
// for everyone else the fields simply do not exist and the scope is pinned server-side.
if (true === $options['can_filter_by_manager']) {
// Triage does not depend on there being anyone to choose from, so this stays even
// when nobody carries a groups role yet.
$fields['unassigned'] = [CheckboxType::class, [
'unassigned' => [CheckboxType::class, [
'label' => 'nur ohne Zuordnung',
'required' => false,
]];
if ([] !== $options['managers']) {
$fields['managedBy'] = [EntityType::class, [
]],
'managedBy' => [EntityType::class, [
'label' => 'Betreuer:in',
'class' => User::class,
'choices' => $options['managers'],
'query_builder' => static fn (UserRepository $er) => $er->createAccommodationBookingManagersQueryBuilder(),
'choice_label' => 'displayName',
'placeholder' => 'alle',
'required' => false,
]];
}
}
return $fields;
]],
];
}
public function configureOptions(OptionsResolver $resolver): void
@@ -79,13 +73,7 @@ class AccommodationBookingFilterType extends AbstractListFilterType
$resolver->setDefaults([
'data_class' => AccommodationBookingFilterDto::class,
'can_filter_by_manager' => false,
'managers' => [],
'accommodations' => [],
]);
$resolver->setAllowedTypes('can_filter_by_manager', 'bool');
$resolver->setAllowedTypes('managers', User::class.'[]');
$resolver->setAllowedTypes('accommodations', Accommodation::class.'[]');
}
protected function searchPlaceholder(): string
src/Form/Model/Filter/AccommodationBookingFilterDto.php
+5 -16
View File
@@ -28,22 +28,16 @@ class AccommodationBookingFilterDto extends AbstractListFilterDto
public ?Accommodation $accommodation = null;
/**
* Whether the manager scope is imposed rather than chosen. A group manager without admin
* rights only ever sees their own bookings, so the chip has to be shown — otherwise the
* short list looks like a bug — but must not offer a way out that would not work.
*/
public bool $managedByLocked = false;
/**
* The list as it presents itself to someone who has not filtered yet.
*
* The still-live statuses — drafts, inquiries awaiting an offer, offers out with the
* customer and bookings awaiting validation, i.e. everything that may still need work —
* nothing whose stay is already over, and, for staff who are not group admins, only the
* bookings they are responsible for.
* bookings they are responsible for by default (they remain free to widen or change this
* through the filter, same as a group admin).
*/
public static function defaults(bool $seesAllBookings, ?User $currentUser): self
public static function defaults(bool $isGroupsAdmin, ?User $currentUser): self
{
$filter = new self();
$filter->status = [
@@ -54,9 +48,8 @@ class AccommodationBookingFilterDto extends AbstractListFilterDto
];
$filter->dateFrom = new \DateTimeImmutable('today');
if (!$seesAllBookings) {
if (!$isGroupsAdmin) {
$filter->managedBy = $currentUser;
$filter->managedByLocked = true;
}
return $filter;
@@ -85,11 +78,7 @@ class AccommodationBookingFilterDto extends AbstractListFilterDto
if ($this->unassigned) {
$chips[] = new ListFilterChip('Betreuer:in', 'keine Zuordnung', ['unassigned']);
} elseif (null !== $this->managedBy) {
$chips[] = new ListFilterChip(
'Betreuer:in',
$this->managedBy->getDisplayName(),
$this->managedByLocked ? [] : ['managedBy'],
);
$chips[] = new ListFilterChip('Betreuer:in', $this->managedBy->getDisplayName(), ['managedBy']);
}
if (null !== $this->accommodation) {
src/Repository/UserRepository.php
+21
View File
@@ -4,10 +4,12 @@ declare(strict_types=1);
namespace App\Repository;
use App\Entity\Groups\AccommodationBooking;
use App\Entity\User;
use App\Form\Model\Filter\UserFilterDto;
use App\Repository\Filter\AppliesListFiltersTrait;
use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
use Doctrine\ORM\Query\Expr\Join;
use Doctrine\ORM\QueryBuilder;
use Doctrine\Persistence\ManagerRegistry;
@@ -67,4 +69,23 @@ class UserRepository extends ServiceEntityRepository
->getQuery()
->getResult();
}
/**
* Everyone worth filtering an accommodation booking by: current groups staff, plus whoever
* a booking is still assigned to even after losing the role — otherwise a booking assigned
* last season would become unfindable the moment its manager is deroled.
*/
public function createAccommodationBookingManagersQueryBuilder(): QueryBuilder
{
return $this->createQueryBuilder('u')
->leftJoin(AccommodationBooking::class, 'booking', Join::ON, 'booking.managedBy = u')
->andWhere('u.roles LIKE :groupsAdmin OR u.roles LIKE :groupsManager OR booking.id IS NOT NULL')
->setParameter('groupsAdmin', '%"ROLE_GROUPS_ADMIN"%')
->setParameter('groupsManager', '%"ROLE_GROUPS_MANAGER"%')
->distinct()
->orderBy('u.firstName')
->addOrderBy('u.lastName')
->addOrderBy('u.email')
;
}
}
tests/Form/Admin/Filter/AccommodationBookingFilterOptionsProviderTest.php
-97
View File
@@ -1,97 +0,0 @@
<?php
declare(strict_types=1);
namespace App\Tests\Form\Admin\Filter;
use App\Entity\User;
use App\Form\Admin\Filter\AccommodationBookingFilterOptionsProvider;
use App\Repository\Groups\AccommodationBookingRepository;
use App\Repository\Groups\AccommodationRepository;
use App\Repository\UserRepository;
use PHPUnit\Framework\TestCase;
class AccommodationBookingFilterOptionsProviderTest extends TestCase
{
public function testSomebodyAssignedWithoutTheRoleStaysFilterable(): void
{
$staff = $this->user(1, '[email protected]');
$formerStaff = $this->user(2, '[email protected]');
$options = $this->provider([$staff], [$formerStaff])->formOptions(true);
self::assertSame(
['[email protected]', '[email protected]'],
array_map(static fn (User $u) => $u->getEmail(), $options['managers']),
'a booking assigned to someone who lost the groups role has to remain findable',
);
}
public function testSomebodyBothAssignedAndOnStaffIsOfferedOnce(): void
{
$manager = $this->user(1, '[email protected]');
$options = $this->provider([$manager], [$manager])->formOptions(true);
self::assertCount(1, $options['managers']);
}
public function testManagersAreSortedByEmail(): void
{
$options = $this->provider(
[$this->user(1, '[email protected]'), $this->user(2, '[email protected]')],
[$this->user(3, '[email protected]')],
)->formOptions(true);
self::assertSame(
['[email protected]', '[email protected]', '[email protected]'],
array_map(static fn (User $u) => $u->getEmail(), $options['managers']),
);
}
public function testStaffWhoCannotSeeOtherBookingsGetNoManagerFilterAtAll(): void
{
$options = $this->provider([$this->user(1, '[email protected]')], [])->formOptions(false);
self::assertFalse($options['can_filter_by_manager']);
self::assertSame([], $options['managers']);
}
public function testTheManagerFilterIsAPermissionNotAConsequenceOfEmptyData(): void
{
// Nobody holds a groups role and nothing is assigned yet — a group admin still gets to
// filter, which is what keeps the "no assignment" triage available on day one.
$options = $this->provider([], [])->formOptions(true);
self::assertTrue($options['can_filter_by_manager']);
self::assertSame([], $options['managers']);
}
/**
* @param User[] $groupsStaff
* @param User[] $assigned
*/
private function provider(array $groupsStaff, array $assigned): AccommodationBookingFilterOptionsProvider
{
$users = $this->createMock(UserRepository::class);
$users->method('findGroupsStaff')->willReturn($groupsStaff);
$bookings = $this->createMock(AccommodationBookingRepository::class);
$bookings->method('findAssignedManagers')->willReturn($assigned);
$accommodations = $this->createMock(AccommodationRepository::class);
$accommodations->method('findBy')->willReturn([]);
return new AccommodationBookingFilterOptionsProvider($users, $bookings, $accommodations);
}
private function user(int $id, string $email): User
{
$user = new User($email);
$idProperty = new \ReflectionProperty(User::class, 'id');
$idProperty->setValue($user, $id);
return $user;
}
}
tests/Form/Model/Filter/AccommodationBookingFilterDtoTest.php
+3 -5
View File
@@ -28,26 +28,24 @@ class AccommodationBookingFilterDtoTest extends TestCase
);
self::assertSame((new \DateTimeImmutable('today'))->format('Y-m-d'), $filter->dateFrom?->format('Y-m-d'));
self::assertNull($filter->managedBy, 'a group admin sees everybody’s bookings');
self::assertFalse($filter->managedByLocked);
}
public function testEverybodyElseStartsPinnedToTheirOwnBookings(): void
public function testEverybodyElseStartsFilteredToTheirOwnBookings(): void
{
$user = new User('[email protected]');
$filter = AccommodationBookingFilterDto::defaults(false, $user);
self::assertSame($user, $filter->managedBy);
self::assertTrue($filter->managedByLocked);
}
public function testThePinnedManagerChipCannotBeDismissed(): void
public function testTheDefaultManagerChipCanBeDismissed(): void
{
$filter = AccommodationBookingFilterDto::defaults(false, new User('[email protected]'));
$chip = $this->chipFor($filter, 'Betreuer:in');
self::assertNotNull($chip);
self::assertFalse($chip->isRemovable());
self::assertTrue($chip->isRemovable());
}
public function testAChosenManagerChipCanBeDismissed(): void