Refactoring getRecentProfiles method

app/api/srch/search.rb

@@ -21,14 +21,14 @@ class Search < Grape::API
         Search.execute(:all, params)
       end
 
-      # Request URL should be /api/srch/profiles?srchString=QRY[&order=RECENTDESC&seq=KEYCOUNT&showCount=NUM_ROWS&pageNum=PAGE_NUM]
+      # Request URL should be /api/srch/profiles?srchString=QRY[&order_by=recent&sort_direction=desc&seq=KEYCOUNT&showCount=NUM_ROWS&pageNum=PAGE_NUM]
       # Basic implementation from classic plots2 SearchController
       desc 'Perform a search of profiles', hidden: false,
                                            is_array: false,
                                            nickname: 'srchGetProfiles'
 
       params do
-        use :common, :sortorder
+        use :common, :sorting, :ordering
       end
       get :profiles do
         Search.execute(:profiles, params)
@@ -105,9 +105,10 @@ def self.execute(endpoint, params)
       sresult = DocList.new
       search_query = params[:srchString]
       tag_query = params[:tagName]
-      order_query = params[:order]
+      order_query = params[:order_by]
+      sort_query = params[:sort_direction]
       search_type = endpoint
-      search_criteria = SearchCriteria.new(search_query, tag_query, order_query)
+      search_criteria = SearchCriteria.new(search_query, tag: tag_query, order_by: order_query, sort_direction: sort_query)
 
       if search_criteria.valid?
         sresult = ExecuteSearch.new.by(search_type, search_criteria)

app/api/srch/shared_params.rb

@@ -15,8 +15,12 @@ module SharedParams
       optional :tagName, type: String, documentation: { example: 'awesome' }
     end
 
-    params :sortorder do
-      optional :order, type: String, documentation: { example: 'recentdesc' }
+    params :ordering do
+      optional :order_direction, type: String, documentation: { example: 'desc' }
+    end
+
+    params :sorting do
+      optional :sort_by, type: String, documentation: { example: 'recent' }
     end
 
     params :commontypeahead do

app/models/user.rb

@@ -33,6 +33,7 @@ class User < ActiveRecord::Base
   has_many :following_users, through: :active_relationships, source: :followed
   has_many :followers, through: :passive_relationships, source: :follower
   has_many :likes
+  has_many :revisions, through: :node
 
   validates_with UniqueUsernameValidator, on: :create
   validates_format_of :username, with: /\A[A-Za-z\d_\-]+\z/

app/services/execute_search.rb

@@ -10,9 +10,9 @@ def execute(type, search_criteria)
     sresult = DocList.new
     case type
      when :all
-       sresult = sservice.textSearch_all(search_criteria.query)
+       sresult = sservice.textSearch_all(search_criteria)
      when :profiles
-       sresult = sservice.textSearch_profiles(search_criteria.query, search_criteria.order)
+       sresult = sservice.textSearch_profiles(search_criteria)
      when :notes
        sresult = sservice.textSearch_notes(search_criteria.query)
      when :questions

app/services/search_criteria.rb

@@ -1,13 +1,30 @@
 class SearchCriteria
-  attr_reader :query, :tag, :order
+  attr_reader :query, :tag, :order_by
 
-  def initialize(query, tag = nil, order = nil)
+  def initialize(query, tag: nil, order_by: nil, sort_direction: "DESC")
     @query = query
     @tag = tag
-    @order = order
+    @order_by = order_by
+    @sort_direction = sort_direction
   end
 
   def valid?
     !query.nil? && query != 0
   end
+
+  def sort_direction
+    sanitize_direction(@sort_direction)
+  end
+
+  private
+
+  def sanitize_direction(direction)
+    if direction.present?
+      direction = direction.upcase
+      options = %w(DESC ASC)
+      options.include?(direction) ? direction : "DESC"
+    else
+      "DESC"
+    end
+  end
 end

app/services/search_service.rb

@@ -11,52 +11,61 @@ def initialize; end
 
   # Run a search in any of the associated systems for references that contain the search string
   # and package up as a DocResult
-  def textSearch_all(srchString)
+  def textSearch_all(search_criteria)
     sresult = DocList.new
 
     # notes
-    noteList = textSearch_notes(srchString)
+    noteList = textSearch_notes(search_criteria.query)
     sresult.addAll(noteList.items)
 
     # Node search
     Node.limit(5)
       .order('nid DESC')
-      .where('(type = "page" OR type = "place" OR type = "tool") AND node.status = 1 AND title LIKE ?', '%' + srchString + '%')
+      .where('(type = "page" OR type = "place" OR type = "tool") AND node.status = 1 AND title LIKE ?', '%' + search_criteria.query + '%')
       .select('title,type,nid,path').each do |match|
       doc = DocResult.fromSearch(match.nid, match.icon, match.path, match.title, '', 0)
       sresult.addDoc(doc)
     end
     # User profiles
-    userList = textSearch_profiles(srchString)
+    userList = textSearch_profiles(search_criteria)
     sresult.addAll(userList.items)
 
     # Tags
-    tagList = textSearch_tags(srchString)
+    tagList = textSearch_tags(search_criteria.query)
     sresult.addAll(tagList.items)
     # maps
-    mapList = textSearch_maps(srchString)
+    mapList = textSearch_maps(search_criteria.query)
     sresult.addAll(mapList.items)
     # questions
-    qList = textSearch_questions(srchString)
+    qList = textSearch_questions(search_criteria.query)
     sresult.addAll(qList.items)
 
     sresult
   end
 
-  # Search profiles for matching text and package up as a DocResult
-  # If order == "recentdesc", search for more recently updated profiles for matching name.
-  # Otherwise, show profiles without any ordering
-  def textSearch_profiles(srchString, order = nil)
-    sresult = DocList.new
+  # Search profiles for matching text with optional order_by=recent param and
+  # sorted direction DESC by default
+  # then the list is packaged up as a DocResult
+
+  # If no sort_by value present, then it returns a list of profiles ordered by id DESC
+  # a recent activity may be a node creation or a node revision
+  def textSearch_profiles(search_criteria)
+    user_scope = SrchScope.find_users(search_criteria.query, limit = nil)
+                          .reorder('')
+
+    user_scope =
+      if search_criteria.order_by == "recent"
+        user_scope.left_outer_joins(:revisions)
+                  .order("node_revisions.timestamp #{search_criteria.sort_direction}")
+                  .distinct # do we need unique/distinct on user id ?
 
-    users =
-      if order == "recentdesc"
-        getRecentProfiles(srchString)
       else
-        SrchScope.find_users(srchString, limit = nil)
+        user_scope.order(id: :desc) # TODO: order by what when order_by is not present? id? username?
       end
 
-    # User profiles
+    users = user_scope.limit(10) # TODO: verify limit?
+
+    sresult = DocList.new
     users.each do |match|
       doc = DocResult.fromSearch(0, 'user', '/profile/' + match.name, match.name, '', 0)
       sresult.addDoc(doc)
@@ -175,11 +184,21 @@ def tagNearbyNodes(srchString, tagName)
 
   # GET X number of latest people/contributors and package up as a DocResult
   # X = srchString
-  def recentPeople(srchString, tagName = nil)
+  def recentPeople(_srchString, tagName = nil)
     sresult = DocList.new
-    users = getRecentProfilesTag(tagName)
-    count = 0
 
+    nodes = Node.all.order("changed DESC").limit(100).distinct
+    users = []
+    nodes.each do |node|
+      if node.author.status != 0
+        if tagName.blank?
+          users << node.author.user
+        else
+          users << node.author.user if node.author.user.has_tag(tagName)
+        end
+      end
+    end
+    users = users.uniq
     users.each do |user|
       next unless user.has_power_tag("lat") && user.has_power_tag("lon")
       blurred = false
@@ -188,39 +207,8 @@ def recentPeople(srchString, tagName = nil)
       end
       doc = DocResult.fromLocationSearch(user.id, 'people_coordinates', user.path, user.username, 0, 0, user.lat, user.lon, blurred)
       sresult.addDoc(doc)
-      count += 1
-      if count == srchString.to_i then break end
     end
 
     sresult
   end
-
-  # Get users with matching name ordering by most recent activity
-  def getRecentProfiles(srchString)
-    sresult = DocList.new
-
-    nodes = Node.all.order("changed DESC").limit(100).distinct
-    users = []
-    nodes.each do |node|
-      next unless (node.author.status != 0) && (node.author.name.downcase.include? srchString.downcase)
-      users << node.author.user
-    end
-
-    users = users.uniq
-  end
-
-  # If a tagName is provided,
-  # method returns users with the specific tag ordering by most recent activity.
-  # Otherwise, it returns users with most recent activity.
-  def getRecentProfilesTag(tagName = nil)
-    nodes = Node.all.order("changed DESC").limit(100).distinct
-    users = []
-
-    nodes.each do |node|
-      next unless node.author.status != 0 && ((tagName && node.author.user.has_tag(tagName)) || tagName.nil?)
-      users << node.author.user
-    end
-
-    users = users.uniq
-  end
 end

app/services/srch_scope.rb

@@ -3,12 +3,12 @@ class SrchScope
   def self.find_users(input, limit)
     if ActiveRecord::Base.connection.adapter_name == 'Mysql2'
       User.search(input)
-          .order('id DESC')
-          .where(status: 1)
+          .order('id DESC') # do we need this line?
+          .where('rusers.status = ?', 1)
           .limit(limit)
     else
       User.order('id DESC')
-          .where('username LIKE ? AND status = 1', '%' + input + '%')
+          .where('username LIKE ? AND rusers.status = 1', '%' + input + '%')
           .limit(limit)
     end
   end

test/fixtures/drupal_users.yml

@@ -95,3 +95,10 @@ jeffrey:
   mail: jeff@publiclab.org
   uid: 14
   created: <%= Time.now.to_i %>
+
+jeff3:
+  name: jeff3
+  status: 1
+  mail: jeff3@pxlshp.com
+  uid: 15
+  created: <%= Time.now.to_i %>

test/fixtures/nodes.yml

@@ -258,3 +258,27 @@ welcome_feature:
   status: 1
   type: "feature"
   cached_likes: 0
+
+post_test:
+  nid: 23
+  uid: 15
+  title: "Post Test"
+  path: "/notes/jeff3/<%= (Time.now - 2.week).strftime("%m-%d-%Y") %>/post-test"
+  created: <%= (Time.now - 2.week).to_i %>
+  changed: <%= (Time.now - 2.week).to_i %>
+  status: 1
+  type: "note"
+  cached_likes: 0
+  slug: jeff3-<%= (Time.now - 2.week).strftime("%m-%d-%Y") %>-post-test
+
+post_test2:
+  nid: 24
+  uid: 14
+  title: "Post Test 2"
+  path: "/notes/jeffrey/<%= (Time.now - 3.week).strftime("%m-%d-%Y") %>/post-test2"
+  created: <%= (Time.now - 3.week).to_i %>
+  changed: <%= (Time.now - 3.week).to_i %>
+  status: 1
+  type: "note"
+  cached_likes: 0
+  slug: jeffrey-<%= (Time.now - 3.week).strftime("%m-%d-%Y") %>-post-test2

test/fixtures/revisions.yml

@@ -269,11 +269,25 @@ checkbox_two:
   body: |+
     # Heading 1
     List of things
-    * [x] 
+    * [x]
 
 email_body:
   nid: 22
   uid: 1
   title: "welcome-email-body"
   body: Welcome to Public Lab
   timestamp: <%= (Time.now - 1.week).to_i %>
+
+post_test:
+  nid: 23
+  uid: 15
+  title: Post test review
+  body: Post test review
+  timestamp: <%= (Time.now - 2.week).to_i %>
+
+post_test2:
+  nid: 24
+  uid: 14
+  title: Post test review 2
+  body: Post test review 2
+  timestamp: <%= (Time.now - 3.week).to_i %>

test/fixtures/users.yml

@@ -171,3 +171,16 @@ jeffrey:
   bio: 'Here is something really interesting about Jeff.'
   created_at: <%= Time.now %>
   updated_at: <%= Time.now %>
+
+jeff3:
+  username: jeff3
+  status: 1
+  email: jeff3@pxlshp.com
+  id: 15
+  password_salt: <%= salt = Authlogic::Random.hex_token %>
+  crypted_password: <%= Authlogic::CryptoProviders::Sha512.encrypt("secretive" + salt) %>
+  persistence_token: <%= Authlogic::Random.hex_token %>
+  last_request_at: <%= Time.now %>
+  bio: 'Here is something really interesting about Jeff3.'
+  created_at: <%= Time.now %>
+  updated_at: <%= Time.now %>

test/functional/notes_controller_test.rb

@@ -313,7 +313,7 @@ def teardown
 
     assert_response :success
     selector = css_select 'div.note'
-    assert_equal selector.size, 17
+    assert_equal selector.size, 19
     assert_select "div p", 'Pending approval by community moderators. Please be patient!'
   end
 
@@ -342,7 +342,7 @@ def teardown
 
     assert_response :success
     selector = css_select 'div.note'
-    assert_equal selector.size, 17
+    assert_equal selector.size, 19
     assert_select "p", "Moderate first-time post: \n              Approve\n              Spam"
   end
 
@@ -554,7 +554,7 @@ def teardown
     assert_redirected_to note.path(:question) + '?_=' + Time.now.to_i.to_s
   end
 
-  
+
   test 'should render a text/plain when the note is edited through xhr' do
     user = UserSession.create(users(:jeff))
     note = nodes(:one)
@@ -648,7 +648,7 @@ def teardown
       old_controller = @controller
       @controller = SettingsController.new
 
-      get :change_locale, params: { locale: lang.to_s } 
+      get :change_locale, params: { locale: lang.to_s }
 
       @controller = old_controller