Don't notify relationships with links during initialization

[mmun]

Fixes #4942.
Fixes #5023.
Fixes #5017.

I also took the liberty of changing https://github.com/emberjs/data/compare/bugfix/4942?expand=1#diff-fd4c34d9a34dfde73cd3413128a3c973L2876 to be more explicit.

If this change is approved, we should reach out to people linking to this issue after a release is cut.

• Bump Ember-Data to 2.14 travis-ci/travis-web#1173
• https://github.com/acquia/ember-drupal-waterwheel/issues/6
• https://github.com/niklaslong/ember-has-many/commit/28708aaf88a2cc452f0b4546746456c2c097290b
• emberobserver/client@7f4c39c
• Refactor to display events on public page via New API fossasia/open-event-frontend#399
• [Glimmer 2] "Backtracking re-render" is now an assertion ember.js#13948 (comment)
• WIP: Upgrade dependencies rust-lang/crates.io#893

[runspired]

where is this true ?

[mmun]

https://github.com/emberjs/data/blob/master/addon/-private/system/relationships/state/create.js#L72

Furthermore, this line and the one in my PR are the only two places that call Relationship#push in the test suite.

[runspired]

oooooh, that this arg was already present implies that at some point we'd thought of this case and lost it

[mmun]

I tried it out on someone's example app and it fixed the bug: #5023 (comment). I'd still like if other users could confirm this fix works for them.

If someone would like to try it out in their app, just change your ember-data dependency in package.json to

"ember-data": "emberjs/data#bugfix/4942",

[tsteuwer]

Will try in the morning on our apps.

[stefanpenner]

Fix looks good. But we will need a test or two

[mmun]

Hmm... I'm not sure of the right way to test this. The problematic assertion is very coupled to rendering so I guess we can write a regression test via a component integration test.

[rtablada]

@mmun if you want to pair a bit on testing it, I've got some time tomorrow and I'm definitely interested in getting more involved in the E-D side of things.

[aureliosaraiva]

@mmun Very nice!

[aureliosaraiva]

I tested in my apps and solves the problem.

[stefanpenner]

@mmun there is an observable difference re: change notifications, so a isolated test with some strategically placed listeners (cp dk's or observers) should do the trick in isolation, outside of rendering.

[mmun]

@stefanpenner Hmm for some reason I thought there wouldn't be change notifications in this case but I'll try that first.

BTW, should I label this for [BUGFIX release]?

[mmun]

@stefanpenner Tests added. I confirmed that they fail before the fix is applied.

[stefanpenner]

BTW, should I label this for [BUGFIX release]?

👍

[stefanpenner]

• beta backport PR: {BACKPORT BETA} [BUGFIX release] Don't notify relationships with links during initial… #5120
• release backport PR: {BACKPORT RELEASE} [BUGFIX release] Don't notify relationships with links during initial… #5121

Let's see how those CI runs go, if they are good we should cut releases. @bmac if they are green, and you happen to have cycles. I would love a hand shipping them. If not, I can likely handle shipping them late tomorrow night.

going to sleep now, see you all in the morning.

[rlivsey]

Just checked my app against master and can confirm this fixes the issues I was seeing in #4942

Awesome stuff, thank you everyone!

[buschtoens]

Thank you so much! This was blocking our upgrades for weeks. ♥️

[bmac]

I've released v2.14.10 and v2.15.0-beta.4 with this bugfix.

[stefanpenner]

@mmun thanks for the fix, @bmac thanks for cutting the release.

My apologies to those affected by this, and how long it took to address.