-
Notifications
You must be signed in to change notification settings - Fork 76
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
Serializer blueprint #122
Serializer blueprint #122
Conversation
Thanks for the PR! Could somebody else with more Ember knowledge take a look at this? Also just to check - this wasn't a bug that snuck in in v3.2.0, was it? |
I don't think it was a bug so much as ember-pouch doesn't act like you'd expect w/r/t relations. I think this #111 fixed it, but then you'd need to create an application seralizer in your app for it to work. I just didn't see it documented anywhere that you really needed a serializer. |
I agree that this should be mentioned in the documentation, so I support merging this. I also wonder whether we should make explicit that people need to save both ends of a relationship because that’s not how people will generally expect Ember Data to work. |
Is this related to #5? |
As far as making it explicit that both ends need to be saved, that is in the documentation now under relationships - > saving. |
@backspace why do you think that ember-data users expect that they only need to save one side? Have I missed something? |
Oops, missed that, @dweremeichik, thanks! @fsmanuel, with Ember Data don’t you normally only have to save the child end of a relationship because in a relational database-style API, it’s the one with the foreign key? |
@fsmanuel @backspace according to the Emberjs documentation you only need to save one side. It shouldn't matter which, see this. Anyone who is new to both Ember and Ember Pouch will assume that what the documentation says is what would be done here (as it was never specified otherwise). Per the recent ember pouch documentation update, this is made more clear for new users. I would agree that it makes sense to include the serializer by default, otherwise it won't work as users expect. |
@backspace @dweremeichik yeah that's true. But it only works for relational database-style API. As far as I know ember data doesn't save both sides (aka send 2 requests to the server). If someone is interested I can assist to bring that feature into ember-pouch. |
That sounds great to me, @fsmanuel! |
Does this PR need any changes in order to get merged? |
I ran your generator locally and it worked as expected. I think @nolanlawson prefers squashed commits so maybe do that and then it can be merged? |
closing this in favor of #124 |
The serializer/relations bug bit me in the ass. Couldn't find anything in the Readme that mentioned the need for a serializer so I added a blueprint and updated the readme.