Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

ActiveRecord default where no longer works #75

Closed
dnagir opened this Issue Nov 8, 2011 · 7 comments

Comments

Projects
None yet
3 participants

dnagir commented Nov 8, 2011

I have an existing Rails 3.1 app with complex scopes that use simple hash conditions like Article.published.

It worked before I added squeel. The problem is that its .to_sql started to generate incorrect SQL:

SELECT articles.* from articles.* WHERE articles.status = articles.approved

Not the WHERE clause and the approved column that doesn't exist. It was specified on the scope:

Article.scope :published, where(:status => :approved)

I would prefer to keep existing AR queries and use squeel here and there.
Or maybe I'm just doing something wrong?

Also tried to config.load_core_extensions :symbol, but it only seems compatibility with MetaWhere.

Owner

ernie commented Nov 8, 2011

The value should be a string, not a symbol. That will fix the issue. Thanks!

Sent from my iPhone

On Nov 7, 2011, at 11:51 PM, Dmytrii Nagirniak
reply@reply.github.com
wrote:

I have an existing Rails 3.1 app with complex scopes that use simple hash conditions like Article.published.

It worked before I added squeel. The problem is that its .to_sql started to generate incorrect SQL:

SELECT articles.* from articles.* WHERE articles.status = articles.approved

Not the WHERE clause and the approved column that doesn't exist. It was specified on the scope:

Article.scope :published, where(:status => :approved)

I would prefer to keep existing AR queries and use squeel here and there.
Or maybe I'm just doing something wrong?

Also tried to config.load_core_extensions :symbol, but it only seems compatibility with MetaWhere.


Reply to this email directly or view it on GitHub:
ernie#75

@ernie ernie closed this Nov 8, 2011

dnagir commented Nov 8, 2011

No, that's not true for ActiveRecord. The value doesn't need to be a string. It only must respond to to_s: https://github.com/rails/rails/blob/master/activerecord/lib/active_record/relation/predicate_builder.rb#L11

There are 2 problems though:

  1. I expected that squell not to change any existing ActiveRecord behaviour when there is no block given. It does change it as you can see (it works without squeel).
  2. I would probably be able to go and change symbols to strings everywhere, but that's quite a bit of code to change. And symbols represent much better what I need (predefined list of items, not an arbitrary string, - states).
Owner

ernie commented Nov 8, 2011

I understand that this isn't the case for stock ActiveRecord -- this is the case for Squeel, however. See #67 for more details and discussion on this one.

dnagir commented Nov 8, 2011

Look, if there is no way of "enhancing" AR without breaking it. Then probably squeel should provide a "save" option.
I'm sure this topic will raise again and again.

I agree with guys there that "enhancement" should improve on something, not change. Also I don't see any reason to restrict the options to strings only. The whole Ruby works based on whether it responds to something or not.

So what I propose is to create and extension "safe" that would swap back the default implementation of AR/Arel.
But you can access squeel methods using something like Article.squeel.where or Article.sq_where or similar.
This should just leave normal AR methods alone, untouched.

dnagir commented Nov 8, 2011

Or maybe just use bang notion in "safe" mode: Article.where!

dnagir commented Nov 8, 2011

Think of it as jQuery.noConflict -> Squeel.no_conflict. It might be a golden spot.

I just encountered this problem and agree that we need a solution for this. Breaking standard AR functionality / behavior seems like a bad idea.

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