josx / ra-data-feathers

A feathers rest client for react-admin
MIT License
157 stars 53 forks source link

id property in put request payload #102

Closed dinodeSimon closed 5 years ago

dinodeSimon commented 5 years ago

We are using PUT not PATCH for updates for a couple of reasons.

When a PUT request comes through via the ra-data-feathers adaptor it includes the "id" field in the payload sent from react-admin to feathers which is not a valid property on the model.

Should this be stripped off as part of the data adaptor since it is only required for the react-admin client and not on the server?

josx commented 5 years ago

To clearify some stuff:

If i understand you well, you are saying that in data is the id field. (in that case we need to take it out from data)

Can you show me your example code and your models?

dinodeSimon commented 5 years ago

Hi José,

Thanks for following up. Apologies, I should have attached a sample originally.

Here is a sample mongoose model with 2 fields let Schema = new mongoose.Schema( { field1: { type: Types.String }, field2: { type: Types.String } }, { collection: "samples" } );

It is using the default mongo / mongoose _id field as the unique identifier which is configured in the restClientOptions along with the option to use PUT instead of patch

const restClientOptions = { id: "_id", usePatch: false };

When going to the edit form which has the 2 fields on it a request is made to get the data. This is the response that is returned from the server

{"_id":"5d001ce77a211f5e2cba5363", "field1":"foo", "field2":"bar", "__v":0}

The data adaptor then adds the id property as required by react-admin return { data: { ...response, id: response[idKey] } };

From the edit screen in react-admin if the fields are updated this is the PUT request that gets sent to http://localhost:3030/samples/5d001ce77a211f5e2cba5363

{"_id":"5d001ce77a211f5e2cba5363", "field1":"biz", "field2":"baz", "__v":0, "id":"5d001ce77a211f5e2cba5363" }

The id field doesn't exist on the model and shouldn't be required for feathers since it knows about the real _id field and only react_admin cares about the id field that is copied from _id during GET requests by the data adaptor.

The feathers update method called for PUT requests includes the id as a parameter so dropping the extra id field from the payload shouldn't stop the update from working

From restClient.js: service.update(params.id, params.data);

The reason this becomes an issue for us is that we are sanitising all incoming requests to ensure that the payload matches what is expected and there are no additional properties. Since id isn't something the server knows about the validation is rejecting the payload.

Obviously we can work around this be ignoring that field specifically on the payload validation or modify the data provider ourselves but it seems like something that might be good to fix in the core data adaptor

josx commented 5 years ago

@dinodeSimon I have done a PR #103 with your request, can you check this out, solves your problem?

dinodeSimon commented 5 years ago

@josx tested PR #103 and it works perfectly. Nice work! Thanks.