Pass $options in knp_pager.before event
#342
Conversation
| @@ -15,7 +15,8 @@ final class BeforeEvent extends Event | |||
| public function __construct( | |||
| private readonly EventDispatcherInterface $eventDispatcher, | |||
| private readonly ArgumentAccessInterface $argumentAccess, | |||
| private readonly ?Connection $connection = null | |||
| private readonly ?Connection $connection = null, | |||
| private array &$options = [], | |||
There was a problem hiding this comment.
do we really need to pass this argument by reference?
| @@ -29,6 +30,11 @@ public function getArgumentAccess(): ArgumentAccessInterface | |||
| return $this->argumentAccess; | |||
| } | |||
|
|
|||
| public function &getOptions(): array | |||
There was a problem hiding this comment.
same objection about reference here
|
I share your reservations regarding the use of references 🤔. I am not quite sure, but my feeling is that the We could follow the same pattern that is used for the WDYT? |
…d reference handling
mpdude
force-pushed
the
options-in-before-event
branch
from
8fac657 to
7163ed1
Compare
|
Test case added. What would be appropriate documentation? |
I don't know. It's your proposal, meant to fit your needs. |
|
I've scanned the current documentation and there is nothing that explains in detail how events work or how they should be used. So, I'd say one has to understand it from looking at the source code, and there is nothing particular I'd suggest to add in the scope of this PR. |
Do you think there's room for improvement? Maybe you can propose a different PR to enhance documentation, explaining a bit how a generic customization (or even your specific customization) can work. It would be much appreciated |
With this PR I'd like to suggest to pass the
$optionsarray with theknp_pager.beforeevent in a way that makes it possible to modify the options from event subscribers.Motivating use case:
I'd like to use values for the sort field parameter that do not expose implementation details in the URL. This is necessary to maintain stable URLs.
For example, instead of using
blog.title+blog.published_date+blog.idas a value in the sort query parameter, usetitle.The translation from
titleto the actual sort criteria could take place in aknp_pager.beforesubscriber; however, it would need to be able to access the options, since I need to pass in information about which pagination use case is being executed (the mapping from query parameter to actual sort expression depends on it).