API: extend /search/user with a filter option

See API docs for more details

app/controllers/api/v1/search_controller.rb

@@ -3,12 +3,20 @@
 module Api
   module V1
     class SearchController < Api::V1::BaseController
+      USER_FILTER_CONTACTS = "contacts"
+      USER_FILTER_RECEIVING_CONTACTS = "contacts:receiving"
+      USER_FILTER_SHARING_CONTACTS = "contacts:sharing"
+      USER_FILTER_ASPECTS_PREFIX = "aspect:"
+      USER_FILTERS_EXACT_MATCH = [USER_FILTER_CONTACTS, USER_FILTER_RECEIVING_CONTACTS,
+                                  USER_FILTER_SHARING_CONTACTS].freeze
+      USER_FILTERS_PREFIX_MATCH = [USER_FILTER_ASPECTS_PREFIX].freeze
+
       before_action do
         require_access_token %w[public:read]
       end
 
-      rescue_from ActionController::ParameterMissing, RuntimeError do
+      rescue_from ActionController::ParameterMissing, RuntimeError do |e|
-        render_error 422, "Search request could not be processed"
+        render_error 422, e.message
       end
 
       def user_index
@@ -36,20 +44,46 @@ module Api
       end
 
       def people_query
-        parameters = params.permit(:tag, :name_or_handle)
+        tag = params[:tag]
-        raise RuntimeError if parameters.keys.length != 1
+        name_or_handle = params[:name_or_handle]
+        raise "Parameters tag and name_or_handle are exclusive" if tag.present? && name_or_handle.present?
 
-        if params.has_key?(:tag)
+        query = if tag.present?
-          Person.profile_tagged_with(params[:tag])
+                  # scope filters to only searchable people already
-        else
+                  Person.profile_tagged_with(tag)
-          connected_only = !private_read?
+                elsif name_or_handle.present?
-          Person.search(
+                  Person.searchable(contacts_read? && current_user) # rubocop:disable Rails/DynamicFindBy
-            params[:name_or_handle],
+                        .find_by_substring(name_or_handle)
-            current_user,
+                else
-            only_contacts: connected_only,
+                  raise "Missing parameter tag or name_or_handle"
-            mutual:        connected_only
+                end
-          )
+
+        query = query.where(closed_account: false)
+
+        user_filters.each do |filter|
+          query = query.contacts_of(current_user) if filter == USER_FILTER_CONTACTS
+
+          if filter == USER_FILTER_RECEIVING_CONTACTS
+            query = query.contacts_of(current_user).where(contacts: {receiving: true})
+          end
+
+          if filter == USER_FILTER_SHARING_CONTACTS
+            query = query.contacts_of(current_user).where(contacts: {sharing: true})
+          end
+
+          if filter.start_with?(USER_FILTER_ASPECTS_PREFIX) # rubocop:disable Style/Next
+            _, ids = filter.split(":", 2)
+            ids = ids.split(",").map {|id|
+              Integer(id) rescue raise("Invalid aspect filter") # rubocop:disable Style/RescueModifier
+            }
+
+            raise "Invalid aspect filter" unless current_user.aspects.where(id: ids).count == ids.size
+
+            query = Person.where(id: query.all_from_aspects(ids, current_user).select(:id))
+          end
         end
+
+        query.distinct
       end
 
       def posts_query
@@ -63,6 +97,22 @@ module Api
       def tags_query
         ActsAsTaggableOn::Tag.autocomplete(params.require(:query))
       end
+
+      def user_filters
+        @user_filters ||= Array(params[:filter]).uniq.tap do |filters|
+          raise "Invalid filter" unless filters.all? {|filter|
+            USER_FILTERS_EXACT_MATCH.include?(filter) ||
+            USER_FILTERS_PREFIX_MATCH.any? {|prefix| filter.start_with?(prefix) }
+          }
+
+          # For now all filters require contacts:read
+          require_access_token %w[contacts:read] unless filters.empty?
+        end
+      end
+
+      def contacts_read?
+        access_token? %w[contacts:read]
+      end
     end
   end
 end

app/models/person.rb

@@ -66,8 +66,15 @@ class Person < ApplicationRecord
   validates :diaspora_handle, :uniqueness => true
 
   scope :searchable, -> (user) {
-    joins(:profile).where("profiles.searchable = true OR contacts.user_id = ?", user.id)
+    if user
+      joins("LEFT OUTER JOIN contacts ON contacts.user_id = #{user.id} AND contacts.person_id = people.id")
+        .joins(:profile)
+        .where("profiles.searchable = true OR contacts.user_id = ?", user.id)
+    else
+      joins(:profile).where(profiles: {searchable: true})
+    end
   }
+
   scope :remote, -> { where('people.owner_id IS NULL') }
   scope :local, -> { where('people.owner_id IS NOT NULL') }
   scope :for_json, -> { select("people.id, people.guid, people.diaspora_handle").includes(:profile) }
@@ -89,6 +96,15 @@ class Person < ApplicationRecord
       .where(aspect_memberships: {aspect_id: aspect_ids}).distinct
   }
 
+  scope :in_all_aspects, ->(aspect_ids) {
+    joins(contacts: :aspect_memberships)
+      .where(aspect_memberships: {aspect_id: aspect_ids})
+  }
+
+  scope :contacts_of, ->(user) {
+    joins(:contacts).where(contacts: {user_id: user.id})
+  }
+
   scope :profile_tagged_with, ->(tag_name) {
     joins(:profile => :tags)
       .where(:tags => {:name => tag_name})
@@ -233,11 +249,9 @@ class Person < ApplicationRecord
     return query if query.is_a?(ActiveRecord::NullRelation)
 
     query = if only_contacts
-              query.joins(:contacts).where(contacts: {user_id: user.id})
+              query.contacts_of(user)
             else
-              query.joins(
+              query.searchable(user)
-                "LEFT OUTER JOIN contacts ON contacts.user_id = #{user.id} AND contacts.person_id = people.id"
-              ).searchable(user)
             end
 
     query = query.where(contacts: {sharing: true, receiving: true}) if mutual

spec/integration/api/search_controller_spec.rb

@@ -6,7 +6,7 @@ describe Api::V1::SearchController do
   let(:auth) {
     FactoryGirl.create(
       :auth_with_default_scopes,
-      scopes: %w[openid public:read public:modify private:read private:modify]
+      scopes: %w[openid public:read public:modify private:read contacts:read private:modify]
     )
   }
 
@@ -55,13 +55,19 @@ describe Api::V1::SearchController do
     end
 
     it "succeeds by tag" do
+      tag = SecureRandom.hex(5)
+      5.times do
+        FactoryGirl.create(:person, profile: FactoryGirl.build(:profile, tag_string: "##{tag}"))
+      end
+      FactoryGirl.create(:person, closed_account: true, profile: FactoryGirl.build(:profile, tag_string: "##{tag}"))
+
       get(
         "/api/v1/search/users",
-        params: {tag: "one", access_token: access_token}
+        params: {tag: tag, access_token: access_token}
       )
       expect(response.status).to eq(200)
       users = response_body_data(response)
-      expect(users.length).to eq(15)
+      expect(users.length).to eq(5)
 
       expect(users.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/users")
     end
@@ -90,6 +96,242 @@ describe Api::V1::SearchController do
       expect(users.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/users")
     end
 
+    context "with a contacts filter" do
+      let(:name) { "A contact" }
+
+      it "only returns contacts" do
+        add_contact(true, false)
+        add_contact(false, true)
+        add_contact(true, true)
+
+        get(
+          "/api/v1/search/users",
+          params: {
+            name_or_handle: name,
+            filter:         "contacts",
+            access_token:   access_token
+          }
+        )
+
+        expect(response.status).to eq(200)
+        users = response_body_data(response)
+        expect(users.length).to eq(3)
+      end
+
+      it "only returns receiving contacts" do
+        add_contact(true, false)
+        add_contact(true, false)
+        add_contact(false, true)
+
+        get(
+          "/api/v1/search/users",
+          params: {
+            name_or_handle: name,
+            filter:         "contacts:receiving",
+            access_token:   access_token
+          }
+        )
+
+        expect(response.status).to eq(200)
+        users = response_body_data(response)
+        expect(users.length).to eq(2)
+      end
+
+      it "only returns sharing contacts" do
+        add_contact(true, false)
+        add_contact(false, true)
+        add_contact(false, true)
+
+        get(
+          "/api/v1/search/users",
+          params: {
+            name_or_handle: name,
+            filter:         "contacts:sharing",
+            access_token:   access_token
+          }
+        )
+
+        expect(response.status).to eq(200)
+        users = response_body_data(response)
+        expect(users.length).to eq(2)
+      end
+
+      it "only returns mutually sharing contacts" do
+        add_contact(true, false)
+        add_contact(false, true)
+        add_contact(true, true)
+
+        get(
+          "/api/v1/search/users",
+          params: {
+            name_or_handle: name,
+            filter:         ["contacts:receiving", "contacts:sharing"],
+            access_token:   access_token
+          }
+        )
+
+        expect(response.status).to eq(200)
+        users = response_body_data(response)
+        expect(users.length).to eq(1)
+      end
+
+      it "fails with an invalid filter" do
+        get(
+          "/api/v1/search/users",
+          params: {
+            name_or_handle: name,
+            filter:         "contacts:thingsiwant",
+            access_token:   access_token
+          }
+        )
+
+        confirm_api_error(response, 422, "Invalid filter")
+      end
+
+      it "fails without contacts:read scope" do
+        get(
+          "/api/v1/search/users",
+          params: {
+            name_or_handle: name,
+            filter:         "contacts",
+            access_token:   access_token_read_only
+          }
+        )
+
+        confirm_api_error(response, 403, "insufficient_scope")
+      end
+
+      def add_contact(receiving, sharing)
+        other = FactoryGirl.create(:user)
+        other.profile.update(first_name: name)
+
+        if receiving
+          aspect = auth.user.aspects.find_or_create_by(name: "Test")
+          auth.user.share_with(other.person, aspect)
+        end
+
+        if sharing # rubocop:disable Style/GuardClause
+          aspect = other.aspects.create(name: "Test")
+          other.share_with(auth.user.person, aspect)
+        end
+      end
+    end
+
+    context "with an aspects filter" do
+      let(:contact_name) { "My aspect contact" }
+      let(:aspect) { auth.user.aspects.create(name: "Test") }
+      let(:second_aspect) { auth.user.aspects.create(name: "Second test") }
+
+      it "only returns members of given aspects" do
+        add_contact(aspect)
+        add_contact(second_aspect)
+
+        get(
+          "/api/v1/search/users",
+          params: {
+            name_or_handle: contact_name,
+            filter:         "aspect:#{aspect.id}",
+            access_token:   access_token
+          }
+        )
+
+        expect(response.status).to eq(200)
+        users = response_body_data(response)
+        expect(users.length).to eq(1)
+
+        get(
+          "/api/v1/search/users",
+          params: {
+            name_or_handle: contact_name,
+            filter:         "aspect:#{aspect.id},#{second_aspect.id}",
+            access_token:   access_token
+          }
+        )
+
+        expect(response.status).to eq(200)
+        users = response_body_data(response)
+        expect(users.length).to eq(2)
+      end
+
+      it "only returns people matching all aspect filters" do
+        add_contact(aspect)
+        add_contact(second_aspect)
+        add_contact(aspect, second_aspect)
+
+        get(
+          "/api/v1/search/users",
+          params: {
+            name_or_handle: contact_name,
+            filter:         ["aspect:#{aspect.id}", "aspect:#{second_aspect.id}"],
+            access_token:   access_token
+          }
+        )
+
+        expect(response.status).to eq(200)
+        users = response_body_data(response)
+        expect(users.length).to eq(1)
+      end
+
+      it "fails with an invalid aspect" do
+        get(
+          "/api/v1/search/users",
+          params: {
+            name_or_handle: contact_name,
+            filter:         "aspect:0",
+            access_token:   access_token
+          }
+        )
+
+        confirm_api_error(response, 422, "Invalid aspect filter")
+
+        get(
+          "/api/v1/search/users",
+          params: {
+            name_or_handle: contact_name,
+            filter:         "aspect:#{aspect.id},0",
+            access_token:   access_token
+          }
+        )
+
+        confirm_api_error(response, 422, "Invalid aspect filter")
+      end
+
+      it "fails without contacts:read scope" do
+        aspect = auth_read_only.user.aspects.create(name: "Test")
+
+        get(
+          "/api/v1/search/users",
+          params: {
+            name_or_handle: contact_name,
+            filter:         "aspect:#{aspect.id}",
+            access_token:   access_token_read_only
+          }
+        )
+
+        confirm_api_error(response, 403, "insufficient_scope")
+      end
+
+      def add_contact(*aspects)
+        other = FactoryGirl.create(:person, profile: FactoryGirl.build(:profile, first_name: contact_name))
+        aspects.each do |aspect|
+          auth.user.share_with(other, aspect)
+        end
+      end
+    end
+
+    it "fails with an invalid filter" do
+      get(
+        "/api/v1/search/users",
+        params: {
+          name_or_handle: "findable",
+          filter:         "thingsiwant",
+          access_token:   access_token
+        }
+      )
+
+      confirm_api_error(response, 422, "Invalid filter")
+    end
+
     it "doesn't return closed accounts" do
       get(
         "/api/v1/search/users",
@@ -128,7 +370,7 @@ describe Api::V1::SearchController do
         "/api/v1/search/users",
         params: {tag: "tag1", name_or_handle: "name", access_token: access_token}
       )
-      confirm_api_error(response, 422, "Search request could not be processed")
+      confirm_api_error(response, 422, "Parameters tag and name_or_handle are exclusive")
     end
 
     it "fails with no fields" do
@@ -136,7 +378,7 @@ describe Api::V1::SearchController do
         "/api/v1/search/users",
         params: {access_token: access_token}
       )
-      confirm_api_error(response, 422, "Search request could not be processed")
+      confirm_api_error(response, 422, "Missing parameter tag or name_or_handle")
     end
 
     it "fails with bad credentials" do
@@ -208,7 +450,7 @@ describe Api::V1::SearchController do
         "/api/v1/search/posts",
         params: {access_token: access_token}
       )
-      confirm_api_error(response, 422, "Search request could not be processed")
+      confirm_api_error(response, 422, "param is missing or the value is empty: tag")
     end
 
     it "fails with bad credentials" do
@@ -256,7 +498,7 @@ describe Api::V1::SearchController do
         "/api/v1/search/tags",
         params: {access_token: access_token}
       )
-      confirm_api_error(response, 422, "Search request could not be processed")
+      confirm_api_error(response, 422, "param is missing or the value is empty: query")
     end
 
     it "fails with bad credentials" do