Remove clear_changes_information in relations #317
Conversation
fzf
commented
Oct 31, 2018
fzf
commented
- Enables passing relationships in create calls
- Existing tests were not working due to stub request matching
- Includes relationships in as_json_api method
* Enables passing relationships in create calls * Existing tests were not working due to stub request matching * Includes relationships in as_json_api method
fzf
force-pushed
the
remove_clear_changes_information
branch
from
c913871 to
894779b
Compare
|
@gaorlov I noted on #307 currently the mock does not trigger and the extra paramters are not sent with the current implementation. The reformatting is to encapsulate the setup mock in a describe block and changes that were necessary to actually mock the web request. Let me know if you have any other questions. |
| "id"=>"9" | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
I don't think that in this case relationships should be serialized
#as_json_api is used for gathering a payload that will be sent to server
and we should not add to payload relationships that weren't changed by user
example:
post = Post.includes(:author).find(1)
post.title = 'changed title'
post.save # author relationship should not be present in payloadThere was a problem hiding this comment.
I can agree with that, this part was more of a side effect of removing the clear_changes_information, if there is another solution that allows relationships to be passed on create and does not change this I am all for it.
There was a problem hiding this comment.
Also, #318 does not address the issues with creation
There was a problem hiding this comment.
@fzf can you please provide some code snippet that explains issue with creation that you have encountered?
There was a problem hiding this comment.
Relationship data is never actually sent, it is cleared before it can be sent. Here is a spec showing that it is a false positive. #319
|
@fzf I agree that some tests have mistakes and some mocks were written incorrect, but I don't think that logic shouldn't be changed. |
