marmelab / react-admin

A frontend Framework for single-page applications on top of REST/GraphQL APIs, using TypeScript, React and Material Design
http://marmelab.com/react-admin
MIT License
24.97k stars 5.25k forks source link

Sparse Field and meta handling different for create/update #10317

Open dricholm opened 5 days ago

dricholm commented 5 days ago

What you were expecting: I'm trying out the new sparseField feature of ra-data-graphql-simple added in #9392. However when setting the property: { meta: { sparseFields: ['id'] } } it's not taken into account for create/update mutations. The cause seems to be because buildCreateUpdateVariables in buildVariables only takes id and data destructured and ignores meta. The unit test for these two query types is written that the meta property is inside the data object, however the types suggest no such difference: UpdateParams still has data: Partial<RecordType> and meta?: any in its type declarations.

In short the below code should still use the sparse fields, but it's completely ignoring it:

updateComment(Resource.MatchComment, {
  id: commentId,
  data: {
    meta: { sparseFields: ['createdAt', 'id', 'matchId', 'employeeId', 'text', 'updatedAt', 'pinned'] },
    text: values.text,
  },
});

What happened instead: Sparse fields are not used for create/update calls from params.meta.sparseFields, they do seem to work on params.data.meta.sparseFields however.

Steps to reproduce: It should be easily reproducible/testable by updating the unit tests in buildVariables.test.ts for CREATE and UPDATE. meta property should be on the top level of the params variable, not inside data.

Related code: Unit tests for create/update should use top-level meta.

-            const params = {
-                data: {
-                    meta: { sparseFields: [] },
-                },
-            };
+            const params = {
+                data: {},
+                meta: { sparseFields: [] },
+            };

Other information:

Environment

slax57 commented 4 days ago

Thank you for the detailed issue and analysis!

It took me some time to get into the issue, but I believe you are correct: there is an inconsistency between the place where react-admin puts the meta value in the create/update call params, and the place where ra-data-graphql-simple expects it to be.

Would you like to open a PR to fix this issue? Thanks.

dricholm commented 4 days ago

Thank you for the detailed issue and analysis!

It took me some time to get into the issue, but I believe you are correct: there is an inconsistency between the place where react-admin puts the meta value in the create/update call params, and the place where ra-data-graphql-simple expects it to be.

Would you like to open a PR to fix this issue? Thanks.

Thank you for the quick response! Yes, I can open a PR. Probably this week or early next week.