-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
docs: add note to resource embedding and vertical filtering section #4446
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
ac79217 to
1ea94b0
Compare
Adds a note about resource embedding and vertical filtering sections when they are combined with mutation queries. This resolves PostgREST#4352. Signed-off-by: Taimoor Zaeem <[email protected]>
1ea94b0 to
3729176
Compare
steve-chavez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't look like this resolves #4352. The problem there is horizontal filtering and not vertical?
#4352 (comment) says that the confusion comes from this section:
Deletions also support Return Representation, Resource Embedding and Vertical Filtering.
On https://postgrest--4446.org.readthedocs.build/en/4446/references/api/tables_views.html
So to summarize, I believe we should:
postgrest/docs/references/api/tables_views.rst
Lines 601 to 604 in bccf7ac
Updates also support: - :ref:`prefer_return` - :ref:`resource_embedding` - Correct to
- :ref:resource_embedding, with the exception of top-level filtering (<link>)
- Correct to
Deletions also support :ref:`prefer_return`, :ref:`resource_embedding` and :ref:`v_filter`. - Change here to a bullet list, same as above "Updates"
- Correct to
- :ref:resource_embedding, with the exception of top-level filtering (<link>)
|
|
||
| .. note:: | ||
|
|
||
| It is **not** possible to perform mutations *on* related resources. When combining mutations and resource embedding, the mutation is performed first and then the related resource is returned in combination with :ref:`prefer_return`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reading the discussion about the breaking change again, I think we should not document this behavior. This is a bug that needs to be fixed by throwing an error accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we consider it a bug if we are actively testing for it (and for a while now)?
postgrest/test/spec/Feature/Query/DeleteSpec.hs
Lines 73 to 78 in bccf7ac
| it "can embed (parent) entities" $ | |
| request methodDelete "/tasks?id=eq.8&select=id,name,project:projects(id)" [("Prefer", "return=representation")] "" | |
| `shouldRespondWith` [json|[{"id":8,"name":"Code OSX","project":{"id":4}}]|] | |
| { matchStatus = 200 | |
| , matchHeaders = ["Content-Range" <:> "*/*"] | |
| } |
Not against removing this behavior, but I think it could still be considered a breaking change due to these tests.
Adds a note about resource embedding and vertical filtering sections that they are top-level operations. This resolves #4352.