Skip to content

Guard for embedded unknown hasMany relationship - #3126

Merged
igorT merged 1 commit into
warp-drive-data:masterfrom
wecc:embedded-unknown-hasMany
Jun 2, 2015
Merged

Guard for embedded unknown hasMany relationship#3126
igorT merged 1 commit into
warp-drive-data:masterfrom
wecc:embedded-unknown-hasMany

Conversation

@wecc

@wecc wecc commented May 31, 2015

Copy link
Copy Markdown
Contributor

Closes #3071

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is this really what we want? (since the relationship is "unknown")

For other serializers it would make sense to exclude unknown relationships from the payload to prevent overwriting. Might not be necessary for embedded since it should be present, c?

ping @igorT @jakesjews

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I do agree that an embedded relationship should always be present but a developer might not necessarily be in control of the API. An alternative could perhaps be an assert so it's easier to debug.

@wecc
wecc force-pushed the embedded-unknown-hasMany branch from 2a153a6 to 98b96cd Compare June 1, 2015 11:48
@wecc

wecc commented Jun 1, 2015

Copy link
Copy Markdown
Contributor Author

@igorT @jakesjews updated with a warning, still embedding it as an empty array.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should probably mention that it's an embedded relationship, and say undefined instead of unknown

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done!

@igorT

igorT commented Jun 1, 2015

Copy link
Copy Markdown
Contributor

Thanks!

@wecc
wecc force-pushed the embedded-unknown-hasMany branch from 98b96cd to 7c21fc8 Compare June 2, 2015 01:20
@wecc
wecc force-pushed the embedded-unknown-hasMany branch from 7c21fc8 to d92c6ed Compare June 2, 2015 01:21
igorT added a commit that referenced this pull request Jun 2, 2015
Guard for embedded unknown hasMany relationship
@igorT
igorT merged commit 5cadf40 into warp-drive-data:master Jun 2, 2015
@ChrisHonniball

Copy link
Copy Markdown

Will this be tagged soon? This is the only thing preventing me from updating my application to Ember v1.12.1. Thanks!

@esbanarango

Copy link
Copy Markdown
Contributor

Same here!

@bmac

bmac commented Jun 3, 2015

Copy link
Copy Markdown
Contributor

Yeah, there have been a lot of changes recently. The plan is to get a release out this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Uncaught TypeError: Cannot read property 'map' of undefined

6 participants