Note: Marking this as a task for the entity system, as my presumption is that JSON:API's usage is fine, we just need to update the documentation. Alternatively this could be seen as a bug in JSON:API if its usage is in fact deemed incorrect.
Per the documentation there is no NOT BETWEEN operator in the entity query system. \Drupal\Core\Entity\Query\QueryAggregateInterface::conditionAggregate() documents BETWEEN but not its inverse.
However, \Drupal\jsonapi\Query\EntityCondition::$allowedOperators does contain NOT BETWEEN, and \Drupal\Tests\jsonapi\Unit\Query\EntityConditionTest does test that it in fact works.
-
Either:
Add the NOT BETWEEN operator to the documentation in \Drupal\Core\Entity\Query\QueryAggregateInterface::conditionAggregate().
Or:
Remove support for NOT BETWEEN from JSON:API.
-
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 3256056-6.patch | 858 bytes | andregp |
Comments
Comment #2
andregp commentedThe project https://www.drupal.org/project/jsonapi is deprecated so I don't believe that the core function (QueryAggregateInterface::conditionAggregate()) should be changed to follow jsonapi. I'm also not sure if it's really needed to correct/update the jsonapi for the same reason, so, IMO I would vote for closing this issue.
Comment #3
andregp commentedSorry, assigned by mistake
Comment #4
tstoeckler@andregp: Thanks for the feedback. I think you are misunderstanding something, though. Or I am misunderstanding your comment, not sure. In any case: The contributed JSON:API module that you linked to is deprecated, yes, but only because it moved into Drupal core. And the JSON:API module in Drupal core contains the same code. So yes, I do believe it makes sense to either adapt the JSON:API usage of the entity query API as it is currently documented or (preferred) to adapt the documentation of the entity query API and, thus, make JSON:API's usage "official".
Comment #5
andregp commented@tstoeckler I'm sorry. You're absolutely right. I was confused and didn't know about the JSON:API being moved to core.
Comment #6
andregp commentedThis patch adds
NOT BETWEENinto the QueryAggregateInterface.php documentation.Comment #7
beatrizrodriguesI'll do the review
Comment #8
beatrizrodriguesSeems good to me, there is no phpcs problems and as there is usage of
NOT BETWEENthe documentation probably needed that indeed. RTBC to me.Comment #9
xjmDocumentation improvements are patch-eligible fixes, so unless this documentation only exists in D10, the fix should be backported all the way to 9.2.x and filed against that branch. 10.0.x should only be used for changes that cannot be made in D9 (e.g., major dependency updates, removing deprecated code). Thanks!
Comment #10
alexpottCommitted and pushed 1cf95460d9b to 10.0.x and f75ef8ce27a to 9.4.x and 5a4f3c8bc7f to 9.3.x. Thanks!
Backported to 9.3.x as that's the current bugfix branch and this is a docs patch.