Fix bug in snippets updating only file_name or content

parent 951fd679
......@@ -53,10 +53,21 @@ class SnippetRepository < ApplicationRecord
def transform_file_entries(files)
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[: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
file_entry
end.compact
end
def file_path_for(file_entry, next_index)
......@@ -111,4 +122,12 @@ class SnippetRepository < ApplicationRecord
err.is_a?(ArgumentError) &&
err.message.downcase.match?(/failed to parse signature/)
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
......@@ -112,8 +112,10 @@ module Snippets
end
def build_actions_from_params(snippet)
[{ previous_path: snippet.file_name_on_repo,
file_path: params[:file_name],
file_name_on_repo = snippet.file_name_on_repo
[{ previous_path: file_name_on_repo,
file_path: params[:file_name] || file_name_on_repo,
content: params[:content] }]
end
end
......
---
title: Fix bug in snippets updating only file_name or content
merge_request: 33375
author:
type: fixed
......@@ -116,18 +116,82 @@ describe SnippetRepository do
end
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] }
it 'does not change commit action' do
specify do
expect(repo).to(
receive(:multi_action).with(
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)
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
shared_examples 'snippet repository with file names' do |*filenames|
......
......@@ -378,6 +378,60 @@ describe Snippets::UpdateService do
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
let_it_be(:project) { create(:project) }
let!(:snippet) { create(:project_snippet, :repository, author: user, project: project) }
......@@ -393,6 +447,8 @@ describe Snippets::UpdateService do
it_behaves_like 'commit operation fails'
it_behaves_like 'committable attributes'
it_behaves_like 'when snippet_files param is present'
it_behaves_like 'only file_name is present'
it_behaves_like 'only content is present'
it_behaves_like 'snippets spam check is performed' do
before do
subject
......@@ -418,6 +474,8 @@ describe Snippets::UpdateService do
it_behaves_like 'commit operation fails'
it_behaves_like 'committable attributes'
it_behaves_like 'when snippet_files param is present'
it_behaves_like 'only file_name is present'
it_behaves_like 'only content is present'
it_behaves_like 'snippets spam check is performed' do
before do
subject
......
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