Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
In a contrib module I need to add a new condition for the query in the TermStorage::loadTree(), the condition depends by a custom field. At the moment it's possible to alter this query using the term_access tag with hook_query_alter() or hook_query_TAG_alter(), but this tag is too general and can affect other queries.
Proposed resolution
Add a new tag, specific just for this query.
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | 2616348-17.patch | 614 bytes | xavier.masson |
Issue fork drupal-2616348
Show commands
Start 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:
compare
Comments
Comment #2
timmillwoodComment #3
alexpottI don;t think adding a tag is a feature. Also this is not in the rc eligible class of changes.
Comment #4
xjmHm, it seems to me like this would not be the best way to do this in D8. Shouldn't it be possible to override the default storage to add the needed condition in the contrib project, without needing to alter the query directly or change core?
If that turns out to not be feasible, it could make sense to add this in 8.0.x since it would indeed be a contrib blocker in that case, and also a fairly harmless/non-disruptive addition. If we do make this change, it would need test coverage.
Comment #5
xjmAdding a few related issues. @jeqq, maybe it would help to share a few more details of the usecase?
Comment #6
alexpottFYI when doing a service override in a contrib module I think we need to establish some good practice. I think we should use service decoration - http://symfony.com/doc/current/components/dependency_injection/advanced....
Comment #17
xavier.massonReroll the patch for the latest 9.2.x
Comment #19
Marios Anagnostopoulos CreditAttribution: Marios Anagnostopoulos as a volunteer commentedWould it be an overkill to maybe dispatch an event containing the query, so that people can subscribe to it?
Comment #20
Marios Anagnostopoulos CreditAttribution: Marios Anagnostopoulos as a volunteer commentedHere is a patch doing the above, surely needs tests, but let's see first if this is the correct approach.
Also this (like the tag solution) only allows altering the query. If someone would want to alter any other part of the process, the best way would be to decorate the Storage... like @alexpott suggested.
Comment #21
Marios Anagnostopoulos CreditAttribution: Marios Anagnostopoulos as a volunteer commentedFixed paths in patch.
Additionally all these approaches still do not make the function a good tool to be used when someone wants to build the tree programmatically.
Maybe we should add a way for people to further configure the query before calling the method.
Maybe setting class variables to the storage or something?? (Which would be the equivalent of passing an options array, but would make the code a bit cleaner.
Comment #22
joachim CreditAttribution: joachim as a volunteer commentedYes, there's already hook_query_TAG_alter() which allows module to alter this query specifically.