Merge branch 'ee-move-settings-oprations-to-ce' into 'master'

Move Settings Operations controller to CE

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

ee/app/controllers/projects/settings/operations_controller.rb → app/controllers/projects/settings/operations_controller.rb

@@ -22,19 +22,20 @@ module Projects
 
       private
 
-      helper_method :tracing_setting
-
-      def tracing_setting
-        @tracing_setting ||= project.tracing_setting || project.build_tracing_setting
+      def update_params
+        params.require(:project).permit(permitted_project_params)
       end
 
-      def update_params
-        params.require(:project).permit(tracing_setting_attributes: [:external_url])
+      # overridden in EE
+      def permitted_project_params
+        {}
       end
 
       def check_license
-        render_404 unless @project.feature_available?(:tracing, current_user)
+        render_404 unless helpers.settings_operations_available?
       end
     end
   end
 end
+
+Projects::Settings::OperationsController.prepend(::EE::Projects::Settings::OperationsController)

app/helpers/projects_helper.rb

@@ -285,6 +285,11 @@ module ProjectsHelper
     !disabled && !compact_mode && Feature.enabled?(:project_list_show_issue_count, default_enabled: true)
   end
 
+  # overridden in EE
+  def settings_operations_available?
+    false
+  end
+
   private
 
   def get_project_nav_tabs(project, current_user)

ee/app/services/projects/operations/update_service.rb → app/services/projects/operations/update_service.rb

@@ -12,17 +12,10 @@ module Projects
       private
 
       def project_update_params
-        tracing_setting_params(params)
-      end
-
-      def tracing_setting_params(params)
-        attr = params[:tracing_setting_attributes]
-        return params unless attr
-
-        destroy = attr[:external_url].blank?
-
-        { tracing_setting_attributes: attr.merge(_destroy: destroy) }
+        {}
       end
     end
   end
 end
+
+Projects::Operations::UpdateService.prepend(::EE::Projects::Operations::UpdateService)

app/views/layouts/nav/sidebar/_project.html.haml

@@ -358,7 +358,10 @@
                   = link_to project_settings_ci_cd_path(@project), title: _('CI / CD') do
                     %span
                       = _('CI / CD')
-              = render_if_exists 'projects/sidebar/settings_operations'
+              - if settings_operations_available?
+                = nav_link(controller: [:operations]) do
+                  = link_to project_settings_operations_path(@project), title: _('Operations') do
+                    = _('Operations')
               - if @project.pages_available?
                 = nav_link(controller: :pages) do
                   = link_to project_pages_path(@project), title: _('Pages') do

app/views/projects/settings/operations/show.html.haml

+- @content_class = 'limit-container-width' unless fluid_layout
+- page_title _('Operations')
+
+= render_if_exists 'projects/settings/operations/tracing'

config/routes/project.rb

@@ -545,6 +545,10 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       ## EE-specific
       resources :managed_licenses, only: [:index, :show, :new, :create, :edit, :update, :destroy]
       ## EE-specific
+
+      namespace :settings do
+        resource :operations, only: [:show, :update]
+      end
     end
 
     resources(:projects,

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

+# frozen_string_literal: true
+
+module EE
+  module Projects
+    module Settings
+      module OperationsController
+        extend ::Gitlab::Utils::Override
+        extend ActiveSupport::Concern
+
+        prepended do
+          helper_method :tracing_setting
+
+          def tracing_setting
+            @tracing_setting ||= project.tracing_setting || project.build_tracing_setting
+          end
+
+          private :tracing_setting
+        end
+
+        override :permitted_project_params
+        def permitted_project_params
+          super.merge(tracing_setting_attributes: [:external_url])
+        end
+      end
+    end
+  end
+end

ee/app/helpers/ee/projects_helper.rb

@@ -202,5 +202,11 @@ module EE
         }
       end
     end
+
+    def settings_operations_available?
+      return true if super
+
+      @project.feature_available?(:tracing, current_user) && can?(current_user, :read_environment, @project)
+    end
   end
 end

ee/app/services/ee/projects/operations/update_service.rb

+# frozen_string_literal: true
+
+module EE
+  module Projects
+    module Operations
+      module UpdateService
+        extend ActiveSupport::Concern
+        extend ::Gitlab::Utils::Override
+
+        override :project_update_params
+        def project_update_params
+          super.merge(tracing_setting_params)
+        end
+
+        private
+
+        def tracing_setting_params
+          attr = params[:tracing_setting_attributes]
+          return {} unless attr
+
+          destroy = attr[:external_url].blank?
+
+          { tracing_setting_attributes: attr.merge(_destroy: destroy) }
+        end
+      end
+    end
+  end
+end

ee/app/views/projects/settings/operations/_tracing.html.haml

+- return unless @project.feature_available?(:tracing, current_user)
+
+- setting = tracing_setting
 - has_jaeger_url = setting.external_url.present?
 
 %section

ee/app/views/projects/settings/operations/show.html.haml

-- @content_class = "limit-container-width" unless fluid_layout
-- page_title _("Operations")
-
-- if @project.feature_available?(:tracing, current_user)
-  = render_if_exists 'projects/settings/operations/tracing', setting: tracing_setting

ee/app/views/projects/sidebar/_settings_operations.html.haml

-- return unless @project.feature_available?(:tracing)
-
-= nav_link(controller: [:operations]) do
-  = link_to project_settings_operations_path(@project), title: _('Operations') do
-    = _('Operations')

ee/config/routes/project.rb

@@ -11,10 +11,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
 
       resource :tracing, only: [:show]
 
-      namespace :settings do
-        resource :operations, only: [:show, :update]
-      end
-
       resources :autocomplete_sources, only: [] do
         collection do
           get 'epics'

spec/controllers/projects/settings/operations_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::Settings::OperationsController do
+  set(:user) { create(:user) }
+  set(:project) { create(:project) }
+
+  before do
+    sign_in(user)
+    project.add_maintainer(user)
+  end
+
+  describe 'GET #show' do
+    it 'returns 404' do
+      get :show, params: project_params(project)
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+
+  describe 'PATCH #update' do
+    it 'returns 404' do
+      patch :update, params: project_params(project)
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+
+  private
+
+  def project_params(project)
+    { namespace_id: project.namespace, project_id: project }
+  end
+end

spec/services/projects/operations/update_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::Operations::UpdateService do
+  set(:user) { create(:user) }
+  set(:project) { create(:project) }
+
+  let(:result) { subject.execute }
+
+  subject { described_class.new(project, user, params) }
+
+  describe '#execute' do
+    context 'with inappropriate params' do
+      let(:params) { { name: '' } }
+
+      let!(:original_name) { project.name }
+
+      it 'ignores params' do
+        expect(result[:status]).to eq(:success)
+        expect(project.reload.name).to eq(original_name)
+      end
+    end
+  end
+end