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

[WIP] Add normalizePayload(modelName, payload) to DS.Store #3838

Closed
wants to merge 1 commit into from
Closed

[WIP] Add normalizePayload(modelName, payload) to DS.Store #3838

wants to merge 1 commit into from

Conversation

pangratz
Copy link
Member

A payload can be normalized by store.normalizePayload() into the JSON
API format Ember Data expects for store.push. The method optionally
takes a primary model name as first argument. If a primary model is
specified, the normalized payloads' primary data contains data for this
model from the payload and all other data is included.


This addresses #3605.

Opened this PR to get some feedback if I understood #3605 correctly and if this is heading the right direction...

A payload can be normalized by `store.normalizePayload()` into the JSON
API format Ember Data expects for `store.push`. The method optionally
takes a primary model name as first argument. If a primary model is
specified, the normalized payloads' primary data contains data for this
model from the payload and all other data is included.
@pangratz pangratz changed the title Add normalizePayload(modelName, payload) to DS.Store [WIP] Add normalizePayload(modelName, payload) to DS.Store Oct 11, 2015
@bmac
Copy link
Member

bmac commented Oct 13, 2015

I like this change. @pangratz do you mind wrapping this code in feature flags?

@workmanw
Copy link

Just a small piece of feedback on the naming. As someone who maintains a custom adapter and serializer, this is kind of makes my head spin. There was a normalizePayload on the JSONSerializer previously, which was deprecated and renamed to normalizeResponse in Ember-Data 1.13. The old normalizePayload would take a server response and normalize it via the serialize. And now I'm still not totally sure what the difference between the current normalizeResponse and the new normalizePayload will be.

@bmac
Copy link
Member

bmac commented Oct 13, 2015

Thanks @workmanw. That is excellent feedback. I've flagged this feature for discussion at the Ember Data weekly meeting to hopefully clarify its role, naming and necessity in the system.

@bmac
Copy link
Member

bmac commented Oct 28, 2015

We chatted about this method in the Ember Data weekly meeting and were a little skeptical about the benefits of adding normalizePayload without a clear overall understanding of the problem space. I think the next steps here are to look enumerate the problems that store#pushPayload, store#normalize and store#normalizePayload solve and try to figure out if there are any gaps or places where APIs can be simplified.

Making pushPayload return a value as @bcardarella suggested in #3576 may also be an acceptable solution to this problem.

@bmac bmac removed the team-review label Oct 28, 2015
@bmac
Copy link
Member

bmac commented Dec 4, 2015

@pangratz I'm going to close this because I think it would be better see pushPayload return a value to solve this issue.

@bmac bmac closed this Dec 4, 2015
@pangratz
Copy link
Member Author

pangratz commented Dec 4, 2015

👍

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.

3 participants