Commit 814652b9 authored by Marc Shaw's avatar Marc Shaw Committed by Stan Hu

Dont change state if we are already in the unchecked merge state

MR: gitlab.com/gitlab-org/gitlab/-/merge_requests/53537
Issue: gitlab.com/gitlab-org/gitlab/-/issues/218410
parent 7ad670fb
...@@ -196,8 +196,8 @@ class MergeRequest < ApplicationRecord ...@@ -196,8 +196,8 @@ class MergeRequest < ApplicationRecord
end end
event :mark_as_unchecked do event :mark_as_unchecked do
transition [:preparing, :can_be_merged, :checking, :unchecked] => :unchecked transition [:preparing, :can_be_merged, :checking] => :unchecked
transition [:cannot_be_merged, :cannot_be_merged_rechecking, :cannot_be_merged_recheck] => :cannot_be_merged_recheck transition [:cannot_be_merged, :cannot_be_merged_rechecking] => :cannot_be_merged_recheck
end end
event :mark_as_checking do event :mark_as_checking do
...@@ -326,7 +326,7 @@ class MergeRequest < ApplicationRecord ...@@ -326,7 +326,7 @@ class MergeRequest < ApplicationRecord
scope :preload_approved_by_users, -> { preload(:approved_by_users) } scope :preload_approved_by_users, -> { preload(:approved_by_users) }
scope :preload_metrics, -> (relation) { preload(metrics: relation) } scope :preload_metrics, -> (relation) { preload(metrics: relation) }
scope :preload_project_and_latest_diff, -> { preload(:source_project, :latest_merge_request_diff) } scope :preload_project_and_latest_diff, -> { preload(:source_project, :latest_merge_request_diff) }
scope :preload_latest_diff_comment, -> { preload(latest_merge_request_diff: :merge_request_diff_commits) } scope :preload_latest_diff_commit, -> { preload(latest_merge_request_diff: :merge_request_diff_commits) }
scope :with_web_entity_associations, -> { preload(:author, :target_project) } scope :with_web_entity_associations, -> { preload(:author, :target_project) }
scope :with_auto_merge_enabled, -> do scope :with_auto_merge_enabled, -> do
......
...@@ -75,7 +75,7 @@ module MergeRequests ...@@ -75,7 +75,7 @@ module MergeRequests
commit_ids = @commits.map(&:id) commit_ids = @commits.map(&:id)
merge_requests = @project.merge_requests.opened merge_requests = @project.merge_requests.opened
.preload_project_and_latest_diff .preload_project_and_latest_diff
.preload_latest_diff_comment .preload_latest_diff_commit
.where(target_branch: @push.branch_name).to_a .where(target_branch: @push.branch_name).to_a
.select(&:diff_head_commit) .select(&:diff_head_commit)
.select do |merge_request| .select do |merge_request|
......
---
title: Remove unneeded transitions on MR for mark_as_unchecked event
merge_request: 53537
author:
type: performance
...@@ -87,7 +87,7 @@ RSpec.describe Gitlab::Git::Push do ...@@ -87,7 +87,7 @@ RSpec.describe Gitlab::Git::Push do
it { is_expected.to be_force_push } it { is_expected.to be_force_push }
end end
context 'when called muiltiple times' do context 'when called mulitiple times' do
it 'does not make make multiple calls to the force push check' do it 'does not make make multiple calls to the force push check' do
expect(Gitlab::Checks::ForcePush).to receive(:force_push?).once expect(Gitlab::Checks::ForcePush).to receive(:force_push?).once
......
...@@ -4105,6 +4105,72 @@ RSpec.describe MergeRequest, factory_default: :keep do ...@@ -4105,6 +4105,72 @@ RSpec.describe MergeRequest, factory_default: :keep do
end end
end end
describe '#mark_as_unchecked' do
subject { create(:merge_request, source_project: project, merge_status: merge_status) }
shared_examples 'for an invalid state transition' do
it 'is not a valid state transition' do
expect { subject.mark_as_unchecked! }.to raise_error(StateMachines::InvalidTransition)
end
end
shared_examples 'for an valid state transition' do
it 'is a valid state transition' do
expect { subject.mark_as_unchecked! }
.to change { subject.merge_status }
.from(merge_status.to_s)
.to(expected_merge_status)
end
end
context 'when the status is unchecked' do
let(:merge_status) { :unchecked }
include_examples 'for an invalid state transition'
end
context 'when the status is checking' do
let(:merge_status) { :checking }
let(:expected_merge_status) { 'unchecked' }
include_examples 'for an valid state transition'
end
context 'when the status is preparing' do
let(:merge_status) { :preparing }
let(:expected_merge_status) { 'unchecked' }
include_examples 'for an valid state transition'
end
context 'when the status is can_be_merged' do
let(:merge_status) { :can_be_merged }
let(:expected_merge_status) { 'unchecked' }
include_examples 'for an valid state transition'
end
context 'when the status is cannot_be_merged_recheck' do
let(:merge_status) { :cannot_be_merged_recheck }
include_examples 'for an invalid state transition'
end
context 'when the status is cannot_be_merged' do
let(:merge_status) { :cannot_be_merged }
let(:expected_merge_status) { 'cannot_be_merged_recheck' }
include_examples 'for an valid state transition'
end
context 'when the status is cannot_be_merged' do
let(:merge_status) { :cannot_be_merged }
let(:expected_merge_status) { 'cannot_be_merged_recheck' }
include_examples 'for an valid state transition'
end
end
describe 'transition to cannot_be_merged' do describe 'transition to cannot_be_merged' do
let(:notification_service) { double(:notification_service) } let(:notification_service) { double(:notification_service) }
let(:todo_service) { double(:todo_service) } let(:todo_service) { double(:todo_service) }
......
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