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

Undefined method where for Array #96

Open
coneybeare opened this Issue Aug 17, 2014 · 13 comments

Comments

Projects
None yet
6 participants
@coneybeare

coneybeare commented Aug 17, 2014

According to your docs and example page, this should work:

irb(main):006:0> user.recommended_thing.where('published = true')
NoMethodError: undefined method `where' for #<Array:0x00000005d45788>

Likes, recommendations, queries all work, but scoping the recommended_thing balks out on the call due to an array being returned instead. How can I debug?

@coneybeare

This comment has been minimized.

Show comment
Hide comment
@coneybeare

coneybeare Aug 17, 2014

Could this have to do with class caching? I can reproduce it continuously in production but not development using the same ruby, rails and recommendable versions.

coneybeare commented Aug 17, 2014

Could this have to do with class caching? I can reproduce it continuously in production but not development using the same ruby, rails and recommendable versions.

@davidcelis

This comment has been minimized.

Show comment
Hide comment
@davidcelis

davidcelis Aug 20, 2014

Owner

Sorry about the slow response; I've been on vacation! Oh man, those docs are completely wrong. They actually reflect how I wanted the interface to be. I must have never corrected those.

In order to preserve how recommendations should be ordered (best to worst), I have recommended_things return an Array as opposed to an ActiveRecord::Relation. This is because some databases don't let you order results by a specific order of a column (in this case, ID). I'd love to be able to make it chainable, but that above problem makes it seem impossible unless I'm missing something. I'm not sure why chaining a where on that in development would work, though...

Sorry about that confusion. I'll correct those docs when I can, but I'm without a computer for the next week. I would accept a PR to remove those bad examples though!

Owner

davidcelis commented Aug 20, 2014

Sorry about the slow response; I've been on vacation! Oh man, those docs are completely wrong. They actually reflect how I wanted the interface to be. I must have never corrected those.

In order to preserve how recommendations should be ordered (best to worst), I have recommended_things return an Array as opposed to an ActiveRecord::Relation. This is because some databases don't let you order results by a specific order of a column (in this case, ID). I'd love to be able to make it chainable, but that above problem makes it seem impossible unless I'm missing something. I'm not sure why chaining a where on that in development would work, though...

Sorry about that confusion. I'll correct those docs when I can, but I'm without a computer for the next week. I would accept a PR to remove those bad examples though!

@coneybeare

This comment has been minimized.

Show comment
Hide comment
@coneybeare

coneybeare Aug 20, 2014

Can you explain this? (in dev server, my prod is where I was getting the error before)

> user.recommended_audio(10, 0)
  Audio Load (0.3ms)  SELECT `audio`.* FROM `audio`  WHERE `audio`.`deleted_at` IS NULL AND 1=0
 => #<ActiveRecord::Relation []> 

coneybeare commented Aug 20, 2014

Can you explain this? (in dev server, my prod is where I was getting the error before)

> user.recommended_audio(10, 0)
  Audio Load (0.3ms)  SELECT `audio`.* FROM `audio`  WHERE `audio`.`deleted_at` IS NULL AND 1=0
 => #<ActiveRecord::Relation []> 
@coneybeare

This comment has been minimized.

Show comment
Hide comment
@coneybeare

coneybeare Aug 20, 2014

Ah, it seems that you return a relation if there are no recommendations, but an array if there are some. This is why it differs on my live data and my relatively sparse dev data. Now that I know I cannot chain the calls, is there a mechanism recommendable has for filtering recommended_thing using any params other than limit and offset?

coneybeare commented Aug 20, 2014

Ah, it seems that you return a relation if there are no recommendations, but an array if there are some. This is why it differs on my live data and my relatively sparse dev data. Now that I know I cannot chain the calls, is there a mechanism recommendable has for filtering recommended_thing using any params other than limit and offset?

@davidcelis

This comment has been minimized.

Show comment
Hide comment
@davidcelis

davidcelis Aug 21, 2014

Owner

Unfortunately no. But that method could (and should) totally be changed to accept a conditions hash that can just be passed through to a where call!

Owner

davidcelis commented Aug 21, 2014

Unfortunately no. But that method could (and should) totally be changed to accept a conditions hash that can just be passed through to a where call!

@inspire22

This comment has been minimized.

Show comment
Hide comment
@inspire22

inspire22 Aug 27, 2014

And then the results could be re-ordered again using the orig. order (minus, of course, the missing rows)

inspire22 commented Aug 27, 2014

And then the results could be re-ordered again using the orig. order (minus, of course, the missing rows)

@davidcelis

This comment has been minimized.

Show comment
Hide comment
@davidcelis

davidcelis Aug 27, 2014

Owner

Update: this is gonna get fixed, just got a new computer today! Once it's set up and I'm back in the swing of things, this'll be the first thing I tackle 👍

Owner

davidcelis commented Aug 27, 2014

Update: this is gonna get fixed, just got a new computer today! Once it's set up and I'm back in the swing of things, this'll be the first thing I tackle 👍

@davidcelis

This comment has been minimized.

Show comment
Hide comment
@davidcelis

davidcelis Sep 2, 2014

Owner

Ah I forgot that this query is entirely ID-based. Right now I just grab the first 10 (by default) IDs sorted by rank in the ZSET that stores recommendations and query based on those. This will be a bit more difficult than I thought, as I'll have to grab the entire ZSET out of Redis and query for all of those records. This is kinda bad for three reasons:

  1. This ZSET can get rather large if your data set is large
  2. Recommendable would have to query for every record present in the ZSET (along with any conditions passed), then sort by ID, then slice off the best 10 (or whatever). That's a bigger query than most people will be asking for, probably
  3. There's a configuration option to keep this set smaller by only storing N recommendations at a time. If that configuration option is set, Recommendable won't know about more than N records, so querying through those by conditions could give very very small sets of recommendations.
Owner

davidcelis commented Sep 2, 2014

Ah I forgot that this query is entirely ID-based. Right now I just grab the first 10 (by default) IDs sorted by rank in the ZSET that stores recommendations and query based on those. This will be a bit more difficult than I thought, as I'll have to grab the entire ZSET out of Redis and query for all of those records. This is kinda bad for three reasons:

  1. This ZSET can get rather large if your data set is large
  2. Recommendable would have to query for every record present in the ZSET (along with any conditions passed), then sort by ID, then slice off the best 10 (or whatever). That's a bigger query than most people will be asking for, probably
  3. There's a configuration option to keep this set smaller by only storing N recommendations at a time. If that configuration option is set, Recommendable won't know about more than N records, so querying through those by conditions could give very very small sets of recommendations.
@glebm

This comment has been minimized.

Show comment
Hide comment
@glebm

glebm Oct 10, 2014

This is because some databases don't let you order results by a specific order of a column (in this case, ID).

To receive a specific ordering from the database in a compatible way you can:

SELECT * FROM users 
ORDER BY `users`.`id`=5 DESC, `users`.`id`=7 DESC, `users`.`id`=3 DESC

There is a gem that can generate this, order_query:

User.seek([:id, [5, 7, 3]]).scope

glebm commented Oct 10, 2014

This is because some databases don't let you order results by a specific order of a column (in this case, ID).

To receive a specific ordering from the database in a compatible way you can:

SELECT * FROM users 
ORDER BY `users`.`id`=5 DESC, `users`.`id`=7 DESC, `users`.`id`=3 DESC

There is a gem that can generate this, order_query:

User.seek([:id, [5, 7, 3]]).scope

davidcelis added a commit that referenced this issue Mar 11, 2015

Return ActiveRecord::Relations for `recommended_*` methods
A supposedly database-compatible way to do this; possibly addresses #96
and #103, but I'd like user feedback first.

Signed-off-by: David Celis <me@davidcel.is>
@davidcelis

This comment has been minimized.

Show comment
Hide comment
@davidcelis

davidcelis Mar 11, 2015

Owner

Hey. I implemented that supposedly database-compatible way to do this in that above commit. Can you please try bundling off of master? I'd like to get this tested across several databases before releasing a new version.

Owner

davidcelis commented Mar 11, 2015

Hey. I implemented that supposedly database-compatible way to do this in that above commit. Can you please try bundling off of master? I'd like to get this tested across several databases before releasing a new version.

@davidcelis

This comment has been minimized.

Show comment
Hide comment
@davidcelis

davidcelis Mar 11, 2015

Owner

Note that with this change, these methods technically won't need to handle limit or offset anymore, so I'd like to remove that logic entirely. Because of that interface change, I'm likely to bump the minor version instead of the patch.

 # instead of user.recommended_books(10, 20)...
user.recommended_books.limit(10).offset(20)
Owner

davidcelis commented Mar 11, 2015

Note that with this change, these methods technically won't need to handle limit or offset anymore, so I'd like to remove that logic entirely. Because of that interface change, I'm likely to bump the minor version instead of the patch.

 # instead of user.recommended_books(10, 20)...
user.recommended_books.limit(10).offset(20)
@davidrv

This comment has been minimized.

Show comment
Hide comment
@davidrv

davidrv Jun 12, 2015

Hey, not working for Mongo (mongoid)...

Getting "undefined method `recommended_apps'". I've tested it in one of my apps from here: https://github.com/davidrv/recommendable/commits/quickfix
and it works until I merge this commit into v2.2.0 stable.

  • rails 4.2.1
  • mongoid 4.0.2

Thanks! :)

davidrv commented Jun 12, 2015

Hey, not working for Mongo (mongoid)...

Getting "undefined method `recommended_apps'". I've tested it in one of my apps from here: https://github.com/davidrv/recommendable/commits/quickfix
and it works until I merge this commit into v2.2.0 stable.

  • rails 4.2.1
  • mongoid 4.0.2

Thanks! :)

@gsdean

This comment has been minimized.

Show comment
Hide comment
@gsdean

gsdean Feb 8, 2018

So did anything ever come from this. Interfacing with an array is less than ideal

gsdean commented Feb 8, 2018

So did anything ever come from this. Interfacing with an array is less than ideal

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