diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2017-12-14 13:27:00 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2017-12-14 13:27:00 +0000 |
commit | b0bc676f209257115a16a0ba6fa736a97693c6fe (patch) | |
tree | 2100fa159f1b0eaafb07147da0cfabd99e7676d2 | |
parent | 57175ecb5060dac145abc048b6e88642bc1aa82b (diff) | |
parent | 8ad412559de71f3030fb4665ff1a8ddfb5211824 (diff) | |
download | gitlab-ce-b0bc676f209257115a16a0ba6fa736a97693c6fe.tar.gz |
Merge branch 'zj-mr-diff-memoization-bug' into 'master'
Clear caches before updating MR diffs
See merge request gitlab-org/gitlab-ce!15916
-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 f627ce260a3..c39789b047d 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 98a39c33319..bb63abd167b 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 |