Suggestion: Make the ID param optional

[dmnelson]

Currently Polo's usage goes by Polo.explore(Product, 1). But, in some cases you might not be wanting to look for an specific ID but for objects that have a specific type of data, for example: Polo.explore(Product.most_popular). Or, in cases where you completely truncate a table and re-populate it often and you can't have static primary key values.

In that case it would be nice to be able to omit the ID parameter. Right now we can workaround this by doing: Polo.explore(Product.most_popular, Product.most_popular.pluck(:id)), but that looks very inefficient.

Another possibility would be to remove completely the ID param and only rely on scopes for that, like Polo.explore(Product.where(id: 1)), but I do see the value of having that param as a way to simplify the usage.

[nettofarah]

Hi, @dmnelson.
I think this issue makes a lot of sense. That was how Polo used to work back in its first days.

The biggest problem with this approach is that the outermost select doesn't get picked up by Polo::Collector if you pass in the ActiveRecord query itself.

So for that reason we have to force Polo to load the parent query again in order for it to collect that initial query. You can see it here: https://github.com/IFTTT/polo/blob/master/lib/polo/collector.rb#L18

I'm open to better solutions though...

[dmnelson]

So, I'm not sure if I understood the issue. Before opening the issue, I only did a small hack on the collector.rb file and tested it out:

base_finder = @base_class.includes(@dependency_tree)
base_finder = base_finder.where(@base_class.primary_key => @id) unless @id.nil?

(In that case we would probably want to rename base_class to scope or relation to be more accurate)

And it worked fine (it even gets the default scope from act_as_paranoid):

[2] pry(main)> Polo.explore(Product.in_stock)
  Product Load (38.0ms)  SELECT "products".* FROM "products" WHERE "products"."deleted_at" IS NULL AND in_stock
  Product Load (6.4ms)  SELECT "products".* FROM "products" WHERE "products"."deleted_at" IS NULL AND in_stock
  Product Load (4.8ms)  SELECT "products".* FROM "products" WHERE "products"."deleted_at" IS NULL AND in_stock
=> ["INSERT INTO...

So, my question is, is there a case where it doesn't work as expected?

[nettofarah]

that's weird, I have seen tests break in the past when that extra db call wasn't around.
Have you tried running all the tests after making that change?

[dmnelson]

Yes, they all passed. I just tried 0.1.0 and it showed two instead of three. But yeah, that's weird, Rails relations are supposed to be lazy evaluated, what could be triggering them so many times?

[nettofarah]

That's a good question.. I think we'd have to dig a little deeper to find out what is going on.