marmelab
Add a GET_COUNT fetch action for graphql dataProvider
In the graphql provider, we have to define a query to fetch the total count of entity for a given resource. Previously, the name of this query was hardcoded.
This PR aims to allow the customization of this query name, such as every other query names.
I'm not sure about where to put the new GET_COUNT constant, tell me if you think there is a better place.
I have exported GET_COUNT from ra-data-graphql-simple to allow an easier use for developers, allowing to import this simbol directly from ra-data-graphql-simple instead of having an explicit dependence on ra-data-graphql in the application code.
Closes #4566
I have a bug, it doesn't work if you don't provide a custom GET_COUNT operation name. Working on it and adding some errors with more useful informations for the developer.
I have updated the unit tests but it shows that we potentially have a breaking change with this feature on the ra-data-graphql package.
ra-data-graphql-simple implicitly relies on a _allResourcesMeta query, but it was not the case of ra-data-graphql. This PR explicitly defines a GET_COUNT operation in ra-data-graphql, so if someone was using ra-data-graphql instead of ra-data-graphql-simple for some reason, it can break if they don't have this query.
I don't know if this is fine or not. If it's not, do you have some inputs about how to implement this without breaking change ?
A quick fix could be to remove the check in isResource. This way, any existing resource without the GET_COUNT operation should be ok, but this means the error report in case of missing GET_COUNT query when using ra-data-graphql-simple will be less explicit and thrown only when a GET_LIST or GET_MANY fetch is actually executed.
After more thought, there is no explicit error message in the isResource, so it bring nothing to make GET_COUNT operation required in ra-data-graphql.
There is no more breaking change. I have just added a more explicit error in ra-data-graphql-simple for missing GET_COUNT operation, which was implicitly required before.
I have also fixed a bug where the params where not filtered out if GET_COUNT query didn't declared them. The #2711 will be closable if this is merged :-)
Just a little update, Is there something I should do to push this PR forward ? We would like to have this feature before deploying our backoffice based on react-admin to production :-)
@fzaninotto @djhi Just a friendly ping :-) Do I have something to do for this PR to be merged ?
it needs rebase, can only be pushed to next, and will have to wait until we overhaul the current GraphQL provider.
React-admin version 3 is feature freeze. Can you please rebase this PR against next, and make it compatible with v4?
I'm closing this PR. I don't use this anymore and I'm not sure if it's useful to spare time on a rebase.
Don't hesitate to comment if you think that it could be interesting to work on it, I could look at it if you want to.