Commit 2b171e66 authored by GitLab Bot's avatar GitLab Bot

Add latest changes from gitlab-org/gitlab@13-0-stable-ee

parent bab5bdce
...@@ -53,10 +53,21 @@ class SnippetRepository < ApplicationRecord ...@@ -53,10 +53,21 @@ class SnippetRepository < ApplicationRecord
def transform_file_entries(files) def transform_file_entries(files)
next_index = get_last_empty_file_index + 1 next_index = get_last_empty_file_index + 1
files.each do |file_entry| files.map do |file_entry|
file_entry[:file_path] = file_path_for(file_entry, next_index) { next_index += 1 } file_entry[:file_path] = file_path_for(file_entry, next_index) { next_index += 1 }
file_entry[:action] = infer_action(file_entry) unless file_entry[:action] file_entry[:action] = infer_action(file_entry) unless file_entry[:action]
file_entry[:action] = file_entry[:action].to_sym
if only_rename_action?(file_entry)
file_entry[:infer_content] = true
elsif empty_update_action?(file_entry)
# There is no need to perform a repository operation
# When the update action has no content
file_entry = nil
end end
file_entry
end.compact
end end
def file_path_for(file_entry, next_index) def file_path_for(file_entry, next_index)
...@@ -111,4 +122,12 @@ class SnippetRepository < ApplicationRecord ...@@ -111,4 +122,12 @@ class SnippetRepository < ApplicationRecord
err.is_a?(ArgumentError) && err.is_a?(ArgumentError) &&
err.message.downcase.match?(/failed to parse signature/) err.message.downcase.match?(/failed to parse signature/)
end end
def only_rename_action?(action)
action[:action] == :move && action[:content].nil?
end
def empty_update_action?(action)
action[:action] == :update && action[:content].nil?
end
end end
...@@ -28,7 +28,7 @@ module Ci ...@@ -28,7 +28,7 @@ module Ci
stop_actions.each do |stop_action| stop_actions.each do |stop_action|
stop_action.play(stop_action.user) stop_action.play(stop_action.user)
rescue => e rescue => e
Gitlab::ErrorTracking.track_error(e, deployable_id: stop_action.id) Gitlab::ErrorTracking.track_exception(e, deployable_id: stop_action.id)
end end
end end
......
...@@ -85,8 +85,10 @@ module Snippets ...@@ -85,8 +85,10 @@ module Snippets
end end
def snippet_files(snippet) def snippet_files(snippet)
[{ previous_path: snippet.file_name_on_repo, file_name_on_repo = snippet.file_name_on_repo
file_path: params[:file_name],
[{ previous_path: file_name_on_repo,
file_path: params[:file_name] || file_name_on_repo,
content: params[:content] }] content: params[:content] }]
end end
......
...@@ -12,7 +12,7 @@ module IncidentManagement ...@@ -12,7 +12,7 @@ module IncidentManagement
return unless project return unless project
new_issue = create_issue(project, alert_payload) new_issue = create_issue(project, alert_payload)
return unless am_alert_id && new_issue.persisted? return unless am_alert_id && new_issue&.persisted?
link_issue_with_alert(am_alert_id, new_issue.id) link_issue_with_alert(am_alert_id, new_issue.id)
end end
...@@ -27,6 +27,7 @@ module IncidentManagement ...@@ -27,6 +27,7 @@ module IncidentManagement
IncidentManagement::CreateIssueService IncidentManagement::CreateIssueService
.new(project, alert_payload) .new(project, alert_payload)
.execute .execute
.dig(:issue)
end end
def link_issue_with_alert(alert_id, issue_id) def link_issue_with_alert(alert_id, issue_id)
......
---
title: Fix ambiguous string concatenation on CleanupProjectsWithMissingNamespace
merge_request: 33497
author:
type: fixed
---
title: Fix NoMethodError by using the correct method to report exceptions to Sentry
merge_request: 33260
author:
type: fixed
---
title: Fix bug in snippets updating only file_name or content
merge_request: 33375
author:
type: fixed
---
title: Fix linking alerts to created issues for the Generic alerts intergration
merge_request: 33647
author:
type: fixed
...@@ -249,8 +249,8 @@ class CleanupProjectsWithMissingNamespace < ActiveRecord::Migration[6.0] ...@@ -249,8 +249,8 @@ class CleanupProjectsWithMissingNamespace < ActiveRecord::Migration[6.0]
-- Names are expected to be unique inside their namespace -- Names are expected to be unique inside their namespace
-- (uniqueness validation on namespace_id, name) -- (uniqueness validation on namespace_id, name)
-- Attach the id to the name and path to make sure that they are unique -- Attach the id to the name and path to make sure that they are unique
name = name || '_' || id, name = name || '_' || id::text,
path = path || '_' || id path = path || '_' || id::text
SQL SQL
end end
end end
......
...@@ -116,18 +116,82 @@ describe SnippetRepository do ...@@ -116,18 +116,82 @@ describe SnippetRepository do
end end
context 'when commit actions are present' do context 'when commit actions are present' do
let(:file_action) { { file_path: 'foo.txt', content: 'foo', action: :foobar } } shared_examples 'uses the expected action' do |action, expected_action|
let(:file_action) { { file_path: 'foo.txt', content: 'foo', action: action } }
let(:data) { [file_action] } let(:data) { [file_action] }
it 'does not change commit action' do specify do
expect(repo).to( expect(repo).to(
receive(:multi_action).with( receive(:multi_action).with(
user, user,
hash_including(actions: array_including(hash_including(action: :foobar))))) hash_including(actions: array_including(hash_including(action: expected_action)))))
snippet_repository.multi_files_action(user, data, commit_opts) snippet_repository.multi_files_action(user, data, commit_opts)
end end
end end
it_behaves_like 'uses the expected action', :foobar, :foobar
context 'when action is a string' do
it_behaves_like 'uses the expected action', 'foobar', :foobar
end
end
end
context 'when move action does not include content' do
let(:previous_path) { 'CHANGELOG' }
let(:new_path) { 'CHANGELOG_new' }
let(:move_action) { { previous_path: previous_path, file_path: new_path, action: action } }
shared_examples 'renames file and does not update content' do
specify do
existing_content = blob_at(snippet, previous_path).data
snippet_repository.multi_files_action(user, [move_action], commit_opts)
blob = blob_at(snippet, new_path)
expect(blob).not_to be_nil
expect(blob.data).to eq existing_content
end
end
context 'when action is not set' do
let(:action) { nil }
it_behaves_like 'renames file and does not update content'
end
context 'when action is set' do
let(:action) { :move }
it_behaves_like 'renames file and does not update content'
end
end
context 'when update action does not include content' do
let(:update_action) { { previous_path: 'CHANGELOG', file_path: 'CHANGELOG', action: action } }
shared_examples 'does not commit anything' do
specify do
last_commit_id = snippet.repository.head_commit.id
snippet_repository.multi_files_action(user, [update_action], commit_opts)
expect(snippet.repository.head_commit.id).to eq last_commit_id
end
end
context 'when action is not set' do
let(:action) { nil }
it_behaves_like 'does not commit anything'
end
context 'when action is set' do
let(:action) { :update }
it_behaves_like 'does not commit anything'
end
end end
shared_examples 'snippet repository with file names' do |*filenames| shared_examples 'snippet repository with file names' do |*filenames|
......
...@@ -222,8 +222,10 @@ describe Ci::StopEnvironmentsService do ...@@ -222,8 +222,10 @@ describe Ci::StopEnvironmentsService do
it 'tracks the exception' do it 'tracks the exception' do
expect(Gitlab::ErrorTracking) expect(Gitlab::ErrorTracking)
.to receive(:track_error) .to receive(:track_exception)
.with(Gitlab::Access::AccessDeniedError, anything).twice .with(Gitlab::Access::AccessDeniedError, anything)
.twice
.and_call_original
subject subject
end end
......
...@@ -302,6 +302,60 @@ describe Snippets::UpdateService do ...@@ -302,6 +302,60 @@ describe Snippets::UpdateService do
end end
end end
shared_examples 'only file_name is present' do
let(:base_opts) do
{
file_name: file_name
}
end
shared_examples 'content is not updated' do
specify do
existing_content = snippet.blobs.first.data
response = subject
snippet = response.payload[:snippet]
blob = snippet.repository.blob_at('master', file_name)
expect(blob).not_to be_nil
expect(response).to be_success
expect(blob.data).to eq existing_content
end
end
context 'when renaming the file_name' do
let(:file_name) { 'new_file_name' }
it_behaves_like 'content is not updated'
end
context 'when file_name does not change' do
let(:file_name) { snippet.blobs.first.path }
it_behaves_like 'content is not updated'
end
end
shared_examples 'only content is present' do
let(:content) { 'new_content' }
let(:base_opts) do
{
content: content
}
end
it 'updates the content' do
response = subject
snippet = response.payload[:snippet]
blob = snippet.repository.blob_at('master', snippet.blobs.first.path)
expect(blob).not_to be_nil
expect(response).to be_success
expect(blob.data).to eq content
end
end
context 'when Project Snippet' do context 'when Project Snippet' do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let!(:snippet) { create(:project_snippet, :repository, author: user, project: project) } let!(:snippet) { create(:project_snippet, :repository, author: user, project: project) }
...@@ -316,6 +370,8 @@ describe Snippets::UpdateService do ...@@ -316,6 +370,8 @@ describe Snippets::UpdateService do
it_behaves_like 'updates repository content' it_behaves_like 'updates repository content'
it_behaves_like 'commit operation fails' it_behaves_like 'commit operation fails'
it_behaves_like 'committable attributes' it_behaves_like 'committable attributes'
it_behaves_like 'only file_name is present'
it_behaves_like 'only content is present'
it_behaves_like 'snippets spam check is performed' do it_behaves_like 'snippets spam check is performed' do
before do before do
subject subject
...@@ -340,6 +396,8 @@ describe Snippets::UpdateService do ...@@ -340,6 +396,8 @@ describe Snippets::UpdateService do
it_behaves_like 'updates repository content' it_behaves_like 'updates repository content'
it_behaves_like 'commit operation fails' it_behaves_like 'commit operation fails'
it_behaves_like 'committable attributes' it_behaves_like 'committable attributes'
it_behaves_like 'only file_name is present'
it_behaves_like 'only content is present'
it_behaves_like 'snippets spam check is performed' do it_behaves_like 'snippets spam check is performed' do
before do before do
subject subject
......
...@@ -7,40 +7,39 @@ describe IncidentManagement::ProcessAlertWorker do ...@@ -7,40 +7,39 @@ describe IncidentManagement::ProcessAlertWorker do
describe '#perform' do describe '#perform' do
let(:alert_management_alert_id) { nil } let(:alert_management_alert_id) { nil }
let(:alert_payload) { { alert: 'payload' } } let(:alert_payload) do
let(:new_issue) { create(:issue, project: project) } {
let(:create_issue_service) { instance_double(IncidentManagement::CreateIssueService, execute: new_issue) } 'annotations' => { 'title' => 'title' },
'startsAt' => Time.now.rfc3339
}
end
let(:created_issue) { Issue.last }
subject { described_class.new.perform(project.id, alert_payload, alert_management_alert_id) } subject { described_class.new.perform(project.id, alert_payload, alert_management_alert_id) }
before do before do
allow(IncidentManagement::CreateIssueService) allow(IncidentManagement::CreateIssueService)
.to receive(:new).with(project, alert_payload) .to receive(:new).with(project, alert_payload)
.and_return(create_issue_service) .and_call_original
end end
it 'calls create issue service' do it 'creates an issue' do
expect(Project).to receive(:find_by_id).and_call_original
expect(IncidentManagement::CreateIssueService) expect(IncidentManagement::CreateIssueService)
.to receive(:new).with(project, alert_payload) .to receive(:new).with(project, alert_payload)
.and_return(create_issue_service)
expect(create_issue_service).to receive(:execute)
subject expect { subject }.to change { Issue.count }.by(1)
end end
context 'with invalid project' do context 'with invalid project' do
let(:invalid_project_id) { 0 } let(:invalid_project_id) { non_existing_record_id }
subject { described_class.new.perform(invalid_project_id, alert_payload) } subject { described_class.new.perform(invalid_project_id, alert_payload) }
it 'does not create issues' do it 'does not create issues' do
expect(Project).to receive(:find_by_id).and_call_original
expect(IncidentManagement::CreateIssueService).not_to receive(:new) expect(IncidentManagement::CreateIssueService).not_to receive(:new)
subject expect { subject }.not_to change { Issue.count }
end end
end end
...@@ -59,7 +58,9 @@ describe IncidentManagement::ProcessAlertWorker do ...@@ -59,7 +58,9 @@ describe IncidentManagement::ProcessAlertWorker do
context 'when alert can be updated' do context 'when alert can be updated' do
it 'updates AlertManagement::Alert#issue_id' do it 'updates AlertManagement::Alert#issue_id' do
expect { subject }.to change { alert.reload.issue_id }.to(new_issue.id) subject
expect(alert.reload.issue_id).to eq(created_issue.id)
end end
it 'does not write a warning to log' do it 'does not write a warning to log' do
...@@ -80,12 +81,12 @@ describe IncidentManagement::ProcessAlertWorker do ...@@ -80,12 +81,12 @@ describe IncidentManagement::ProcessAlertWorker do
expect { subject }.not_to change { alert.reload.issue_id } expect { subject }.not_to change { alert.reload.issue_id }
end end
it 'writes a worning to log' do it 'logs a warning' do
subject subject
expect(Gitlab::AppLogger).to have_received(:warn).with( expect(Gitlab::AppLogger).to have_received(:warn).with(
message: 'Cannot link an Issue with Alert', message: 'Cannot link an Issue with Alert',
issue_id: new_issue.id, issue_id: created_issue.id,
alert_id: alert_management_alert_id, alert_id: alert_management_alert_id,
alert_errors: { hosts: ['hosts array is over 255 chars'] } alert_errors: { hosts: ['hosts array is over 255 chars'] }
) )
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment