Skip to content
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

[OData Handler] Add default filter param of ID for all queries on collections when talking to Microsoft Graph #787

Open
glennblock opened this issue Jul 31, 2020 · 7 comments
Labels

Comments

@glennblock
Copy link

glennblock commented Jul 31, 2020

Every entity in OData carries a unique Entity ID. In Graph we also have a default id property on every entity. When querying against a collection, the id can be used in the url as part of the path. Currently there is no way to filter on the id other than using the Filter property on QueryOptions. Without having id first class, I cannot currently create a request to with the following urls

https://graph.microsoft.com/v1.0/me/drive/items/F8B2FD72406FB218!6801/versions

or

https://graph.microsoft.com/v1.0/me/drive/items('F8B2FD72406FB218!6801')/versions

If id is added as a first class param and implemented such that the id is used in the path (using either of the two approaches) then I could use the following GraphQL query to get that url

{
  me {
   drive {
     items(id:"F8B2FD72406FB218!6801") 
     {
       versions {
         id
       }
     }
    }
  }
}

Adding this behavior would enhance overall drill down with GraphQL on OData feeds.

@glennblock glennblock changed the title [OData Handler] add default filter param of ID for all queries on collections [OData Handler] Add default filter param of ID for all queries on collections Jul 31, 2020
@ardatan
Copy link
Owner

ardatan commented Jul 31, 2020

There might be primary keys with different names other than (id) and I also think people can extend their OData schema with singletons to get single object instead of arrays(entityset). Maybe we should leave this as it is now because you are able to extend the schema using additionalTypeDefs and additionalResolvers in Mesh for more cases.

Other than that, we can improve queryOptions to have stronger typings instead of passing a string statement like "id eq Foo" not only for primary keys but also all other queryable fields.

What do you think?

@glennblock
Copy link
Author

glennblock commented Jul 31, 2020

@ardatan thanks and I can see the concern. in Graph we have more uniform guidelines and strive for consistency. Granted we are not the only feed. :-)

The CSDL should indicate which property is the entity id right? So couldn’t you determine based on the entity which prop to add? Maybe it is not part of QueryParams but loose.

I agree that you should not block things supported by the spec. Also agree that having the ability to specify more props to filter on is good.

The entityid though has some special treatment as in the url I displayed.

Also it may make sense to have config switches to allow certain behavior to be turned on or off.

Thoughts?

@glennblock
Copy link
Author

glennblock commented Aug 2, 2020

@ardatan just looking through our Graph $metadata, for Microsoft Graph all entities ultimately derive from entity which defines the key as id. So for those using the handler to talk to Graph, the id prop makes sense.

From the metadata

<EntityType Name="entity" Abstract="true">
  <Key>
    <PropertyRef Name="id" />
  </Key>
  <Property Name="id" Type="Edm.String" Nullable="false" />
</EntityType>

@glennblock glennblock changed the title [OData Handler] Add default filter param of ID for all queries on collections [OData Handler] Add default filter param of ID for all queries on collections when talking to Microsoft Graph Aug 2, 2020
@glennblock
Copy link
Author

@ardatan I updated the description to make it Graph-specific

@ardatan
Copy link
Owner

ardatan commented Sep 12, 2020

@glennblock Available in @graphql-mesh/odata@0.5.1 !

@Ant59
Copy link

Ant59 commented Oct 9, 2020

Other than that, we can improve queryOptions to have stronger typings instead of passing a string statement like "id eq Foo" not only for primary keys but also all other queryable fields.

This would be fantastic. Something like how Hasura allows for filtering, it would be great if GraphQL Mesh could have typed filtering.

eg

{
  contact(queryOptions: {filter: {favourite_colour: { eq: "Blue" }}) {
    name
  }
}

Which would resolve to $filter: "favourite_colour eq 'Blue'" in OData.

@glennblock
Copy link
Author

@Ant59 agreed, I had also raised this with @ardatan. The filter feels like a hack and very unnatural for developers who don't know or WANT to know OData. We should file a separate issue on that.

@theguild-bot theguild-bot mentioned this issue Aug 11, 2022
@theguild-bot theguild-bot mentioned this issue Sep 28, 2023
This was referenced Apr 30, 2024
This was referenced May 7, 2024
klippx pushed a commit to klippx/graphql-mesh that referenced this issue Oct 9, 2024
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants