diff options
author | Robert Speicher <robert@gitlab.com> | 2017-09-29 16:26:51 +0000 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2017-09-29 16:26:51 +0000 |
commit | 1ecf48c5d4626e54cbc1b80f7570fcd0c9514055 (patch) | |
tree | 83947f5969d2aff8ef195e387c0694e1e6f84f40 | |
parent | 06a5a331b4be1a9a59fe786c374334c0dbf557c0 (diff) | |
parent | 3bdb0f1a0c71b0b1239e754bca3501c1478ee5cd (diff) | |
download | gitlab-ce-1ecf48c5d4626e54cbc1b80f7570fcd0c9514055.tar.gz |
Merge branch '10-0-stable-patch-3' into '10-0-stable'
Prepare 10.0.3 release
See merge request gitlab-org/gitlab-ce!14568
21 files changed, 305 insertions, 90 deletions
diff --git a/app/assets/javascripts/files_comment_button.js b/app/assets/javascripts/files_comment_button.js index d02e4cd5876..a00d29a845a 100644 --- a/app/assets/javascripts/files_comment_button.js +++ b/app/assets/javascripts/files_comment_button.js @@ -7,6 +7,8 @@ * causes reflows, visit https://gist.github.com/paulirish/5d52fb081b3570c81e3a */ +import Cookies from 'js-cookie'; + const LINE_NUMBER_CLASS = 'diff-line-num'; const UNFOLDABLE_LINE_CLASS = 'js-unfold'; const NO_COMMENT_CLASS = 'no-comment-btn'; @@ -27,9 +29,7 @@ export default { this.userCanCreateNote = $diffFile.closest(DIFF_CONTAINER_SELECTOR).data('can-create-note') === ''; } - if (typeof notes !== 'undefined' && !this.isParallelView) { - this.isParallelView = notes.isParallelView && notes.isParallelView(); - } + this.isParallelView = Cookies.get('diff_view') === 'parallel'; if (this.userCanCreateNote) { $diffFile.on('mouseover', LINE_COLUMN_CLASSES, e => this.showButton(this.isParallelView, e)) diff --git a/app/models/repository.rb b/app/models/repository.rb index 6ed33e0c268..0315fed24c5 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -488,13 +488,7 @@ class Repository def exists? return false unless full_path - Gitlab::GitalyClient.migrate(:repository_exists) do |enabled| - if enabled - raw_repository.exists? - else - refs_directory_exists? - end - end + raw_repository.exists? end cache_method :exists? @@ -1123,12 +1117,6 @@ class Repository blob.data end - def refs_directory_exists? - circuit_breaker.perform do - File.exist?(File.join(path_to_repo, 'refs')) - end - end - def cache # TODO: should we use UUIDs here? We could move repositories without clearing this cache @cache ||= RepositoryCache.new(full_path, @project.id) @@ -1186,10 +1174,6 @@ class Repository Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', Gitlab::GlRepository.gl_repository(project, false)) end - def circuit_breaker - @circuit_breaker ||= Gitlab::Git::Storage::CircuitBreaker.for_storage(project.repository_storage) - end - def find_commits_by_message_by_shelling_out(query, ref, path, limit, offset) ref ||= root_ref diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index b2b6c5627fb..505ea7bf5bf 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -14,13 +14,13 @@ module MergeRequests @merge_request = merge_request unless @merge_request.mergeable? - return log_merge_error('Merge request is not mergeable', save_message_on_model: true) + return handle_merge_error(log_message: 'Merge request is not mergeable', save_message_on_model: true) end @source = find_merge_source unless @source - return log_merge_error('No source for merge', save_message_on_model: true) + return handle_merge_error(log_message: 'No source for merge', save_message_on_model: true) end merge_request.in_locked_state do @@ -31,8 +31,7 @@ module MergeRequests end end rescue MergeError => e - clean_merge_jid - log_merge_error(e.message, save_message_on_model: true) + handle_merge_error(log_message: e.message, save_message_on_model: true) end private @@ -80,10 +79,16 @@ module MergeRequests @merge_request.force_remove_source_branch? ? @merge_request.author : current_user end - def log_merge_error(message, save_message_on_model: false) - Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{message}") + # Logs merge error message and cleans `MergeRequest#merge_jid`. + # + def handle_merge_error(log_message:, save_message_on_model: false) + Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{log_message}") - @merge_request.update(merge_error: message) if save_message_on_model + if save_message_on_model + @merge_request.update(merge_error: log_message, merge_jid: nil) + else + clean_merge_jid + end end def merge_request_info diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index 261a8bfa200..b1d6bac4d4a 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -14,6 +14,7 @@ module MergeRequests notification_service.merge_mr(merge_request, current_user) execute_hooks(merge_request, 'merge') invalidate_cache_counts(merge_request, users: merge_request.assignees) + merge_request.update_project_counter_caches end private diff --git a/changelogs/unreleased/37467-helper-method-from-users-endpoint-overrides-api-helper-method.yml b/changelogs/unreleased/37467-helper-method-from-users-endpoint-overrides-api-helper-method.yml new file mode 100644 index 00000000000..1984ec6e81c --- /dev/null +++ b/changelogs/unreleased/37467-helper-method-from-users-endpoint-overrides-api-helper-method.yml @@ -0,0 +1,5 @@ +--- +title: find_user Users helper method no longer overrides find_user API helper method. +merge_request: 14418 +author: +type: fixed diff --git a/changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml b/changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml new file mode 100644 index 00000000000..f3c39827590 --- /dev/null +++ b/changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml @@ -0,0 +1,5 @@ +--- +title: Fix 500 error on merged merge requests when GitLab is restored from a backup +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml b/changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml new file mode 100644 index 00000000000..43dec51029b --- /dev/null +++ b/changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml @@ -0,0 +1,5 @@ +--- +title: Adjust MRs being stuck on "process of being merged" for more than 2 hours +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/commit-side-by-side-comment.yml b/changelogs/unreleased/commit-side-by-side-comment.yml new file mode 100644 index 00000000000..f9bea285a77 --- /dev/null +++ b/changelogs/unreleased/commit-side-by-side-comment.yml @@ -0,0 +1,5 @@ +--- +title: Fixed commenting on side-by-side commit diff +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/dm-api-unauthorized.yml b/changelogs/unreleased/dm-api-unauthorized.yml new file mode 100644 index 00000000000..26b45bd4c40 --- /dev/null +++ b/changelogs/unreleased/dm-api-unauthorized.yml @@ -0,0 +1,5 @@ +--- +title: Make sure API responds with 401 when invalid authentication info is provided +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/fix-mr-sidebar-counter-after-merge.yml b/changelogs/unreleased/fix-mr-sidebar-counter-after-merge.yml new file mode 100644 index 00000000000..22a3efb8b1e --- /dev/null +++ b/changelogs/unreleased/fix-mr-sidebar-counter-after-merge.yml @@ -0,0 +1,5 @@ +--- +title: Fix merge request counter updates after merge +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/zj-repo-gitaly.yml b/changelogs/unreleased/zj-repo-gitaly.yml new file mode 100644 index 00000000000..634f6ba1b8b --- /dev/null +++ b/changelogs/unreleased/zj-repo-gitaly.yml @@ -0,0 +1,5 @@ +--- +title: Gitaly RepositoryExists remains opt-in for all method calls +merge_request: +author: +type: fixed diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index c4c0fdda665..e79f988f549 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -75,7 +75,7 @@ module API raise RevokedError when AccessTokenValidationService::VALID - @current_user = User.find(access_token.resource_owner_id) + User.find(access_token.resource_owner_id) end end @@ -84,11 +84,13 @@ module API return nil unless token_string.present? - find_user_by_authentication_token(token_string) || find_user_by_personal_access_token(token_string, scopes) - end + user = + find_user_by_authentication_token(token_string) || + find_user_by_personal_access_token(token_string, scopes) + + raise UnauthorizedError unless user - def current_user - @current_user + user end private @@ -107,7 +109,16 @@ module API end def find_access_token - @access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods) + return @access_token if defined?(@access_token) + + token = Doorkeeper::OAuth::Token.from_request(doorkeeper_request, *Doorkeeper.configuration.access_token_methods) + return @access_token = nil unless token + + @access_token = Doorkeeper::AccessToken.by_token(token) + raise UnauthorizedError unless @access_token + + @access_token.revoke_previous_refresh_token! + @access_token end def doorkeeper_request @@ -169,6 +180,7 @@ module API TokenNotFoundError = Class.new(StandardError) ExpiredError = Class.new(StandardError) RevokedError = Class.new(StandardError) + UnauthorizedError = Class.new(StandardError) class InsufficientScopeError < StandardError attr_reader :scopes diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 00dbc2aee7a..1e8475ba3ec 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -3,6 +3,8 @@ module API include Gitlab::Utils include Helpers::Pagination + UnauthorizedError = Class.new(StandardError) + SUDO_HEADER = "HTTP_SUDO".freeze SUDO_PARAM = :sudo @@ -139,7 +141,7 @@ module API end def authenticate! - unauthorized! unless current_user && can?(initial_current_user, :access_api) + unauthorized! unless current_user end def authenticate_non_get! @@ -397,19 +399,27 @@ module API def initial_current_user return @initial_current_user if defined?(@initial_current_user) - Gitlab::Auth::UniqueIpsLimiter.limit_user! do - @initial_current_user ||= find_user_by_private_token(scopes: scopes_registered_for_endpoint) - @initial_current_user ||= doorkeeper_guard(scopes: scopes_registered_for_endpoint) - @initial_current_user ||= find_user_from_warden - - unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed? - @initial_current_user = nil - end - @initial_current_user + begin + @initial_current_user = Gitlab::Auth::UniqueIpsLimiter.limit_user! { find_current_user } + rescue APIGuard::UnauthorizedError, UnauthorizedError + unauthorized! end end + def find_current_user + user = + find_user_by_private_token(scopes: scopes_registered_for_endpoint) || + doorkeeper_guard(scopes: scopes_registered_for_endpoint) || + find_user_from_warden + + return nil unless user + + raise UnauthorizedError unless Gitlab::UserAccess.new(user).allowed? && user.can?(:access_api) + + user + end + def sudo! return unless sudo_identifier return unless initial_current_user diff --git a/lib/api/users.rb b/lib/api/users.rb index bdebda58d3f..77ac24ec68d 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -11,7 +11,7 @@ module API end helpers do - def find_user(params) + def find_user_by_id(params) id = params[:user_id] || params[:id] User.find_by(id: id) || not_found!('User') end @@ -430,7 +430,7 @@ module API resource :impersonation_tokens do helpers do def finder(options = {}) - user = find_user(params) + user = find_user_by_id(params) PersonalAccessTokensFinder.new({ user: user, impersonation: true }.merge(options)) end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index efa13590a2c..6c639f286ee 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -72,8 +72,6 @@ module Gitlab delegate :empty?, to: :rugged - delegate :exists?, to: :gitaly_repository_client - def ==(other) path == other.path end @@ -101,6 +99,18 @@ module Gitlab @circuit_breaker ||= Gitlab::Git::Storage::CircuitBreaker.for_storage(storage) end + def exists? + Gitlab::GitalyClient.migrate(:repository_exists) do |enabled| + if enabled + gitaly_repository_client.exists? + else + circuit_breaker.perform do + File.exist?(File.join(@path, 'refs')) + end + end + end + end + # Returns an Array of branch names # sorted by name ASC def branch_names @@ -805,7 +815,11 @@ module Gitlab if start_repository == self yield commit(start_branch_name) else - sha = start_repository.commit(start_branch_name).sha + start_commit = start_repository.commit(start_branch_name) + + return yield nil unless start_commit + + sha = start_commit.sha if branch_commit = commit(sha) yield branch_commit @@ -834,8 +848,9 @@ module Gitlab with_repo_branch_commit(source_repository, source_branch) do |commit| if commit write_ref(local_ref, commit.sha) + true else - raise Rugged::ReferenceError, 'source repository is empty' + false end end end diff --git a/spec/features/projects/commit/diff_notes_spec.rb b/spec/features/projects/commit/diff_notes_spec.rb new file mode 100644 index 00000000000..f0fe4e00acc --- /dev/null +++ b/spec/features/projects/commit/diff_notes_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +feature 'Commit diff', :js do + include RepoHelpers + + let(:user) { create(:user) } + let(:project) { create(:project, :public, :repository) } + + before do + project.add_master(user) + sign_in user + end + + %w(inline parallel).each do |view| + context "#{view} view" do + before do + visit project_commit_path(project, sample_commit.id, view: view) + end + + it "adds comment to diff" do + diff_line_num = first('.diff-line-num.new') + + diff_line_num.trigger('mouseover') + diff_line_num.find('.js-add-diff-note-button').trigger('click') + + page.within(first('.diff-viewer')) do + find('.js-note-text').set 'test comment' + + click_button 'Comment' + + expect(page).to have_content('test comment') + end + end + end + end +end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 556a148c3bc..6c014fac2b0 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1332,6 +1332,84 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + describe '#with_repo_branch_commit' do + context 'when comparing with the same repository' do + let(:start_repository) { repository } + + context 'when the branch exists' do + let(:start_branch_name) { 'master' } + + it 'yields the commit' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(an_instance_of(Gitlab::Git::Commit)) + end + end + + context 'when the branch does not exist' do + let(:start_branch_name) { 'definitely-not-master' } + + it 'yields nil' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(nil) + end + end + end + + context 'when comparing with another repository' do + let(:start_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + + context 'when the branch exists' do + let(:start_branch_name) { 'master' } + + it 'yields the commit' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(an_instance_of(Gitlab::Git::Commit)) + end + end + + context 'when the branch does not exist' do + let(:start_branch_name) { 'definitely-not-master' } + + it 'yields nil' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(nil) + end + end + end + end + + describe '#fetch_source_branch' do + let(:local_ref) { 'refs/merge-requests/1/head' } + + context 'when the branch exists' do + let(:source_branch) { 'master' } + + it 'writes the ref' do + expect(repository).to receive(:write_ref).with(local_ref, /\h{40}/) + + repository.fetch_source_branch(repository, source_branch, local_ref) + end + + it 'returns true' do + expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(true) + end + end + + context 'when the branch does not exist' do + let(:source_branch) { 'definitely-not-master' } + + it 'does not write the ref' do + expect(repository).not_to receive(:write_ref) + + repository.fetch_source_branch(repository, source_branch, local_ref) + end + + it 'returns false' do + expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(false) + end + end + end + def create_remote_branch(repository, remote_name, branch_name, source_branch_name) source_branch = repository.branches.find { |branch| branch.name == source_branch_name } rugged = repository.rugged diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index d4006fe71a2..98c49d3364c 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -159,18 +159,25 @@ describe API::Helpers do end describe "when authenticating using a user's private token" do - it "returns nil for an invalid token" do + it "returns a 401 response for an invalid token" do env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token' allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { false } - expect(current_user).to be_nil + expect { current_user }.to raise_error /401/ end - it "returns nil for a user without access" do + it "returns a 401 response for a user without access" do env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user.private_token allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false) - expect(current_user).to be_nil + expect { current_user }.to raise_error /401/ + end + + it 'returns a 401 response for a user who is blocked' do + user.block! + env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user.private_token + + expect { current_user }.to raise_error /401/ end it "leaves user as is when sudo not specified" do @@ -193,24 +200,31 @@ describe API::Helpers do allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { false } end - it "returns nil for an invalid token" do + it "returns a 401 response for an invalid token" do env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token' - expect(current_user).to be_nil + expect { current_user }.to raise_error /401/ end - it "returns nil for a user without access" do + it "returns a 401 response for a user without access" do env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false) - expect(current_user).to be_nil + expect { current_user }.to raise_error /401/ + end + + it 'returns a 401 response for a user who is blocked' do + user.block! + env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token + + expect { current_user }.to raise_error /401/ end - it "returns nil for a token without the appropriate scope" do + it "returns a 401 response for a token without the appropriate scope" do personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user']) env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token - expect(current_user).to be_nil + expect { current_user }.to raise_error /401/ end it "leaves user as is when sudo not specified" do @@ -226,14 +240,14 @@ describe API::Helpers do personal_access_token.revoke! env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token - expect(current_user).to be_nil + expect { current_user }.to raise_error /401/ end it 'does not allow expired tokens' do personal_access_token.update_attributes!(expires_at: 1.day.ago) env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token - expect(current_user).to be_nil + expect { current_user }.to raise_error /401/ end end @@ -351,6 +365,18 @@ describe API::Helpers do end end end + + context 'when user is blocked' do + before do + user.block! + end + + it 'changes current_user to sudo' do + set_env(admin, user.id) + + expect(current_user).to eq(user) + end + end end context 'with regular user' do @@ -490,11 +516,10 @@ describe API::Helpers do context 'current_user is nil' do before do expect_any_instance_of(self.class).to receive(:current_user).and_return(nil) - allow_any_instance_of(self.class).to receive(:initial_current_user).and_return(nil) end it 'returns a 401 response' do - expect { authenticate! }.to raise_error '401 - {"message"=>"401 Unauthorized"}' + expect { authenticate! }.to raise_error /401/ end end @@ -502,35 +527,12 @@ describe API::Helpers do let(:user) { build(:user) } before do - expect_any_instance_of(self.class).to receive(:current_user).at_least(:once).and_return(user) - expect_any_instance_of(self.class).to receive(:initial_current_user).and_return(user) + expect_any_instance_of(self.class).to receive(:current_user).and_return(user) end it 'does not raise an error' do expect { authenticate! }.not_to raise_error end end - - context 'current_user is blocked' do - let(:user) { build(:user, :blocked) } - - before do - expect_any_instance_of(self.class).to receive(:current_user).at_least(:once).and_return(user) - end - - it 'raises an error' do - expect_any_instance_of(self.class).to receive(:initial_current_user).and_return(user) - - expect { authenticate! }.to raise_error '401 - {"message"=>"401 Unauthorized"}' - end - - it "doesn't raise an error if an admin user is impersonating a blocked user (via sudo)" do - admin_user = build(:user, :admin) - - expect_any_instance_of(self.class).to receive(:initial_current_user).and_return(admin_user) - - expect { authenticate! }.not_to raise_error - end - end end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 37cb95a16e3..53552dcd67a 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -128,6 +128,15 @@ describe API::Users do end context "when admin" do + context 'when sudo is defined' do + it 'does not return 500' do + admin_personal_access_token = create(:personal_access_token, user: admin).token + get api("/users?private_token=#{admin_personal_access_token}&sudo=#{user.id}", admin) + + expect(response).to have_http_status(:success) + end + end + it "returns an array of users" do get api("/users", admin) diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index b60136064b7..80213d093f1 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -13,20 +13,21 @@ describe MergeRequests::MergeService do describe '#execute' do context 'MergeRequest#merge_jid' do + let(:service) do + described_class.new(project, user, commit_message: 'Awesome message') + end + before do merge_request.update_column(:merge_jid, 'hash-123') end it 'is cleaned when no error is raised' do - service = described_class.new(project, user, commit_message: 'Awesome message') - service.execute(merge_request) expect(merge_request.reload.merge_jid).to be_nil end it 'is cleaned when expected error is raised' do - service = described_class.new(project, user, commit_message: 'Awesome message') allow(service).to receive(:commit).and_raise(described_class::MergeError) service.execute(merge_request) @@ -34,6 +35,22 @@ describe MergeRequests::MergeService do expect(merge_request.reload.merge_jid).to be_nil end + it 'is cleaned when merge request is not mergeable' do + allow(merge_request).to receive(:mergeable?).and_return(false) + + service.execute(merge_request) + + expect(merge_request.reload.merge_jid).to be_nil + end + + it 'is cleaned when no source is found' do + allow(merge_request).to receive(:diff_head_sha).and_return(nil) + + service.execute(merge_request) + + expect(merge_request.reload.merge_jid).to be_nil + end + it 'is not cleaned when unexpected error is raised' do service = described_class.new(project, user, commit_message: 'Awesome message') allow(service).to receive(:commit).and_raise(StandardError) diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index a37cdab8928..d2bd05d921f 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -11,5 +11,16 @@ describe MergeRequests::PostMergeService do describe '#execute' do it_behaves_like 'cache counters invalidator' + + it 'refreshes the number of open merge requests for a valid MR', :use_clean_rails_memory_store_caching do + # Cache the counter before the MR changed state. + project.open_merge_requests_count + merge_request.update!(state: 'merged') + + service = described_class.new(project, user, {}) + + expect { service.execute(merge_request) } + .to change { project.open_merge_requests_count }.from(1).to(0) + end end end |