squeel
squeel copied to clipboard
activerecord-hackery
broken hash conditions when querying for an activerecord object on a joined table
Not really a pull request, just a test. Seems like squeel breaks the following expressions:
Comment.joins(:article).where(articles: {person_id: person})
Comment.joins(:article).where(articles: {person: person})
These will both raise an exception telling us that 'articles' cannot be converted to a class.
The fix looks like it would be pretty involved. Essentially squeel is trying to replicate AR predicate builder, but it might be better offer using it, and somehow converting the resulting arel back into something it can work with because trying to keep the behavior up to date and matching might be a pain.
Looks like a dup of https://github.com/activerecord-hackery/squeel/issues/313 which was closed, but this behavior definitely fails.
@dontfidget Thank you very much. Does it mean AR can pass this test? Squeel has a long history when AR was not as good as current. So it provided another way to build predicates to Arel. From Rails 3.0 to 4.2, AR changed a lot and is changing a lot, so it's harder to keep the same behaviors with AR. I'd like to consider your suggestion to use AR's code as possible as we can. It needs lots of works. Any information you provide would be helpful :)
I assume AR can pass the test. It was in my app and it started failing when I added squeel. Unfortunately, I don't know any more about AR than taking a cursory glance and seeing that there was something called a predicate builder. I don't know how hard it would to use it. In general, I don't understand why squeel is involved at all for a where clause that isn't passed a block, but I guess it must because it is built on top of arel rather than AR. Anyhow, I just worked the problem by writing it as:
Comment.joins(:article).where('articles.person_id' => person})
Incidentally, the reason this syntax is handy is that you can pass both a relation and a single AR record to it, which I've found convenient when defining scopes.
hum... you can also use squeel like this:
Comment.joins { article }.where { article.person_id == person }
It's also convenient when defining scopes :)
That true, but I'm not in the habit of using squeel unless I really need it to make things more readable, so my workaround has the property that it equally readable and works whether I'm using squeel or not. So far, at least on rails 4.1.x, calling squeel production ready is a bit of a stretch, so I currently don't want to commit to it more than I have to. The fact that it breaks my existing code is a bit terrifying, although by using the master branch of squeel and fixing a few things here and there, I think I'm going to run with it because I do much appreciate the syntax improvements it offers over arel. I'm looking forward to using it more and greatly appreciate all the work you've done so far to make it work at all on rails 4.
Andrew
On Wed, Sep 10, 2014 at 10:14 PM, Xiang Li [email protected] wrote:
hum... you can also use squeel like this:
Comment.joins { article }.where { article.person_id == person }
It's also convenient when defining scopes :)
— Reply to this email directly or view it on GitHub https://github.com/activerecord-hackery/squeel/pull/338#issuecomment-55220319 .
Yes, one of Squeel's goal is to make users couldn't detect the existence of Squeel when they are not using them. If you find more compatible problems, just tell me, I would schedule them one by one. Then I would try to find a way to refactor the code. Thank you.
