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

Need a warning in Readme #235

Closed
tranvictor opened this issue Apr 8, 2016 · 3 comments
Closed

Need a warning in Readme #235

tranvictor opened this issue Apr 8, 2016 · 3 comments

Comments

@tranvictor
Copy link
Contributor

Using enumerize with Mongoid can mislead to something pretty hard to figure out which is writer? method.

In more details, if we use writer as a valid value for a field and predicates is true, prefix is false then developers are likely to use writer? to check if the field has value of writer. In fact, this method mostly returns false because it is defined by Mongoid.

Explanation: Enumerize delegates the writer? message to Enumerize:Value, this value is inherited from String which is internally extended by Mongoid with writer? defined. On the other hand, when developer check method(:writer?), they would still see that the method is from enumerize that seems legit making it pretty hard to track down.

Proposal: We should have a strong warning for developers about this on Readme because writer is not a rare value to use. For website dealing with publishing content (news, magazine, ad network, affiliate platform, ...), developers can easily fall into using writer as a role of an user.

@nashby
Copy link
Member

nashby commented Apr 27, 2016

@tranvictor nice catch, thank you for filling the issue! Care to send a pull request please?

@tranvictor
Copy link
Contributor Author

@nashby sure, i will file a PR soon.

@flvrone
Copy link
Contributor

flvrone commented Sep 18, 2018

@nashby , @tranvictor , @aki77 , guys, I just have noticed that mongoid already has some method called with_type, don't know what it does yet, but,

it actually means that for quite frequent scenario when models have enumerable field called type, and have scope for it using scope: true, it's bad, for 2 reasons:

  1. it overrides original with_type method on model class,

  2. it does not override it for Mongoid::Criteria objects, which causes errors when trying to use generated scope in a chain after any other scope (which returns Mongoid::Criteria object, of course).

So I believe we got to have this warning as well.

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

No branches or pull requests

4 participants
@nashby @tranvictor @flvrone and others