fix: support calling save multiple times

[jacobq]

Cherry-picked from #246

[jacobq]

FWIW trailing whitespace keeps getting trimmed automatically by my IDE because of the .editorconfig settings associated with this project. I know it's not nice to have styling changes mixed in with other changes, but it's a pain to manually undo this, so if someone really cares about this please let me know and I'll submit a separate whitespace / style clean-up PR first.

[jlami]

Whitespaces are not that bad in the diffs. But is there a way to let Travis check the failing test commit alone? Would be good to see the failing test there before merging the solution. I think cherry picking and force updating made it skip/forget about commit 9bfde5c

[jacobq]

Would be good to see the failing test there before merging the solution.

It is pretty easy to see by running the test locally. There has also been an open issue about it for nearly a year. But sure, I'll push a revert commit (218c8cd) just so you can see it on CI. Error message appears as:

Error: Attempted to handle event becameError on <(subclass of DS.Model):ember1377:E> while in state root.loaded.saved.

[jlami]

Thanks, this just makes it easier to review without having to run things locally.

[jacobq]

The part of this fix that I really don't like are these 2 lines:
d7bfcc2#diff-8bef95280660598dc2f4437f447292e2R481-R482

It does seem to work, but I suspect there is a cleaner, if not easier, way to do this.

[jlami]

You could kind of unwrap updateRecord.

var data = this._recordToData(store, type, snapshot);
data.rev = records[Object.keys(records)[0]][0].rev;
return this.get('db').rel.save(this.getRecordTypeName(type), data);

But that still introduces double code and keeps the Object.keys part. And you would have to handle this._init before that point. Maybe the Object.keys kan be replaced with some serializer code, but I'm not sure. But at least Object.keys should be stable across ember-data versions. The __attributes part I'm more worried about.

You could make a new version of updateRecord that takes the snapshotted data which we can update the rev for to prevent the double code.

[jacobq]

Another way to do it is to use store.peekRecord(...) then .setProperties(...) and .save() though I am not sure if that is any better.

[jlami]

Hmmm, re-entering the save of ember-data itself feels really dangerous, and might even infinite loop?

[jacobq]

might even infinite loop?

The if that checks if changes need to be applied should prevent that.

[jacobq]

Any objections to merging this? It seems to work, and I'm not planning to make more changes unless requested.

[backspace]

Thanks for all your work on this, @jacobq, I’m sorry it’s gone stale. I’m updating an old application and have run into some of these errors. For now I’ve been sticking with an old Ember Data but eventually I’ll face updating that and will be checking out your proposed solution, as my initial experiment produced a huge number of inFlight errors in the test suite.

[jacobq]

Thanks for all your work on this, @jacobq, I’m sorry it’s gone stale. I’m updating an old application and have run into some of these errors. For now I’ve been sticking with an old Ember Data but eventually I’ll face updating that and will be checking out your proposed solution, as my initial experiment produced a huge number of inFlight errors in the test suite.

You're welcome & no worries! FWIW, I feel like my FOSS life has entirely gone stale -- been overwhelmed by other aspects of life for the past couple years. Good luck & thanks for picking this up again. I ended up using ember-indexeddb instead, IIRC, and it did the job. Pouch seems like a cool concept though, and if I hadn't run into problems I didn't fully understand I probably would've kept using it.