Hobo / hobo

The web app builder for Rails (moved from tablatom/hobo)
http://hobocentral.net
103 stars 39 forks source link

.apply_scopes MUST be called before .where #152

Open enwood opened 9 years ago

enwood commented 9 years ago

Ignacio Responds: Nice find! The implementation of apply_scopes is unfortunately old and fragile. We really need to replace it with something stronger based on the latest ActiveRecord/Arel syntax.

Tim's Submission: It seems that it is very important to call .apply_scopes BEFORE calling a .where clause, especially if the scope is effectively empty.

Calling .apply_scopes with an empty scope AFTER a .where clause causes the .where clause to be ignored!

An example:

In an #index function, I watch for search and sorting scopes:

  def index
    scopes = {                                                               
      :search => [params[:search], :user_name, :search_text], 
      :order_by => parse_sort_param(:id, :date_searched, :user_name, :matches, :search_text)
    }
    @searches = Search
         .where('date_searched >= ? AND date_searched <= ?', params[:start_date], params[:end_date] )
         .apply_scopes(scopes).order('date_searched DESC')
         .paginate(:per_page => 200, :page => params[:page])

I've found, though, that if my request does not include a search and an order_by parameter, the call to apply_scopes causes the .where clause to be ignored!

For example, this is the request to filter by date:

GET "/admin/searches?start_date=04%2F01%2F2015&end_date=04%2F30%2F2015"

And this is the SQL that's generated:

[INFO ] Parameters: {"start_date"=>"04/01/2015", "end_date"=>"04/30/2015"} [DEBUG] Search Load (98.4ms) EXEC sp_executesql N'SELECT TOP (200) [__rnt].* FROM ( SELECT ROW_NUMBER() OVER (ORDER BY date_searched DESC) AS [__rn], [searches].* FROM [searches] ) AS [__rnt] WHERE [__rnt].[__rn] > (0) ORDER BY [__rnt].[__rn] ASC'

How bizarre!

But, if it does include a search parameter to be used by apply_scopes:

[INFO ] GET "/admin/searches?start_date=04%2F01%2F2015&end_date=04%2F30%2F2015&search=Tim"

I get this SQL:

[DEBUG] Search Load (8.8ms) EXEC sp_executesql N'SELECT TOP (200) [__rnt].* FROM ( SELECT ROW_NUMBER() OVER (ORDER BY date_searched DESC) AS [__rn], [searches].* FROM [searches] WHERE (date_searched >= N''04/01/2015'' AND date_searched <= N''04/30/2015'') AND (((searches.user_name LIKE N''%Tim%'') OR (searches.search_text LIKE N''%Tim%''))) ) AS [__rnt] WHERE [__rnt].[__rn] > (0) ORDER BY [__rnt].[__rn] ASC'

SOLUTION:

If I reverse the order of the .where and the .apply_scopes, it works as expected, which is completely unintuitive!

@searches = Search
  .apply_scopes(scopes)
  .order('date_searched DESC')
  .where('date_searched >= ? AND date_searched <= ?', params[:start_date], params[:end_date] )

[DEBUG]   Search Load (6.5ms)  EXEC sp_executesql N'SELECT TOP (200) [__rnt].* FROM ( SELECT ROW_NUMBER() OVER (ORDER BY date_searched DESC) AS [__rn], [searches].* FROM [searches] WHERE (date_searched >= N''04/01/2015'' AND date_searched <= N''04/30/2015'') ) AS [__rnt] WHERE [__rnt].[__rn] > (0) ORDER BY [__rnt].[__rn] ASC'

In thinking about crafting this query, it made more sense to me to reduce the overall set first (with the .where clause), and then apply any search (or sorting) within that.

iox commented 9 years ago

Thanks for the great bug report!