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

Refactor common snippet controller methods

See merge request gitlab-org/gitlab!34266

app/controllers/concerns/find_snippet.rb

+# frozen_string_literal: true
+
+module FindSnippet
+  extend ActiveSupport::Concern
+  include Gitlab::Utils::StrongMemoize
+
+  private
+
+  # rubocop:disable CodeReuse/ActiveRecord
+  def snippet
+    strong_memoize(:snippet) do
+      snippet_klass.inc_relations_for_view.find_by(id: snippet_id)
+    end
+  end
+  # rubocop:enable CodeReuse/ActiveRecord
+
+  def snippet_klass
+    raise NotImplementedError
+  end
+
+  def snippet_id
+    params[:id]
+  end
+end

app/controllers/concerns/snippet_authorizations.rb

+# frozen_string_literal: true
+
+module SnippetAuthorizations
+  extend ActiveSupport::Concern
+
+  private
+
+  def authorize_read_snippet!
+    return render_404 unless can?(current_user, :read_snippet, snippet)
+  end
+
+  def authorize_update_snippet!
+    return render_404 unless can?(current_user, :update_snippet, snippet)
+  end
+
+  def authorize_admin_snippet!
+    return render_404 unless can?(current_user, :admin_snippet, snippet)
+  end
+
+  def authorize_create_snippet!
+    return render_404 unless can?(current_user, :create_snippet)
+  end
+end

app/controllers/concerns/snippets_actions.rb

@@ -3,9 +3,18 @@
 module SnippetsActions
   extend ActiveSupport::Concern
   include SendsBlob
+  include RendersNotes
+  include RendersBlob
+  include PaginatedCollection
+  include Gitlab::NoteableMetadata
 
   included do
+    skip_before_action :verify_authenticity_token,
+      if: -> { action_name == 'show' && js_request? }
+
     before_action :redirect_if_binary, only: [:edit, :update]
+
+    respond_to :html
   end
 
   def edit
@@ -43,6 +52,58 @@ module SnippetsActions
     request.format.js?
   end
 
+  # rubocop:disable Gitlab/ModuleWithInstanceVariables
+  def show
+    conditionally_expand_blob(blob)
+
+    respond_to do |format|
+      format.html do
+        @note = Note.new(noteable: @snippet, project: @snippet.project)
+        @noteable = @snippet
+
+        @discussions = @snippet.discussions
+        @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes), @noteable)
+        render 'show'
+      end
+
+      format.json do
+        render_blob_json(blob)
+      end
+
+      format.js do
+        if @snippet.embeddable?
+          render 'shared/snippets/show'
+        else
+          head :not_found
+        end
+      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
 
   def content_disposition

app/controllers/projects/snippets/application_controller.rb

+# frozen_string_literal: true
+
+class Projects::Snippets::ApplicationController < Projects::ApplicationController
+  include FindSnippet
+  include SnippetAuthorizations
+
+  private
+
+  # This overrides the default snippet create authorization
+  # because ProjectSnippets are checked against the project rather
+  # than the user
+  def authorize_create_snippet!
+    return render_404 unless can?(current_user, :create_snippet, project)
+  end
+
+  def snippet_klass
+    ProjectSnippet
+  end
+end

app/controllers/projects/snippets_controller.rb

 # frozen_string_literal: true
 
-class Projects::SnippetsController < Projects::ApplicationController
-  include RendersNotes
+class Projects::SnippetsController < Projects::Snippets::ApplicationController
+  include SnippetsActions
   include ToggleAwardEmoji
   include SpammableActions
-  include SnippetsActions
-  include RendersBlob
-  include PaginatedCollection
-  include Gitlab::NoteableMetadata
-
-  skip_before_action :verify_authenticity_token,
-    if: -> { action_name == 'show' && js_request? }
 
   before_action :check_snippets_available!
+
   before_action :snippet, only: [:show, :edit, :destroy, :update, :raw, :toggle_award_emoji, :mark_as_spam]
 
-  # Allow create snippet
   before_action :authorize_create_snippet!, only: [:new, :create]
-
-  # Allow read any snippet
   before_action :authorize_read_snippet!, except: [:new, :create, :index]
-
-  # Allow modify snippet
   before_action :authorize_update_snippet!, only: [:edit, :update]
-
-  # Allow destroy snippet
   before_action :authorize_admin_snippet!, only: [:destroy]
 
-  respond_to :html
-
   def index
     @snippet_counts = Snippets::CountService
       .new(current_user, project: @project)
@@ -56,61 +41,8 @@ class Projects::SnippetsController < Projects::ApplicationController
     handle_repository_error(:new)
   end
 
-  def update
-    update_params = snippet_params.merge(spammable_params)
-
-    service_response = Snippets::UpdateService.new(project, current_user, update_params).execute(@snippet)
-    @snippet = service_response.payload[:snippet]
-
-    handle_repository_error(:edit)
-  end
-
-  def show
-    conditionally_expand_blob(blob)
-
-    respond_to do |format|
-      format.html do
-        @note = @project.notes.new(noteable: @snippet)
-        @noteable = @snippet
-
-        @discussions = @snippet.discussions
-        @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes), @noteable)
-        render 'show'
-      end
-
-      format.json do
-        render_blob_json(blob)
-      end
-
-      format.js do
-        if @snippet.embeddable?
-          render 'shared/snippets/show'
-        else
-          head :not_found
-        end
-      end
-    end
-  end
-
-  def destroy
-    service_response = Snippets::DestroyService.new(current_user, @snippet).execute
-
-    if service_response.success?
-      redirect_to project_snippets_path(project), status: :found
-    elsif service_response.http_status == 403
-      access_denied!
-    else
-      redirect_to project_snippet_path(project, @snippet),
-                  status: :found,
-                  alert: service_response.message
-    end
-  end
-
   protected
 
-  def snippet
-    @snippet ||= @project.snippets.inc_relations_for_view.find(params[:id])
-  end
   alias_method :awardable, :snippet
   alias_method :spammable, :snippet
 
@@ -118,18 +50,6 @@ class Projects::SnippetsController < Projects::ApplicationController
     project_snippet_path(@project, @snippet)
   end
 
-  def authorize_read_snippet!
-    return render_404 unless can?(current_user, :read_snippet, @snippet)
-  end
-
-  def authorize_update_snippet!
-    return render_404 unless can?(current_user, :update_snippet, @snippet)
-  end
-
-  def authorize_admin_snippet!
-    return render_404 unless can?(current_user, :admin_snippet, @snippet)
-  end
-
   def snippet_params
     params.require(:project_snippet).permit(:title, :content, :file_name, :private, :visibility_level, :description)
   end

app/controllers/snippets/application_controller.rb

+# frozen_string_literal: true
+
+class Snippets::ApplicationController < ApplicationController
+  include FindSnippet
+  include SnippetAuthorizations
+
+  private
+
+  def authorize_read_snippet!
+    return if can?(current_user, :read_snippet, snippet)
+
+    if current_user
+      render_404
+    else
+      authenticate_user!
+    end
+  end
+
+  def snippet_klass
+    PersonalSnippet
+  end
+end

app/controllers/snippets_controller.rb

 # frozen_string_literal: true
 
-class SnippetsController < ApplicationController
-  include RendersNotes
-  include ToggleAwardEmoji
-  include SpammableActions
+class SnippetsController < Snippets::ApplicationController
   include SnippetsActions
-  include RendersBlob
   include PreviewMarkdown
-  include PaginatedCollection
-  include Gitlab::NoteableMetadata
-
-  skip_before_action :verify_authenticity_token,
-    if: -> { action_name == 'show' && js_request? }
+  include ToggleAwardEmoji
+  include SpammableActions
 
-  before_action :snippet, only: [:show, :edit, :destroy, :update, :raw]
+  before_action :snippet, only: [:show, :edit, :destroy, :update, :raw, :toggle_award_emoji, :mark_as_spam]
 
   before_action :authorize_create_snippet!, only: [:new, :create]
   before_action :authorize_read_snippet!, only: [:show, :raw]
@@ -23,7 +16,6 @@ class SnippetsController < ApplicationController
   skip_before_action :authenticate_user!, only: [:index, :show, :raw]
 
   layout 'snippets'
-  respond_to :html
 
   def index
     if params[:username].present?
@@ -60,62 +52,8 @@ class SnippetsController < ApplicationController
     end
   end
 
-  def update
-    service_response = Snippets::UpdateService.new(nil, current_user, snippet_params).execute(@snippet)
-    @snippet = service_response.payload[:snippet]
-
-    handle_repository_error(:edit)
-  end
-
-  def show
-    conditionally_expand_blob(blob)
-
-    respond_to do |format|
-      format.html do
-        @note = Note.new(noteable: @snippet)
-        @noteable = @snippet
-
-        @discussions = @snippet.discussions
-        @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes), @noteable)
-        render 'show'
-      end
-
-      format.json do
-        render_blob_json(blob)
-      end
-
-      format.js do
-        if @snippet.embeddable?
-          render 'shared/snippets/show'
-        else
-          head :not_found
-        end
-      end
-    end
-  end
-
-  def destroy
-    service_response = Snippets::DestroyService.new(current_user, @snippet).execute
-
-    if service_response.success?
-      redirect_to dashboard_snippets_path, status: :found
-    elsif service_response.http_status == 403
-      access_denied!
-    else
-      redirect_to snippet_path(@snippet),
-                  status: :found,
-                  alert: service_response.message
-    end
-  end
-
   protected
 
-  # rubocop: disable CodeReuse/ActiveRecord
-  def snippet
-    @snippet ||= PersonalSnippet.inc_relations_for_view.find_by(id: params[:id])
-  end
-  # rubocop: enable CodeReuse/ActiveRecord
-
   alias_method :awardable, :snippet
   alias_method :spammable, :snippet
 
@@ -123,28 +61,6 @@ class SnippetsController < ApplicationController
     snippet_path(@snippet)
   end
 
-  def authorize_read_snippet!
-    return if can?(current_user, :read_snippet, @snippet)
-
-    if current_user
-      render_404
-    else
-      authenticate_user!
-    end
-  end
-
-  def authorize_update_snippet!
-    return render_404 unless can?(current_user, :update_snippet, @snippet)
-  end
-
-  def authorize_admin_snippet!
-    return render_404 unless can?(current_user, :admin_snippet, @snippet)
-  end
-
-  def authorize_create_snippet!
-    return render_404 unless can?(current_user, :create_snippet)
-  end
-
   def snippet_params
     params.require(:personal_snippet).permit(:title, :content, :file_name, :private, :visibility_level, :description).merge(spammable_params)
   end

app/helpers/gitlab_routing_helper.rb

@@ -245,6 +245,14 @@ module GitlabRoutingHelper
     end
   end
 
+  def gitlab_dashboard_snippets_path(snippet, *args)
+    if snippet.is_a?(ProjectSnippet)
+      project_snippets_path(snippet.project, *args)
+    else
+      dashboard_snippets_path
+    end
+  end
+
   def gitlab_raw_snippet_path(snippet, *args)
     if snippet.is_a?(ProjectSnippet)
       raw_project_snippet_path(snippet.project, snippet, *args)

app/services/projects/destroy_service.rb

@@ -64,7 +64,7 @@ module Projects
     end
 
     def remove_snippets
-      response = Snippets::BulkDestroyService.new(current_user, project.snippets).execute
+      response = ::Snippets::BulkDestroyService.new(current_user, project.snippets).execute
 
       response.success?
     end

spec/helpers/gitlab_routing_helper_spec.rb

@@ -238,6 +238,16 @@ describe GitlabRoutingHelper do
         expect(gitlab_toggle_award_emoji_snippet_url(personal_snippet)).to eq("http://test.host/snippets/#{personal_snippet.id}/toggle_award_emoji")
       end
     end
+
+    describe '#gitlab_dashboard_snippets_path' do
+      it 'returns the personal snippets dashboard path' do
+        expect(gitlab_dashboard_snippets_path(personal_snippet)).to eq("/dashboard/snippets")
+      end
+
+      it 'returns the project snippets dashboard path' do
+        expect(gitlab_dashboard_snippets_path(project_snippet)).to eq("/#{project_snippet.project.full_path}/snippets")
+      end
+    end
   end
 
   context 'wikis' do