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

Allow more configuration in introspection to determine which types should be react-admin resources #4933

Closed
bayareacoder opened this issue Jun 11, 2020 · 3 comments

Comments

@bayareacoder
Copy link

bayareacoder commented Jun 11, 2020

Is your feature request related to a problem? Please describe.
To detect a GraphQL type as a resource, the current introspection requires that there is a query in the GraphQL API with a singular name of the resource AND a plural name. While you can set the operation names of these queries in an option, you cannot change the requirement that BOTH need to be present for a type to be detected as a resource due to this fixed function in introspection.js:

  const isResource = (type) =>
    queries.some(
      (query) => query.name === options.operationNames[GET_LIST](type)
    )
    &&
    queries.some(
      (query) => query.name === options.operationNames[GET_ONE](type)
   );

Any whitelisting of resources via include is only applied in filterTypesByIncludeOrExclude AFTER the above. This is a problem with APIs that do not support a plural or singular version of a query.

Describe the solution you'd like
The include list should override any filtering done by isResource

Describe alternatives you've considered
You could pass in a completely custom resolveIntrospection to fix this, but seems overkill

Additional context
This goes together with issue#4932 to allow multiple GraphQL queries for a single react-admin verb. For instance, both changes are needed to support GraphQL APIs that support GET_MANY or event GET_LIST only from multiple GET_ONE fetches (eg in case all ids are known outside the API) as they would not have a plural query, and then require multiple queries to provide the data required for GET_LIST or GET_MANY

@djhi
Copy link
Collaborator

djhi commented Jun 12, 2020

The solution is to build your own GraphQL dataProvider. Use the existing ones as starting points.

@djhi djhi closed this as completed Jun 12, 2020
@bayareacoder
Copy link
Author

bayareacoder commented Jun 13, 2020

Sure but the proposal is to make small changes to ra-data-graphql so it can be used as-is with these kind of APIs. The change is simply to isResource, in file introspection.js:

  const isResource = (type) => {
    // whitelist any type that is in `include` list
    if (filterTypesByIncludeExclude(options)) return true;
    // otherwise we require there is a GraphQL query for both the operation name of GET_LIST and GET_ONE in order
    // for the type to be a resource
    return (
      queries.some(
        (query) => query.name === options.operationNames[GET_LIST](type)
      ) &&
      queries.some(
        (query) => query.name === options.operationNames[GET_ONE](type)
      )
    );
  }

@Kilometers42
Copy link
Contributor

@djhi I am trying to understand why the decision was made to make all resources implement both a GET_LIST and GET_ONE. I am thinking of porting over an existing admin site I have to use react admin and there are a couple of cases where a resource only has a GET_ONE. It seems like @bayareacoder's suggestion is a good one so I could easily overwrite the default behavior in these one-offs. Is there another workaround for this use case that I am missing?

I understand that I could fix this by writing my own provider but I am happy with the rest of the functionality of ra-data-provider and that's a fair bit of work. I am happy to take the idea that @bayareacoder has and turn it into a PR if it would be accepted!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants