Commit 75dfe32c authored by Felipe Artur's avatar Felipe Artur

Fill TestReport#issue_id field when creating new objects

Part of the plan to deprecate Requirement objects.
We will keep both requirement_id, and issue_id in in sync
until Requirement gets removed.

Changelog: other
EE: true
parent b6df7992
# 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
adb95bc78104382fb1d3af2c2775b4b5bd23394b4260c3a97667b4bd7917e0da
\ No newline at end of file
...@@ -18884,8 +18884,7 @@ CREATE TABLE requirements_management_test_reports ( ...@@ -18884,8 +18884,7 @@ CREATE TABLE requirements_management_test_reports (
author_id bigint, author_id bigint,
state smallint NOT NULL, state smallint NOT NULL,
build_id bigint, build_id bigint,
issue_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 CREATE SEQUENCE requirements_management_test_reports_id_seq
...@@ -10,8 +10,13 @@ module RequirementsManagement ...@@ -10,8 +10,13 @@ module RequirementsManagement
belongs_to :build, class_name: 'Ci::Build' belongs_to :build, class_name: 'Ci::Build'
belongs_to :requirement_issue, class_name: 'Issue', foreign_key: :issue_id 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 validates :state, presence: true
validate :only_one_requirement_association
validate :only_requirement_type_issue validate :only_requirement_type_issue
enum state: { passed: 1, failed: 2 } enum state: { passed: 1, failed: 2 }
...@@ -36,6 +41,7 @@ module RequirementsManagement ...@@ -36,6 +41,7 @@ module RequirementsManagement
def build_report(author: nil, state:, requirement:, build: nil, timestamp: Time.current) def build_report(author: nil, state:, requirement:, build: nil, timestamp: Time.current)
new( new(
requirement_id: requirement.id, requirement_id: requirement.id,
issue_id: requirement.issue_id,
build_id: build&.id, build_id: build&.id,
author_id: build&.user_id || author&.id, author_id: build&.user_id || author&.id,
created_at: timestamp, created_at: timestamp,
...@@ -47,7 +53,7 @@ module RequirementsManagement ...@@ -47,7 +53,7 @@ module RequirementsManagement
def passed_reports_for_all_requirements(build, timestamp) def passed_reports_for_all_requirements(build, timestamp)
[].tap do |reports| [].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) reports << build_report(state: :passed, requirement: requirement, build: build, timestamp: timestamp)
end end
end end
...@@ -58,7 +64,7 @@ module RequirementsManagement ...@@ -58,7 +64,7 @@ module RequirementsManagement
iids = ci_report.requirements.keys iids = ci_report.requirements.keys
break [] if iids.empty? 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 # ignore anything with any unexpected state
new_state = ci_report.requirements[requirement.iid.to_s] new_state = ci_report.requirements[requirement.iid.to_s]
next unless states.key?(new_state) next unless states.key?(new_state)
...@@ -69,10 +75,6 @@ module RequirementsManagement ...@@ -69,10 +75,6 @@ module RequirementsManagement
end 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 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? 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 end
......
...@@ -22,7 +22,7 @@ RSpec.describe Issue do ...@@ -22,7 +22,7 @@ RSpec.describe Issue do
context 'for an issue with associated test report' do context 'for an issue with associated test report' do
let_it_be(:requirement_issue) do let_it_be(:requirement_issue) do
ri = create(:requirement_issue) ri = create(:requirement_issue)
create(:test_report, requirement_issue: ri, requirement: nil) create(:test_report, requirement_issue: ri, requirement: create(:requirement))
ri ri
end end
......
...@@ -17,33 +17,13 @@ RSpec.describe RequirementsManagement::TestReport do ...@@ -17,33 +17,13 @@ RSpec.describe RequirementsManagement::TestReport do
let(:requirement) { build(:requirement) } let(:requirement) { build(:requirement) }
let(:requirement_issue) { build(:requirement_issue) } 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(:state) }
it { is_expected.to validate_presence_of(:requirement) }
context 'requirements associations' do context 'requirements associations' do
subject { build(:test_report, requirement: requirement_arg, requirement_issue: requirement_issue_arg) } 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 context 'when only requirement is set' do
let(:requirement_arg) { requirement } let(:requirement_arg) { requirement }
let(:requirement_issue_arg) { nil } let(:requirement_issue_arg) { nil }
...@@ -111,7 +91,7 @@ RSpec.describe RequirementsManagement::TestReport do ...@@ -111,7 +91,7 @@ RSpec.describe RequirementsManagement::TestReport do
end end
it 'creates test report with expected status for each open requirement' do 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) requirement2 = create(:requirement, state: :opened, project: project)
create(:requirement, state: :opened) # different project create(:requirement, state: :opened) # different project
create(:requirement, state: :archived, project: project) # archived create(:requirement, state: :archived, project: project) # archived
...@@ -119,8 +99,10 @@ RSpec.describe RequirementsManagement::TestReport do ...@@ -119,8 +99,10 @@ RSpec.describe RequirementsManagement::TestReport do
expect { subject }.to change { RequirementsManagement::TestReport.count }.by(2) expect { subject }.to change { RequirementsManagement::TestReport.count }.by(2)
reports = RequirementsManagement::TestReport.where(build: build) reports = RequirementsManagement::TestReport.where(build: build)
expect(reports).to match_array([ expect(reports).to match_array([
have_attributes(requirement: requirement1, have_attributes(requirement: requirement1,
requirement_issue: requirement1.requirement_issue,
author: build.user, author: build.user,
state: 'passed'), state: 'passed'),
have_attributes(requirement: requirement2, have_attributes(requirement: requirement2,
...@@ -155,7 +137,8 @@ RSpec.describe RequirementsManagement::TestReport do ...@@ -155,7 +137,8 @@ RSpec.describe RequirementsManagement::TestReport do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:build_author) { create(:user) } let_it_be(:build_author) { create(:user) }
let_it_be(:build) { create(:ci_build, author: build_author) } 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 } let(:now) { Time.current }
...@@ -166,6 +149,7 @@ RSpec.describe RequirementsManagement::TestReport do ...@@ -166,6 +149,7 @@ RSpec.describe RequirementsManagement::TestReport do
expect(test_report.author).to eq(build.author) expect(test_report.author).to eq(build.author)
expect(test_report.build).to eq(build) expect(test_report.build).to eq(build)
expect(test_report.requirement).to eq(requirement) 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.state).to eq('failed')
expect(test_report.created_at).to eq(now) expect(test_report.created_at).to eq(now)
end end
...@@ -178,6 +162,7 @@ RSpec.describe RequirementsManagement::TestReport do ...@@ -178,6 +162,7 @@ RSpec.describe RequirementsManagement::TestReport do
expect(test_report.author).to eq(user) expect(test_report.author).to eq(user)
expect(test_report.build).to eq(nil) expect(test_report.build).to eq(nil)
expect(test_report.requirement).to eq(requirement) 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.state).to eq('passed')
expect(test_report.created_at).to eq(now) expect(test_report.created_at).to eq(now)
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment