Make sure new record are not pushed twice when parent is saved before.

[olivierchatry]

No description provided.

[koemeet]

@olivierchatry Does it also update the record again when new data is returned by the server (such as the id for example)?

[olivierchatry]

yes, it should. it was really because the same record would be pushed twice because of some handling of isNew in the relationship. You can also simply 'reload' the record every-time you save ( when ALL relations has been saved ) and it should fix the issue.

[koemeet]

Nice, would be cool to see this in core 👍

[bmac]

@olivierchatry did you run into an issue that required this if check? Under the hood members does a similar check which should be preventing the record from being added twice.

[olivierchatry]

Yes, using firebase, record get duplicate, see test.
On Feb 12, 2016 1:29 AM, "Brendan McLoughlin" [email protected]
wrote:

In addon/-private/system/relationships/state/relationship.js
#4154 (comment):

@@ -173,7 +173,9 @@ Relationship.prototype = {
//TODO(Igor) make this less abysmally slow
this.members = this.canonicalMembers.copy();
for (i=0; i<newRecords.length; i++) {

•  this.members.add(newRecords[i]);
  

•  if (!this.members.has(newRecords[i])) {
  

•    this.members.add(newRecords[i]);
  

•  }
  

@olivierchatry https://github.com/olivierchatry did you run into an
issue that required this if check? Under the hood members does a similar
check
https://github.com/emberjs/ember.js/blob/master/packages/ember-metal/lib/map.js#L113
which should be preventing the record from being added twice.

—
Reply to this email directly or view it on GitHub
https://github.com/emberjs/data/pull/4154/files#r52690836.

[bmac]

When I remove this if check all of the tests still pass.

[olivierchatry]

So, what do I need to do to make it OK ?

[bmac]

3 things:

1. I'd like to get @igorT to review this pr.
2. I think the change in addon/-private/system/relationships/state/relationship.js can be reverted.
3. Would you mind squashing this pr down to 1 commit and prefixing the commit message with [BUGFIX beta] so it can be pulled into the 2.4 beta branch.

[olivierchatry]

I will revert the relationship one as it is redundant. How do you want me
to squash down the PR ? create a new one ?

On Fri, Feb 12, 2016 at 7:02 PM, Brendan McLoughlin <
[email protected]> wrote:

3 things:

1. I'd like to get @igorT https://github.com/igorT to review this pr.
2. I think the change in
   addon/-private/system/relationships/state/relationship.js can be reverted.
3. Would you mind squashing this pr down to 1 commit and prefixing the
   commit message with [BUGFIX beta] so it can be pulled into the 2.4 beta
   branch.

—
Reply to this email directly or view it on GitHub
#4154 (comment).

[bmac]

@olivierchatry you can rebase your branch locally and then force push it to github and it will update this pr.

Here is a good short tutorial on how to perform a rebase locally: https://egghead.io/lessons/javascript-how-to-squash-multiple-git-commits

[olivierchatry]

Am turning a bit crazy, wondering if it would not be easier to create a new branch and a new pull request from it.

[koemeet]

@olivierchatry Do the following commands to squash your stuff:

git rebase -i HEAD~6 # 6 is how many commits you want to rebase

It will open vim or something equalvant. Then replace pick with s on the last 5 commits and save + exit the file. Note that you leave the first commit on pick.

Now make a good commit message, everything with # will be ignored. Also save + exit this file.

Your history is now been altered to one single commit, now run git push -f to force push your changes to the remote.

[olivierchatry]

stupid me, was missing the -f on the push ... thanks a lot !

On Fri, Feb 12, 2016 at 10:19 PM, Steffen Brem [email protected]
wrote:

@olivierchatry https://github.com/olivierchatry Do the following
commands to squash your stuff:

git rebase -i HEAD~6

It will open vim or something equalvant. Then replace pick with s on the
last 5 commits and save + exit the file.

Your history is now been altered to one single commit, now run git push -f
to force push your changes to the remote.

—
Reply to this email directly or view it on GitHub
#4154 (comment).

[sirvine]

I notice this isn't in 2.5.0-beta1. Should we expect this to appear in a release soon?

[bmac]

I'll make sure this gets included in the 2.5.0-beta.2 release.