Merge branch 'jh-drop_jid_null_constraint' into 'master'

Allow group import states to be created without job ID

See merge request gitlab-org/gitlab!33181

app/models/group_import_state.rb

@@ -5,7 +5,8 @@ class GroupImportState < ApplicationRecord
 
   belongs_to :group, inverse_of: :import_state
 
-  validates :group, :status, :jid, presence: true
+  validates :group, :status, presence: true
+  validates :jid, presence: true, if: -> { started? || finished? }
 
   state_machine :status, initial: :created do
     state :created, value: 0

changelogs/unreleased/jh-drop_jid_null_constraint.yml

+---
+title: Remove null constraint for JID in GroupImportState
+merge_request: 33181
+author:
+type: changed

db/migrate/20200526142550_drop_null_constraint_on_group_import_state_jid.rb

+# frozen_string_literal: true
+
+class DropNullConstraintOnGroupImportStateJid < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def up
+    change_column_null :group_import_states, :jid, true
+  end
+
+  def down
+    # No-op -- null values could have been added after this this constraint was removed.
+  end
+end

db/structure.sql

@@ -3187,7 +3187,7 @@ CREATE TABLE public.group_import_states (
     created_at timestamp with time zone NOT NULL,
     updated_at timestamp with time zone NOT NULL,
     status smallint DEFAULT 0 NOT NULL,
-    jid text NOT NULL,
+    jid text,
     last_error text,
     CONSTRAINT check_87b58f6b30 CHECK ((char_length(last_error) <= 255)),
     CONSTRAINT check_96558fff96 CHECK ((char_length(jid) <= 100))
@@ -13816,6 +13816,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200526000407
 20200526013844
 20200526120714
+20200526142550
 20200526153844
 20200526164946
 20200526164947

spec/factories/group_import_states.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :group_import_state, class: 'GroupImportState', traits: %i[created] do
+    association :group, factory: :group
+
+    trait :created do
+      status { 0 }
+    end
+
+    trait :started do
+      status { 1 }
+      sequence(:jid) { |n| "group_import_state_#{n}" }
+    end
+
+    trait :finished do
+      status { 2 }
+      sequence(:jid) { |n| "group_import_state_#{n}" }
+    end
+
+    trait :failed do
+      status { -1 }
+    end
+  end
+end

spec/models/group_import_state_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe GroupImportState do
+  describe 'validations' do
+    let_it_be(:group) { create(:group) }
+
+    it { is_expected.to validate_presence_of(:group) }
+    it { is_expected.to validate_presence_of(:status) }
+
+    it 'can be created without a jid' do
+      import_state = build(:group_import_state, :created, group: group, jid: nil)
+
+      expect(import_state).to be_valid
+    end
+
+    it 'cannot be started without a jid' do
+      import_state = build(:group_import_state, :started, group: group, jid: nil)
+
+      expect(import_state).not_to be_valid
+      expect(import_state.errors[:jid]).to include "can't be blank"
+    end
+
+    it 'cannot be finished without a jid' do
+      import_state = build(:group_import_state, :finished, group: group, jid: nil)
+
+      expect(import_state).not_to be_valid
+      expect(import_state.errors[:jid]).to include "can't be blank"
+    end
+
+    it 'can fail without a jid' do
+      import_state = build(:group_import_state, :failed, group: group, jid: nil)
+
+      expect(import_state).to be_valid
+    end
+  end
+end