Merge branch 'master' into '29497-pages-custom-domain-dns-verification'

app/assets/javascripts/filtered_search/dropdown_user.js

@@ -14,7 +14,6 @@ export default class DropdownUser extends FilteredSearchDropdown {
         endpoint: `${gon.relative_url_root || ''}/autocomplete/users.json`,
         searchKey: 'search',
         params: {
-          per_page: 20,
           active: true,
           group_id: this.getGroupId(),
           project_id: this.getProjectId(),

app/assets/javascripts/users_select.js

@@ -39,7 +39,6 @@ function UsersSelect(currentUser, els, options = {}) {
       options.showCurrentUser = $dropdown.data('currentUser');
       options.todoFilter = $dropdown.data('todoFilter');
       options.todoStateFilter = $dropdown.data('todoStateFilter');
-      options.perPage = $dropdown.data('perPage');
       showNullUser = $dropdown.data('nullUser');
       defaultNullUser = $dropdown.data('nullUserDefault');
       showMenuAbove = $dropdown.data('showMenuAbove');
@@ -669,7 +668,6 @@ UsersSelect.prototype.users = function(query, options, callback) {
   const url = this.buildUrl(this.usersPath);
   const params = {
     search: query,
-    per_page: options.perPage || 20,
     active: true,
     project_id: options.projectId || null,
     group_id: options.groupId || null,

app/controllers/projects/clusters/gcp_controller.rb

@@ -40,9 +40,9 @@ class Projects::Clusters::GcpController < Projects::ApplicationController
   def verify_billing
     case google_project_billing_status
     when nil
-      flash[:alert] = _('We could not verify that one of your projects on GCP has billing enabled. Please try again.')
+      flash.now[:alert] = _('We could not verify that one of your projects on GCP has billing enabled. Please try again.')
     when false
-      flash[:alert] = _('Please <a href=%{link_to_billing} target="_blank" rel="noopener noreferrer">enable billing for one of your projects to be able to create a Kubernetes cluster</a>, then try again.').html_safe % { link_to_billing: "https://console.cloud.google.com/freetrial?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral" }
+      flash.now[:alert] = _('Please <a href=%{link_to_billing} target="_blank" rel="noopener noreferrer">enable billing for one of your projects to be able to create a Kubernetes cluster</a>, then try again.').html_safe % { link_to_billing: "https://console.cloud.google.com/freetrial?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral" }
     when true
       return
     end

app/finders/autocomplete_users_finder.rb

 class AutocompleteUsersFinder
+  # The number of users to display in the results is hardcoded to 20, and
+  # pagination is not supported. This ensures that performance remains
+  # consistent and removes the need for implementing keyset pagination to ensure
+  # good performance.
+  LIMIT = 20
+
   attr_reader :current_user, :project, :group, :search, :skip_users,
-              :page, :per_page, :author_id, :params
+              :author_id, :params
 
   def initialize(params:, current_user:, project:, group:)
     @current_user = current_user
@@ -8,8 +14,6 @@ class AutocompleteUsersFinder
     @group = group
     @search = params[:search]
     @skip_users = params[:skip_users]
-    @page = params[:page]
-    @per_page = params[:per_page]
     @author_id = params[:author_id]
     @params = params
   end
@@ -20,7 +24,7 @@ class AutocompleteUsersFinder
     items = items.reorder(:name)
     items = items.search(search) if search.present?
     items = items.where.not(id: skip_users) if skip_users.present?
-    items = items.page(page).per(per_page)
+    items = items.limit(LIMIT)
 
     if params[:todo_filter].present? && current_user
       items = items.todo_authors(current_user.id, params[:todo_state_filter])
@@ -52,9 +56,13 @@ class AutocompleteUsersFinder
   end
 
   def users_from_project
-    user_ids = project.team.users.pluck(:id)
-    user_ids << author_id if author_id.present?
+    if author_id.present?
+      union = Gitlab::SQL::Union
+        .new([project.authorized_users, User.where(id: author_id)])
 
-    User.where(id: user_ids)
+      User.from("(#{union.to_sql}) #{User.table_name}")
+    else
+      project.authorized_users
+    end
   end
 end

app/models/user.rb

@@ -327,8 +327,8 @@ class User < ActiveRecord::Base
       SQL
 
       where(
-        fuzzy_arel_match(:name, query)
-          .or(fuzzy_arel_match(:username, query))
+        fuzzy_arel_match(:name, query, lower_exact_match: true)
+          .or(fuzzy_arel_match(:username, query, lower_exact_match: true))
           .or(arel_table[:email].eq(query))
       ).reorder(order % { query: ActiveRecord::Base.connection.quote(query) }, :name)
     end

app/services/projects/autocomplete_service.rb

@@ -50,16 +50,7 @@ module Projects
 
       return [] unless noteable&.is_a?(Issuable)
 
-      opts = {
-        project: project,
-        issuable: noteable,
-        current_user: current_user
-      }
-      QuickActions::InterpretService.command_definitions.map do |definition|
-        next unless definition.available?(opts)
-
-        definition.to_h(opts)
-      end.compact
+      QuickActions::InterpretService.new(project, current_user).available_commands(noteable)
     end
   end
 end

app/services/quick_actions/interpret_service.rb

@@ -7,6 +7,18 @@ module QuickActions
     SHRUG = '¯\\＿(ツ)＿/¯'.freeze
     TABLEFLIP = '(╯°□°)╯︵ ┻━┻'.freeze
 
+    # Takes an issuable and returns an array of all the available commands
+    # represented with .to_h
+    def available_commands(issuable)
+      @issuable = issuable
+
+      self.class.command_definitions.map do |definition|
+        next unless definition.available?(self)
+
+        definition.to_h(self)
+      end.compact
+    end
+
     # Takes a text and interprets the commands that are extracted from it.
     # Returns the content without commands, and hash of changes to be applied to a record.
     def execute(content, issuable)
@@ -15,8 +27,8 @@ module QuickActions
       @issuable = issuable
       @updates = {}
 
-      content, commands = extractor.extract_commands(content, context)
-      extract_updates(commands, context)
+      content, commands = extractor.extract_commands(content)
+      extract_updates(commands)
 
       [content, @updates]
     end
@@ -28,8 +40,8 @@ module QuickActions
 
       @issuable = issuable
 
-      content, commands = extractor.extract_commands(content, context)
-      commands = explain_commands(commands, context)
+      content, commands = extractor.extract_commands(content)
+      commands = explain_commands(commands)
       [content, commands]
     end
 
@@ -157,11 +169,11 @@ module QuickActions
     params '%"milestone"'
     condition do
       current_user.can?(:"admin_#{issuable.to_ability_name}", project) &&
-        project.milestones.active.any?
+        find_milestones(project, state: 'active').any?
     end
     parse_params do |milestone_param|
       extract_references(milestone_param, :milestone).first ||
-        project.milestones.find_by(title: milestone_param.strip)
+        find_milestones(project, title: milestone_param.strip).first
     end
     command :milestone do |milestone|
       @updates[:milestone_id] = milestone.id if milestone
@@ -544,6 +556,10 @@ module QuickActions
       users
     end
 
+    def find_milestones(project, params = {})
+      MilestonesFinder.new(params.merge(project_ids: [project.id], group_ids: [project.group&.id])).execute
+    end
+
     def find_labels(labels_param)
       extract_references(labels_param, :label) |
         LabelsFinder.new(current_user, project_id: project.id, name: labels_param.split).execute
@@ -557,21 +573,21 @@ module QuickActions
       find_labels(labels_param).map(&:id)
     end
 
-    def explain_commands(commands, opts)
+    def explain_commands(commands)
       commands.map do |name, arg|
         definition = self.class.definition_by_name(name)
         next unless definition
 
-        definition.explain(self, opts, arg)
+        definition.explain(self, arg)
       end.compact
     end
 
-    def extract_updates(commands, opts)
+    def extract_updates(commands)
       commands.each do |name, arg|
         definition = self.class.definition_by_name(name)
         next unless definition
 
-        definition.execute(self, opts, arg)
+        definition.execute(self, arg)
       end
     end
 
@@ -581,14 +597,5 @@ module QuickActions
 
       ext.references(type)
     end
-
-    def context
-      {
-        issuable: issuable,
-        current_user: current_user,
-        project: project,
-        params: params
-      }
-    end
   end
 end

app/workers/stuck_import_jobs_worker.rb

@@ -16,43 +16,41 @@ class StuckImportJobsWorker
   private
 
   def mark_projects_without_jid_as_failed!
-    started_projects_without_jid.each do |project|
+    enqueued_projects_without_jid.each do |project|
       project.mark_import_as_failed(error_message)
     end.count
   end
 
   def mark_projects_with_jid_as_failed!
-    completed_jids_count = 0
+    jids_and_ids = enqueued_projects_with_jid.pluck(:import_jid, :id).to_h
 
-    started_projects_with_jid.find_in_batches(batch_size: 500) do |group|
-      jids = group.map(&:import_jid)
+    # Find the jobs that aren't currently running or that exceeded the threshold.
+    completed_jids = Gitlab::SidekiqStatus.completed_jids(jids_and_ids.keys)
+    return unless completed_jids.any?
 
-      # Find the jobs that aren't currently running or that exceeded the threshold.
-      completed_jids = Gitlab::SidekiqStatus.completed_jids(jids).to_set
+    completed_project_ids = jids_and_ids.values_at(*completed_jids)
 
-      if completed_jids.any?
-        completed_jids_count += completed_jids.count
-        group.each do |project|
-          project.mark_import_as_failed(error_message) if completed_jids.include?(project.import_jid)
-        end
+    # We select the projects again, because they may have transitioned from
+    # scheduled/started to finished/failed while we were looking up their Sidekiq status.
+    completed_projects = enqueued_projects_with_jid.where(id: completed_project_ids)
 
-        Rails.logger.info("Marked stuck import jobs as failed. JIDs: #{completed_jids.to_a.join(', ')}")
-      end
-    end
+    Rails.logger.info("Marked stuck import jobs as failed. JIDs: #{completed_projects.map(&:import_jid).join(', ')}")
 
-    completed_jids_count
+    completed_projects.each do |project|
+      project.mark_import_as_failed(error_message)
+    end.count
   end
 
-  def started_projects
-    Project.with_import_status(:started)
+  def enqueued_projects
+    Project.with_import_status(:scheduled, :started)
   end
 
-  def started_projects_with_jid
-    started_projects.where.not(import_jid: nil)
+  def enqueued_projects_with_jid
+    enqueued_projects.where.not(import_jid: nil)
   end
 
-  def started_projects_without_jid
-    started_projects.where(import_jid: nil)
+  def enqueued_projects_without_jid
+    enqueued_projects.where(import_jid: nil)
   end
 
   def error_message

changelogs/unreleased/42545-milestion-quick-actions-for-groups.yml

+---
+title: Allows the usage of /milestone quick action for group milestones
+merge_request: 17239
+author: Jacopo Beschi @jacopo-beschi
+type: fixed

changelogs/unreleased/43496-error-message-for-gke-clusters-persists-in-the-next-page.yml

+---
+title: Do not persist Google Project verification flash errors after a page reload
+merge_request: 17299
+author:
+type: fixed

changelogs/unreleased/dm-stuck-import-jobs-verify.yml

+---
+title: Verify project import status again before marking as failed
+merge_request:
+author:
+type: fixed

changelogs/unreleased/users-autocomplete.yml

+---
+title: Improve performance of searching for and autocompleting of users
+merge_request:
+author:
+type: performance

db/migrate/20180215181245_users_name_lower_index.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class UsersNameLowerIndex < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+  INDEX_NAME = 'index_on_users_name_lower'
+
+  disable_ddl_transaction!
+
+  def up
+    return unless Gitlab::Database.postgresql?
+
+    # On GitLab.com this produces an index with a size of roughly 60 MB.
+    execute "CREATE INDEX CONCURRENTLY #{INDEX_NAME} ON users (LOWER(name))"
+  end
+
+  def down
+    return unless Gitlab::Database.postgresql?
+
+    if supports_drop_index_concurrently?
+      execute "DROP INDEX CONCURRENTLY IF EXISTS #{INDEX_NAME}"
+    else
+      execute "DROP INDEX IF EXISTS #{INDEX_NAME}"
+    end
+  end
+end

lib/gitlab/quick_actions/command_definition.rb

@@ -24,15 +24,14 @@ module Gitlab
         action_block.nil?
       end
 
-      def available?(opts)
+      def available?(context)
         return true unless condition_block
 
-        context = OpenStruct.new(opts)
         context.instance_exec(&condition_block)
       end
 
-      def explain(context, opts, arg)
-        return unless available?(opts)
+      def explain(context, arg)
+        return unless available?(context)
 
         if explanation.respond_to?(:call)
           execute_block(explanation, context, arg)
@@ -41,15 +40,13 @@ module Gitlab
         end
       end
 
-      def execute(context, opts, arg)
-        return if noop? || !available?(opts)
+      def execute(context, arg)
+        return if noop? || !available?(context)
 
         execute_block(action_block, context, arg)
       end
 
-      def to_h(opts)
-        context = OpenStruct.new(opts)
-
+      def to_h(context)
         desc = description
         if desc.respond_to?(:call)
           desc = context.instance_exec(&desc) rescue ''

lib/gitlab/quick_actions/dsl.rb

@@ -62,9 +62,8 @@ module Gitlab
 
         # Allows to define conditions that must be met in order for the command
         # to be returned by `.command_names` & `.command_definitions`.
-        # It accepts a block that will be evaluated with the context given to
-        # `CommandDefintion#to_h`.
-        #
+        # It accepts a block that will be evaluated with the context
+        # of a QuickActions::InterpretService instance
         # Example:
         #
         #   condition do

lib/gitlab/quick_actions/extractor.rb

@@ -29,7 +29,7 @@ module Gitlab
       # commands = extractor.extract_commands(msg) #=> [['labels', '~foo ~"bar baz"']]
       # msg #=> "hello\nworld"
       # ```
-      def extract_commands(content, opts = {})
+      def extract_commands(content)
         return [content, []] unless content
 
         content = content.dup
@@ -37,7 +37,7 @@ module Gitlab
         commands = []
 
         content.delete!("\r")
-        content.gsub!(commands_regex(opts)) do
+        content.gsub!(commands_regex) do
           if $~[:cmd]
             commands << [$~[:cmd], $~[:arg]].reject(&:blank?)
             ''
@@ -60,8 +60,8 @@ module Gitlab
       # It looks something like:
       #
       #   /^\/(?<cmd>close|reopen|...)(?:( |$))(?<arg>[^\/\n]*)(?:\n|$)/
-      def commands_regex(opts)
-        names = command_names(opts).map(&:to_s)
+      def commands_regex
+        names = command_names.map(&:to_s)
 
         @commands_regex ||= %r{
             (?<code>
@@ -133,7 +133,7 @@ module Gitlab
         [content, commands]
       end
 
-      def command_names(opts)
+      def command_names
         command_definitions.flat_map do |command|
           next if command.noop?
 

lib/gitlab/sql/pattern.rb

@@ -25,7 +25,11 @@ module Gitlab
           query.length >= MIN_CHARS_FOR_PARTIAL_MATCHING
         end
 
-        def fuzzy_arel_match(column, query)
+        # column - The column name to search in.
+        # query - The text to search for.
+        # lower_exact_match - When set to `true` we'll fall back to using
+        #                     `LOWER(column) = query` instead of using `ILIKE`.
+        def fuzzy_arel_match(column, query, lower_exact_match: false)
           query = query.squish
           return nil unless query.present?
 
@@ -36,7 +40,13 @@ module Gitlab
           else
             # No words of at least 3 chars, but we can search for an exact
             # case insensitive match with the query as a whole
-            arel_table[column].matches(sanitize_sql_like(query))
+            if lower_exact_match
+              Arel::Nodes::NamedFunction
+                .new('LOWER', [arel_table[column]])
+                .eq(query)
+            else
+              arel_table[column].matches(sanitize_sql_like(query))
+            end
           end
         end
 

lib/tasks/migrate/setup_postgresql.rake

@@ -8,6 +8,7 @@ task setup_postgresql: :environment do
   require Rails.root.join('db/migrate/20170503185032_index_redirect_routes_path_for_like')
   require Rails.root.join('db/migrate/20171220191323_add_index_on_namespaces_lower_name.rb')
   require Rails.root.join('db/migrate/20180113220114_rework_redirect_routes_indexes.rb')
+  require Rails.root.join('db/migrate/20180215181245_users_name_lower_index.rb')
 
   NamespacesProjectsPathLowerIndexes.new.up
   AddUsersLowerUsernameEmailIndexes.new.up
@@ -17,4 +18,5 @@ task setup_postgresql: :environment do
   IndexRedirectRoutesPathForLike.new.up
   AddIndexOnNamespacesLowerName.new.up
   ReworkRedirectRoutesIndexes.new.up
+  UsersNameLowerIndex.new.up
 end

spec/controllers/autocomplete_controller_spec.rb

@@ -109,15 +109,17 @@ describe AutocompleteController do
     end
 
     context 'limited users per page' do
-      let(:per_page) { 2 }
-
       before do
+        25.times do
+          create(:user)
+        end
+
         sign_in(user)
-        get(:users, per_page: per_page)
+        get(:users)
       end
 
       it { expect(json_response).to be_kind_of(Array) }
-      it { expect(json_response.size).to eq(per_page) }
+      it { expect(json_response.size).to eq(20) }
     end
 
     context 'unauthenticated user' do

spec/controllers/projects/clusters/gcp_controller_spec.rb

@@ -161,7 +161,7 @@ describe Projects::Clusters::GcpController do
           it 'renders the cluster form with an error' do
             go
 
-            expect(response).to set_flash[:alert]
+            expect(response).to set_flash.now[:alert]
             expect(response).to render_template('new')
           end
         end

spec/lib/gitlab/quick_actions/command_definition_spec.rb

@@ -40,7 +40,7 @@ describe Gitlab::QuickActions::CommandDefinition do
   end
 
   describe "#available?" do
-    let(:opts) { { go: false } }
+    let(:opts) { OpenStruct.new(go: false) }
 
     context "when the command has a condition block" do
       before do
@@ -78,7 +78,7 @@ describe Gitlab::QuickActions::CommandDefinition do
       it "doesn't execute the command" do
         expect(context).not_to receive(:instance_exec)
 
-        subject.execute(context, {}, nil)
+        subject.execute(context, nil)
 
         expect(context.run).to be false
       end
@@ -95,7 +95,7 @@ describe Gitlab::QuickActions::CommandDefinition do
         end
 
         it "doesn't execute the command" do
-          subject.execute(context, {}, nil)
+          subject.execute(context, nil)
 
           expect(context.run).to be false
         end
@@ -109,7 +109,7 @@ describe Gitlab::QuickActions::CommandDefinition do
 
           context "when the command is provided an argument" do
             it "executes the command" do
-              subject.execute(context, {}, true)
+              subject.execute(context, true)
 
               expect(context.run).to be true
             end
@@ -117,7 +117,7 @@ describe Gitlab::QuickActions::CommandDefinition do
 
           context "when the command is not provided an argument" do
             it "executes the command" do
-              subject.execute(context, {}, nil)
+              subject.execute(context, nil)
 
               expect(context.run).to be true
             end
@@ -131,7 +131,7 @@ describe Gitlab::QuickActions::CommandDefinition do
 
           context "when the command is provided an argument" do
             it "executes the command" do
-              subject.execute(context, {}, true)
+              subject.execute(context, true)
 
               expect(context.run).to be true
             end
@@ -139,7 +139,7 @@ describe Gitlab::QuickActions::CommandDefinition do
 
           context "when the command is not provided an argument" do
             it "doesn't execute the command" do
-              subject.execute(context, {}, nil)
+              subject.execute(context, nil)
 
               expect(context.run).to be false
             end
@@ -153,7 +153,7 @@ describe Gitlab::QuickActions::CommandDefinition do
 
           context "when the command is provided an argument" do
             it "executes the command" do
-              subject.execute(context, {}, true)
+              subject.execute(context, true)
 
               expect(context.run).to be true
             end
@@ -161,7 +161,7 @@ describe Gitlab::QuickActions::CommandDefinition do
 
           context "when the command is not provided an argument" do
             it "executes the command" do
-              subject.execute(context, {}, nil)
+              subject.execute(context, nil)
 
               expect(context.run).to be true
             end
@@ -175,7 +175,7 @@ describe Gitlab::QuickActions::CommandDefinition do
           end
 
           it 'executes the command passing the parsed param' do
-            subject.execute(context, {}, 'something   ')
+            subject.execute(context, 'something   ')
 
             expect(context.received_arg).to eq('something')
           end
@@ -192,7 +192,7 @@ describe Gitlab::QuickActions::CommandDefinition do
       end
 
       it 'returns nil' do
-        result = subject.explain({}, {}, nil)
+        result = subject.explain({}, nil)
 
         expect(result).to be_nil
       end
@@ -204,7 +204,7 @@ describe Gitlab::QuickActions::CommandDefinition do
       end
 
       it 'returns this static string' do
-        result = subject.explain({}, {}, nil)
+        result = subject.explain({}, nil)
 
         expect(result).to eq 'Explanation'
       end
@@ -216,7 +216,7 @@ describe Gitlab::QuickActions::CommandDefinition do
       end
 
       it 'invokes the proc' do
-        result = subject.explain({}, {}, 'explanation')
+        result = subject.explain({}, 'explanation')
 
         expect(result).to eq 'Dynamic explanation'
       end

spec/lib/gitlab/quick_actions/dsl_spec.rb

@@ -76,7 +76,7 @@ describe Gitlab::QuickActions::Dsl do
 
       expect(dynamic_description_def.name).to eq(:dynamic_description)
       expect(dynamic_description_def.aliases).to eq([])
-      expect(dynamic_description_def.to_h(noteable: 'issue')[:description]).to eq('A dynamic description for ISSUE')
+      expect(dynamic_description_def.to_h(OpenStruct.new(noteable: 'issue'))[:description]).to eq('A dynamic description for ISSUE')
       expect(dynamic_description_def.explanation).to eq('')
       expect(dynamic_description_def.params).to eq(['The first argument', 'The second argument'])
       expect(dynamic_description_def.condition_block).to be_nil

spec/lib/gitlab/sql/pattern_spec.rb

@@ -154,6 +154,12 @@ describe Gitlab::SQL::Pattern do
       it 'returns a single equality condition' do
         expect(fuzzy_arel_match.to_sql).to match(/title.*I?LIKE 'fo'/)
       end
+
+      it 'uses LOWER instead of ILIKE when LOWER is enabled' do
+        rel = Issue.fuzzy_arel_match(:title, query, lower_exact_match: true)
+
+        expect(rel.to_sql).to match(/LOWER\(.*title.*\).*=.*'fo'/)
+      end
     end
 
     context 'with two words both equal to 3 chars' do

spec/services/quick_actions/interpret_service_spec.rb

@@ -522,6 +522,22 @@ describe QuickActions::InterpretService do
       let(:issuable) { merge_request }
     end
 
+    context 'only group milestones available' do
+      let(:group) { create(:group) }
+      let(:project) { create(:project, :public, namespace: group) }
+      let(:milestone) { create(:milestone, group: group, title: '10.0') }
+
+      it_behaves_like 'milestone command' do
+        let(:content) { "/milestone %#{milestone.title}" }
+        let(:issuable) { issue }
+      end
+
+      it_behaves_like 'milestone command' do
+        let(:content) { "/milestone %#{milestone.title}" }
+        let(:issuable) { merge_request }
+      end
+    end
+
     it_behaves_like 'remove_milestone command' do
       let(:content) { '/remove_milestone' }
       let(:issuable) { issue }

spec/workers/stuck_import_jobs_worker_spec.rb

@@ -2,35 +2,59 @@ require 'spec_helper'
 
 describe StuckImportJobsWorker do
   let(:worker) { described_class.new }
-  let(:exclusive_lease_uuid) { SecureRandom.uuid }
 
-  before do
-    allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(exclusive_lease_uuid)
-  end
+  shared_examples 'project import job detection' do
+    context 'when the job has completed' do
+      context 'when the import status was already updated' do
+        before do
+          allow(Gitlab::SidekiqStatus).to receive(:completed_jids) do
+            project.import_start
+            project.import_finish
 
-  describe 'with started import_status' do
-    let(:project) { create(:project, :import_started, import_jid: '123') }
+            [project.import_jid]
+          end
+        end
+
+        it 'does not mark the project as failed' do
+          worker.perform
+
+          expect(project.reload.import_status).to eq('finished')
+        end
+      end
+
+      context 'when the import status was not updated' do
+        before do
+          allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return([project.import_jid])
+        end
 
-    describe 'long running import' do
-      it 'marks the project as failed' do
-        allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return(['123'])
+        it 'marks the project as failed' do
+          worker.perform
 
-        expect { worker.perform }.to change { project.reload.import_status }.to('failed')
+          expect(project.reload.import_status).to eq('failed')
+        end
       end
     end
 
-    describe 'running import' do
-      it 'does not mark the project as failed' do
+    context 'when the job is still in Sidekiq' do
+      before do
         allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return([])
+      end
 
+      it 'does not mark the project as failed' do
         expect { worker.perform }.not_to change { project.reload.import_status }
       end
+    end
+  end
 
-      describe 'import without import_jid' do
-        it 'marks the project as failed' do
-          expect { worker.perform }.to change { project.reload.import_status }.to('failed')
-        end
-      end
+  describe 'with scheduled import_status' do
+    it_behaves_like 'project import job detection' do
+      let(:project) { create(:project, :import_scheduled, import_jid: '123') }
+    end
+  end
+
+  describe 'with started import_status' do
+    it_behaves_like 'project import job detection' do
+      let(:project) { create(:project, :import_started, import_jid: '123') }
     end
   end
 end