Merge branch '323779-requirement-issue-association' into 'master'

Relate Test Reports and Requirement Issues

See merge request gitlab-org/gitlab!63512

db/migrate/20210608103230_add_issue_id_to_test_report.rb

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

db/migrate/20210608103235_add_issue_index_to_test_report.rb

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

db/migrate/20210608110752_change_column_null_test_report_requirement.rb

+# frozen_string_literal: true
+
+class ChangeColumnNullTestReportRequirement < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  TARGET_TABLE = :requirements_management_test_reports
+
+  def up
+    with_lock_retries do
+      change_column_null TARGET_TABLE, :requirement_id, true
+    end
+  end
+
+  def down
+    # no-op as it's difficult to revert
+  end
+end

db/migrate/20210608110760_add_requirement_test_reports_foreign_key.rb

+# frozen_string_literal: true
+
+class AddRequirementTestReportsForeignKey < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  TARGET_TABLE = :requirements_management_test_reports
+  CONSTRAINT_NAME = 'requirements_test_reports_requirement_id_xor_issue_id'
+
+  def up
+    add_concurrent_foreign_key TARGET_TABLE, :issues, column: :issue_id
+
+    add_check_constraint(TARGET_TABLE, 'num_nonnulls(requirement_id, issue_id) = 1', CONSTRAINT_NAME)
+  end
+
+  def down
+    remove_check_constraint TARGET_TABLE, CONSTRAINT_NAME
+
+    with_lock_retries do
+      remove_foreign_key_if_exists(TARGET_TABLE, column: :issue_id)
+    end
+  end
+end

db/schema_migrations/20210608103230

+12d8de65d287cf29fa2761264c42eb42e7fe2a5b36c279e623d93897503b5313
\ No newline at end of file

db/schema_migrations/20210608103235

+fc503b8e9672eb5638d2cb3468c8df4d9c0d998332909351121ace04d3f7214a
\ No newline at end of file

db/schema_migrations/20210608110752

+cbe4cff5937f3ba39a4aeeed78dcc6dc6ece212b01b16bfcd61ccf4a20427dcc
\ No newline at end of file

db/schema_migrations/20210608110760

+b84505713afce3bf0673329a3a4eaf85a00d4f8948f56d43d365d6cc47ef629c
\ No newline at end of file

db/structure.sql

@@ -17498,10 +17498,12 @@ ALTER SEQUENCE requirements_id_seq OWNED BY requirements.id;
 CREATE TABLE requirements_management_test_reports (
     id bigint NOT NULL,
     created_at timestamp with time zone NOT NULL,
-    requirement_id bigint NOT NULL,
+    requirement_id bigint,
     author_id bigint,
     state smallint NOT NULL,
-    build_id bigint
+    build_id bigint,
+    issue_id bigint,
+    CONSTRAINT requirements_test_reports_requirement_id_xor_issue_id CHECK ((num_nonnulls(requirement_id, issue_id) = 1))
 );
 
 CREATE SEQUENCE requirements_management_test_reports_id_seq
@@ -24489,6 +24491,8 @@ CREATE INDEX index_requirements_management_test_reports_on_author_id ON requirem
 
 CREATE INDEX index_requirements_management_test_reports_on_build_id ON requirements_management_test_reports USING btree (build_id);
 
+CREATE INDEX index_requirements_management_test_reports_on_issue_id ON requirements_management_test_reports USING btree (issue_id);
+
 CREATE INDEX index_requirements_management_test_reports_on_requirement_id ON requirements_management_test_reports USING btree (requirement_id);
 
 CREATE INDEX index_requirements_on_author_id ON requirements USING btree (author_id);
@@ -25826,6 +25830,9 @@ ALTER TABLE ONLY vulnerabilities
 ALTER TABLE ONLY bulk_import_entities
     ADD CONSTRAINT fk_88c725229f FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY requirements_management_test_reports
+    ADD CONSTRAINT fk_88f30752fc FOREIGN KEY (issue_id) REFERENCES issues(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY issues
     ADD CONSTRAINT fk_899c8f3231 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 

ee/app/models/requirements_management/test_report.rb

@@ -8,8 +8,11 @@ module RequirementsManagement
     belongs_to :requirement, inverse_of: :test_reports
     belongs_to :author, inverse_of: :test_reports, class_name: 'User'
     belongs_to :build, class_name: 'Ci::Build'
+    belongs_to :requirement_issue, class_name: 'Issue', foreign_key: :issue_id
 
-    validates :requirement, :state, presence: true
+    validates :state, presence: true
+    validate :only_one_requirement_association
+    validate :only_requirement_type_issue
 
     enum state: { passed: 1, failed: 2 }
 
@@ -65,5 +68,13 @@ module RequirementsManagement
         end
       end
     end
+
+    def only_one_requirement_association
+      errors.add(:base, 'Must be associated with either a RequirementsManagement::Requirement OR an Issue of type `requirement`, but not both') unless !!requirement ^ !!requirement_issue
+    end
+
+    def only_requirement_type_issue
+      errors.add(:requirement_issue, 'must be an issue of type `Requirement`') if requirement_issue && !requirement_issue.requirement?
+    end
   end
 end

ee/spec/models/requirements_management/test_report_spec.rb

@@ -8,14 +8,69 @@ RSpec.describe RequirementsManagement::TestReport do
 
     it { is_expected.to belong_to(:author).class_name('User') }
     it { is_expected.to belong_to(:requirement) }
+    it { is_expected.to belong_to(:requirement_issue) }
     it { is_expected.to belong_to(:build) }
   end
 
   describe 'validations' do
     subject { build(:test_report) }
 
-    it { is_expected.to validate_presence_of(:requirement) }
+    let(:requirement) { build(:requirement) }
+    let(:requirement_issue) { build(:requirement_issue) }
+    let(:requirement_error) { /Must be associated with either a RequirementsManagement::Requirement OR an Issue of type `requirement`, but not both/ }
+
     it { is_expected.to validate_presence_of(:state) }
+
+    context 'requirements associations' do
+      subject { build(:test_report, requirement: requirement_arg, requirement_issue: requirement_issue_arg) }
+
+      context 'when both are set' do
+        let(:requirement_arg) { requirement }
+        let(:requirement_issue_arg) { requirement_issue }
+
+        specify do
+          expect(subject).not_to be_valid
+          expect(subject.errors.messages[:base]).to include(requirement_error)
+        end
+      end
+
+      context 'when neither are set' do
+        let(:requirement_arg) { nil }
+        let(:requirement_issue_arg) { nil }
+
+        specify do
+          expect(subject).not_to be_valid
+          expect(subject.errors.messages[:base]).to include(requirement_error)
+        end
+      end
+
+      context 'when only requirement is set' do
+        let(:requirement_arg) { requirement }
+        let(:requirement_issue_arg) { nil }
+
+        specify { expect(subject).to be_valid }
+      end
+
+      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 non-requirement issue' do
+          let(:invalid_issue) { build(: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 an issue of type `Requirement`/)
+          end
+        end
+      end
+    end
   end
 
   describe 'scopes' do