Commit da6775a2 authored by Phil Hughes's avatar Phil Hughes

Defaults reviewer state to attention_required

If the attention required feature flag is enabled
then the state for assignees and reviewers
gets defaulted to `attention_required`
parent b148b97b
......@@ -13,5 +13,13 @@ module MergeRequestReviewerState
validates :state,
presence: true,
inclusion: { in: self.states.keys }
after_initialize :set_state, unless: :persisted?
def set_state
if Feature.enabled?(:mr_attention_requests, self.merge_request&.project, default_enabled: :yaml)
self.state = :attention_required
end
end
end
end
......@@ -78,7 +78,7 @@ RSpec.describe GitlabSchema.types['UserMergeRequestInteraction'] do
merge_request.reviewers << user
end
it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['UNREVIEWED'].value) }
it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['ATTENTION_REQUIRED'].value) }
it 'implies not reviewed' do
expect(resolve(:reviewed)).to be false
......@@ -87,7 +87,8 @@ RSpec.describe GitlabSchema.types['UserMergeRequestInteraction'] do
context 'when the user has provided a review' do
before do
merge_request.merge_request_reviewers.create!(reviewer: user, state: MergeRequestReviewer.states['reviewed'])
reviewer = merge_request.merge_request_reviewers.create!(reviewer: user)
reviewer.update!(state: MergeRequestReviewer.states['reviewed'])
end
it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['REVIEWED'].value) }
......
......@@ -39,4 +39,6 @@ RSpec.describe MergeRequestAssignee do
end
it_behaves_like 'having unique enum values'
it_behaves_like 'having reviewer state'
end
......@@ -9,6 +9,8 @@ RSpec.describe MergeRequestReviewer do
it_behaves_like 'having unique enum values'
it_behaves_like 'having reviewer state'
describe 'associations' do
it { is_expected.to belong_to(:merge_request).class_name('MergeRequest') }
it { is_expected.to belong_to(:reviewer).class_name('User').inverse_of(:merge_request_reviewers) }
......
......@@ -61,7 +61,7 @@ RSpec.describe ::Users::MergeRequestInteraction do
merge_request.reviewers << user
end
it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['UNREVIEWED'].value) }
it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['ATTENTION_REQUIRED'].value) }
it 'implies not reviewed' do
expect(interaction).not_to be_reviewed
......@@ -70,7 +70,8 @@ RSpec.describe ::Users::MergeRequestInteraction do
context 'when the user has provided a review' do
before do
merge_request.merge_request_reviewers.create!(reviewer: user, state: MergeRequestReviewer.states['reviewed'])
reviewer = merge_request.merge_request_reviewers.create!(reviewer: user)
reviewer.update!(state: MergeRequestReviewer.states['reviewed'])
end
it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['REVIEWED'].value) }
......
......@@ -347,7 +347,7 @@ RSpec.describe 'getting merge request information nested in a project' do
expect(interaction_data).to contain_exactly a_hash_including(
'canMerge' => false,
'canUpdate' => can_update,
'reviewState' => unreviewed,
'reviewState' => attention_required,
'reviewed' => false,
'approved' => false
)
......@@ -380,8 +380,8 @@ RSpec.describe 'getting merge request information nested in a project' do
describe 'scalability' do
let_it_be(:other_users) { create_list(:user, 3) }
let(:unreviewed) do
{ 'reviewState' => 'UNREVIEWED' }
let(:attention_required) do
{ 'reviewState' => 'ATTENTION_REQUIRED' }
end
let(:reviewed) do
......@@ -413,9 +413,9 @@ RSpec.describe 'getting merge request information nested in a project' do
expect { post_graphql(query) }.not_to exceed_query_limit(baseline)
expect(interaction_data).to contain_exactly(
include(unreviewed),
include(unreviewed),
include(unreviewed),
include(attention_required),
include(attention_required),
include(attention_required),
include(reviewed)
)
end
......@@ -444,7 +444,7 @@ RSpec.describe 'getting merge request information nested in a project' do
it_behaves_like 'when requesting information about MR interactions' do
let(:field) { :reviewers }
let(:unreviewed) { 'UNREVIEWED' }
let(:attention_required) { 'ATTENTION_REQUIRED' }
let(:can_update) { false }
def assign_user(user)
......@@ -454,7 +454,7 @@ RSpec.describe 'getting merge request information nested in a project' do
it_behaves_like 'when requesting information about MR interactions' do
let(:field) { :assignees }
let(:unreviewed) { nil }
let(:attention_required) { nil }
let(:can_update) { true } # assignees can update MRs
def assign_user(user)
......
# frozen_string_literal: true
RSpec.shared_examples 'having reviewer state' do
describe 'mr_attention_requests feature flag is disabled' do
before do
stub_feature_flags(mr_attention_requests: false)
end
it { is_expected.to have_attributes(state: 'unreviewed') }
end
describe 'mr_attention_requests feature flag is enabled' do
it { is_expected.to have_attributes(state: 'attention_required') }
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