Clear caches before updating MR diffs
The hook ordering influenced the diffs being generated as these used values from before the update due to the memoization still being in place. This commit reorders them and tests against this behaviour.
This commit is contained in:
parent
097b067844
commit
8ad412559d
3 changed files with 25 additions and 14 deletions
|
@ -53,8 +53,8 @@ class MergeRequest < ActiveRecord::Base
|
||||||
serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize
|
serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize
|
||||||
|
|
||||||
after_create :ensure_merge_request_diff, unless: :importing?
|
after_create :ensure_merge_request_diff, unless: :importing?
|
||||||
after_update :reload_diff_if_branch_changed
|
|
||||||
after_update :clear_memoized_shas
|
after_update :clear_memoized_shas
|
||||||
|
after_update :reload_diff_if_branch_changed
|
||||||
|
|
||||||
# When this attribute is true some MR validation is ignored
|
# When this attribute is true some MR validation is ignored
|
||||||
# It allows us to close or modify broken merge requests
|
# It allows us to close or modify broken merge requests
|
||||||
|
|
|
@ -601,30 +601,30 @@ describe MergeRequest do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#can_remove_source_branch?' do
|
describe '#can_remove_source_branch?' do
|
||||||
let(:user) { create(:user) }
|
set(:user) { create(:user) }
|
||||||
let(:user2) { create(:user) }
|
set(:merge_request) { create(:merge_request, :simple) }
|
||||||
|
|
||||||
|
subject { merge_request }
|
||||||
|
|
||||||
before do
|
before do
|
||||||
subject.source_project.team << [user, :master]
|
subject.source_project.add_master(user)
|
||||||
|
|
||||||
subject.source_branch = "feature"
|
|
||||||
subject.target_branch = "master"
|
|
||||||
subject.save!
|
|
||||||
end
|
end
|
||||||
|
|
||||||
it "can't be removed when its a protected branch" do
|
it "can't be removed when its a protected branch" do
|
||||||
allow(ProtectedBranch).to receive(:protected?).and_return(true)
|
allow(ProtectedBranch).to receive(:protected?).and_return(true)
|
||||||
|
|
||||||
expect(subject.can_remove_source_branch?(user)).to be_falsey
|
expect(subject.can_remove_source_branch?(user)).to be_falsey
|
||||||
end
|
end
|
||||||
|
|
||||||
it "can't remove a root ref" do
|
it "can't remove a root ref" do
|
||||||
subject.source_branch = "master"
|
subject.update(source_branch: 'master', target_branch: 'feature')
|
||||||
subject.target_branch = "feature"
|
|
||||||
|
|
||||||
expect(subject.can_remove_source_branch?(user)).to be_falsey
|
expect(subject.can_remove_source_branch?(user)).to be_falsey
|
||||||
end
|
end
|
||||||
|
|
||||||
it "is unable to remove the source branch for a project the user cannot push to" do
|
it "is unable to remove the source branch for a project the user cannot push to" do
|
||||||
|
user2 = create(:user)
|
||||||
|
|
||||||
expect(subject.can_remove_source_branch?(user2)).to be_falsey
|
expect(subject.can_remove_source_branch?(user2)).to be_falsey
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -635,6 +635,7 @@ describe MergeRequest do
|
||||||
end
|
end
|
||||||
|
|
||||||
it "cannot be removed if the last commit is not also the head of the source branch" do
|
it "cannot be removed if the last commit is not also the head of the source branch" do
|
||||||
|
subject.clear_memoized_shas
|
||||||
subject.source_branch = "lfs"
|
subject.source_branch = "lfs"
|
||||||
|
|
||||||
expect(subject.can_remove_source_branch?(user)).to be_falsey
|
expect(subject.can_remove_source_branch?(user)).to be_falsey
|
||||||
|
@ -1405,6 +1406,16 @@ describe MergeRequest do
|
||||||
|
|
||||||
subject.reload_diff
|
subject.reload_diff
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'when using the after_update hook to update' do
|
||||||
|
context 'when the branches are updated' do
|
||||||
|
it 'uses the new heads to generate the diff' do
|
||||||
|
expect { subject.update!(source_branch: subject.target_branch, target_branch: subject.source_branch) }
|
||||||
|
.to change { subject.merge_request_diff.start_commit_sha }
|
||||||
|
.and change { subject.merge_request_diff.head_commit_sha }
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#update_diff_discussion_positions' do
|
describe '#update_diff_discussion_positions' do
|
||||||
|
|
|
@ -1,14 +1,14 @@
|
||||||
require 'spec_helper'
|
require 'spec_helper'
|
||||||
|
|
||||||
describe MergeRequests::MergeService do
|
describe MergeRequests::MergeService do
|
||||||
let(:user) { create(:user) }
|
set(:user) { create(:user) }
|
||||||
let(:user2) { create(:user) }
|
set(:user2) { create(:user) }
|
||||||
let(:merge_request) { create(:merge_request, :simple, author: user2, assignee: user2) }
|
let(:merge_request) { create(:merge_request, :simple, author: user2, assignee: user2) }
|
||||||
let(:project) { merge_request.project }
|
let(:project) { merge_request.project }
|
||||||
|
|
||||||
before do
|
before do
|
||||||
project.team << [user, :master]
|
project.add_master(user)
|
||||||
project.team << [user2, :developer]
|
project.add_developer(user2)
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#execute' do
|
describe '#execute' do
|
||||||
|
|
Loading…
Reference in a new issue