diff options
author | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2017-12-13 18:14:51 +0100 |
---|---|---|
committer | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2017-12-14 11:22:34 +0100 |
commit | 8ad412559de71f3030fb4665ff1a8ddfb5211824 (patch) | |
tree | 2da9da164c19af2b34151ae09d6dccf2bfdf386c | |
parent | 097b06784432b22844b85ed7c4af5d750ce306aa (diff) | |
download | gitlab-ce-8ad412559de71f3030fb4665ff1a8ddfb5211824.tar.gz |
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.
-rw-r--r-- | app/models/merge_request.rb | 2 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 29 | ||||
-rw-r--r-- | spec/services/merge_requests/merge_service_spec.rb | 8 |
3 files changed, 25 insertions, 14 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 26a3388602a..c76a87fc6a5 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -53,8 +53,8 @@ class MergeRequest < ActiveRecord::Base serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize after_create :ensure_merge_request_diff, unless: :importing? - after_update :reload_diff_if_branch_changed after_update :clear_memoized_shas + after_update :reload_diff_if_branch_changed # When this attribute is true some MR validation is ignored # It allows us to close or modify broken merge requests diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 6b98d013ded..d036fa3d3e9 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -601,30 +601,30 @@ describe MergeRequest do end describe '#can_remove_source_branch?' do - let(:user) { create(:user) } - let(:user2) { create(:user) } + set(:user) { create(:user) } + set(:merge_request) { create(:merge_request, :simple) } - before do - subject.source_project.team << [user, :master] + subject { merge_request } - subject.source_branch = "feature" - subject.target_branch = "master" - subject.save! + before do + subject.source_project.add_master(user) end it "can't be removed when its a protected branch" do allow(ProtectedBranch).to receive(:protected?).and_return(true) + expect(subject.can_remove_source_branch?(user)).to be_falsey end it "can't remove a root ref" do - subject.source_branch = "master" - subject.target_branch = "feature" + subject.update(source_branch: 'master', target_branch: 'feature') expect(subject.can_remove_source_branch?(user)).to be_falsey end 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 end @@ -635,6 +635,7 @@ describe MergeRequest do end 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" expect(subject.can_remove_source_branch?(user)).to be_falsey @@ -1405,6 +1406,16 @@ describe MergeRequest do subject.reload_diff 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 describe '#update_diff_discussion_positions' do diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index f86f1ac2443..c38ddf4612b 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -1,14 +1,14 @@ require 'spec_helper' describe MergeRequests::MergeService do - let(:user) { create(:user) } - let(:user2) { create(:user) } + set(:user) { create(:user) } + set(:user2) { create(:user) } let(:merge_request) { create(:merge_request, :simple, author: user2, assignee: user2) } let(:project) { merge_request.project } before do - project.team << [user, :master] - project.team << [user2, :developer] + project.add_master(user) + project.add_developer(user2) end describe '#execute' do |