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

Add a method for normalizing a payload instead of a single resource #3605

Closed
jonkoops opened this issue Jul 29, 2015 · 7 comments
Closed

Add a method for normalizing a payload instead of a single resource #3605

jonkoops opened this issue Jul 29, 2015 · 7 comments

Comments

@jonkoops
Copy link

I am building a Service that takes care of authenticating a user in my application. The Service does a request to the API, normalizes the data, pushes and gets the record from the store and resolves with said record of an authenticated user.

The issue I am encountering is that there is no way to normalize the payload without manually stripping the data hash from the root of the server response. This is because store.normalize() accepts only a single resource and not a payload like JSON API where the type and primary data is known. The Service now needs to know what the format of the payload is because the API response needs to be modified, which means that if the API response changes this code will break.

After some discussion with @wecc and @igorT a suggestion popped up that it might be an idea to add a normalizePayload method which should take an optional modelName parameter to determine which serializer will be used and a inputPayload argument to pass the payload (much the same as store.pushPayload).

Current situation:

var payload = store.normalize('user', response.data);
var record = store.push(payload);

Proposed solution:

var payload = store.normalizePayload(response); // Or optionally store.normalizePayload('user', response);
var record = store.push(payload);
@fotinakis
Copy link

I think I just hit the same thing you described—it looks like store.push only normalizes the JSON API primary data, not included resources, so relationships included with my user object die, for example: "No model was found for 'subscriptions' (because subscriptions in the JSON API response does not get normalized to subscription in ember-data internals).

I was sort of able to get this to work as a workaround, though with a bunch of deprecation warnings:

var userId = payload.data.id;  // Grab now because pushPayload modifies the hash.
store.pushPayload(payload);
var userRecord = store.peekRecord('user', userId);

To fix this, shouldn't store.push(payload) just normalize full compound documents by default?

@wecc
Copy link
Contributor

wecc commented Jul 29, 2015

I think I just hit the same thing you described—it looks like store.push only normalizes the JSON API primary data

Just for the record, store.push expects fully normalized data, it does no normalization on it's own.

@fotinakis
Copy link

Ah makes sense—let me reframe then, shouldn't store.normalize normalize full compound documents by default?

I'd expect this to work: store.push(store.normalize('user', payload)) but that also fails with the above No model was found for 'subscriptions' error because the compound resources are not normalized.

@wecc
Copy link
Contributor

wecc commented Jul 29, 2015

I'd say that's exactly the issue :) store.normalize only normalizes a single record/resource but in this scenario we want to normalize a full payload AND return the result when pushing it. There's currently no good way of doing that.

@fotinakis
Copy link

Hah got it—after that and reading a bunch of the ember-data source, I see now that the OP was asking for exactly that all along. :) A normalizePayload method would solve all of this.

@sly7-7
Copy link
Contributor

sly7-7 commented Feb 10, 2016

Closing, since #3838 was closed too, and if I understand well, store.pushPayload() (with the ds-pushpayload-return flag enabled), does exactly what is requested here. Please reopen if I'm missing something

@sly7-7 sly7-7 closed this as completed Feb 10, 2016
@jonkoops
Copy link
Author

@sly7-7 Looks good to me. Thanks for checking up on this issue.

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

No branches or pull requests

4 participants