Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass lid from relationship data to get record data #7425

Merged
merged 1 commit into from
Apr 8, 2021
Merged

Pass lid from relationship data to get record data #7425

merged 1 commit into from
Apr 8, 2021

Conversation

azhiv
Copy link
Contributor

@azhiv azhiv commented Feb 8, 2021

This PR aims to fix #7424.
The issue appeared in the sideposting scenario:

  • model A has belongs to relationship to model B.
  • instance of model A and instance of model B are created on the client,
  • model A is saved along with model B sent in include section of payload; the linking is done through lid
  • server persisted both models and sent the response with server-generated id along with lid sent by the client.
  • when belongsTo.updateData() was called while the response payload was pushed to the store it couldn't match the record which was already in the store with the record data contained in the relationship data because lid was omitted.

Add a test which ensures that the belongsTo() relationship has correct status in the sideposting with lid scenario.

@azhiv azhiv closed this Feb 9, 2021
@azhiv azhiv reopened this Feb 9, 2021
@azhiv
Copy link
Contributor Author

azhiv commented Feb 9, 2021

@snewcomer Can you please review this PR?

Copy link
Contributor

@runspired runspired left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM but one cleanup desired for the test.

@snewcomer snewcomer added the Bug label Feb 12, 2021
Copy link
Contributor

@runspired runspired left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with this and will merge once tests pass (had to rekick them) but want to give @igorT a heads up before merging as this is the sort of thing that can sometimes introduce subtle issues.

The issue appeared in the sideposting scenario:

- model A has belongs to relationship to model B.

- instance of model A and instance of model B are created on the client,

- model A is saved along with model B sent in `include` section of
  payload; the linking is done through `lid`

- server persisted both models and sent the response with
  server-generated `id` along with `lid` sent by the client.

- when belongsTo.updateData() was called while the response payload was
  pushed to the store it couldn't match the record which was already in
  the store with the record data contained in the relationship data
  because `lid` was omitted.

Add a test which ensures that the belongsTo() relationship has correct
status in the sideposting with lid scenario.
@andreyfel
Copy link
Contributor

@igorT the branch is green! Can we get it merged?

@runspired runspired merged commit 0ab470c into emberjs:master Apr 8, 2021
@runspired runspired added 🏷️ bug This PR primarily fixes a reported issue and removed Bug labels Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bug This PR primarily fixes a reported issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sideposting with lid leaves relationship in invalid state (belongsTo case)
4 participants