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

Namespace conflict with model methods #112

Open
gbuesing opened this Issue Dec 14, 2015 · 4 comments

Comments

Projects
None yet
3 participants
@gbuesing
Contributor

gbuesing commented Dec 14, 2015

My User model recommends :places, but it already has an Active Record association for recommended_places, so I am forced to access Recommendable recommendations via:

user.send :recommended_for, Place

... which is a bit clunky. To avoid this, and other potential namespace collisions for existing model methods, I suggest supporting an optional recommendable_ prefix on Recommendable finder methods, e.g.:

user.recommendable_recommended_places

Would be easy to implement -- we'd just need to add an optional check in the method regexes for the namespace. Existing finder methods would still work -- you'd only need to prepend recommendable_ if you had a namespace issue.

@Nuru

This comment has been minimized.

Show comment
Hide comment
@Nuru

Nuru Dec 17, 2015

I have a similar but different name collision: some of my models already implement a recommendable? function that of course has a different meaning. You might want to look into how elasticsearch-rails handles potential name collisions and do something similar.

Nuru commented Dec 17, 2015

I have a similar but different name collision: some of my models already implement a recommendable? function that of course has a different meaning. You might want to look into how elasticsearch-rails handles potential name collisions and do something similar.

@davidcelis

This comment has been minimized.

Show comment
Hide comment
@davidcelis

davidcelis Dec 17, 2015

Owner

@Nuru Thanks for that link; I like that idea a lot. I'll take a look at implementing something similar soon. In the meantime, I'm glad you found a workaround @gbuesing!

Owner

davidcelis commented Dec 17, 2015

@Nuru Thanks for that link; I like that idea a lot. I'll take a look at implementing something similar soon. In the meantime, I'm glad you found a workaround @gbuesing!

@Nuru

This comment has been minimized.

Show comment
Hide comment
@Nuru

Nuru Dec 17, 2015

@davidcelis There's also the issue that if a ratable class is within a module, e.g. Product::Song, then the generated methods are not callable directly because they contain a /. One ends up having to use user.send('recommended_product/song'). Since these all end up calling recommended_for anyway, perhaps make that public instead of private. (While you are at it, you could make liked_ids_for(klass) public, too.)

Nuru commented Dec 17, 2015

@davidcelis There's also the issue that if a ratable class is within a module, e.g. Product::Song, then the generated methods are not callable directly because they contain a /. One ends up having to use user.send('recommended_product/song'). Since these all end up calling recommended_for anyway, perhaps make that public instead of private. (While you are at it, you could make liked_ids_for(klass) public, too.)

@davidcelis

This comment has been minimized.

Show comment
Hide comment
@davidcelis

davidcelis Dec 17, 2015

Owner

I completely forgot about that issue! 😳

Owner

davidcelis commented Dec 17, 2015

I completely forgot about that issue! 😳

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