Skip to content

G
gitlab-ce
Commit 187a3a0d authored by Fabio Pitino's avatar Fabio Pitino
Browse files
Options

Merge branch 'vij-snippet-controller-cleanup' into 'master' 

Clean up Snippet controllers

See merge request gitlab-org/gitlab!44718
parents 2d10a4da b91a5b12
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 29 additions and 662 deletions
app/controllers/concerns/snippets_actions.rb
View file @ 187a3a0d
......@@ -17,13 +17,7 @@ module SnippetsActions
respond_to :html
end
def edit
# We need to load some info from the existing blob
snippet.content = blob.data
snippet.file_name = blob.path
render 'edit'
end
def edit; end
# This endpoint is being replaced by Snippets::BlobController#raw
# Support for old raw links will be maintainted via this action but
......@@ -55,7 +49,6 @@ module SnippetsActions
def show
respond_to do |format|
format.html do
conditionally_expand_blob(blob)
@note = Note.new(noteable: @snippet, project: @snippet.project)
@noteable = @snippet
......@@ -80,29 +73,6 @@ module SnippetsActions
end
end
end
def update
update_params = snippet_params.merge(spammable_params)
service_response = Snippets::UpdateService.new(@snippet.project, current_user, update_params).execute(@snippet)
@snippet = service_response.payload[:snippet]
handle_repository_error(:edit)
end
def destroy
service_response = Snippets::DestroyService.new(current_user, @snippet).execute
if service_response.success?
redirect_to gitlab_dashboard_snippets_path(@snippet), status: :found
elsif service_response.http_status == 403
access_denied!
else
redirect_to gitlab_snippet_path(@snippet),
status: :found,
alert: service_response.message
end
end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
private
......@@ -124,12 +94,4 @@ module SnippetsActions
def convert_line_endings(content)
params[:line_ending] == 'raw' ? content : content.gsub(/\r\n/, "\n")
end
def handle_repository_error(action)
errors = Array(snippet.errors.delete(:repository))
flash.now[:alert] = errors.first if errors.present?
recaptcha_check_with_fallback(errors.empty?) { render action }
end
end
app/controllers/projects/snippets_controller.rb
View file @ 187a3a0d
......@@ -7,12 +7,11 @@ class Projects::SnippetsController < Projects::Snippets::ApplicationController
before_action :check_snippets_available!
before_action :snippet, only: [:show, :edit, :destroy, :update, :raw, :toggle_award_emoji, :mark_as_spam]
before_action :snippet, only: [:show, :edit, :raw, :toggle_award_emoji, :mark_as_spam]
before_action :authorize_create_snippet!, only: [:new, :create]
before_action :authorize_read_snippet!, except: [:new, :create, :index]
before_action :authorize_update_snippet!, only: [:edit, :update]
before_action :authorize_admin_snippet!, only: [:destroy]
before_action :authorize_create_snippet!, only: :new
before_action :authorize_read_snippet!, except: [:new, :index]
before_action :authorize_update_snippet!, only: :edit
def index
@snippet_counts = ::Snippets::CountService
......@@ -33,14 +32,6 @@ class Projects::SnippetsController < Projects::Snippets::ApplicationController
@snippet = @noteable = @project.snippets.build
end
def create
create_params = snippet_params.merge(spammable_params)
service_response = ::Snippets::CreateService.new(project, current_user, create_params).execute
@snippet = service_response.payload[:snippet]
handle_repository_error(:new)
end
protected
alias_method :awardable, :snippet
......@@ -49,8 +40,4 @@ class Projects::SnippetsController < Projects::Snippets::ApplicationController
def spammable_path
project_snippet_path(@project, @snippet)
end
def snippet_params
params.require(:project_snippet).permit(:title, :content, :file_name, :private, :visibility_level, :description)
end
end
app/controllers/snippets_controller.rb
View file @ 187a3a0d
......@@ -6,12 +6,11 @@ class SnippetsController < Snippets::ApplicationController
include ToggleAwardEmoji
include SpammableActions
before_action :snippet, only: [:show, :edit, :destroy, :update, :raw, :toggle_award_emoji, :mark_as_spam]
before_action :snippet, only: [:show, :edit, :raw, :toggle_award_emoji, :mark_as_spam]
before_action :authorize_create_snippet!, only: [:new, :create]
before_action :authorize_create_snippet!, only: :new
before_action :authorize_read_snippet!, only: [:show, :raw]
before_action :authorize_update_snippet!, only: [:edit, :update]
before_action :authorize_admin_snippet!, only: [:destroy]
before_action :authorize_update_snippet!, only: :edit
skip_before_action :authenticate_user!, only: [:index, :show, :raw]
......@@ -40,18 +39,6 @@ class SnippetsController < Snippets::ApplicationController
@snippet = PersonalSnippet.new
end
def create
create_params = snippet_params.merge(files: params.delete(:files))
service_response = Snippets::CreateService.new(nil, current_user, create_params).execute
@snippet = service_response.payload[:snippet]
if service_response.error? && @snippet.errors[:repository].present?
handle_repository_error(:new)
else
recaptcha_check_with_fallback { render :new }
end
end
protected
alias_method :awardable, :snippet
......@@ -60,8 +47,4 @@ class SnippetsController < Snippets::ApplicationController
def spammable_path
snippet_path(@snippet)
end
def snippet_params
params.require(:personal_snippet).permit(:title, :content, :file_name, :private, :visibility_level, :description).merge(spammable_params)
end
end
config/routes/project.rb
View file @ 187a3a0d
......@@ -368,7 +368,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
resource :jira, only: [:show], controller: :jira
end
resources :snippets, concerns: :awardable, constraints: { id: /\d+/ } do
resources :snippets, except: [:create, :update, :destroy], concerns: :awardable, constraints: { id: /\d+/ } do
member do
get :raw
post :mark_as_spam
......
config/routes/snippets.rb
View file @ 187a3a0d
resources :snippets, concerns: :awardable do
resources :snippets, except: [:create, :update, :destroy], concerns: :awardable, constraints: { id: /\d+/ } do
member do
get :raw
post :mark_as_spam
......
spec/controllers/projects/snippets_controller_spec.rb
View file @ 187a3a0d
......@@ -82,215 +82,6 @@ RSpec.describe Projects::SnippetsController do
end
end
describe 'POST #create' do
def create_snippet(project, snippet_params = {}, additional_params = {})
sign_in(user)
project.add_developer(user)
post :create, params: {
namespace_id: project.namespace.to_param,
project_id: project,
project_snippet: { title: 'Title', content: 'Content', description: 'Description' }.merge(snippet_params)
}.merge(additional_params)
Snippet.last
end
it 'creates the snippet correctly' do
snippet = create_snippet(project, visibility_level: Snippet::PRIVATE)
expect(snippet.title).to eq('Title')
expect(snippet.content).to eq('Content')
expect(snippet.description).to eq('Description')
end
context 'when the snippet is spam' do
before do
allow_next_instance_of(Spam::AkismetService) do |instance|
allow(instance).to receive(:spam?).and_return(true)
end
end
context 'when the snippet is private' do
it 'creates the snippet' do
expect { create_snippet(project, visibility_level: Snippet::PRIVATE) }
.to change { Snippet.count }.by(1)
end
end
context 'when the snippet is public' do
it 'rejects the snippet' do
expect { create_snippet(project, visibility_level: Snippet::PUBLIC) }
.not_to change { Snippet.count }
expect(response).to render_template(:new)
end
it 'creates a spam log' do
expect { create_snippet(project, visibility_level: Snippet::PUBLIC) }
.to log_spam(title: 'Title', user_id: user.id, noteable_type: 'ProjectSnippet')
end
it 'renders :new with reCAPTCHA disabled' do
stub_application_setting(recaptcha_enabled: false)
create_snippet(project, visibility_level: Snippet::PUBLIC)
expect(response).to render_template(:new)
end
context 'reCAPTCHA enabled' do
before do
stub_application_setting(recaptcha_enabled: true)
end
it 'renders :verify with reCAPTCHA enabled' do
create_snippet(project, visibility_level: Snippet::PUBLIC)
expect(response).to render_template(:verify)
end
it 'renders snippet page when reCAPTCHA verified' do
spammy_title = 'Whatever'
spam_logs = create_list(:spam_log, 2, user: user, title: spammy_title)
create_snippet(project,
{ visibility_level: Snippet::PUBLIC },
{ spam_log_id: spam_logs.last.id,
recaptcha_verification: true })
expect(response).to redirect_to(project_snippet_path(project, Snippet.last))
end
end
end
end
end
describe 'PUT #update' do
let(:visibility_level) { Snippet::PUBLIC }
let(:snippet) { create :project_snippet, author: user, project: project, visibility_level: visibility_level }
def update_snippet(snippet_params = {}, additional_params = {})
sign_in(user)
project.add_developer(user)
put :update, params: {
namespace_id: project.namespace.to_param,
project_id: project,
id: snippet,
project_snippet: { title: 'Title', content: 'Content' }.merge(snippet_params)
}.merge(additional_params)
snippet.reload
end
context 'when the snippet is spam' do
before do
allow_next_instance_of(Spam::AkismetService) do |instance|
allow(instance).to receive(:spam?).and_return(true)
end
end
context 'when the snippet is private' do
let(:visibility_level) { Snippet::PRIVATE }
it 'updates the snippet' do
expect { update_snippet(title: 'Foo') }
.to change { snippet.reload.title }.to('Foo')
end
end
context 'when the snippet is public' do
it 'rejects the snippet' do
expect { update_snippet(title: 'Foo') }
.not_to change { snippet.reload.title }
end
it 'creates a spam log' do
expect { update_snippet(title: 'Foo') }
.to log_spam(title: 'Foo', user_id: user.id, noteable_type: 'ProjectSnippet')
end
it 'renders :edit with reCAPTCHA disabled' do
stub_application_setting(recaptcha_enabled: false)
update_snippet(title: 'Foo')
expect(response).to render_template(:edit)
end
context 'reCAPTCHA enabled' do
before do
stub_application_setting(recaptcha_enabled: true)
end
it 'renders :verify with reCAPTCHA enabled' do
update_snippet(title: 'Foo')
expect(response).to render_template(:verify)
end
it 'renders snippet page when reCAPTCHA verified' do
spammy_title = 'Whatever'
spam_logs = create_list(:spam_log, 2, user: user, title: spammy_title)
snippet = update_snippet({ title: spammy_title },
{ spam_log_id: spam_logs.last.id,
recaptcha_verification: true })
expect(response).to redirect_to(project_snippet_path(project, snippet))
end
end
end
context 'when the private snippet is made public' do
let(:visibility_level) { Snippet::PRIVATE }
it 'rejects the snippet' do
expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) }
.not_to change { snippet.reload.title }
end
it 'creates a spam log' do
expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) }
.to log_spam(title: 'Foo', user_id: user.id, noteable_type: 'ProjectSnippet')
end
it 'renders :edit with reCAPTCHA disabled' do
stub_application_setting(recaptcha_enabled: false)
update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC)
expect(response).to render_template(:edit)
end
context 'reCAPTCHA enabled' do
before do
stub_application_setting(recaptcha_enabled: true)
end
it 'renders :verify' do
update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC)
expect(response).to render_template(:verify)
end
it 'renders snippet page' do
spammy_title = 'Whatever'
spam_logs = create_list(:spam_log, 2, user: user, title: spammy_title)
snippet = update_snippet({ title: spammy_title, visibility_level: Snippet::PUBLIC },
{ spam_log_id: spam_logs.last.id,
recaptcha_verification: true })
expect(response).to redirect_to(project_snippet_path(project, snippet))
end
end
end
end
end
describe 'POST #mark_as_spam' do
let_it_be(:snippet) { create(:project_snippet, :private, project: project, author: user) }
......@@ -329,12 +120,6 @@ RSpec.describe Projects::SnippetsController do
expect(assigns(:snippet)).to eq(project_snippet)
expect(response).to have_gitlab_http_status(:ok)
end
it 'renders the blob from the repository' do
subject
expect(assigns(:blob)).to eq(project_snippet.blobs.first)
end
end
%w[show raw].each do |action|
......@@ -395,6 +180,16 @@ RSpec.describe Projects::SnippetsController do
end
end
describe 'GET #show as JSON' do
it 'renders the blob from the repository' do
project_snippet = create(:project_snippet, :public, :repository, project: project, author: user)
get :show, params: { namespace_id: project.namespace, project_id: project, id: project_snippet.to_param }, format: :json
expect(assigns(:blob)).to eq(project_snippet.blobs.first)
end
end
describe "GET #show for embeddable content" do
let(:project_snippet) { create(:project_snippet, :repository, snippet_permission, project: project, author: user) }
let(:extra_params) { {} }
......@@ -533,62 +328,4 @@ RSpec.describe Projects::SnippetsController do
it_behaves_like 'content disposition headers'
end
end
describe 'DELETE #destroy' do
let_it_be(:snippet) { create(:project_snippet, :private, project: project, author: user) }
let(:params) do
{
namespace_id: project.namespace.to_param,
project_id: project,
id: snippet.to_param
}
end
subject { delete :destroy, params: params }
context 'when current user has ability to destroy the snippet' do
before do
sign_in(user)
end
it 'removes the snippet' do
subject
expect { snippet.reload }.to raise_error(ActiveRecord::RecordNotFound)
end
context 'when snippet is succesfuly destroyed' do
it 'redirects to the project snippets page' do
subject
expect(response).to redirect_to(project_snippets_path(project))
end
end
context 'when snippet is not destroyed' do
before do
allow(snippet).to receive(:destroy).and_return(false)
controller.instance_variable_set(:@snippet, snippet)
end
it 'renders the snippet page with errors' do
subject
expect(flash[:alert]).to eq('Failed to remove snippet.')
expect(response).to redirect_to(project_snippet_path(project, snippet))
end
end
end
context 'when current_user does not have ability to destroy the snippet' do
it 'responds with status 404' do
sign_in(other_user)
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
end
spec/routing/project_routing_spec.rb
View file @ 187a3a0d
......@@ -306,12 +306,9 @@ RSpec.describe 'project routing' do
end
# raw_project_snippet GET /:project_id/snippets/:id/raw(.:format) snippets#raw
# project_snippets GET /:project_id/snippets(.:format) snippets#index
# POST /:project_id/snippets(.:format) snippets#create
# new_project_snippet GET /:project_id/snippets/new(.:format) snippets#new
# edit_project_snippet GET /:project_id/snippets/:id/edit(.:format) snippets#edit
# project_snippet GET /:project_id/snippets/:id(.:format) snippets#show
# PUT /:project_id/snippets/:id(.:format) snippets#update
# DELETE /:project_id/snippets/:id(.:format) snippets#destroy
describe SnippetsController, 'routing' do
it 'to #raw' do
expect(get('/gitlab/gitlabhq/-/snippets/1/raw')).to route_to('projects/snippets#raw', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1')
......@@ -321,10 +318,6 @@ RSpec.describe 'project routing' do
expect(get('/gitlab/gitlabhq/-/snippets')).to route_to('projects/snippets#index', namespace_id: 'gitlab', project_id: 'gitlabhq')
end
it 'to #create' do
expect(post('/gitlab/gitlabhq/-/snippets')).to route_to('projects/snippets#create', namespace_id: 'gitlab', project_id: 'gitlabhq')
end
it 'to #new' do
expect(get('/gitlab/gitlabhq/-/snippets/new')).to route_to('projects/snippets#new', namespace_id: 'gitlab', project_id: 'gitlabhq')
end
......@@ -337,14 +330,6 @@ RSpec.describe 'project routing' do
expect(get('/gitlab/gitlabhq/-/snippets/1')).to route_to('projects/snippets#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1')
end
it 'to #update' do
expect(put('/gitlab/gitlabhq/-/snippets/1')).to route_to('projects/snippets#update', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1')
end
it 'to #destroy' do
expect(delete('/gitlab/gitlabhq/-/snippets/1')).to route_to('projects/snippets#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1')
end
it 'to #show from unscope routing' do
expect(get('/gitlab/gitlabhq/snippets/1')).to route_to('projects/snippets#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1')
end
......
spec/routing/routing_spec.rb
View file @ 187a3a0d
......@@ -61,12 +61,9 @@ RSpec.describe "Mounted Apps", "routing" do
end
# snippets GET /snippets(.:format) snippets#index
# POST /snippets(.:format) snippets#create
# new_snippet GET /snippets/new(.:format) snippets#new
# edit_snippet GET /snippets/:id/edit(.:format) snippets#edit
# snippet GET /snippets/:id(.:format) snippets#show
# PUT /snippets/:id(.:format) snippets#update
# DELETE /snippets/:id(.:format) snippets#destroy
RSpec.describe SnippetsController, "routing" do
it "to #raw" do
expect(get("/-/snippets/1/raw")).to route_to('snippets#raw', id: '1')
......@@ -76,10 +73,6 @@ RSpec.describe SnippetsController, "routing" do
expect(get("/-/snippets")).to route_to('snippets#index')
end
it "to #create" do
expect(post("/-/snippets")).to route_to('snippets#create')
end
it "to #new" do
expect(get("/-/snippets/new")).to route_to('snippets#new')
end
......@@ -92,14 +85,6 @@ RSpec.describe SnippetsController, "routing" do
expect(get("/-/snippets/1")).to route_to('snippets#show', id: '1')
end
it "to #update" do
expect(put("/-/snippets/1")).to route_to('snippets#update', id: '1')
end
it "to #destroy" do
expect(delete("/-/snippets/1")).to route_to('snippets#destroy', id: '1')
end
it 'to #show from unscoped routing' do
expect(get("/snippets/1")).to route_to('snippets#show', id: '1')
end
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7