Skip to content
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

ActiveRecord scope with first returns all records #13647

Closed
tak2siva opened this issue Jan 9, 2014 · 11 comments
Closed

ActiveRecord scope with first returns all records #13647

tak2siva opened this issue Jan 9, 2014 · 11 comments

Comments

@tak2siva
Copy link

tak2siva commented Jan 9, 2014

This is the model I have

class TestModel < ActiveRecord::Base
 scope :fail, -> { where('1=0').first}
 scope :pass, -> { where('1=1').first}
end

On accessing fail, I get unexpected results(i.e it retrieves all records of TestModel ) instead of returning nil

2.0.0-p247 :003 >   TestModel.fail
  TestModel Load (17.5ms)  SELECT `test_models`.* FROM `test_models` WHERE (1=0) ORDER BY `test_models`.`id` ASC LIMIT 1
  TestModel Load (7.1ms)  SELECT `test_models`.* FROM `test_models`
 => #<ActiveRecord::Relation [#<TestModel id: 1, created_at: "2014-01-06 05:03:32", updated_at: "2014-01-06 05:03:32">, #<TestModel id: 2, created_at: "2014-01-06 05:03:36", updated_at: "2014-01-06 05:03:36">]> 
2.0.0-p247 :004 > 
2.0.0-p247 :005 >  

I'm using Rails 4.0 and MySQL 5.1

@carlosantoniodasilva
Copy link
Member

Scopes must return relation objects, so we are not supposed to be returning the objects themselves with first/last/find or any other.

The idea behind falling back to returning all is that in case your scope returns nil (which it does in your fail example), you can still use it to chain with other scopes: See this example:

class Post < ActiveRecord::Base
  scope :by_author, -> author { where(author: author) if author }
  scope :by_tag, -> tag { where(tag: tag) if tag }
end

Post.by_author(params[:author]).by_tag(params[:tag])

In case params[:author] is nil, the scope chaining would just work.

Check out this post for more details on scopes, and the api docs for scope.

Thanks!

/cc @rafaelfranca this might be of your interest :)

@tak2siva
Copy link
Author

@carlosantoniodasilva what if params[:author] is not nil and returns zero records

@carlosantoniodasilva
Copy link
Member

@tak2siva the important part is that it's going to return a relation, which will later be fetched (not inside the scope), and whether it's gonna return 0 or 1 or 10 records shouldn't affect the expected behavior. However, returning something that's not a relation, ie with first that returns the AR object, will break this.

@loicteixeira
Copy link

Hi there. I recently ran into the same issue, and while I understand the intent, I think the current implementation of scope is a little bit off as both claims (is only syntactic sugar and always return a relation) are false, not to mention the dangerous nature of returning all.

Below are details on each of those points. I would be happy to hear your thoughts on them @carlosantoniodasilva.

Always a relation

Given this model

class Post < ActiveRecord::Base
  scope :most_recent, -> { where(status: 'published').first }
end

If there are no published post, the result of the scope will be nil and fallback to all.

However, if there is one, the result of the scope will be an instance of Post which will be truthy, therefore returning the Post instance, not all, breaking the always return a relation promise.

Dangerous

Since the scope applies some conditions on the query, returning all (in other words unscoped) is dangerous, it should probably return an empty relation instead.

Syntactic sugar

Since scope adds some behaviour (scope || all at the end of the method), this is not simple syntactic sugar. It should either stand true to that claim and remove any extra behaviour (also removing extensions would be a bit of a shame) or the documentation should emphasis on the differences it introduce.

@pooja-mane
Copy link

pooja-mane commented Aug 11, 2016

I have a same issue related to scope.this is my code
scope :current, -> { where.(current: true).try(:first) }
when i run code for model then it gives me correct output, but for particular records which contains current as false value it return all records whichever it has.
I wrote class method instead of scope like this
def self.current
where(current: true).try(:first)
end
then it works correctly.
I have a question scope internally gets converted into class method then why does it happen if write a try(:first) in scope and in class method it works properly.

@agalloch
Copy link

Basically, scope is meant to be used solely when performing non-destructive operations (based) on the result set.

Should this be added to the documentation?

Also, should the doc for Rails 4.2 be updated with

If it returns nil or false, an all scope is returned instead.

given this behaviour is there as well?

@ChristinaXT
Copy link

I want to create a scope that says
scope :users_with_most_requests, _> {order(name: :desc)} . but I want to limit the name to one. The name with the most. How can I accomplish this? I have tried so many alternatives that my brain can't seem to produce any new ideas

@ChristinaXT
Copy link

This is the correct code I have so far
`scope :users_with_most_requests, -> { order(name: :desc)}

@carlosantoniodasilva
Copy link
Member

@ChristinaXT it sounds like you're ordering records by name in descending order, which doesn't quite match the "users with most requests" scope name... feels like something is missing. Do you have multiple models you're trying to connect here to achieve this "most requests" logic?

You can always apply a limit to the relation, for example order().limit(X), but that still returns a relation that can be used in the scope. "relation" basically means a collection of records. If you want a single record, I think you're looking for a combination of the scope (to build the order / condition you need), and then take (or first/last) to fetch that single record from the scope/relation. I hope that helps.

@ChristinaXT
Copy link

ChristinaXT commented Jun 15, 2019 via email

@CoderMiguel
Copy link

I would like to understand better why all records is favorable, when we have the ability to return none as an ActiveRecord::Relation, when the results are in fact none. Returning all results in this case breaks the logic for exists? as well.

    # activerecord/lib/active_record/relation/query_methods.rb

    # Returns a chainable relation with zero records.
    #
    # The returned relation implements the Null Object pattern. It is an
    # object with defined null behavior and always returns an empty array of
    # records without querying the database.
    #
    # Any subsequent condition chained to the returned relation will continue
    # generating an empty relation and will not fire any query to the database.
    #
    # Used in cases where a method or scope could return zero records but the
    # result needs to be chainable.
    #
    # For example:
    #
    #   @posts = current_user.visible_posts.where(name: params[:name])
    #   # the visible_posts method is expected to return a chainable Relation
    #
    #   def visible_posts
    #     case role
    #     when 'Country Manager'
    #       Post.where(country: country)
    #     when 'Reviewer'
    #       Post.published
    #     when 'Bad User'
    #       Post.none # It can't be chained if [] is returned.
    #     end
    #   end
    #
    def none
      spawn.none!
    end

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

No branches or pull requests

7 participants