UserSearch refactor

user_search.rb

class UserSearch
  def self.search(term, topic_id = nil)
    scope = User.scoped 

    if topic_id
      scope = scope.joins("
        LEFT JOIN (
          SELECT DISTINCT(posts.user_id)
          FROM posts
          WHERE topic_id = #{topic_id.to_i}
        ) s ON s.user_id = users.id
      ")

      scope = scope.order('s.user_id IS NULL')
    end

    scope = scope.order('
      last_seen_at IS NULL ASC,
      last_seen_at DESC,
      username ASC
    ')

    scope = scope.where("
      username_lower LIKE :term_like OR
      TO_TSVECTOR('simple', name) @@
      TO_TSQUERY('simple',
        REGEXP_REPLACE(
          REGEXP_REPLACE(
            CAST(PLAINTO_TSQUERY(:term) AS text)
            ,'\''(?: |$)', ':*''', 'g'),
        '''', '', 'g')
    )", term: term, term_like: "#{term.downcase}%")
    
    scope.limit(20)
  end
end

Agree

1. Check to see if fabricator has an equivalent of FactoryGirl's build_stubbed. Reducing database interactions is the best way to get a fast test suite.
2. This is fair to some degree, but you're racking up a ton of technical debt and it's much easier to educate developers on the way in than herd cats once you hit scale.
3. It's tough to even read -- are you just regexping out noise and then doing a fulltext? If so, you can move the sanitization to Ruby and then stuff this complexity below a class method.
4. It does.
5. Yes, but contributors may not. That's one of the tools I think most Rails developers take as a given.
6. I'll take another stab at it when I have time.

Disagree

0. There is debate around rvm vs rbenv but there is zero debate around the fact that you need to choose one of the two. As it is, you're totally at the mercy of whatever insane local configuration each developer has, and you WILL cause conflicts and hard-to-track bugs given your current (lack of) approach.
1. The lack of an ability to export a partial index is not an excuse for letting schema.rb be out of date. You're missing entire tables, and that will be quite painful for future developers.
2. Egh, it's not in the tests. I'll take another look later.
3. Entirely subjective. I hear you, though, that .joins(...) should behave more like .where(...).
4. find_by_sql is not a "supported active record concept." It's a code smell and only to be used in really, really dire cases and when implementing gems. The official Rails docs are explicit about this:

If you’re used to using raw SQL to find database records, then you will generally find that there are better ways to carry out the same operations in Rails. Active Record insulates you from the need to use SQL in most cases.

http://guides.rubyonrails.org/active_record_querying.html

And, no, stating that it's "just a reinvention of the ActiveRecord wheel" is emphatically not a statement of taste, whatever you think about it. Every single method you've implemented has a corresponding method in ActiveRecord with the SAME NAME that does the SAME THING. Your library is a strict subset of the built-in ORM that lacks the flexibility and community testing that ActiveRecord has received over the years.

Don't get me wrong, AR is not perfect. It can be improved and it has shortcomings. However, I see absolutely nothing in your library that indicates you hit a wall with AR or saw some huge shortcoming.

9. I never suggested that you introduce scopes that are not needed. I suggested refactoring this method further so the scopes are implemented as classmethods (faux scopes). I suspect you will get some reuse out of them down the road, but the arguments for testability, granularity and readability are sufficient without reuse, in my opinion.
10. Generally Rails apps don't specify the select clause because it introduces undesirable coupling between the views and models, especially since there's no security or performance argument for doing so (that I'm aware of).

PS Just to be clear, I think it's great that you're open-sourcing this project and using Rails. I strongly prefer to be direct in the interest of time.

I think @blowmage's fork is an improvement over what I produced. See it here:

https://gist.github.com/blowmage/4755700