summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Speicher <robert@gitlab.com>2017-09-29 16:26:51 +0000
committerRobert Speicher <robert@gitlab.com>2017-09-29 16:26:51 +0000
commit1ecf48c5d4626e54cbc1b80f7570fcd0c9514055 (patch)
tree83947f5969d2aff8ef195e387c0694e1e6f84f40
parent06a5a331b4be1a9a59fe786c374334c0dbf557c0 (diff)
parent3bdb0f1a0c71b0b1239e754bca3501c1478ee5cd (diff)
downloadgitlab-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
-rw-r--r--app/assets/javascripts/files_comment_button.js6
-rw-r--r--app/models/repository.rb18
-rw-r--r--app/services/merge_requests/merge_service.rb19
-rw-r--r--app/services/merge_requests/post_merge_service.rb1
-rw-r--r--changelogs/unreleased/37467-helper-method-from-users-endpoint-overrides-api-helper-method.yml5
-rw-r--r--changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml5
-rw-r--r--changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml5
-rw-r--r--changelogs/unreleased/commit-side-by-side-comment.yml5
-rw-r--r--changelogs/unreleased/dm-api-unauthorized.yml5
-rw-r--r--changelogs/unreleased/fix-mr-sidebar-counter-after-merge.yml5
-rw-r--r--changelogs/unreleased/zj-repo-gitaly.yml5
-rw-r--r--lib/api/api_guard.rb24
-rw-r--r--lib/api/helpers.rb30
-rw-r--r--lib/api/users.rb4
-rw-r--r--lib/gitlab/git/repository.rb23
-rw-r--r--spec/features/projects/commit/diff_notes_spec.rb36
-rw-r--r--spec/lib/gitlab/git/repository_spec.rb78
-rw-r--r--spec/requests/api/helpers_spec.rb78
-rw-r--r--spec/requests/api/users_spec.rb9
-rw-r--r--spec/services/merge_requests/merge_service_spec.rb23
-rw-r--r--spec/services/merge_requests/post_merge_service_spec.rb11
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