Skip to content
This repository has been archived by the owner on Mar 30, 2022. It is now read-only.

ActiveRecord default where no longer works #75

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

ActiveRecord default where no longer works #75

dnagir opened this issue Nov 8, 2011 · 7 comments

Comments

@dnagir
Copy link

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.

@ernie
Copy link
Contributor

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:
#75

@ernie ernie closed this as completed Nov 8, 2011
@dnagir
Copy link
Author

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).

@ernie
Copy link
Contributor

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
Copy link
Author

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
Copy link
Author

dnagir commented Nov 8, 2011

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

@dnagir
Copy link
Author

dnagir commented Nov 8, 2011

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

@cvshepherd
Copy link

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 subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants