Could/should isPristine be promise/proxy promise aware?

[richbrown]

I am not really sure if this is a bug, a feature request or just my misunderstanding of the intended design.

I have two Ember Data models that are related to each other:

export default class FieldTemplate extends Model {
   @belongsTo fieldType;
}

export default class FieldType extends Model {}

The issue I am experiencing occurs when I use a changeset in an hbs form and change the fieldType value, it makes the changeset dirty. However, when I change it back to the original fieldType the changeset is still dirty, as in the following pseudo code:

let fieldTemplateChangeset = Changeset(fieldTemplate);
fieldTemplateChangeset.isPristine //is true (asexpected)

fieldTemplateChangeset.fieldType = differentFieldType;
fieldTemplateChangeset.isPristine //is false (as expected)

fieldTemplateChangeset.fieldType = originalFieldType;
fieldTemplateChangeset.isPristine //is false (not as expected)

I believe this is because of the comparison in validated-changeset BufferedChangeset#_setProperty where oldValue !== value. The oldValue is a proxy to the original promise and the new value is a resolved model that has been set on the changeset. So in this example the oldValue.content === value //is true.

So my question is should this comparison be promise aware, or I am misusing the changeset?

I have worked around this by making the relationship @belongsTo('field-type', {async: false}) fieldType and that works fine, but I suspect other aspects of the app I am working on may need similar functionality but I would prefer not to make the relationship async: false

[snewcomer]

@richbrown Is it possible to await the fieldType on the fieldTemplate before creating the Changeset? I think in general we expect all of the state to be settled before creating the changeset. Only APIs like save support async data.

[richbrown]

@snewcomer thank you for pointing me in the right direction so kindly. I have done that and with a few little other tweaks, mainly notifyPropertyChange() for various overidden properties I have got it working. Thank again for your help and my apologies for wasting your time.

[snewcomer]

@richbrown No wasting time here! I always learn something new talking with people like you.

mainly notifyPropertyChange() for various overidden properties I have got it working

Could you detail this more in depth? What is the problem and the approximate solution? We might be able to improve something in this area.

[richbrown]

@snewcomer thank you and your response is timely as it happens as I spoke out of turn and my solution doesn't work in all contexts. So without confusing the issue too much I have included my original Changeset below. The basic premise of the problem is largely as @sandstorm described in issue #551.

I am working on a nested interface for ModuleTemplate whereby there are four nested routes that represent properties/relationships of ModuleTemplate and are represented in the interface by tabs. Changes to any one of these properties or relationships will dirty the ModuleTemplate. This is represented by a notification to tell the user that the ModuleTemplate is dirty and needs saving to persist the changes, along with showing/hiding relevant save and rollback buttons.

Therefore, in this instance, as you can hopefully see below, I am overriding the fieldTemplates, isDirty, isPristine properties together with the rollback() and save() methods to take account of changes made to fieldTemplates i.e. added, removed or changes to an individual fieldTemplate. I have to use notifyPropertyChange() to ensure that these changes are propagated to the observers of fieldTemplates, isDirty, isPristine.

I have tried awaiting the fieldTemplate.fieldType property within the _buildFieldTemplateChangesets() method in various ways e.g. awaiting, trying to set it, using an ember-concurrency task, overriding in the model etc. But I can't get it working and I am still faced with the issue that at the point validated-changeset tests the oldValue !== value oldValue is a proxy and value is resolved object.

import { EmberChangeset, Changeset } from 'ember-changeset';
import { task } from 'ember-concurrency-decorators';
import { notifyPropertyChange } from '@ember/object';

export default class ModuleTemplateChangeset extends EmberChangeset {
  _fieldTemplates = [];
  _fieldTemplateChangesetKeys = ['name', 'fieldType', 'defaultValue', 'listOptions'];

  get fieldTemplates() {
    return this._fieldTemplates.rejectBy('isDeleted');
  }

  get isDirty() {
    return !this.isPristine;
  }

  get isPristine() {
    return super.isPristine &&
      this.areFieldTemplatesPristine;
  }

  get areFieldTemplatesPristine() {
    return this._fieldTemplates.isEvery('isPristine') &&
      !this._fieldTemplates.isAny('isNew') &&
      !this._fieldTemplates.isAny('isDeleted')
  }

  constructor() {
    super(...arguments);
    this._buildFieldTemplateChangesets();
  }

  addFieldTemplate(fieldTemplate) {
    const changeset = Changeset(fieldTemplate, null, null, { changesetKeys: this._fieldTemplateChangesetKeys });
    this._fieldTemplates.pushObject(changeset);
    this._notifyFieldTemplateChanges();
  }

  removeFieldTemplate(fieldTemplateChangeset) {
    // I don't think that ember-changeset supports deletion so
    // we need to go to the underlying object to propagate the 
    // change
    fieldTemplateChangeset.data.deleteRecord();
    this._notifyFieldTemplateChanges();
  }

  @task
  *save() {
    let fieldTemplatesToSave =
      this._fieldTemplates.filter(fieldTemplate => {
        return fieldTemplate.isDirty || fieldTemplate.isNew || fieldTemplate.isDeleted;
      });
    let fieldTemplatePromises = fieldTemplatesToSave.map((fieldTemplate) => fieldTemplate.save());

    try {
      let result = yield Promise.all([super.save(...arguments), fieldTemplatePromises]);
      let deletedFieldTemplates = fieldTemplatesToSave.filterBy('isDeleted');
      this._fieldTemplates.removeObjects(deletedFieldTemplates);
      this._notifyFieldTemplateChanges();
      return result;
    }
    catch (e) {
      console.log(e);
    }
  }

  rollback() {
    super.rollback(...arguments);

    let newFieldTemplates = [];
    let deletedFieldTemplates = [];

    this._fieldTemplates.forEach((fieldTemplate) => {
      if (fieldTemplate.isNew) {
        newFieldTemplates.pushObject(fieldTemplate);
      }
      else if (fieldTemplate.isDeleted) {
        deletedFieldTemplates.pushObject(fieldTemplate);
      }
      else if (fieldTemplate.isDirty) {
        fieldTemplate.rollback();
      }
    })

    this._fieldTemplates.removeObjects(newFieldTemplates);

    // I don't think that ember-changeset supports deletion so
    // we need to go to the underlying object to propagate the 
    // change
    deletedFieldTemplates.forEach((fieldTemplate) => fieldTemplate.data.rollbackAttributes());
  }

  _buildFieldTemplateChangesets() {
    this._fieldTemplates = this.data.fieldTemplates.map(fieldTemplate => {
      return Changeset(fieldTemplate, null, null, { changesetKeys: this._fieldTemplateChangesetKeys });
    });
  }

  _notifyFieldTemplateChanges() {
    notifyPropertyChange(this, 'fieldTemplates');
    notifyPropertyChange(this, 'isDirty');
    notifyPropertyChange(this, 'isPristine');
  }

}