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

Fix for #375 - toJSON should include IDs of unregistered models #376

Merged
merged 4 commits into from Oct 29, 2013

Conversation

Projects
None yet
2 participants
@philfreo
Collaborator

philfreo commented Jul 10, 2013

Possible fix for #375

Side effect (seems fine to me) is that it also continues to return IDs of deleted models, which
seems less magic and more expected to me - also allows for 'soft deletes' where an object itself
is deleted but other objects can still have references to it.

@philfreo

This comment has been minimized.

Show comment
Hide comment
@philfreo

philfreo Aug 19, 2013

Collaborator

@PaulUithol @DouweM Thoughts on this?

Collaborator

philfreo commented Aug 19, 2013

@PaulUithol @DouweM Thoughts on this?

philfreo added some commits Jul 10, 2013

Fix for #375 - toJSON should include IDs of unregistered models
Side effect (seems fine to me) is that it also continues to return IDs of deleted models, which
seems less magic and more expected to me - also allows for 'soft deletes' where an object itself
is deleted but other objects can still have references to it.
@philfreo

This comment has been minimized.

Show comment
Hide comment
@philfreo

philfreo Oct 29, 2013

Collaborator

Rebased against latest master.

Collaborator

philfreo commented Oct 29, 2013

Rebased against latest master.

@PaulUithol

This comment has been minimized.

Show comment
Hide comment
@PaulUithol

PaulUithol Oct 29, 2013

Owner

Was on the fence about this one for a bit; but seems quite fine to me. Merging.

Owner

PaulUithol commented Oct 29, 2013

Was on the fence about this one for a bit; but seems quite fine to me. Merging.

PaulUithol added a commit that referenced this pull request Oct 29, 2013

Merge pull request #376 from elasticsales/master
Fix for #375 - toJSON should include IDs of unregistered models

@PaulUithol PaulUithol merged commit 6ae1e60 into PaulUithol:master Oct 29, 2013

@philfreo

This comment has been minimized.

Show comment
Hide comment
@philfreo

philfreo Oct 29, 2013

Collaborator

thanks!

Collaborator

philfreo commented Oct 29, 2013

thanks!

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