Skip to content

Conversation

@ViniTou
Copy link
Contributor

@ViniTou ViniTou commented Jun 18, 2025

🎫 Issue IBX-10170

Description:

For QA:

Documentation:

public static function getSubscribedEvents(): array
{
return [
ControllerArgumentsEvent::class => 'onControllerArgumentsEvent',
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't kernel.controller event more appropriate?

This event is dispatched after the controller has been resolved but before executing it. It's useful to initialize things later needed by the controller...

kernel.controller_arguments has a different purpose.

This event is dispatched just before a controller is called. It's useful to configure the arguments that are going to be passed to the controller.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, it could be to early and arguments for controller are still not set. @adamwojs , did you tried mentioned one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked this out, and it seems that everything is working properly wirh kernel.controller.

2ba7b59

public function onControllerArgumentsEvent(ControllerArgumentsEvent $event): void
{
$controller = $event->getController();
if (is_array($controller) && $controller[0] instanceof AccessCheckController) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure this can also be an invoke'able object (with __invoke method). While we ourselves never use controllers like that, there is nothing stoping someone else from doing so - since it's in Contracts namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was just moving this from admin-ui, but you are right - fixed.
Also, string callables with static methods are also possible so I tried to handle those as well.

@ViniTou ViniTou force-pushed the ibx-10170-access-check branch from f42b5e7 to 5b9f72d Compare June 24, 2025 10:48
@ViniTou ViniTou force-pushed the ibx-10170-access-check branch from 5b9f72d to 0daae07 Compare June 24, 2025 10:50
@ViniTou ViniTou requested a review from Steveb-p June 24, 2025 12:01
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 4, 2025

@ViniTou ViniTou requested a review from konradoboza July 4, 2025 09:02
@juskora
Copy link

juskora commented Jul 10, 2025

QA Approved on Ibexa DXP Commerce 4.6-dev.

@konradoboza konradoboza merged commit cfff315 into main Jul 10, 2025
14 checks passed
@konradoboza konradoboza deleted the ibx-10170-access-check branch July 10, 2025 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants