Add presence validation to content and title in snippet rest endpoints

parent 079c18c1
---
title: Add presence validation to content and title in snippet rest endpoints
merge_request: 32522
author:
type: changed
...@@ -55,9 +55,9 @@ module API ...@@ -55,9 +55,9 @@ module API
success Entities::ProjectSnippet success Entities::ProjectSnippet
end end
params do params do
requires :title, type: String, desc: 'The title of the snippet' requires :title, type: String, allow_blank: false, desc: 'The title of the snippet'
requires :file_name, type: String, desc: 'The file name of the snippet' requires :file_name, type: String, desc: 'The file name of the snippet'
optional :content, type: String, allow_blank: false, desc: 'The content of the snippet' requires :content, type: String, allow_blank: false, desc: 'The content of the snippet'
optional :description, type: String, desc: 'The description of a snippet' optional :description, type: String, desc: 'The description of a snippet'
requires :visibility, type: String, requires :visibility, type: String,
values: Gitlab::VisibilityLevel.string_values, values: Gitlab::VisibilityLevel.string_values,
...@@ -84,7 +84,7 @@ module API ...@@ -84,7 +84,7 @@ module API
end end
params do params do
requires :snippet_id, type: Integer, desc: 'The ID of a project snippet' requires :snippet_id, type: Integer, desc: 'The ID of a project snippet'
optional :title, type: String, desc: 'The title of the snippet' optional :title, type: String, allow_blank: false, desc: 'The title of the snippet'
optional :file_name, type: String, desc: 'The file name of the snippet' optional :file_name, type: String, desc: 'The file name of the snippet'
optional :content, type: String, allow_blank: false, desc: 'The content of the snippet' optional :content, type: String, allow_blank: false, desc: 'The content of the snippet'
optional :description, type: String, desc: 'The description of a snippet' optional :description, type: String, desc: 'The description of a snippet'
......
...@@ -65,9 +65,9 @@ module API ...@@ -65,9 +65,9 @@ module API
success Entities::PersonalSnippet success Entities::PersonalSnippet
end end
params do params do
requires :title, type: String, desc: 'The title of a snippet' requires :title, type: String, allow_blank: false, desc: 'The title of a snippet'
requires :file_name, type: String, desc: 'The name of a snippet file' requires :file_name, type: String, desc: 'The name of a snippet file'
requires :content, type: String, desc: 'The content of a snippet' requires :content, type: String, allow_blank: false, desc: 'The content of a snippet'
optional :description, type: String, desc: 'The description of a snippet' optional :description, type: String, desc: 'The description of a snippet'
optional :visibility, type: String, optional :visibility, type: String,
values: Gitlab::VisibilityLevel.string_values, values: Gitlab::VisibilityLevel.string_values,
...@@ -96,9 +96,9 @@ module API ...@@ -96,9 +96,9 @@ module API
end end
params do params do
requires :id, type: Integer, desc: 'The ID of a snippet' requires :id, type: Integer, desc: 'The ID of a snippet'
optional :title, type: String, desc: 'The title of a snippet' optional :title, type: String, allow_blank: false, desc: 'The title of a snippet'
optional :file_name, type: String, desc: 'The name of a snippet file' optional :file_name, type: String, desc: 'The name of a snippet file'
optional :content, type: String, desc: 'The content of a snippet' optional :content, type: String, allow_blank: false, desc: 'The content of a snippet'
optional :description, type: String, desc: 'The description of a snippet' optional :description, type: String, desc: 'The description of a snippet'
optional :visibility, type: String, optional :visibility, type: String,
values: Gitlab::VisibilityLevel.string_values, values: Gitlab::VisibilityLevel.string_values,
......
...@@ -216,12 +216,22 @@ describe API::ProjectSnippets do ...@@ -216,12 +216,22 @@ describe API::ProjectSnippets do
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
it 'returns 400 for empty content field' do it 'returns 400 if content is blank' do
params[:content] = '' params[:content] = ''
post api("/projects/#{project.id}/snippets/", admin), params: params post api("/projects/#{project.id}/snippets/", admin), params: params
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['error']).to eq 'content is empty'
end
it 'returns 400 if title is blank' do
params[:title] = ''
post api("/projects/#{project.id}/snippets/", admin), params: params
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['error']).to eq 'title is empty'
end end
context 'when save fails because the repository could not be created' do context 'when save fails because the repository could not be created' do
...@@ -323,12 +333,19 @@ describe API::ProjectSnippets do ...@@ -323,12 +333,19 @@ describe API::ProjectSnippets do
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
it 'returns 400 for empty content field' do it 'returns 400 if content is blank' do
update_snippet(params: { content: '' }) update_snippet(params: { content: '' })
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
it 'returns 400 if title is blank' do
update_snippet(params: { title: '' })
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['error']).to eq 'title is empty'
end
it_behaves_like 'update with repository actions' do it_behaves_like 'update with repository actions' do
let(:snippet_without_repo) { create(:project_snippet, author: admin, project: project, visibility_level: visibility_level) } let(:snippet_without_repo) { create(:project_snippet, author: admin, project: project, visibility_level: visibility_level) }
end end
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require 'spec_helper' require 'spec_helper'
describe API::Snippets do describe API::Snippets do
let!(:user) { create(:user) } let_it_be(:user) { create(:user) }
describe 'GET /snippets/' do describe 'GET /snippets/' do
it 'returns snippets available' do it 'returns snippets available' do
...@@ -195,7 +195,7 @@ describe API::Snippets do ...@@ -195,7 +195,7 @@ describe API::Snippets do
end end
describe 'POST /snippets/' do describe 'POST /snippets/' do
let(:params) do let(:base_params) do
{ {
title: 'Test Title', title: 'Test Title',
file_name: 'test.rb', file_name: 'test.rb',
...@@ -204,12 +204,14 @@ describe API::Snippets do ...@@ -204,12 +204,14 @@ describe API::Snippets do
visibility: 'public' visibility: 'public'
} }
end end
let(:params) { base_params.merge(extra_params) }
let(:extra_params) { {} }
subject { post api("/snippets/", user), params: params }
shared_examples 'snippet creation' do shared_examples 'snippet creation' do
let(:snippet) { Snippet.find(json_response["id"]) } let(:snippet) { Snippet.find(json_response["id"]) }
subject { post api("/snippets/", user), params: params }
it 'creates a new snippet' do it 'creates a new snippet' do
expect do expect do
subject subject
...@@ -253,7 +255,7 @@ describe API::Snippets do ...@@ -253,7 +255,7 @@ describe API::Snippets do
let(:user) { create(:user, :external) } let(:user) { create(:user, :external) }
it 'does not create a new snippet' do it 'does not create a new snippet' do
post api("/snippets/", user), params: params subject
expect(response).to have_gitlab_http_status(:forbidden) expect(response).to have_gitlab_http_status(:forbidden)
end end
...@@ -262,17 +264,27 @@ describe API::Snippets do ...@@ -262,17 +264,27 @@ describe API::Snippets do
it 'returns 400 for missing parameters' do it 'returns 400 for missing parameters' do
params.delete(:title) params.delete(:title)
post api("/snippets/", user), params: params subject
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
it 'returns 400 for validation errors' do it 'returns 400 if content is blank' do
params[:content] = ''
subject
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['error']).to eq 'content is empty'
end
it 'returns 400 if title is blank' do
params[:title] = '' params[:title] = ''
post api("/snippets/", user), params: params subject
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['error']).to eq 'title is empty'
end end
context 'when save fails because the repository could not be created' do context 'when save fails because the repository could not be created' do
...@@ -283,17 +295,13 @@ describe API::Snippets do ...@@ -283,17 +295,13 @@ describe API::Snippets do
end end
it 'returns 400' do it 'returns 400' do
post api("/snippets/", user), params: params subject
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
end end
context 'when the snippet is spam' do context 'when the snippet is spam' do
def create_snippet(snippet_params = {})
post api('/snippets', user), params: params.merge(snippet_params)
end
before do before do
allow_next_instance_of(Spam::AkismetService) do |instance| allow_next_instance_of(Spam::AkismetService) do |instance|
allow(instance).to receive(:spam?).and_return(true) allow(instance).to receive(:spam?).and_return(true)
...@@ -301,23 +309,25 @@ describe API::Snippets do ...@@ -301,23 +309,25 @@ describe API::Snippets do
end end
context 'when the snippet is private' do context 'when the snippet is private' do
let(:extra_params) { { visibility: 'private' } }
it 'creates the snippet' do it 'creates the snippet' do
expect { create_snippet(visibility: 'private') } expect { subject }.to change { Snippet.count }.by(1)
.to change { Snippet.count }.by(1)
end end
end end
context 'when the snippet is public' do context 'when the snippet is public' do
let(:extra_params) { { visibility: 'public' } }
it 'rejects the shippet' do it 'rejects the shippet' do
expect { create_snippet(visibility: 'public') } expect { subject }.not_to change { Snippet.count }
.not_to change { Snippet.count }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['message']).to eq({ "error" => "Spam detected" }) expect(json_response['message']).to eq({ "error" => "Spam detected" })
end end
it 'creates a spam log' do it 'creates a spam log' do
expect { create_snippet(visibility: 'public') } expect { subject }
.to log_spam(title: 'Test Title', user_id: user.id, noteable_type: 'PersonalSnippet') .to log_spam(title: 'Test Title', user_id: user.id, noteable_type: 'PersonalSnippet')
end end
end end
...@@ -325,8 +335,9 @@ describe API::Snippets do ...@@ -325,8 +335,9 @@ describe API::Snippets do
end end
describe 'PUT /snippets/:id' do describe 'PUT /snippets/:id' do
let_it_be(:other_user) { create(:user) }
let(:visibility_level) { Snippet::PUBLIC } let(:visibility_level) { Snippet::PUBLIC }
let(:other_user) { create(:user) }
let(:snippet) do let(:snippet) do
create(:personal_snippet, :repository, author: user, visibility_level: visibility_level) create(:personal_snippet, :repository, author: user, visibility_level: visibility_level)
end end
...@@ -378,10 +389,18 @@ describe API::Snippets do ...@@ -378,10 +389,18 @@ describe API::Snippets do
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
it 'returns 400 for validation errors' do it 'returns 400 if content is blank' do
update_snippet(params: { content: '' })
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['error']).to eq 'content is empty'
end
it 'returns 400 if title is blank' do
update_snippet(params: { title: '' }) update_snippet(params: { title: '' })
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['error']).to eq 'title is empty'
end end
it_behaves_like 'update with repository actions' do it_behaves_like 'update with repository actions' do
......
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