Skip to content
Merged
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file.

## [Unreleased]

- [#260](https://github.com/os2display/display-api-service/pull/260)
- Changed how exceptions are handled in InstantBook.

## [2.5.1] - 2025-06-23

- [#245](https://github.com/os2display/display-api-service/pull/245)
Expand Down
20 changes: 16 additions & 4 deletions src/Controller/InteractiveController.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@
use App\Entity\ScreenUser;
use App\Entity\Tenant\Slide;
use App\Entity\User;
use App\Exceptions\NotFoundException;
use App\Exceptions\InteractiveSlideException;
use App\Service\InteractiveSlideService;
use Symfony\Bundle\SecurityBundle\Security;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Attribute\AsController;
use Symfony\Component\HttpKernel\Exception\HttpException;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;

#[AsController]
final readonly class InteractiveController
Expand All @@ -26,14 +28,24 @@ public function __invoke(Request $request, Slide $slide): JsonResponse
{
$requestBody = $request->toArray();

$interactionRequest = $this->interactiveSlideService->parseRequestBody($requestBody);
try {
$interactionRequest = $this->interactiveSlideService->parseRequestBody($requestBody);
} catch (InteractiveSlideException $e) {
throw new HttpException($e->getCode(), $e->getMessage());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use API Platforms Exception to status. That way we have one line of config instead of having to catch the domain exceptions multiple places.

}

$user = $this->security->getUser();

if (!($user instanceof User || $user instanceof ScreenUser)) {
throw new NotFoundException('User not found');
throw new NotFoundHttpException('User not found');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Questions:

  • We only have these two user types, so how is this even relevant?
  • If it is relevant it seems "Access denied" is the proper exception to throw
  • Should this only be allowed for ScreenUsers or are there legitimate use cases for normal users or service accounts?

}

try {
$actionResult = $this->interactiveSlideService->performAction($user, $slide, $interactionRequest);
} catch (InteractiveSlideException $e) {
throw new HttpException($e->getCode(), $e->getMessage());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use "Exception to status" as above

}

return new JsonResponse($this->interactiveSlideService->performAction($user, $slide, $interactionRequest));
return new JsonResponse($actionResult);
}
}
Loading