diff --git a/src/Controller/Organizations/TicketsController.php b/src/Controller/Organizations/TicketsController.php index f9b2f2bb..03ca91ff 100644 --- a/src/Controller/Organizations/TicketsController.php +++ b/src/Controller/Organizations/TicketsController.php @@ -22,6 +22,7 @@ use App\Service\ActorsLister; use App\TicketActivity\MessageEvent; use App\TicketActivity\TicketEvent; +use App\Utils\ArrayHelper; use App\Utils\ConstraintErrorsFormatter; use App\Utils\Pagination; use App\Utils\Time; @@ -228,7 +229,10 @@ public function create( ]); } - $requester = $userRepository->findOneBy(['uid' => $requesterUid]); + $requester = ArrayHelper::find($allUsers, function ($user) use ($requesterUid): bool { + return $user->getUid() === $requesterUid; + }); + if (!$requester) { return $this->renderBadRequest('organizations/tickets/new.html.twig', [ 'organization' => $organization, @@ -249,29 +253,11 @@ public function create( ]); } + $assignee = null; if ($assigneeUid) { - $assignee = $userRepository->findOneBy(['uid' => $assigneeUid]); - if (!$assignee) { - return $this->renderBadRequest('organizations/tickets/new.html.twig', [ - 'organization' => $organization, - 'title' => $title, - 'message' => $messageContent, - 'type' => $type, - 'requesterUid' => $requesterUid, - 'assigneeUid' => $assigneeUid, - 'isResolved' => $isResolved, - 'urgency' => $urgency, - 'impact' => $impact, - 'priority' => $priority, - 'allUsers' => $allUsers, - 'agents' => $agents, - 'errors' => [ - 'assignee' => $translator->trans('ticket.assignee.invalid', [], 'errors'), - ], - ]); - } - } else { - $assignee = null; + $assignee = ArrayHelper::find($agents, function ($agent) use ($assigneeUid): bool { + return $agent->getUid() === $assigneeUid; + }); } $ticket = new Ticket(); diff --git a/src/Controller/Tickets/ActorsController.php b/src/Controller/Tickets/ActorsController.php index f5f99d3d..05070885 100644 --- a/src/Controller/Tickets/ActorsController.php +++ b/src/Controller/Tickets/ActorsController.php @@ -12,6 +12,7 @@ use App\Repository\UserRepository; use App\Service\ActorsLister; use App\TicketActivity\TicketEvent; +use App\Utils\ArrayHelper; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Request; @@ -95,7 +96,10 @@ public function update( ]); } - $requester = $userRepository->findOneBy(['uid' => $requesterUid]); + $requester = ArrayHelper::find($allUsers, function ($user) use ($requesterUid): bool { + return $user->getUid() === $requesterUid; + }); + if (!$requester) { return $this->renderBadRequest('tickets/actors/edit.html.twig', [ 'ticket' => $ticket, @@ -109,22 +113,11 @@ public function update( ]); } + $assignee = null; if ($assigneeUid) { - $assignee = $userRepository->findOneBy(['uid' => $assigneeUid]); - if (!$assignee) { - return $this->renderBadRequest('tickets/actors/edit.html.twig', [ - 'ticket' => $ticket, - 'requesterUid' => $requesterUid, - 'assigneeUid' => $assigneeUid, - 'allUsers' => $allUsers, - 'agents' => $agents, - 'errors' => [ - 'assignee' => $translator->trans('ticket.assignee.invalid', [], 'errors'), - ], - ]); - } - } else { - $assignee = null; + $assignee = ArrayHelper::find($agents, function ($agent) use ($assigneeUid): bool { + return $agent->getUid() === $assigneeUid; + }); } $previousAssignee = $ticket->getAssignee(); diff --git a/tests/AuthorizationHelper.php b/tests/AuthorizationHelper.php index 6a2eaa81..19a4b7ca 100644 --- a/tests/AuthorizationHelper.php +++ b/tests/AuthorizationHelper.php @@ -55,9 +55,14 @@ public function grantAdmin(User $user, array $permissions): void /** * @param string[] $permissions + * @param 'user'|'agent' $type */ - public function grantOrga(User $user, array $permissions, ?Organization $organization = null): void - { + public function grantOrga( + User $user, + array $permissions, + ?Organization $organization = null, + string $type = 'agent', + ): void { if (empty($permissions)) { return; } @@ -72,12 +77,12 @@ public function grantOrga(User $user, array $permissions, ?Organization $organiz /** @var \App\Repository\AuthorizationRepository $authorizationRepo */ $authorizationRepo = $entityManager->getRepository(Authorization::class); - $permissions = Role::sanitizePermissions('agent', $permissions); + $permissions = Role::sanitizePermissions($type, $permissions); $role = new Role(); $role->setName(Random::hex(10)); $role->setDescription('The role description'); - $role->setType('agent'); + $role->setType($type); $role->setPermissions($permissions); $roleRepo->save($role); diff --git a/tests/Controller/Organizations/TicketsControllerTest.php b/tests/Controller/Organizations/TicketsControllerTest.php index 818e430c..cfe72420 100644 --- a/tests/Controller/Organizations/TicketsControllerTest.php +++ b/tests/Controller/Organizations/TicketsControllerTest.php @@ -195,7 +195,7 @@ public function testPostCreateCreatesATicketAndRedirects(): void list( $user, $requester, - ) = UserFactory::createMany(3); + ) = UserFactory::createMany(2); $client->loginUser($user->object()); $organization = OrganizationFactory::createOne(); $this->grantOrga($user->object(), [ @@ -205,6 +205,9 @@ public function testPostCreateCreatesATicketAndRedirects(): void 'orga:update:tickets:actors', 'orga:update:tickets:priority', ], $organization->object()); + $this->grantOrga($requester->object(), [ + 'orga:create:tickets', + ], $organization->object()); $title = 'My ticket'; $messageContent = 'My message'; @@ -351,15 +354,14 @@ public function testPostCreateAttachesDocumentsToMessage(): void $this->assertSame($message->getUid(), $messageDocument2->getMessage()->getUid()); } - public function testPostCreateCanAssignsAUser(): void + public function testPostCreateCanAssignAUser(): void { $now = new \DateTimeImmutable('2022-11-02'); $client = static::createClient(); list( $user, - $requester, $assignee - ) = UserFactory::createMany(3); + ) = UserFactory::createMany(2); $client->loginUser($user->object()); $organization = OrganizationFactory::createOne(); $this->grantOrga($user->object(), [ @@ -369,6 +371,9 @@ public function testPostCreateCanAssignsAUser(): void 'orga:update:tickets:actors', 'orga:update:tickets:priority', ], $organization->object()); + $this->grantOrga($assignee->object(), [ + 'orga:create:tickets', + ], $organization->object()); $title = 'My ticket'; $messageContent = 'My message'; @@ -378,7 +383,7 @@ public function testPostCreateCanAssignsAUser(): void $client->request('POST', "/organizations/{$organization->getUid()}/tickets/new", [ '_csrf_token' => $this->generateCsrfToken($client, 'create organization ticket'), 'title' => $title, - 'requesterUid' => $requester->getUid(), + 'requesterUid' => $user->getUid(), 'assigneeUid' => $assignee->getUid(), 'type' => 'incident', 'urgency' => 'high', @@ -399,10 +404,7 @@ public function testPostCreateCanAssignsAUser(): void public function testPostCreateCanMarkATicketAsResolved(): void { $client = static::createClient(); - list( - $user, - $requester - ) = UserFactory::createMany(2); + $user = UserFactory::createOne(); $client->loginUser($user->object()); $organization = OrganizationFactory::createOne(); $this->grantOrga($user->object(), [ @@ -418,7 +420,7 @@ public function testPostCreateCanMarkATicketAsResolved(): void $client->request('POST', "/organizations/{$organization->getUid()}/tickets/new", [ '_csrf_token' => $this->generateCsrfToken($client, 'create organization ticket'), 'title' => $title, - 'requesterUid' => $requester->getUid(), + 'requesterUid' => $user->getUid(), 'message' => $messageContent, 'isResolved' => true, ]); @@ -472,12 +474,14 @@ public function testPostCreateCanCreateATicketWithMinimalPermissions(): void $this->assertSame('webapp', $message->getVia()); } - public function testPostCreateFailsIfTitleIsEmpty(): void + public function testPostCreateDoesNotAssignIfUserIsNotAgent(): void { $now = new \DateTimeImmutable('2022-11-02'); - Time::freeze($now); $client = static::createClient(); - $user = UserFactory::createOne(); + list( + $user, + $assignee + ) = UserFactory::createMany(2); $client->loginUser($user->object()); $organization = OrganizationFactory::createOne(); $this->grantOrga($user->object(), [ @@ -487,25 +491,66 @@ public function testPostCreateFailsIfTitleIsEmpty(): void 'orga:update:tickets:actors', 'orga:update:tickets:priority', ], $organization->object()); - $title = ''; + $this->grantOrga($assignee->object(), [ + 'orga:create:tickets', + ], $organization->object(), 'user'); + $title = 'My ticket'; $messageContent = 'My message'; - $this->assertSame(0, TicketFactory::count()); - $client->request('POST', "/organizations/{$organization->getUid()}/tickets/new", [ '_csrf_token' => $this->generateCsrfToken($client, 'create organization ticket'), 'title' => $title, 'requesterUid' => $user->getUid(), + 'assigneeUid' => $assignee->getUid(), + 'type' => 'incident', + 'urgency' => 'high', + 'impact' => 'high', + 'priority' => 'high', + 'message' => $messageContent, + ]); + + $this->assertSame(1, TicketFactory::count()); + $this->assertSame(1, MessageFactory::count()); + + $ticket = TicketFactory::first(); + $this->assertResponseRedirects("/tickets/{$ticket->getUid()}", 302); + $this->assertSame('new', $ticket->getStatus()); + $this->assertNull($ticket->getAssignee()); + } + + public function testPostCreateFailsIfRequesterIsNotInOrganization(): void + { + $now = new \DateTimeImmutable('2022-11-02'); + $client = static::createClient(); + list( + $user, + $requester, + ) = UserFactory::createMany(2); + $client->loginUser($user->object()); + $organization = OrganizationFactory::createOne(); + $this->grantOrga($user->object(), [ + 'orga:create:tickets', + 'orga:update:tickets:status', + 'orga:update:tickets:type', + 'orga:update:tickets:actors', + 'orga:update:tickets:priority', + ], $organization->object()); + $title = 'My ticket'; + $messageContent = 'My message'; + + $client->request('POST', "/organizations/{$organization->getUid()}/tickets/new", [ + '_csrf_token' => $this->generateCsrfToken($client, 'create organization ticket'), + 'title' => $title, + 'requesterUid' => $requester->getUid(), 'message' => $messageContent, ]); - Time::unfreeze(); $this->assertSame(0, TicketFactory::count()); $this->assertSame(0, MessageFactory::count()); - $this->assertSelectorTextContains('#title-error', 'Enter a title'); + $this->assertSelectorTextContains('#requester-error', 'Select a user from the list'); } - public function testPostCreateFailsIfTitleIsTooLong(): void + public function testPostCreateFailsIfTitleIsEmpty(): void { $now = new \DateTimeImmutable('2022-11-02'); Time::freeze($now); @@ -520,7 +565,7 @@ public function testPostCreateFailsIfTitleIsTooLong(): void 'orga:update:tickets:actors', 'orga:update:tickets:priority', ], $organization->object()); - $title = str_repeat('a', 256); + $title = ''; $messageContent = 'My message'; $this->assertSame(0, TicketFactory::count()); @@ -535,10 +580,10 @@ public function testPostCreateFailsIfTitleIsTooLong(): void Time::unfreeze(); $this->assertSame(0, TicketFactory::count()); $this->assertSame(0, MessageFactory::count()); - $this->assertSelectorTextContains('#title-error', 'Enter a title of less than 255 characters'); + $this->assertSelectorTextContains('#title-error', 'Enter a title'); } - public function testPostCreateFailsIfMessageIsEmpty(): void + public function testPostCreateFailsIfTitleIsTooLong(): void { $now = new \DateTimeImmutable('2022-11-02'); Time::freeze($now); @@ -553,8 +598,8 @@ public function testPostCreateFailsIfMessageIsEmpty(): void 'orga:update:tickets:actors', 'orga:update:tickets:priority', ], $organization->object()); - $title = 'My ticket'; - $messageContent = ''; + $title = str_repeat('a', 256); + $messageContent = 'My message'; $this->assertSame(0, TicketFactory::count()); @@ -568,10 +613,10 @@ public function testPostCreateFailsIfMessageIsEmpty(): void Time::unfreeze(); $this->assertSame(0, TicketFactory::count()); $this->assertSame(0, MessageFactory::count()); - $this->assertSelectorTextContains('#message-error', 'Enter a message'); + $this->assertSelectorTextContains('#title-error', 'Enter a title of less than 255 characters'); } - public function testPostCreateFailsIfRequesterIsInvalid(): void + public function testPostCreateFailsIfMessageIsEmpty(): void { $now = new \DateTimeImmutable('2022-11-02'); Time::freeze($now); @@ -587,24 +632,24 @@ public function testPostCreateFailsIfRequesterIsInvalid(): void 'orga:update:tickets:priority', ], $organization->object()); $title = 'My ticket'; - $messageContent = 'My message'; + $messageContent = ''; $this->assertSame(0, TicketFactory::count()); $client->request('POST', "/organizations/{$organization->getUid()}/tickets/new", [ '_csrf_token' => $this->generateCsrfToken($client, 'create organization ticket'), 'title' => $title, - 'requesterUid' => 'not an uid', + 'requesterUid' => $user->getUid(), 'message' => $messageContent, ]); Time::unfreeze(); $this->assertSame(0, TicketFactory::count()); $this->assertSame(0, MessageFactory::count()); - $this->assertSelectorTextContains('#requester-error', 'Select a user from the list'); + $this->assertSelectorTextContains('#message-error', 'Enter a message'); } - public function testPostCreateFailsIfAssigneeIsInvalid(): void + public function testPostCreateFailsIfRequesterIsInvalid(): void { $now = new \DateTimeImmutable('2022-11-02'); Time::freeze($now); @@ -627,15 +672,14 @@ public function testPostCreateFailsIfAssigneeIsInvalid(): void $client->request('POST', "/organizations/{$organization->getUid()}/tickets/new", [ '_csrf_token' => $this->generateCsrfToken($client, 'create organization ticket'), 'title' => $title, - 'requesterUid' => $user->getUid(), - 'assigneeUid' => 'not an uid', + 'requesterUid' => 'not an uid', 'message' => $messageContent, ]); Time::unfreeze(); $this->assertSame(0, TicketFactory::count()); $this->assertSame(0, MessageFactory::count()); - $this->assertSelectorTextContains('#assignee-error', 'Select a user from the list'); + $this->assertSelectorTextContains('#requester-error', 'Select a user from the list'); } public function testPostCreateFailsIfCsrfTokenIsInvalid(): void diff --git a/tests/Controller/Tickets/ActorsControllerTest.php b/tests/Controller/Tickets/ActorsControllerTest.php index f237d78b..68d7a817 100644 --- a/tests/Controller/Tickets/ActorsControllerTest.php +++ b/tests/Controller/Tickets/ActorsControllerTest.php @@ -96,6 +96,8 @@ public function testPostUpdateSavesTicketAndRedirects(): void ) = UserFactory::createMany(3); $client->loginUser($user->object()); $this->grantOrga($user->object(), ['orga:update:tickets:actors']); + $this->grantOrga($requester->object(), ['orga:create:tickets']); + $this->grantOrga($assignee->object(), ['orga:create:tickets']); $ticket = TicketFactory::createOne([ 'createdBy' => $user, 'requester' => null, @@ -120,10 +122,10 @@ public function testPostUpdateAcceptsEmptyAssignee(): void list( $user, $requester, - $assignee, - ) = UserFactory::createMany(3); + ) = UserFactory::createMany(2); $client->loginUser($user->object()); $this->grantOrga($user->object(), ['orga:update:tickets:actors']); + $this->grantOrga($requester->object(), ['orga:create:tickets']); $ticket = TicketFactory::createOne([ 'createdBy' => $user, 'requester' => null, @@ -142,34 +144,43 @@ public function testPostUpdateAcceptsEmptyAssignee(): void $this->assertNull($ticket->getAssignee()); } - public function testPostUpdateFailsIfRequesterIsInvalid(): void + public function testPostUpdateSetsNullIfAssigneeIsNotAgent(): void { $client = static::createClient(); - $user = UserFactory::createOne(); + list( + $user, + $requester, + $assignee, + ) = UserFactory::createMany(3); $client->loginUser($user->object()); $this->grantOrga($user->object(), ['orga:update:tickets:actors']); + $this->grantOrga($requester->object(), ['orga:create:tickets']); + $this->grantOrga($assignee->object(), ['orga:create:tickets'], type: 'user'); $ticket = TicketFactory::createOne([ 'createdBy' => $user, 'requester' => null, - 'assignee' => null, + 'assignee' => $user, ]); $client->request('POST', "/tickets/{$ticket->getUid()}/actors/edit", [ '_csrf_token' => $this->generateCsrfToken($client, 'update ticket actors'), - 'requesterUid' => 'not an uid', - 'assigneeUid' => $user->getUid(), + 'requesterUid' => $requester->getUid(), + 'assigneeUid' => $assignee->getUid(), ]); - $this->assertSelectorTextContains('#requester-error', 'Select a user from the list'); + $this->assertResponseRedirects("/tickets/{$ticket->getUid()}", 302); $ticket->refresh(); - $this->assertNull($ticket->getRequester()); + $this->assertSame($requester->getUid(), $ticket->getRequester()->getUid()); $this->assertNull($ticket->getAssignee()); } - public function testPostUpdateFailsIfAssigneeIsInvalid(): void + public function testPostUpdateFailsIfRequesterIsNotInOrganization(): void { $client = static::createClient(); - $user = UserFactory::createOne(); + list( + $user, + $requester, + ) = UserFactory::createMany(2); $client->loginUser($user->object()); $this->grantOrga($user->object(), ['orga:update:tickets:actors']); $ticket = TicketFactory::createOne([ @@ -180,11 +191,11 @@ public function testPostUpdateFailsIfAssigneeIsInvalid(): void $client->request('POST', "/tickets/{$ticket->getUid()}/actors/edit", [ '_csrf_token' => $this->generateCsrfToken($client, 'update ticket actors'), - 'requesterUid' => $user->getUid(), - 'assigneeUid' => 'not an uid', + 'requesterUid' => $requester->getUid(), + 'assigneeUid' => $user->getUid(), ]); - $this->assertSelectorTextContains('#assignee-error', 'Select a user from the list'); + $this->assertSelectorTextContains('#requester-error', 'Select a user from the list'); $ticket->refresh(); $this->assertNull($ticket->getRequester()); $this->assertNull($ticket->getAssignee()); @@ -200,6 +211,8 @@ public function testPostUpdateFailsIfCsrfTokenIsInvalid(): void ) = UserFactory::createMany(3); $client->loginUser($user->object()); $this->grantOrga($user->object(), ['orga:update:tickets:actors']); + $this->grantOrga($requester->object(), ['orga:create:tickets']); + $this->grantOrga($assignee->object(), ['orga:create:tickets']); $ticket = TicketFactory::createOne([ 'createdBy' => $user, 'requester' => null, @@ -229,6 +242,8 @@ public function testPostUpdateFailsIfAccessIsForbidden(): void $assignee, ) = UserFactory::createMany(3); $client->loginUser($user->object()); + $this->grantOrga($requester->object(), ['orga:create:tickets']); + $this->grantOrga($assignee->object(), ['orga:create:tickets']); $ticket = TicketFactory::createOne([ 'createdBy' => $user, 'requester' => null, @@ -255,6 +270,8 @@ public function testPostUpdateFailsIfAccessToTicketIsForbidden(): void ) = UserFactory::createMany(3); $client->loginUser($user->object()); $this->grantOrga($user->object(), ['orga:update:tickets:actors']); + $this->grantOrga($requester->object(), ['orga:create:tickets']); + $this->grantOrga($assignee->object(), ['orga:create:tickets']); $ticket = TicketFactory::createOne([ 'requester' => null, 'assignee' => null, diff --git a/translations/errors+intl-icu.en_GB.yaml b/translations/errors+intl-icu.en_GB.yaml index bb48cea8..f6560e4f 100644 --- a/translations/errors+intl-icu.en_GB.yaml +++ b/translations/errors+intl-icu.en_GB.yaml @@ -37,7 +37,6 @@ role.name.required: 'Enter a name.' team.name.already_used: 'Enter a different name, a team already has this name.' team.name.max_chars: 'Enter a name of less than {limit} characters.' team.name.required: 'Enter a name.' -ticket.assignee.invalid: 'Select a user from the list.' ticket.impact.invalid: 'Select an impact from the list.' ticket.priority.invalid: 'Select a priority from the list.' ticket.requester.invalid: 'Select a user from the list.' diff --git a/translations/errors+intl-icu.fr_FR.yaml b/translations/errors+intl-icu.fr_FR.yaml index b5478176..96dd035a 100644 --- a/translations/errors+intl-icu.fr_FR.yaml +++ b/translations/errors+intl-icu.fr_FR.yaml @@ -37,7 +37,6 @@ role.name.required: 'Saisissez un nom.' team.name.already_used: 'Saisissez un nom différent, une équipe porte déjà ce même nom.' team.name.max_chars: 'Saisissez un nom de moins de {limit} caractères.' team.name.required: 'Saisissez un nom.' -ticket.assignee.invalid: 'Sélectionnez une personne de la liste.' ticket.impact.invalid: 'Sélectionnez un impact de la liste.' ticket.priority.invalid: 'Sélectionnez une priorité de la liste.' ticket.requester.invalid: 'Sélectionnez une personne de la liste.'