Merge branch '118797-epics-by-iid-graphql-pd' into 'master'

Add possibility to search for partial iids on EpicResolver

See merge request gitlab-org/gitlab!24673

db/migrate/20200225111018_add_index_for_group_and_iid_search_to_epics.rb

+# frozen_string_literal: true
+
+class AddIndexForGroupAndIidSearchToEpics < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'index_epics_on_group_id_and_iid_varchar_pattern'
+
+  disable_ddl_transaction!
+
+  def up
+    disable_statement_timeout do
+      execute "CREATE INDEX CONCURRENTLY \"#{INDEX_NAME}\" ON epics (group_id, CAST(iid AS VARCHAR) varchar_pattern_ops);"
+    end
+  end
+
+  def down
+    disable_statement_timeout do
+      remove_concurrent_index_by_name :epics, INDEX_NAME
+    end
+  end
+end

db/schema.rb

@@ -1581,6 +1581,7 @@ ActiveRecord::Schema.define(version: 2020_02_27_165129) do
     t.integer "start_date_sourcing_epic_id"
     t.integer "due_date_sourcing_epic_id"
     t.integer "health_status", limit: 2
+    t.index "group_id, ((iid)::character varying) varchar_pattern_ops", name: "index_epics_on_group_id_and_iid_varchar_pattern"
     t.index ["assignee_id"], name: "index_epics_on_assignee_id"
     t.index ["author_id"], name: "index_epics_on_author_id"
     t.index ["closed_by_id"], name: "index_epics_on_closed_by_id"

doc/api/graphql/reference/gitlab_schema.graphql

@@ -1890,6 +1890,11 @@ type Epic implements Noteable {
     """
     iid: ID
 
+    """
+    Filter epics by iid for autocomplete
+    """
+    iidStartsWith: String
+
     """
     List of IIDs of epics, e.g., [1, 2]
     """
@@ -2941,6 +2946,11 @@ type Group {
     """
     iid: ID
 
+    """
+    Filter epics by iid for autocomplete
+    """
+    iidStartsWith: String
+
     """
     List of IIDs of epics, e.g., [1, 2]
     """
@@ -3008,6 +3018,11 @@ type Group {
     """
     iid: ID
 
+    """
+    Filter epics by iid for autocomplete
+    """
+    iidStartsWith: String
+
     """
     List of IIDs of epics, e.g., [1, 2]
     """

doc/api/graphql/reference/gitlab_schema.json

@@ -3513,6 +3513,16 @@
                     }
                   },
                   "defaultValue": null
+                },
+                {
+                  "name": "iidStartsWith",
+                  "description": "Filter epics by iid for autocomplete",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "String",
+                    "ofType": null
+                  },
+                  "defaultValue": null
                 }
               ],
               "type": {
@@ -3633,6 +3643,16 @@
                   },
                   "defaultValue": null
                 },
+                {
+                  "name": "iidStartsWith",
+                  "description": "Filter epics by iid for autocomplete",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "String",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
                 {
                   "name": "after",
                   "description": "Returns the elements in the list that come after the specified cursor.",
@@ -4866,6 +4886,16 @@
                   },
                   "defaultValue": null
                 },
+                {
+                  "name": "iidStartsWith",
+                  "description": "Filter epics by iid for autocomplete",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "String",
+                    "ofType": null
+                  },
+                  "defaultValue": null
+                },
                 {
                   "name": "after",
                   "description": "Returns the elements in the list that come after the specified cursor.",

ee/app/finders/epics_finder.rb

@@ -18,10 +18,13 @@
 #   updated_before: datetime
 #   include_ancestor_groups: boolean
 #   include_descendant_groups: boolean
+#   starts_with_iid: string (containing a number)
 
 class EpicsFinder < IssuableFinder
   include TimeFrameFilter
 
+  IID_STARTS_WITH_PATTERN = %r{\A(\d)+\z}.freeze
+
   def self.scalar_params
     @scalar_params ||= %i[
       parent_id
@@ -38,6 +41,10 @@ class EpicsFinder < IssuableFinder
     @array_params ||= { label_name: [] }
   end
 
+  def self.valid_iid_query?(query)
+    query.match?(IID_STARTS_WITH_PATTERN)
+  end
+
   def klass
     Epic
   end
@@ -55,6 +62,7 @@ class EpicsFinder < IssuableFinder
     items = by_label(items)
     items = by_parent(items)
     items = by_iids(items)
+    items = starts_with_iid(items)
 
     sort(items)
   end
@@ -91,6 +99,15 @@ class EpicsFinder < IssuableFinder
 
   private
 
+  def starts_with_iid(items)
+    return items unless params[:iid_starts_with].present?
+
+    query = params[:iid_starts_with]
+    raise ArgumentError unless self.class.valid_iid_query?(query)
+
+    items.iid_starts_with(query)
+  end
+
   def related_groups
     include_ancestors = params.fetch(:include_ancestor_groups, false)
     include_descendants = params.fetch(:include_descendant_groups, true)

ee/app/graphql/resolvers/epic_resolver.rb

@@ -32,6 +32,10 @@ module Resolvers
              required: false,
              description: 'Filter epics by labels'
 
+    argument :iid_starts_with, GraphQL::STRING_TYPE,
+             required: false,
+             description: 'Filter epics by iid for autocomplete'
+
     type Types::EpicType, null: true
 
     def resolve(**args)
@@ -41,6 +45,7 @@ module Resolvers
       return [] unless epic_feature_enabled?
 
       validate_timeframe_params!(args)
+      validate_iid_starts_with_query!(args)
 
       find_epics(transform_args(args))
     end
@@ -58,10 +63,10 @@ module Resolvers
     end
 
     def transform_args(args)
-      transformed             = args.dup
-      transformed[:group_id]  = group.id
-      transformed[:parent_id] = parent.id if parent
-      transformed[:iids]    ||= [args[:iid]].compact
+      transformed               = args.dup
+      transformed[:group_id]    = group.id
+      transformed[:parent_id]   = parent.id if parent
+      transformed[:iids]      ||= [args[:iid]].compact
 
       transformed
     end
@@ -81,6 +86,16 @@ module Resolvers
       parent.group
     end
 
+    def validate_iid_starts_with_query!(args)
+      return unless args[:iid_starts_with].present?
+
+      iid_starts_with_query = args[:iid_starts_with]
+
+      unless EpicsFinder.valid_iid_query?(iid_starts_with_query)
+        raise Gitlab::Graphql::Errors::ArgumentError, "#{iid_starts_with_query} is not a valid iid search term"
+      end
+    end
+
     # If we're querying for multiple iids and selecting issues, then ideally
     # we want to batch the epic and issue queries into one to reduce N+1 and memory.
     # https://gitlab.com/gitlab-org/gitlab/issues/11841

ee/app/models/ee/epic.rb

@@ -66,6 +66,7 @@ module EE
       scope :in_milestone, -> (milestone_id) { joins(:issues).where(issues: { milestone_id: milestone_id }) }
       scope :in_issues, -> (issues) { joins(:epic_issues).where(epic_issues: { issue_id: issues }).distinct }
       scope :has_parent, -> { where.not(parent_id: nil) }
+      scope :iid_starts_with, -> (query) { where("CAST(iid AS VARCHAR) LIKE ?", "#{sanitize_sql_like(query)}%") }
 
       scope :within_timeframe, -> (start_date, end_date) do
         where('start_date is not NULL or end_date is not NULL')

ee/changelogs/unreleased/118797-epics-by-iid-graphql-pd.yml

+---
+title: Add possibility to search for epics by partial iids using GraphQL
+merge_request: 24673
+author:
+type: added

ee/spec/finders/epics_finder_spec.rb

@@ -235,10 +235,65 @@ describe EpicsFinder do
             expect(epics(params)).to contain_exactly(epic1)
           end
         end
+
+        context 'when using iid starts with query' do
+          let!(:epic1) { create(:epic, :opened, group: group, iid: '11') }
+          let!(:epic2) { create(:epic, :opened, group: group, iid: '1112') }
+          let!(:epic3) { create(:epic, :closed, group: group, iid: '9978') }
+          let!(:epic4) { create(:epic, :closed, group: another_group, iid: '111') }
+
+          it 'returns the expected epics if just the first two numbers are given' do
+            params = { iid_starts_with: '11' }
+
+            expect(epics(params)).to contain_exactly(epic1, epic2)
+          end
+
+          it 'returns the expected epics if the exact id is given' do
+            params = { iid_starts_with: '1112' }
+
+            expect(epics(params)).to contain_exactly(epic2)
+          end
+
+          it 'is empty if the last number is given' do
+            params = { iid_starts_with: '8' }
+
+            expect(epics(params)).to be_empty
+          end
+
+          it 'fails if iid_starts_with contains a non-numeric string' do
+            expect { epics({ iid_starts_with: 'foo' }) }.to raise_error(ArgumentError)
+          end
+
+          it 'fails if iid_starts_with contains a non-numeric string with line breaks' do
+            expect { epics({ iid_starts_with: "foo\n1" }) }.to raise_error(ArgumentError)
+          end
+
+          it 'fails if iid_starts_with contains a string which contains a negative number' do
+            expect { epics(iid_starts_with: '-1') }.to raise_error(ArgumentError)
+          end
+        end
       end
     end
   end
 
+  describe '.valid_iid_query?' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:query, :expected_result) do
+      "foo" | false
+      "-1" | false
+      "1\nfoo" | false
+      "foo\n1" | false
+      "1" | true
+    end
+
+    with_them do
+      subject { described_class.valid_iid_query?(query) }
+
+      it { is_expected.to eq(expected_result) }
+    end
+  end
+
   describe '#row_count' do
     let(:label) { create(:label) }
     let(:label2) { create(:label) }

ee/spec/graphql/resolvers/epic_resolver_spec.rb

@@ -194,6 +194,51 @@ describe Resolvers::EpicResolver do
           expect(resolve_epics).to contain_exactly(epic1, epic2, epic3, epic4)
         end
       end
+
+      context 'with partial iids' do
+        let!(:other_group) { create(:group, :private) }
+
+        let!(:epic3) { create(:epic, group: group, iid: '1122') }
+        let!(:epic4) { create(:epic, group: group, iid: '132') }
+        let!(:epic5) { create(:epic, group: group, iid: '62') }
+        let!(:epic6) { create(:epic, group: other_group, iid: '11999') }
+
+        it 'returns the expected epics if just the first number of iid is requested' do
+          epics = resolve_epics(iid_starts_with: '1')
+
+          expect(epics).to contain_exactly(epic3, epic4)
+        end
+
+        it 'returns the expected epics if first two numbers of iid are requested' do
+          epics = resolve_epics(iid_starts_with: '11')
+
+          expect(epics).to contain_exactly(epic3)
+        end
+
+        it 'returns the expected epics if last two numbers of iid are given' do
+          epics = resolve_epics(iid_starts_with: '32')
+
+          expect(epics).to be_empty
+        end
+
+        it 'returns the expected epics if exact number of iid is given' do
+          epics = resolve_epics(iid_starts_with: '62')
+
+          expect(epics).to contain_exactly(epic5)
+        end
+
+        it 'fails if iid_starts_with contains a non-numeric string' do
+          expect { resolve_epics(iid_starts_with: 'foo') }.to raise_error(Gitlab::Graphql::Errors::ArgumentError, 'foo is not a valid iid search term')
+        end
+
+        it 'fails if iid_starts_with contains a non-numeric string with line breaks' do
+          expect { resolve_epics(iid_starts_with: "foo\n1") }.to raise_error(Gitlab::Graphql::Errors::ArgumentError, "foo\n1 is not a valid iid search term")
+        end
+
+        it 'fails if iid_starts_with contains a string which contains a negative number' do
+          expect { resolve_epics(iid_starts_with: '-1') }.to raise_error(Gitlab::Graphql::Errors::ArgumentError, '-1 is not a valid iid search term')
+        end
+      end
     end
   end
 

spec/models/concerns/issuable_spec.rb

@@ -56,8 +56,6 @@ describe Issuable do
   end
 
   describe "Scope" do
-    subject { build(:issue) }
-
     it { expect(issuable_class).to respond_to(:opened) }
     it { expect(issuable_class).to respond_to(:closed) }
     it { expect(issuable_class).to respond_to(:assigned) }