Merge records in update

[jsherbert]

Re: updateStart and updateSuccess. The API says the updated records are 'merged with the current state'. I took this to mean that the results of the update operation are merged with the current record, i.e. if the application state for the resource looks like

[0: {id: 'exampleId', width: 10}]

and we dispatch an action thus:

updateStart({id: 'exampleId', height: 20})

the updated state would look like:

[0: {id: 'exampleId', width: 10, height: 20}]

As it is, though, the width property is lost, and I'm getting

[0: {id: 'exampleId', height: 10}]

Is this expected behaviour, that is to say, update operations update records in the current state, using the given key, but overwrite the record contents, rather than merging them? Or am I using the library incorrectly? I'll submit code to reproduce if needed.

Thanks for your help, this library looks like just what I need!

[sporto]

Yes this is the intended behaviour, but is not clear as you say. I will update the read me.

[jsherbert]

Thanks for your reply!

I wonder whether this is desirable behaviour. In order to dispatch updateStart or updateSuccess, the user must have merged their desired changes with the entire preexisting record, or lose their existing model data.

This action almost always necessitates a call to a selector, because it's unlikely that the part of the app making the update call knows about the entire model. As in the example above, a form that edits width but not height will need to pull the model beforehand and merge before calling updateStart or updateSuccess:

/**
 * @param {{id: number, width: string}} data
 */
const onSubmit = data => {
  const originalRecord = findById(data.id)
  const newRecord = originalRecord.merge(data)
  store.dispatch(updateStart(newRecord))
}

Whereas if we were merging in the library, this operation would look like

/**
 * @param {{id: number, width: string}} data
 */
const onSubmit = data => {
  store.dispatch(updateStart(data))
}

I'm probably missing something, but is there a use case in which not merging (the current behaviour) is necessary or desirable? If not, I'd be happy to submit a PR either changing this behaviour or making it configurable.

[GuillaumeJasmin]

Is there going to be a future update for this ?
Maybe an option like this: updateSuccess(item, {merge: true}) ?

[sporto]

A PR would be very welcome.

We already have the second attribute for passing arbitray data to the reducer, updateSuccess(item, data)
So this will mean that this config object will need to go as a third param.

If this should be considered more a global setting for a collection then it should go to actionCreatorsFor e.g.

reduxCrud.actionCreatorsFor('users', {mergeRecordOnUpdate: true});

[yagudaev]

Any updates on this? I keep seeing this pop up in our codebase with things like:

reduxCrud.actionCreatorsFor('photos').updateSucces({ ...photo, fileName: newFileName })  // OMG why doesn't redux crud do this....

We have a

fetchSuccess(records, { replace: true })

In the newest version of redux-crud. It would be nice to see an added merge option

[sporto]

No updates, sorry, unless someone would like to make a PR is unlikely to happen

[d3dc]

I did a quick and dirty add of a partial update feature because its a pattern our APIs use a lot.

I figured I'd chime in here and see if there was still interest . I think this library is still excellent for a quick yarn add redux-crud to get tested reducers in any prototype.

I realized I didn't implement anything for updateStart or updateError and went back to look at those. It doesn't seem like updateStart is any different, but correct me if I'm wrong.

---

After opening this back up I started thinking about REST and how this relates. Ideologically, a whole entity response is better.

In my case I'm updating entities that are children of a larger one and don't want to do the round trip. (For instance profile images like @yagudaev, where all I get back is the url)

However, If these children end up being dependencies of other fields, wouldn't it be more ideal to do a fresh fetch() instead of patching in only the changes the client knows about? Maybe my uploaded image needs to generate thumbnails and add those somewhere else.

I wonder if it would be better to put this into a higher-order abstraction - something that can merge with a definable strategy external to the reducer and has access to the state... Like a thunk...

I think in the perfect scenario, your entity comes from outside the store. In most use cases, the view knows about it to take a slice of it or something and should just patch it itself.

If you're building an application complex enough to decouple forms from the entity, I think I would actually propose sagas or side-effects to make this something you don't think about.

A basic saga for this would be:

• dispatch a USERS_PARTIAL_UPDATE_SUCCESS action with your changes as the payload
• start the saga
  • run selectors on the state
  • get the strategy for the record or collection somehow (e.g.refresh or merge)
  • dispatch the actions needed for strategy to happen.

[jsherbert]

I thought the use case here was to negate the need for users to dig an object out of the store on update to remove some boilerplate for optimistic updates. That's certainly the problem I refer to above.

The solution to that problem might look something like -

On update, the updateStart reducer pulls the related record out of the store, merges it with the changes in the payload, and updates the state. This newly merged record represents the record as we expect to see it once the server has merged it.

On success, once the response comes back from a POST request, it's usually a whole and authoritative representation of the merged model, so I can't think of a reason why we'd want a partial update at this point - we simply drop the model into place as usual.

On failure, we roll back as usual.

Am I missing something? I feel like there's a lot of complexity in your answer that I wasn't expecting (side effects, etc.), @d3dc, and it might be because we're addressing two different use cases.

[sporto]

Reading this I think that the most flexible approach would be to be able to specify a merging strategy for updateStart and updateSuccess. Maybe you pass a function for each of these. In this way is up to you how to handle each case.