ampersand-sync icon indicating copy to clipboard operation
ampersand-sync copied to clipboard

dirty checking on patch?

Open yocontra opened this issue 10 years ago • 5 comments

it seems a little weird that you have to specify which fields go up when you save a model and want to patch only what has changed

Currently this will send the entire model:

model.save(null, {patch: true});

this will send the attributes that changed, but we have to manually specify the keys and values:

model.save({
  fieldOne: 123,
  fieldTwo: 123
}, {patch: true});

which is just as bad as not even using the model

request.patch('/api/users/' + model._id)
  .send({
    fieldOne: 123,
    fieldTwo: 123
  });

yocontra avatar Feb 24 '15 22:02 yocontra

The save method with patch:true imitates how Backbone behaves in this scenario.

There are 2 easy options to run dirty checking:

  • model.changedAttributes - changes since the last set. My experience is that this doesn't work with forms that run multiple sets, as only the last change since set is saved.
if (!this.model.hasChanged()) {
  return console.log('Nothing to update');
}
this.model.save(this.model.changedAttributes()), {patch:true));
  • Have a copy of the original model before any changes, and run a diff:
//..
initialize: function() {
  this.originalModel = this.model.clone();
}
//...
onSave: function() {
 var changedAttrs = this.model.changedAttributes(this.originalModel.toJSON());
  if (!changedAttrs || !_.size(changedAttrs)) {
    return console.log('Nothing to update');
  }
  this.model.save(changedAttrs, {patch:true));
 //...
}

Another option is to integrate a plugin that tracks changes since the last save, something like this one: https://github.com/NYTimes/backbone.trackit

I tend to agree it's a lot of work to do a pretty common task

pgilad avatar Apr 04 '15 06:04 pgilad

This is something that needs to be resolved on ampersand-model. amp-sync is just a wrapper for xhr

pgilad avatar Aug 08 '15 20:08 pgilad

Is ampersand trying to improve where backbone fails or is it set on keeping failure parity?

yocontra avatar Aug 09 '15 22:08 yocontra

Well played :) It doesn't change the fact that its by no means the responsibility of ampersand-sync. ampersand-sync is a function. It doesn't have instances, it doesn't have state. Calculating the difference here would be a total breach of encapsulation.

We're talking about a reasonable feature here, but a feature for ampersand-model.

The usecase might not be that common to add substantial code to ampersand-model, so IMHO the best solution would be another small requireable module that'd add the functionality to models. Totally doable.

naugtur avatar Aug 10 '15 20:08 naugtur

Also @pgilad it doesn't seem like ampersand models have a .clone fn, just FYI

yocontra avatar Sep 02 '15 21:09 yocontra