danielspaniel / ember-data-change-tracker

extending ember data to track and rollback changes including objects and associations
MIT License
106 stars 47 forks source link

Attributes set at create-time are always changed #22

Open mwpastore opened 7 years ago

mwpastore commented 7 years ago

Consider the model:

import Model from 'ember-data/model';
import attr from 'ember-data/attr';

export default Model.extend({
  country: attr('string'), // e.g. US
  region: attr('string'), // e.g. Illinois
  locality: attr('string'), // e.g. Chicago
  street: attr('string'), // e.g. 1060 W Addison St
  postalCode: attr('string') // e.g. 60613
});

What I'm finding is that any attributes that are set at create-time are always changed, even if auto is set to false, e.g.:

let address = store.createRecord('address', { country: 'AR' });
address.didChange('country'); // => true

I would expect the tracker to start after the record is created when auto is true. Even more frustrating, I can't set auto to false and call address.startTrack() after createRecord() to do it manually.

danielspaniel commented 7 years ago

This is a really good question.

I think I remember me thinking that when you create a model it's state is: root.loaded.created.uncommitted

so that in essence, every attribute is considered changed. The time to call startTrack is when the model is in root.loaded.saved state ( before you change stuff , after it is saved ).

So, you have to save before you can startTracking.
Therefore, on a createRecord you can't start tracking, until you save.

I have had a case where I created a model with certain properties, just like you are. Then I showed the model in a form. And I wanted to know if that model had changed from what I originally set it up to be. I think that is what you are trying to do.

Sorry to say, I forgot what I did to solve that one .. but I can look it up if you are interested. One thing is for sure, I was not able to use change tracker. It was custom hocus pocus.

ming-codes commented 7 years ago

@danielspaniel It'll be great if you can share your solution. We're having the same problem.

danielspaniel commented 7 years ago
// utility methods
export function valuesEqual(value1, value2) {
  return (value1 && value1.toString()) === (value2 && value2.toString());
}

export function valuesChanged(value1, value2) {
  let valuesBlank = isEmpty(value1) && isEmpty(value2);
  return !(valuesBlank || valuesEqual(value1, value2));
}

// mixin used in controller
export default Ember.Mixin.create({ 
  initialModelProperties: {},

  setupInitialProperties() {
    let model = this.get('model');
    // using ember-data-change-tracker for changed, but you can use changedAttributes 
    // too if it is only attributes you are checking
    let changedKeys = Object.keys(model.changed());  
    this.set('initialModelProperties', model.getProperties(changedKeys));
  },

  // when you need to see if anything changed
  haveUnsavedChanges() { 
    let props = this.get('initialModelProperties') || {},
        propKeys = Object.keys(props),
        model = this.get('model'),
        changedKeys = Object.keys(model.changed()),
        searchKeys = Ember.merge(propKeys, changedKeys);
    return searchKeys.any((key) => valuesChanged(model.get(key), props[key]));
  }
});

// here is a route that makes a new model as its "model" and passes that to controller
export default Ember.Route.extend({
  model() {
    let code = this.get('session.code');
    let openDate = moment().startOf('day').toDate();
    // i am setting up the model with some initial properties
    return this.store.createRecord('my-model', { openDate, code });
  },

  setupController(controller, model) {
    controller.set('my-model', model);
    controller.setupInitialProperties();  // tell controller to setup initial properties ( from mixin above ) 
  }
});

How's that?

mwpastore commented 7 years ago

Hi @danielspaniel, following up on this, I've noticed that scalar attributes set at create-time are always changed, but relationships set at create-time are not. Is there any way to make this more consistent?

import Model from 'ember-data/model';
import attr from 'ember-data/attr';
import { belongsTo } from 'ember-data/relationships';

export default Model.extend({
  country: attr('string'), // e.g. US
  region: attr('string'), // e.g. Illinois
  locality: attr('string'), // e.g. Chicago
  street: attr('string'), // e.g. 1060 W Addison St
  postalCode: attr('string'), // e.g. 60613

  organization: belongsTo({ inverse: 'addresses' })
});
let organization = store.peekRecord('organization', 1);
let address = store.createRecord('address', { country: 'AR', organization });
address.didChange('country'); // => true
address.didChange('organization'); // => false
danielspaniel commented 7 years ago

@mwpastore, I am scared to look, and I don't have my teddy bear to hold on to today. Do you mind snooping around and seeing why this happens?

danielspaniel commented 7 years ago

This is now "fixed" in the latest change tracker v0.6.2. EmberData was doing the change tracking for attributes for me, and so, what I do now is set the saved value that tracker knows about to undefined ( just like ED does for attributes ). Then, if there is any value you set when creating the model or when altering a newly created model, you will get 'true' for didChange.

This does not solve the "Shangri-La" scenario that you were originally looking for where I save the attributes that are setup when you create the model as the base values, and that scenario is 100% different than this one.

This follows the EmberData 'convention' ( what it does ) and says everything you set on a new model is changed because the original state is assumed to be undefined.

mwpastore commented 7 years ago

Hi Daniel, I meant to write back on Friday but got sidetracked. Thank you so much for this fix, it totally solved my issue. I hope it doesn't compromise your conception of how the changeTracker is supposed to work.

You're right that this is a separate issue and I probably should have opened up a new issue report for you. I was able to work around the original issue, but my "workaround" ran afoul of the inconsistency between the handling of attributes and relationships, so I'm just happy to have a solution here. Thanks again.

danielspaniel commented 7 years ago

Your welcome Mike, and I realized the other day that I actually could do the thing where create attributes are set up for new record the way you want them to ( they would be not be considered changed ) etc.

The thing is that this is different than what ember data default does. So it would be a special mode ( I am thinking )

where you would create record and call => model.startTrack() and if the model is new .. it would wipe the slate and call current attributes the current state ( and ignore what ember data "changedAttributes" say )

created records can't be rolled back , so it would not cause an trouble there.

anyway .. this would solve a huge headache ( and all the ridiculous hocus pokus from the hacking I showed a few messages above this one )

I don't really need this but if you need it we can ponder the goodness or badness of such a concept.