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

Don't override mongoid original update. #40

Merged
merged 1 commit into from
Sep 28, 2015
Merged

Don't override mongoid original update. #40

merged 1 commit into from
Sep 28, 2015

Conversation

simi
Copy link
Owner

@simi simi commented Sep 28, 2015

fixes #39

@simi
Copy link
Owner Author

simi commented Sep 28, 2015

/cc @dblock @johnnyshields

@simi simi force-pushed the use-custom-update branch from 7833aad to cd5f864 Compare September 28, 2015 11:48
@dblock
Copy link
Contributor

dblock commented Sep 28, 2015

It needs an entry in https://github.com/simi/mongoid_paranoia/blob/master/CHANGELOG.md, please. And do write a test. It's contrived, but good discipline.

@simi
Copy link
Owner Author

simi commented Sep 28, 2015

Yes, but it is hard to write proper test for this. I can open some regression test spec file for those cases. But I don't want to write update test again, since there's one in Mongoid test suite.

What about the test case testing paranoia is not overriding methods except those we want (like destroy) ❓ That should cover this problem and the future unwanted overrides.

@maxjacobson
Copy link

That sounds like a good approach to me!

@dblock
Copy link
Contributor

dblock commented Sep 28, 2015

👍

simi added a commit that referenced this pull request Sep 28, 2015
Don't override mongoid original update.
@simi simi merged commit f793ee2 into master Sep 28, 2015
@simi
Copy link
Owner Author

simi commented Sep 28, 2015

@maxjacobson @dblock this was released as 0.2.1. I'll open issue for the rest (testing the overriding).

@simi simi deleted the use-custom-update branch September 28, 2015 21:32
@maxjacobson
Copy link

Thanks @simi

@simi
Copy link
Owner Author

simi commented Sep 28, 2015

Thanks for the report!

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.

including Mongoid::Paranoia changes the update method from public to private
3 participants