Merge branch 'issue_345840-fill_test_reports_issue_id' into 'master'

Fill TestReport#issue_id field when creating new objects

See merge request gitlab-org/gitlab!74923

db/migrate/20211119170805_remove_test_report_requirement_issue_constraint.rb

+# frozen_string_literal: true
+
+class RemoveTestReportRequirementIssueConstraint < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  TARGET_TABLE = :requirements_management_test_reports
+  CONSTRAINT_NAME = 'requirements_test_reports_requirement_id_xor_issue_id'
+
+  def up
+    remove_check_constraint TARGET_TABLE, CONSTRAINT_NAME
+  end
+
+  def down
+    add_check_constraint(TARGET_TABLE, 'num_nonnulls(requirement_id, issue_id) = 1', CONSTRAINT_NAME)
+  end
+end

db/schema_migrations/20211119170805

+adb95bc78104382fb1d3af2c2775b4b5bd23394b4260c3a97667b4bd7917e0da
\ No newline at end of file

db/structure.sql

@@ -18888,8 +18888,7 @@ CREATE TABLE requirements_management_test_reports (
     author_id bigint,
     state smallint NOT NULL,
     build_id bigint,
-    issue_id bigint,
-    CONSTRAINT requirements_test_reports_requirement_id_xor_issue_id CHECK ((num_nonnulls(requirement_id, issue_id) = 1))
+    issue_id bigint
 );
 
 CREATE SEQUENCE requirements_management_test_reports_id_seq

ee/app/models/requirements_management/test_report.rb

@@ -10,8 +10,13 @@ module RequirementsManagement
     belongs_to :build, class_name: 'Ci::Build'
     belongs_to :requirement_issue, class_name: 'Issue', foreign_key: :issue_id
 
+    # RequirementsManagement::Requirement object is going to be deprecated
+    # and replaced with Issue of type requirement.
+    # For now we need to keep both columns in sync until Requirement object is removed.
+    # More information in: https://gitlab.com/groups/gitlab-org/-/epics/7148
+    validates :requirement, presence: true
+
     validates :state, presence: true
-    validate :only_one_requirement_association
     validate :only_requirement_type_issue
 
     enum state: { passed: 1, failed: 2 }
@@ -36,6 +41,7 @@ module RequirementsManagement
       def build_report(author: nil, state:, requirement:, build: nil, timestamp: Time.current)
         new(
           requirement_id: requirement.id,
+          issue_id: requirement.issue_id,
           build_id: build&.id,
           author_id: build&.user_id || author&.id,
           created_at: timestamp,
@@ -47,7 +53,7 @@ module RequirementsManagement
 
       def passed_reports_for_all_requirements(build, timestamp)
         [].tap do |reports|
-          build.project.requirements.opened.select(:id).find_each do |requirement|
+          build.project.requirements.opened.select(:id, :issue_id).find_each do |requirement|
             reports << build_report(state: :passed, requirement: requirement, build: build, timestamp: timestamp)
           end
         end
@@ -58,7 +64,7 @@ module RequirementsManagement
           iids = ci_report.requirements.keys
           break [] if iids.empty?
 
-          build.project.requirements.opened.select(:id, :iid).where(iid: iids).each do |requirement|
+          build.project.requirements.opened.select(:id, :iid, :issue_id).where(iid: iids).each do |requirement|
             # ignore anything with any unexpected state
             new_state = ci_report.requirements[requirement.iid.to_s]
             next unless states.key?(new_state)
@@ -69,10 +75,6 @@ module RequirementsManagement
       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 a `requirement`. You cannot associate a Test Report with a #{requirement_issue.issue_type}.") if requirement_issue && !requirement_issue.requirement? && will_save_change_to_issue_id?
     end

ee/spec/models/issue_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe Issue do
     context 'for an issue with associated test report' do
       let_it_be(:requirement_issue) do
         ri = create(:requirement_issue)
-        create(:test_report, requirement_issue: ri, requirement: nil)
+        create(:test_report, requirement_issue: ri, requirement: create(:requirement))
         ri
       end
 

ee/spec/models/requirements_management/test_report_spec.rb

@@ -17,33 +17,13 @@ RSpec.describe RequirementsManagement::TestReport do
 
     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) }
+    it { is_expected.to validate_presence_of(:requirement) }
 
     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 }
@@ -111,7 +91,7 @@ RSpec.describe RequirementsManagement::TestReport do
         end
 
         it 'creates test report with expected status for each open requirement' do
-          requirement1 = create(:requirement, state: :opened, project: project)
+          requirement1 = create(:requirement, state: :opened, project: project, requirement_issue: create(:requirement_issue))
           requirement2 = create(:requirement, state: :opened, project: project)
           create(:requirement, state: :opened) # different project
           create(:requirement, state: :archived, project: project) # archived
@@ -119,8 +99,10 @@ RSpec.describe RequirementsManagement::TestReport do
           expect { subject }.to change { RequirementsManagement::TestReport.count }.by(2)
 
           reports = RequirementsManagement::TestReport.where(build: build)
+
           expect(reports).to match_array([
             have_attributes(requirement: requirement1,
+                            requirement_issue: requirement1.requirement_issue,
                             author: build.user,
                             state: 'passed'),
             have_attributes(requirement: requirement2,
@@ -155,7 +137,8 @@ RSpec.describe RequirementsManagement::TestReport do
     let_it_be(:user) { create(:user) }
     let_it_be(:build_author) { create(:user) }
     let_it_be(:build) { create(:ci_build, author: build_author) }
-    let_it_be(:requirement) { create(:requirement, state: :opened) }
+    let_it_be(:requirement_issue) { create(:requirement_issue)}
+    let_it_be(:requirement) { create(:requirement, requirement_issue: requirement_issue, state: :opened) }
 
     let(:now) { Time.current }
 
@@ -166,6 +149,7 @@ RSpec.describe RequirementsManagement::TestReport do
         expect(test_report.author).to eq(build.author)
         expect(test_report.build).to eq(build)
         expect(test_report.requirement).to eq(requirement)
+        expect(test_report.requirement_issue).to eq(requirement.requirement_issue)
         expect(test_report.state).to eq('failed')
         expect(test_report.created_at).to eq(now)
       end
@@ -178,6 +162,7 @@ RSpec.describe RequirementsManagement::TestReport do
         expect(test_report.author).to eq(user)
         expect(test_report.build).to eq(nil)
         expect(test_report.requirement).to eq(requirement)
+        expect(test_report.requirement_issue).to eq(requirement.requirement_issue)
         expect(test_report.state).to eq('passed')
         expect(test_report.created_at).to eq(now)
       end