Merge branch '6020-extract-ee-specific-controller-lines-ee' into 'master'

Resolve "Extract EE specific files/lines for some controllers"

Closes #6020

See merge request gitlab-org/gitlab-ee!5739

app/controllers/profiles/keys_controller.rb

@@ -23,7 +23,7 @@ class Profiles::KeysController < Profiles::ApplicationController
 
   def destroy
     @key = current_user.keys.find(params[:id])
-    @key.destroy unless @key.is_a? LDAPKey
+    Keys::DestroyService.new(current_user).execute(@key)
 
     respond_to do |format|
       format.html { redirect_to profile_keys_url, status: 302 }

app/controllers/projects/prometheus/metrics_controller.rb

 module Projects
   module Prometheus
     class MetricsController < Projects::ApplicationController
+      prepend EE::Projects::Prometheus::MetricsController
+
       before_action :authorize_admin_project!
       before_action :require_prometheus_metrics!
 
@@ -18,85 +20,8 @@ module Projects
         end
       end
 
-      def validate_query
-        respond_to do |format|
-          format.json do
-            result = prometheus_adapter.query(:validate, params[:query])
-
-            if result.any?
-              render json: result
-            else
-              head :no_content
-            end
-          end
-        end
-      end
-
-      def new
-        @metric = project.prometheus_metrics.new
-      end
-
-      def index
-        respond_to do |format|
-          format.json do
-            metrics = project.prometheus_metrics
-            response = {}
-            if metrics.any?
-              response[:metrics] = PrometheusMetricSerializer.new(project: project)
-                                     .represent(metrics.order(created_at: :asc))
-            end
-
-            render json: response
-          end
-        end
-      end
-
-      def create
-        @metric = project.prometheus_metrics.create(metrics_params)
-        if @metric.persisted?
-          redirect_to edit_project_service_path(project, PrometheusService),
-                      notice: 'Metric was successfully added.'
-        else
-          render 'new'
-        end
-      end
-
-      def update
-        @metric = project.prometheus_metrics.find(params[:id])
-        @metric.update(metrics_params)
-
-        if @metric.persisted?
-          redirect_to edit_project_service_path(project, PrometheusService),
-                      notice: 'Metric was successfully updated.'
-        else
-          render 'edit'
-        end
-      end
-
-      def edit
-        @metric = project.prometheus_metrics.find(params[:id])
-      end
-
-      def destroy
-        metric = project.prometheus_metrics.find(params[:id])
-        metric.destroy
-
-        respond_to do |format|
-          format.html do
-            redirect_to edit_project_service_path(project, PrometheusService), status: 303
-          end
-          format.json do
-            head :ok
-          end
-        end
-      end
-
       private
 
-      def metrics_params
-        params.require(:prometheus_metric).permit(:title, :query, :y_label, :unit, :legend, :group)
-      end
-
       def prometheus_adapter
         @prometheus_adapter ||= ::Prometheus::AdapterService.new(project).prometheus_adapter
       end

app/controllers/projects/settings/integrations_controller.rb

 module Projects
   module Settings
     class IntegrationsController < Projects::ApplicationController
+      prepend EE::Projects::Settings::IntegrationsController
+
       include ServiceParams
 
       before_action :authorize_admin_project!
@@ -18,17 +20,7 @@ module Projects
 
       # Returns a list of services that should be hidden from the list
       def service_exceptions
-        exceptions = @project.disabled_services.dup
-
-        exceptions << slack_service
-      end
-
-      def slack_service
-        if Gitlab::CurrentSettings.slack_app_enabled
-          'slack_slash_commands'
-        else
-          'gitlab_slack_application'
-        end
+        @project.disabled_services.dup
       end
     end
   end

app/models/project.rb

@@ -1014,7 +1014,7 @@ class Project < ActiveRecord::Base
 
     available_services_names = Service.available_services_names - exceptions
 
-    available_services_names.map do |service_name|
+    available_services = available_services_names.map do |service_name|
       service = find_service(services, service_name)
 
       if service
@@ -1031,6 +1031,14 @@ class Project < ActiveRecord::Base
         end
       end
     end
+
+    available_services.reject do |service|
+      disabled_services.include?(service.to_param)
+    end
+  end
+
+  def disabled_services
+    []
   end
 
   def find_or_initialize_service(name)

app/services/keys/base_service.rb

@@ -2,7 +2,7 @@ module Keys
   class BaseService
     attr_accessor :user, :params
 
-    def initialize(user, params)
+    def initialize(user, params = {})
       @user, @params = user, params
       @ip_address = @params.delete(:ip_address)
     end

app/services/keys/destroy_service.rb

+module Keys
+  class DestroyService < ::Keys::BaseService
+    prepend EE::Keys::DestroyService
+
+    def execute(key)
+      key.destroy if destroy_possible?(key)
+    end
+
+    # overriden in EE::Keys::DestroyService
+    def destroy_possible?(key)
+      true
+    end
+  end
+end

ee/app/controllers/ee/projects/prometheus/metrics_controller.rb

+module EE
+  module Projects
+    module Prometheus
+      module MetricsController
+        def validate_query
+          respond_to do |format|
+            format.json do
+              result = prometheus_adapter.query(:validate, params[:query])
+
+              if result.any?
+                render json: result
+              else
+                head :no_content
+              end
+            end
+          end
+        end
+
+        def new
+          @metric = project.prometheus_metrics.new # rubocop:disable Gitlab/ModuleWithInstanceVariables
+        end
+
+        def index
+          respond_to do |format|
+            format.json do
+              metrics = project.prometheus_metrics
+              response = {}
+              if metrics.any?
+                response[:metrics] = ::PrometheusMetricSerializer.new(project: project)
+                                       .represent(metrics.order(created_at: :asc))
+              end
+
+              render json: response
+            end
+          end
+        end
+
+        def create
+          @metric = project.prometheus_metrics.create(metrics_params) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+          if @metric.persisted? # rubocop:disable Gitlab/ModuleWithInstanceVariables
+            redirect_to edit_project_service_path(project, ::PrometheusService),
+                        notice: 'Metric was successfully added.'
+          else
+            render 'new'
+          end
+        end
+
+        def update
+          @metric = project.prometheus_metrics.find(params[:id]) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+          @metric.update(metrics_params) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+
+          if @metric.persisted? # rubocop:disable Gitlab/ModuleWithInstanceVariables
+            redirect_to edit_project_service_path(project, ::PrometheusService),
+                        notice: 'Metric was successfully updated.'
+          else
+            render 'edit'
+          end
+        end
+
+        def edit
+          @metric = project.prometheus_metrics.find(params[:id]) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+        end
+
+        def destroy
+          metric = project.prometheus_metrics.find(params[:id])
+          metric.destroy
+
+          respond_to do |format|
+            format.html do
+              redirect_to edit_project_service_path(project, ::PrometheusService), status: 303
+            end
+            format.json do
+              head :ok
+            end
+          end
+        end
+
+        private
+
+        def metrics_params
+          params.require(:prometheus_metric).permit(:title, :query, :y_label, :unit, :legend, :group)
+        end
+      end
+    end
+  end
+end

ee/app/controllers/ee/projects/settings/integrations_controller.rb

+module EE
+  module Projects
+    module Settings
+      module IntegrationsController
+        extend ::Gitlab::Utils::Override
+
+        private
+
+        override :service_exceptions
+        def service_exceptions
+          super << slack_service
+        end
+
+        def slack_service
+          if ::Gitlab::CurrentSettings.slack_app_enabled
+            'slack_slash_commands'
+          else
+            'gitlab_slack_application'
+          end
+        end
+      end
+    end
+  end
+end

ee/app/models/ee/project.rb

@@ -448,15 +448,7 @@ module EE
       ).create
     end
 
-    # Override to reject disabled services
-    def find_or_initialize_services(exceptions: [])
-      available_services = super
-
-      available_services.reject do |service|
-        disabled_services.include?(service.to_param)
-      end
-    end
-
+    override :disabled_services
     def disabled_services
       strong_memoize(:disabled_services) do
         disabled_services = []

ee/app/services/ee/keys/destroy_service.rb

+module EE
+  module Keys
+    module DestroyService
+      extend ::Gitlab::Utils::Override
+
+      override :destroy_possible?
+      def destroy_possible?(key)
+        super && !key.is_a?(LDAPKey)
+      end
+    end
+  end
+end

ee/spec/factories/ldap_keys.rb

+FactoryBot.define do
+  factory :ldap_key, parent: :key, class: 'LDAPKey'
+end

ee/spec/services/ee/keys/destroy_service_spec.rb

+require 'spec_helper'
+
+describe Keys::DestroyService do
+  let(:user) { create(:user) }
+  let(:key) { create(:ldap_key) }
+
+  subject { described_class.new(user) }
+
+  it 'does not destroy LDAP key' do
+    key = create(:ldap_key)
+
+    expect { subject.execute(key) }.not_to change(Key, :count)
+    expect(key).not_to be_destroyed
+  end
+end

spec/services/keys/destroy_service_spec.rb

+require 'spec_helper'
+
+describe Keys::DestroyService do
+  let(:user) { create(:user) }
+
+  subject { described_class.new(user) }
+
+  it 'destroys a key' do
+    key = create(:key)
+
+    expect { subject.execute(key) }.to change(Key, :count).by(-1)
+  end
+end