I require to alter the filter params before they are processed. There was no clean way to do it besides overriding the class or do it in a later event.
Add a AlterSearchQueryFiltersEvent that is triggered after QueryParamBuilder builds the filters, but before it builds the rest of the query (i.e.: before the filters are processed).
None.
None.
Adds an AlterSearchQueryFiltersEvent that is triggered after QueryParamBuilder builds the filters, but before it builds the rest of the query.
None.
Added an AlterSearchQueryFiltersEvent to modify filters created by QueryParamBuilder, before they are processed.
| Comment | File | Size | Author |
|---|
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
marckwee commentedComment #5
mparker17@marckwee, thank you for the contribution! This sounds like a great idea to me!
I committed your code to a merge request to make it easier for me to review. I took a very quick look at your patch: the changes seem clear to me, but I haven't yet had a chance to run the changed code... I may have more feedback when I do.
We should add a change record to assist people upgrading from 8.x-7.x. Note we have a draft, not-complete-yet change record with some information on events that existed in the 8.x-7.x and their equivalents in 8.0.x (if applicable) — not sure if we need to update that, or create a new change record.
I didn't see any automated tests in your patch... we prefer to accept merge requests that have passing automated tests. Automated tests ultimately benefit you, because they ensure that future changes to this module (i.e.: by other people) will not break the functionality that your site depends on!
Comment #6
mparker17Briefly checking the status of this issue after releasing 8.0.0-alpha5...
Comment #7
mparker17I should move this to "Needs work" as well; sorry for the noise.
Comment #9
mparker17I've updated the issue summary, added tests, a change record, and created a 9.0.x branch (because I'll merge that first). I've tagged this issue so I remember to merge both branches, and publish the change record.
Now that tests are in place, I'd like to rename the event from
FilterParamsEventtoAlterSearchQueryFiltersEventto make it clearer when this event is dispatched, so I'm going to do that next (then I will update the issue summary and change record).Comment #10
mparker17Okay, tests are passing, so this is ready for review.
@marckwee, I would really appreciate it if you could test the new changes, to make sure they work. Note you will have to alter the class name of the event you're listening to (i.e.: from
FilterParamsEventtoAlterSearchQueryFiltersEvent). If it works, then please change the status of this ticket to "Reviewed & tested by the community".Thank you very much in advance!
Comment #11
mparker17(unassigning: anyone can provide feedback/community review!)
Comment #12
fathershawnCode looks correct
Comment #13
mparker17Awesome, thanks @fathershawn, I will merge this shortly!
Comment #14
mparker17There was a merge conflict with #3554964: Add an event to modify query parameters created by QueryParamBuilder so I rebased the branches. The conflict was in test code, so if the tests pass, then I'm happy to merge - there's no need for a re-review.
Comment #16
mparker17Merged to 9.0.x; change record published. Merging to 8.0.x next.
Comment #18
mparker17Merged to 8.0.x now. I'll update this issue when this issue has been released. Thanks everyone!
Comment #20
mparker17The changes in this issue were released in elasticsearch_connector-9.0.0-alpha3, and elasticsearch_connector-8.0.0-alpha7