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

Fixes an issue where removing with a query object would cause an error #512

Merged
merged 1 commit into from
Dec 14, 2013

Conversation

ben-ng
Copy link

@ben-ng ben-ng commented Dec 13, 2013

Thanks @zerocle for reporting this

Fixes this issue: https://github.com/zerocle/GeddyMultiDeleteError

Not yet merging this one in because I'm not sure if creating a copy of the argument is the best way to fix this.

Since model sends the object to the event handler, so there's the chance that the user (in this case geddy) changes that object and screws up what model expected.

Also, I feel like it should be model's job to perform the copy before passing it as an argument to emit.

Thoughts @mde?

@mde
Copy link
Contributor

mde commented Dec 14, 2013

This looks fine. The RT code is going to be revamped for the next release anyhow.

ben-ng pushed a commit that referenced this pull request Dec 14, 2013
Fixes an issue where removing with a query object would cause an error
@ben-ng ben-ng merged commit 53f66f5 into master Dec 14, 2013
@ben-ng ben-ng deleted the fix-remove-query branch December 14, 2013 02:22
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.

2 participants