Merge branch '323779-3-issue_id_on_requirement' into 'master'

Relate Requirements and Requirement Issues

See merge request gitlab-org/gitlab!64189

db/migrate/20210615064342_add_issue_id_to_requirement.rb

+# frozen_string_literal: true
+
+class AddIssueIdToRequirement < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  def up
+    with_lock_retries do
+      add_column :requirements, :issue_id, :bigint, null: true
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :requirements, :issue_id
+    end
+  end
+end

db/migrate/20210616110748_add_issue_index_to_requirement.rb

+# frozen_string_literal: true
+
+class AddIssueIndexToRequirement < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_requirements_on_issue_id'
+
+  def up
+    add_concurrent_index :requirements, :issue_id, name: INDEX_NAME, unique: true
+  end
+
+  def down
+    remove_concurrent_index_by_name :requirements, INDEX_NAME
+  end
+end

db/migrate/20210616111311_add_issue_requirement_foreign_key.rb

+# frozen_string_literal: true
+
+class AddIssueRequirementForeignKey < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  TARGET_TABLE = :requirements
+
+  def up
+    add_concurrent_foreign_key TARGET_TABLE, :issues, column: :issue_id
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key_if_exists(TARGET_TABLE, column: :issue_id)
+    end
+  end
+end

db/schema_migrations/20210615064342

+509f30d8772e24efc52c5aa12ebcde084f7ded8d228109bbbdda2f21d3235512
\ No newline at end of file

db/schema_migrations/20210616110748

+8207eb9917b4d02f39cd9e9eca9ec0e001266b25b3378f09e4e8c27ff22b6e73
\ No newline at end of file

db/schema_migrations/20210616111311

+fd014b505ecd162c232da23a10c34dc4b1f1dbe8fe357a0f20585479b25d50bc
\ No newline at end of file

db/structure.sql

@@ -17477,6 +17477,7 @@ CREATE TABLE requirements (
     title_html text,
     description text,
     description_html text,
+    issue_id bigint,
     CONSTRAINT check_785ae25b9d CHECK ((char_length(description) <= 10000))
 );
 
@@ -24504,6 +24505,8 @@ CREATE INDEX index_requirements_on_author_id ON requirements USING btree (author
 
 CREATE INDEX index_requirements_on_created_at ON requirements USING btree (created_at);
 
+CREATE UNIQUE INDEX index_requirements_on_issue_id ON requirements USING btree (issue_id);
+
 CREATE INDEX index_requirements_on_project_id ON requirements USING btree (project_id);
 
 CREATE UNIQUE INDEX index_requirements_on_project_id_and_iid ON requirements USING btree (project_id, iid) WHERE (project_id IS NOT NULL);
@@ -25828,6 +25831,9 @@ ALTER TABLE ONLY experiment_subjects
 ALTER TABLE ONLY merge_request_diffs
     ADD CONSTRAINT fk_8483f3258f FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY requirements
+    ADD CONSTRAINT fk_85044baef0 FOREIGN KEY (issue_id) REFERENCES issues(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY ci_pipelines
     ADD CONSTRAINT fk_86635dbd80 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 

ee/app/models/concerns/issue_widgets/acts_like_requirement.rb

@@ -8,6 +8,7 @@ module IssueWidgets
       # This will mean that non-Requirement issues essentially ignore this relationship and always return []
       has_many :test_reports, -> { joins(:requirement_issue).where(issues: { issue_type: Issue.issue_types[:requirement] }) },
                foreign_key: :issue_id, inverse_of: :requirement_issue, class_name: 'RequirementsManagement::TestReport'
+      has_one :requirement, class_name: 'RequirementsManagement::Requirement'
     end
   end
 end

ee/app/models/requirements_management/requirement.rb

@@ -20,6 +20,9 @@ module RequirementsManagement
 
     belongs_to :author, inverse_of: :requirements, class_name: 'User'
     belongs_to :project, inverse_of: :requirements
+    belongs_to :requirement_issue, class_name: 'Issue', foreign_key: :issue_id
+
+    validates :issue_id, uniqueness: true, allow_nil: true
 
     has_many :test_reports, inverse_of: :requirement
 
@@ -30,6 +33,8 @@ module RequirementsManagement
     validates :title, length: { maximum: Issuable::TITLE_LENGTH_MAX }
     validates :title_html, length: { maximum: Issuable::TITLE_HTML_LENGTH_MAX }, allow_blank: true
 
+    validate :only_requirement_type_issue
+
     enum state: { opened: 1, archived: 2 }
 
     scope :for_iid, -> (iid) { where(iid: iid) }
@@ -91,5 +96,9 @@ module RequirementsManagement
     def last_test_report_manually_created?
       latest_report&.build.nil?
     end
+
+    def only_requirement_type_issue
+      errors.add(:requirement_issue, "must be a `requirement`. You cannot associate a Requirement with an issue of type #{requirement_issue.issue_type}.") if requirement_issue && !requirement_issue.requirement? && will_save_change_to_issue_id?
+    end
   end
 end

ee/spec/models/issue_spec.rb

@@ -15,6 +15,7 @@ RSpec.describe Issue do
     it { is_expected.to have_one(:issuable_sla) }
     it { is_expected.to have_many(:metric_images) }
 
+    it { is_expected.to have_one(:requirement) }
     it { is_expected.to have_many(:test_reports) }
 
     context 'for an issue with associated test report' do

ee/spec/models/requirements_management/requirement_spec.rb

@@ -11,6 +11,8 @@ RSpec.describe RequirementsManagement::Requirement do
 
     it { is_expected.to belong_to(:author).class_name('User') }
     it { is_expected.to belong_to(:project) }
+
+    it_behaves_like 'a model with a requirement issue association'
   end
 
   describe 'validations' do
@@ -22,6 +24,24 @@ RSpec.describe RequirementsManagement::Requirement do
 
     it { is_expected.to validate_length_of(:title).is_at_most(::Issuable::TITLE_LENGTH_MAX) }
     it { is_expected.to validate_length_of(:title_html).is_at_most(::Issuable::TITLE_HTML_LENGTH_MAX) }
+
+    context 'with requirement issue' do
+      let(:ri) { create(:requirement_issue) }
+
+      subject { build(:requirement, requirement_issue: ri) }
+
+      it { is_expected.to validate_uniqueness_of(:issue_id).allow_nil }
+    end
+
+    it 'is limited to a unique requirement_issue' do
+      requirement_issue = create(:requirement_issue)
+      requirement = create(:requirement, requirement_issue: requirement_issue)
+      requirement.save!
+
+      subject.requirement_issue = requirement_issue
+
+      expect(subject).not_to be_valid
+    end
   end
 
   describe 'scopes' do

ee/spec/models/requirements_management/test_report_spec.rb

@@ -54,39 +54,7 @@ RSpec.describe RequirementsManagement::TestReport do
       context 'when only requirement issue is set' do
         let(:requirement_arg) { nil }
 
-        context 'when the requirement issue is of type requirement' do
-          let(:requirement_issue_arg) { requirement_issue }
-
-          specify { expect(subject).to be_valid }
-        end
-
-        context 'when requirement issue is not of requirement type' do
-          let(:invalid_issue) { create(:issue) }
-          let(:requirement_issue_arg) { invalid_issue }
-
-          specify do
-            expect(subject).not_to be_valid
-            expect(subject.errors.messages[:requirement_issue]).to include(/You cannot associate a Test Report with a issue/)
-          end
-
-          context 'when requirement issue is invalid but the type field is not dirty' do
-            let(:requirement_arg) { nil }
-            let(:requirement_issue_arg) { requirement_issue }
-
-            before do
-              subject.save!
-
-              # simulate the issue type changing in the background, which will be allowed
-              # the state is technically "invalid" (there are test reports associated with a non-requirement issue)
-              # but we don't want to prevent updating other fields
-              requirement_issue.update_attribute(:issue_type, :incident)
-            end
-
-            specify do
-              expect(subject).to be_valid
-            end
-          end
-        end
+        it_behaves_like 'a model with a requirement issue association'
       end
     end
   end

ee/spec/support/shared_examples/models/requirement_issues_examples.rb

+# frozen_string_literal: true
+
+shared_examples 'a model with a requirement issue association' do
+  describe 'requirement issue association' do
+    subject { build(:requirement, requirement_issue: requirement_issue_arg) }
+
+    let(:requirement_issue) { build(:requirement_issue) }
+
+    context 'when the requirement issue is of type requirement' do
+      let(:requirement_issue_arg) { requirement_issue }
+
+      specify { expect(subject).to be_valid }
+    end
+
+    context 'when requirement issue is not of requirement type' do
+      let(:invalid_issue) { create(:issue) }
+      let(:requirement_issue_arg) { invalid_issue }
+
+      specify do
+        expect(subject).not_to be_valid
+        expect(subject.errors.messages[:requirement_issue]).to include(/must be a `requirement`/)
+      end
+
+      context 'when requirement issue is invalid but the type field is not dirty' do
+        let(:requirement_arg) { nil }
+        let(:requirement_issue_arg) { requirement_issue }
+
+        before do
+          subject.save!
+
+          # simulate the issue type changing in the background, which will be allowed
+          # the state is technically "invalid" (there are test reports associated with a non-requirement issue)
+          # but we don't want to prevent updating other fields
+          requirement_issue.update_attribute(:issue_type, :incident)
+        end
+
+        specify do
+          expect(subject).to be_valid
+        end
+
+        it { is_expected.to be_valid }
+      end
+    end
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -55,6 +55,7 @@ issues:
 - note_authors
 - issue_email_participants
 - test_reports
+- requirement
 events:
 - author
 - project