adopted-ember-addons / ember-data-model-fragments

Ember Data addon to support nested JSON documents
MIT License
370 stars 114 forks source link

Fix fragment attributes in save response being ignored #404

Closed dwickern closed 2 years ago

dwickern commented 3 years ago

Steps to reproduce:

  1. Modify a fragment attribute and save
  2. The server returns a different value for the attribute in its response

Expected: the fragment attribute should have the new value from the server (consistent with regular ED attributes outside of fragments) Actual: the fragment attribute still has the locally modified value

The root cause seems to be willCommit not being called for fragments. willCommit is normally responsible for moving _attributes into _inFlightAttributes: https://github.com/emberjs/data/blob/f56b22c4d6eefab8915ca48979f7d813f612971d/packages/record-data/addon/-private/record-data.ts#L120-L123

This is important because _changedKeys prefers to take _attributes over attributes in the response body: https://github.com/emberjs/data/blob/f56b22c4d6eefab8915ca48979f7d813f612971d/packages/record-data/addon/-private/record-data.ts#L763-L769

patocallaghan commented 2 years ago

@dwickern I came across this same issue myself in #418. Have you been using this fix in a fork or anything?

knownasilya commented 2 years ago

@patocallaghan if you can test this out, I'll merge it if it's good.

dwickern commented 2 years ago

@patocallaghan I've patched it locally:

// app/initializers/ember-data-model-fragments-workaround.js
import { RecordData } from 'ember-data/-private';

export function initialize() {
  const didCommit = RecordData.prototype.didCommit;
  RecordData.prototype.didCommit = function didCommitWorkaround(data) {
    if (this._attributes) {
      // willCommit was never called
      this._inFlightAttributes = this._attributes;
      this._attributes = null;
    }
    return didCommit.call(this, data);
  };
}

export default {
  initialize,
  after: 'fragmentTransform', // after ember-data-model-fragments is initialized
};
patocallaghan commented 2 years ago

Thanks @dwickern 👍

@knownasilya I've verified this fixes the issue in our app so I think this fix will do for now to unblock some folks 👍

patocallaghan commented 2 years ago

Released in https://github.com/adopted-ember-addons/ember-data-model-fragments/releases/tag/v5.0.0-beta.3