recommendable
recommendable copied to clipboard
user.bookmarks, user.likes and user.dislikes fail unless my recommendable class has an 'id' key
id
is hard coded as the primary key. Nothing wrong normally but I have this legacy database ported to Rails that doesn't respond to @instance.id
.
It fails in many levels but the first would be here: https://github.com/davidcelis/recommendable/blob/master/lib/recommendable/rater/bookmarker.rb#L74
If my primary key is a string like 'AAPL' then calling to_i
on it returns 0. Hence requesting all bookmarks for a specific user will trigger:
Company.where(id: 0)
Where there are no columns id
for companies AND there wouldn't be an id for 0.
Would be great if either we could pass as an option to recommends
or if it could look at ActiveRecord relationships:
has_many :companies, primary_key: :ticker
Current workaround:
Recommendable.configure do |config|
config.orm = :mongoid #avoids the .to_i
end
Then call:
ids = current_user.send(:bookmarked_ids_for, :company)
Company.where(ticker: ids)
Legacy databases are an edge case, I think.
That being said, perhaps it's time to introduce a per-recommendable configuration, kind of like what you suggested with the latter, and just allow a recommendable declaration to be one class with an options hash:
recommends :things, options = {}
This would also presumably fail for non-legacy databases using the new Rails 4 Postgres UUID PK.
I've been thinking about this as well, but it raises more questions. How many people do we think are actually going to jump on the UUID PK thing? It's not the default, my guess is many wouldn't configure it and would just stay with auto-incrementing integers. Unless your app is dealing with a ton of data, auto-incrementing integers really aren't that big of a deal.
That being said, it does seem like assuming that the ID is an integer is a problem. I'll provide a patch sometime in the next few days that does type checking on the column before doing mutations (i.e. ActiveRecord has user.column_for_attribute('id').type
)