diff options
author | LUKE BENNETT <lbennett@gitlab.com> | 2018-01-02 15:09:51 +0000 |
---|---|---|
committer | LUKE BENNETT <lbennett@gitlab.com> | 2018-01-02 15:09:51 +0000 |
commit | daf88e02c76e59f04f322663e2df2d9b18959b78 (patch) | |
tree | 6be04271609791c1004c940b9542440f2849b0cc | |
parent | 5fbc5f2fc0b044e4ebdf5ae744bc2bf752c2e874 (diff) | |
parent | f5c2ad7fd86a5801aad4c2fb58104d212e0ae6a1 (diff) | |
download | gitlab-ce-daf88e02c76e59f04f322663e2df2d9b18959b78.tar.gz |
Merge branch '10-3-stable-patch-3' into '10-3-stable'
Prepare 10.3.3 release
See merge request gitlab-org/gitlab-ce!16163
-rw-r--r-- | app/models/diff_discussion.rb | 9 | ||||
-rw-r--r-- | app/views/shared/_recaptcha_form.html.haml | 3 | ||||
-rw-r--r-- | changelogs/unreleased/41492-mr-comment-fix.yml | 5 | ||||
-rw-r--r-- | changelogs/unreleased/sh-fix-spam-update-404.yml | 5 | ||||
-rw-r--r-- | changelogs/unreleased/sh-handle-orphaned-deploy-keys.yml | 5 | ||||
-rw-r--r-- | lib/api/internal.rb | 9 | ||||
-rw-r--r-- | lib/gitlab/git/repository.rb | 4 | ||||
-rw-r--r-- | spec/models/diff_discussion_spec.rb | 48 | ||||
-rw-r--r-- | spec/requests/api/internal_spec.rb | 10 | ||||
-rw-r--r-- | spec/services/system_note_service_spec.rb | 32 |
10 files changed, 108 insertions, 22 deletions
diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb index d67b16584a4..bd6af622bfb 100644 --- a/app/models/diff_discussion.rb +++ b/app/models/diff_discussion.rb @@ -23,8 +23,13 @@ class DiffDiscussion < Discussion def merge_request_version_params return unless for_merge_request? + version_params = get_params + + return version_params unless on_merge_request_commit? && commit_id + + version_params ||= {} version_params.tap do |params| - params[:commit_id] = commit_id if on_merge_request_commit? + params[:commit_id] = commit_id end end @@ -37,7 +42,7 @@ class DiffDiscussion < Discussion private - def version_params + def get_params return {} if active? noteable.version_params_for(position.diff_refs) diff --git a/app/views/shared/_recaptcha_form.html.haml b/app/views/shared/_recaptcha_form.html.haml index 0e816870f15..93a4301f366 100644 --- a/app/views/shared/_recaptcha_form.html.haml +++ b/app/views/shared/_recaptcha_form.html.haml @@ -1,9 +1,10 @@ - resource_name = spammable.class.model_name.singular - humanized_resource_name = spammable.class.model_name.human.downcase - script = local_assigns.fetch(:script, true) +- method = params[:action] == 'create' ? :post : :put - has_submit = local_assigns.fetch(:has_submit, true) -= form_for resource_name, method: :post, html: { class: 'recaptcha-form js-recaptcha-form' } do |f| += form_for resource_name, method: method, html: { class: 'recaptcha-form js-recaptcha-form' } do |f| .recaptcha - params[resource_name].each do |field, value| = hidden_field(resource_name, field, value: value) diff --git a/changelogs/unreleased/41492-mr-comment-fix.yml b/changelogs/unreleased/41492-mr-comment-fix.yml new file mode 100644 index 00000000000..45ddc16aad0 --- /dev/null +++ b/changelogs/unreleased/41492-mr-comment-fix.yml @@ -0,0 +1,5 @@ +--- +title: Fix links to old commits in merge request comments +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/sh-fix-spam-update-404.yml b/changelogs/unreleased/sh-fix-spam-update-404.yml new file mode 100644 index 00000000000..13daec35ecf --- /dev/null +++ b/changelogs/unreleased/sh-fix-spam-update-404.yml @@ -0,0 +1,5 @@ +--- +title: Fix 404 errors after a user edits an issue description and solves the reCAPTCHA +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/sh-handle-orphaned-deploy-keys.yml b/changelogs/unreleased/sh-handle-orphaned-deploy-keys.yml new file mode 100644 index 00000000000..7d3b622534e --- /dev/null +++ b/changelogs/unreleased/sh-handle-orphaned-deploy-keys.yml @@ -0,0 +1,5 @@ +--- +title: Gracefully handle orphaned write deploy keys in /internal/post_receive +merge_request: +author: +type: fixed diff --git a/lib/api/internal.rb b/lib/api/internal.rb index ccaaeca10d4..79b302aae70 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -190,9 +190,12 @@ module API project = Gitlab::GlRepository.parse(params[:gl_repository]).first user = identify(params[:identifier]) - redirect_message = Gitlab::Checks::ProjectMoved.fetch_redirect_message(user.id, project.id) - if redirect_message - output[:redirected_message] = redirect_message + + # A user is not guaranteed to be returned; an orphaned write deploy + # key could be used + if user + redirect_message = Gitlab::Checks::ProjectMoved.fetch_redirect_message(user.id, project.id) + output[:redirected_message] = redirect_message if redirect_message end output diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index fbdd646ea13..da67683fd15 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -104,7 +104,7 @@ module Gitlab end def exists? - Gitlab::GitalyClient.migrate(:repository_exists, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |enabled| + Gitlab::GitalyClient.migrate(:repository_exists) do |enabled| if enabled gitaly_repository_client.exists? else @@ -166,7 +166,7 @@ module Gitlab end def local_branches(sort_by: nil) - gitaly_migrate(:local_branches, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| + gitaly_migrate(:local_branches) do |is_enabled| if is_enabled gitaly_ref_client.local_branches(sort_by: sort_by) else diff --git a/spec/models/diff_discussion_spec.rb b/spec/models/diff_discussion_spec.rb index fa02434b0fd..50b19000799 100644 --- a/spec/models/diff_discussion_spec.rb +++ b/spec/models/diff_discussion_spec.rb @@ -47,8 +47,20 @@ describe DiffDiscussion do diff_note.save! end - it 'returns the diff ID for the version to show' do - expect(subject.merge_request_version_params).to eq(diff_id: merge_request_diff1.id) + context 'when commit_id is not present' do + it 'returns the diff ID for the version to show' do + expect(subject.merge_request_version_params).to eq(diff_id: merge_request_diff1.id) + end + end + + context 'when commit_id is present' do + before do + diff_note.update_attribute(:commit_id, 'commit_123') + end + + it 'includes the commit_id in the result' do + expect(subject.merge_request_version_params).to eq(diff_id: merge_request_diff1.id, commit_id: 'commit_123') + end end end @@ -70,8 +82,20 @@ describe DiffDiscussion do diff_note.save! end - it 'returns the diff ID and start sha of the versions to compare' do - expect(subject.merge_request_version_params).to eq(diff_id: merge_request_diff3.id, start_sha: merge_request_diff1.head_commit_sha) + context 'when commit_id is not present' do + it 'returns the diff ID and start sha of the versions to compare' do + expect(subject.merge_request_version_params).to eq(diff_id: merge_request_diff3.id, start_sha: merge_request_diff1.head_commit_sha) + end + end + + context 'when commit_id is present' do + before do + diff_note.update_attribute(:commit_id, 'commit_123') + end + + it 'includes the commit_id in the result' do + expect(subject.merge_request_version_params).to eq(diff_id: merge_request_diff3.id, start_sha: merge_request_diff1.head_commit_sha, commit_id: 'commit_123') + end end end @@ -83,8 +107,20 @@ describe DiffDiscussion do diff_note.save! end - it 'returns nil' do - expect(subject.merge_request_version_params).to be_nil + context 'when commit_id is not present' do + it 'returns empty hash' do + expect(subject.merge_request_version_params).to eq(nil) + end + end + + context 'when commit_id is present' do + before do + diff_note.update_attribute(:commit_id, 'commit_123') + end + + it 'returns the commit_id' do + expect(subject.merge_request_version_params).to eq(commit_id: 'commit_123') + end end end end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 3c31980b273..c352285b1bf 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -784,6 +784,16 @@ describe API::Internal do expect(json_response["redirected_message"]).to eq(project_moved.redirect_message) end end + + context 'with an orphaned write deploy key' do + it 'does not try to notify that project moved' do + allow_any_instance_of(Gitlab::Identifier).to receive(:identify).and_return(nil) + + post api("/internal/post_receive"), valid_params + + expect(response).to have_gitlab_http_status(200) + end + end end describe 'POST /internal/pre_receive' do diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 47412110b4b..17f4136c2b0 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe SystemNoteService do include Gitlab::Routing + include RepoHelpers set(:group) { create(:group) } set(:project) { create(:project, :repository, group: group) } @@ -1070,17 +1071,32 @@ describe SystemNoteService do let(:action) { 'outdated' } end - it 'creates a new note in the discussion' do - # we need to completely rebuild the merge request object, or the `@discussions` on the merge request are not reloaded. - expect { subject }.to change { reloaded_merge_request.discussions.first.notes.size }.by(1) + context 'when the change_position is valid for the discussion' do + it 'creates a new note in the discussion' do + # we need to completely rebuild the merge request object, or the `@discussions` on the merge request are not reloaded. + expect { subject }.to change { reloaded_merge_request.discussions.first.notes.size }.by(1) + end + + it 'links to the diff in the system note' do + expect(subject.note).to include('version 1') + + diff_id = merge_request.merge_request_diff.id + line_code = change_position.line_code(project.repository) + expect(subject.note).to include(diffs_project_merge_request_url(project, merge_request, diff_id: diff_id, anchor: line_code)) + end end - it 'links to the diff in the system note' do - expect(subject.note).to include('version 1') + context 'when the change_position is invalid for the discussion' do + let(:change_position) { project.commit(sample_commit.id) } - diff_id = merge_request.merge_request_diff.id - line_code = change_position.line_code(project.repository) - expect(subject.note).to include(diffs_project_merge_request_url(project, merge_request, diff_id: diff_id, anchor: line_code)) + it 'creates a new note in the discussion' do + # we need to completely rebuild the merge request object, or the `@discussions` on the merge request are not reloaded. + expect { subject }.to change { reloaded_merge_request.discussions.first.notes.size }.by(1) + end + + it 'does not create a link' do + expect(subject.note).to eq('changed this line in version 1 of the diff') + end end end |