Merge pull request #1334 from ElixirTeSS/fix-person-autocomplete

Also query profiles in people autocomplete

Author: fbacall

app/controllers/autocomplete_controller.rb

@@ -7,15 +7,31 @@ def suggestions
   end
 
   def people_suggestions
-    people = Person.query(params[:query])
-    suggestions = people.map do |p|
-      { value: p.name,
-        data: {
-          orcid: p.orcid,
-          profile_id: p.profile_id
+    people = Person.query(params[:query], 20)
+    profiles = Profile.query(params[:query], 20)
+    unique_map = {}
+    people.each do |p|
+      unique_map[p.profile_id || p.orcid || p.name] =
+        { value: p.name,
+          data: {
+            orcid: p.orcid,
+            profile_id: p.profile_id
+          }
         }
-      }
     end
+    profiles.each do |p|
+      orcid = p.orcid_authenticated? ? p.orcid : nil
+      unique_map.delete(orcid) if orcid
+      unique_map[p.id] =
+        { value: p.full_name,
+          data: {
+            orcid: orcid,
+            profile_id: p.id
+          }
+        }
+    end
+
+    suggestions = unique_map.values.sort_by { |s| [s[:value].downcase, s[:data][:orcid] || 'z'] }
     respond_with({ suggestions: suggestions })
   end
 end

app/models/profile.rb

@@ -62,6 +62,23 @@ def authenticate_orcid(orcid)
     out
   end
 
+  def self.visible
+    joins(:user).merge(User.visible)
+  end
+
+  # For autocomplete
+  def self.starting_with(query)
+    where('lower(firstname) LIKE ?', "#{query.downcase}%").or(where('lower(surname) LIKE ?', "#{query.downcase}%"))
+  end
+
+  def self.query(query, limit = nil)
+    q = visible.where.not(orcid: nil).where(orcid_authenticated: true)
+               .select(:id, :firstname, :surname, :orcid, :orcid_authenticated)
+               .starting_with(query).distinct
+    q = q.limit(limit) if limit
+    q.order(firstname: :asc, surname: :asc, orcid: :asc)
+  end
+
   private
 
   def check_public

test/controllers/autocomplete_controller_test.rb

@@ -51,14 +51,14 @@ class AutocompleteControllerTest < ActionController::TestCase
     res = JSON.parse(response.body)
     suggestions = res['suggestions']
     assert_equal 3, suggestions.length, "Should be 3 - 2 with ORCIDs and 1 without. Should not include duplicates."
-    assert_equal ['0000-0002-1694-233X', '0000-0002-1825-0097', nil], suggestions.map { |s| s['data']['orcid'] }
-    assert_equal ['John Doe', 'John Doe', 'John Doe'], suggestions.map { |s| s['value'] }
+    assert_equal ['0000-0002-1694-233X', nil, '0000-0002-1825-0097'], suggestions.map { |s| s['data']['orcid'] }
+    assert_equal ['John Doe', 'John Doe', 'Josiah Carberry'], suggestions.map { |s| s['value'] }, "Should use Josiah Carberry's name from his profile rather than John Doe from the author"
 
     get :people_suggestions, params: { query: 'j' }, format: :json
     assert_response :success
     res = JSON.parse(response.body)
     suggestions = res['suggestions']
-    assert_equal ['jane Doe', 'John Doe', 'John Doe', 'John Doe'], suggestions.map { |s| s['value'] }
+    assert_equal ['jane Doe', 'John Doe', 'John Doe', 'Josiah Carberry'], suggestions.map { |s| s['value'] }
 
     get :people_suggestions, params: { query: 'FRED' }, format: :json
     assert_response :success

test/models/profile_test.rb

@@ -154,4 +154,51 @@ class ProfileTest < ActiveSupport::TestCase
       refute Profile.new.authenticate_orcid('0009-0006-0987-5702')
     end
   end
+
+  test 'visible' do
+    visible = Profile.visible
+    assert_includes visible, profiles(:one)
+    assert_includes visible, profiles(:two)
+    refute_includes visible, profiles(:basic_user_profile)
+    refute_includes visible, profiles(:banned_user_profile)
+  end
+
+  test 'starting_with' do
+    regi = Profile.starting_with('regi')
+    assert_includes regi, profiles(:one)
+    refute_includes regi, profiles(:two)
+
+    user = Profile.starting_with('user')
+    assert_includes user, profiles(:one)
+    refute_includes user, profiles(:two)
+
+    bla = Profile.starting_with('bla')
+    refute_includes bla, profiles(:one)
+    refute_includes bla, profiles(:two)
+
+    ad = Profile.starting_with('ad')
+    refute_includes ad, profiles(:two)
+    assert_includes ad, profiles(:three)
+    assert_includes ad, profiles(:admin_trainer_profile)
+  end
+
+  test 'query' do
+    josi = Profile.query('josi')
+    assert_equal 1, josi.length
+    assert_includes josi, profiles(:trainer_one_profile).becomes(Profile) # Need to cast back to Profile because it is a Trainer otherwise
+
+    # Excludes unauthenticated orcids
+    unauth = profiles(:trainer_two_profile)
+    assert_includes Profile.starting_with('luci'), unauth
+    unauth_q = Profile.query('luci')
+    assert_equal 0, unauth_q.length
+
+    # Excludes non-visible:
+    banned = profiles(:banned_user_profile)
+    banned.update!(firstname: 'Banned')
+    assert banned.authenticate_orcid('0009-0006-0987-5702')
+    assert_includes Profile.starting_with('banned'), banned
+    banned_q = Profile.query('Banned')
+    assert_equal 0, banned_q.length
+  end
 end