Merge branch 'expose-ca-services-via-api' into 'master'

Expose stage services via API

See merge request gitlab-org/gitlab!18208

ee/app/controllers/analytics/cycle_analytics/stages_controller.rb

@@ -6,10 +6,11 @@ module Analytics
       check_feature_flag Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG
 
       before_action :load_group
-      before_action :authorize_access!
 
       def index
-        result = stage_list_service.execute
+        return render_403 unless can?(current_user, :read_group_cycle_analytics, @group)
+
+        result = list_service.execute
 
         if result.success?
           render json: cycle_analytics_configuration(result.payload[:stages])
@@ -18,23 +19,55 @@ module Analytics
         end
       end
 
-      private
+      def create
+        return render_403 unless can?(current_user, :create_group_stage, @group)
+
+        render_stage_service_result(create_service.execute)
+      end
+
+      def update
+        return render_403 unless can?(current_user, :update_group_stage, @group)
 
-      def authorize_access!
-        render_403 unless can?(current_user, :read_group_cycle_analytics, @group)
+        render_stage_service_result(update_service.execute)
       end
 
+      def destroy
+        return render_403 unless can?(current_user, :delete_group_stage, @group)
+
+        render_stage_service_result(delete_service.execute)
+      end
+
+      private
+
       def cycle_analytics_configuration(stages)
         stage_presenters = stages.map { |s| StagePresenter.new(s) }
 
         Analytics::CycleAnalytics::ConfigurationEntity.new(stages: stage_presenters)
       end
 
-      def stage_list_service
-        Analytics::CycleAnalytics::Stages::ListService.new(
-          parent: @group,
-          current_user: current_user
-        )
+      def list_service
+        Stages::ListService.new(parent: @group, current_user: current_user)
+      end
+
+      def create_service
+        Stages::CreateService.new(parent: @group, current_user: current_user, params: params.permit(:name, :start_event_identifier, :end_event_identifier))
+      end
+
+      def update_service
+        Stages::UpdateService.new(parent: @group, current_user: current_user, params: params.permit(:name, :start_event_identifier, :end_event_identifier, :id))
+      end
+
+      def delete_service
+        Stages::DeleteService.new(parent: @group, current_user: current_user, params: params.permit(:id))
+      end
+
+      def render_stage_service_result(result)
+        if result.success?
+          stage = StagePresenter.new(result.payload[:stage])
+          render json: Analytics::CycleAnalytics::StageEntity.new(stage), status: result.http_status
+        else
+          render json: { message: result.message, errors: result.payload[:errors] }, status: result.http_status
+        end
       end
     end
   end

ee/app/serializers/analytics/cycle_analytics/stage_entity.rb

@@ -8,8 +8,8 @@ module Analytics
       expose :description
       expose :id
       expose :custom
-      expose :start_event_identifier, if: :custom?
-      expose :end_event_identifier, if: :custom?
+      expose :start_event_identifier, if: -> (s) { s.custom? }
+      expose :end_event_identifier, if: -> (s) { s.custom? }
 
       def id
         object.id || object.name

ee/app/services/analytics/cycle_analytics/stages/base_service.rb

@@ -2,52 +2,54 @@
 
 module Analytics
   module CycleAnalytics
-    class BaseService
-      include Gitlab::Allowable
+    module Stages
+      class BaseService
+        include Gitlab::Allowable
 
-      def initialize(parent:, current_user:, params: {})
-        @parent = parent
-        @current_user = current_user
-      end
+        def initialize(parent:, current_user:, params: {})
+          @parent = parent
+          @current_user = current_user
+        end
 
-      def execute
-        raise NotImplementedError
-      end
+        def execute
+          raise NotImplementedError
+        end
 
-      private
+        private
 
-      attr_reader :parent, :current_user, :params
+        attr_reader :parent, :current_user, :params
 
-      def success(stage, http_status = :created)
-        ServiceResponse.success(payload: { stage: stage }, http_status: http_status)
-      end
+        def success(stage, http_status = :created)
+          ServiceResponse.success(payload: { stage: stage }, http_status: http_status)
+        end
 
-      def error(stage)
-        ServiceResponse.error(message: 'Invalid parameters', payload: { errors: stage.errors }, http_status: :unprocessable_entity)
-      end
+        def error(stage)
+          ServiceResponse.error(message: 'Invalid parameters', payload: { errors: stage.errors }, http_status: :unprocessable_entity)
+        end
 
-      def not_found
-        ServiceResponse.error(message: 'Stage not found', payload: {}, http_status: :not_found)
-      end
+        def not_found
+          ServiceResponse.error(message: 'Stage not found', payload: {}, http_status: :not_found)
+        end
 
-      def forbidden
-        ServiceResponse.error(message: 'Forbidden', payload: {}, http_status: :forbidden)
-      end
+        def forbidden
+          ServiceResponse.error(message: 'Forbidden', payload: {}, http_status: :forbidden)
+        end
 
-      def persist_default_stages!
-        persisted_default_stages = parent.cycle_analytics_stages.default_stages
+        def persist_default_stages!
+          persisted_default_stages = parent.cycle_analytics_stages.default_stages
 
-        # make sure that we persist default stages only once
-        stages_to_persist = build_default_stages.select do |new_default_stage|
-          !persisted_default_stages.find { |s| s.name.eql?(new_default_stage.name) }
-        end
+          # make sure that we persist default stages only once
+          stages_to_persist = build_default_stages.select do |new_default_stage|
+            !persisted_default_stages.find { |s| s.name.eql?(new_default_stage.name) }
+          end
 
-        stages_to_persist.each(&:save!)
-      end
+          stages_to_persist.each(&:save!)
+        end
 
-      def build_default_stages
-        Gitlab::Analytics::CycleAnalytics::DefaultStages.all.map do |params|
-          parent.cycle_analytics_stages.build(params)
+        def build_default_stages
+          Gitlab::Analytics::CycleAnalytics::DefaultStages.all.map do |params|
+            parent.cycle_analytics_stages.build(params)
+          end
         end
       end
     end

ee/app/services/analytics/cycle_analytics/stages/delete_service.rb

+# frozen_string_literal: true
+
+module Analytics
+  module CycleAnalytics
+    module Stages
+      class DeleteService < BaseService
+        def initialize(parent:, current_user:, params:)
+          super
+
+          @stage = Analytics::CycleAnalytics::StageFinder.new(parent: parent, stage_id: params[:id]).execute
+        end
+
+        def execute
+          return forbidden if !can?(current_user, :delete_group_stage, parent) || @stage.default_stage?
+
+          @stage.destroy!
+
+          success(@stage, :ok)
+        end
+      end
+    end
+  end
+end

ee/config/routes/analytics.rb

@@ -10,7 +10,7 @@ namespace :analytics do
   constraints(::Constraints::FeatureConstrainer.new(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG)) do
     resource :cycle_analytics, only: :show
     namespace :cycle_analytics do
-      resources :stages, only: [:index]
+      resources :stages, only: [:index, :create, :update, :destroy]
     end
   end
 

ee/spec/controllers/analytics/cycle_analytics/stages_controller_spec.rb

@@ -3,12 +3,10 @@
 require 'spec_helper'
 
 describe Analytics::CycleAnalytics::StagesController do
-  let(:user) { create(:user) }
-  let(:group) { create(:group) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group, refind: true) { create(:group) }
   let(:params) { { group_id: group.full_path } }
 
-  subject { get :index, params: params }
-
   before do
     stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => true)
     stub_licensed_features(cycle_analytics_for_groups: true)
@@ -17,86 +15,148 @@ describe Analytics::CycleAnalytics::StagesController do
     sign_in(user)
   end
 
-  it 'succeeds' do
-    subject
+  describe 'GET `index`' do
+    subject { get :index, params: params }
 
-    expect(response).to be_successful
-    expect(response).to match_response_schema('analytics/cycle_analytics/stages', dir: 'ee')
-  end
+    it 'succeeds' do
+      subject
 
-  it 'returns correct start events' do
-    subject
+      expect(response).to be_successful
+      expect(response).to match_response_schema('analytics/cycle_analytics/stages', dir: 'ee')
+    end
 
-    response_start_events = json_response['stages'].map { |s| s['start_event_identifier'] }
-    start_events = Gitlab::Analytics::CycleAnalytics::DefaultStages.all.map { |s| s['start_event_identifier'] }
+    it 'returns correct start events' do
+      subject
 
-    expect(response_start_events).to eq(start_events)
-  end
+      response_start_events = json_response['stages'].map { |s| s['start_event_identifier'] }
+      start_events = Gitlab::Analytics::CycleAnalytics::DefaultStages.all.map { |s| s['start_event_identifier'] }
 
-  it 'returns correct event names' do
-    subject
+      expect(response_start_events).to eq(start_events)
+    end
 
-    response_event_names = json_response['events'].map { |s| s['name'] }
-    event_names = Gitlab::Analytics::CycleAnalytics::StageEvents.events.map(&:name)
+    it 'returns correct event names' do
+      subject
 
-    expect(response_event_names).to eq(event_names)
-  end
+      response_event_names = json_response['events'].map { |s| s['name'] }
+      event_names = Gitlab::Analytics::CycleAnalytics::StageEvents.events.map(&:name)
 
-  it 'succeeds for subgroups' do
-    subgroup = create(:group, parent: group)
-    params[:group_id] = subgroup.full_path
+      expect(response_event_names).to eq(event_names)
+    end
 
-    subject
+    it 'succeeds for subgroups' do
+      subgroup = create(:group, parent: group)
+      params[:group_id] = subgroup.full_path
 
-    expect(response).to be_successful
-  end
+      subject
 
-  it 'renders 404 when group_id is not provided' do
-    params[:group_id] = nil
+      expect(response).to be_successful
+    end
 
-    subject
+    it 'renders `forbidden` based on the response of the service object' do
+      expect_any_instance_of(Analytics::CycleAnalytics::Stages::ListService).to receive(:can?).and_return(false)
 
-    expect(response).to have_gitlab_http_status(:not_found)
+      subject
+
+      expect(response).to have_gitlab_http_status(:forbidden)
+    end
+
+    include_examples 'group permission check on the controller level'
   end
 
-  it 'renders 404 when group is missing' do
-    params[:group_id] = 'missing_group'
+  describe 'POST `create`' do
+    subject { post :create, params: params }
 
-    subject
+    include_examples 'group permission check on the controller level'
 
-    expect(response).to have_gitlab_http_status(:not_found)
-  end
+    context 'when valid parameters are given' do
+      before do
+        params.merge!({
+          name: 'my new stage',
+          start_event_identifier: :merge_request_created,
+          end_event_identifier: :merge_request_merged
+        })
+      end
 
-  it 'renders 404 when feature flag is disabled' do
-    stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => false)
+      it 'creates the stage' do
+        subject
 
-    subject
+        expect(response).to be_successful
+        expect(response).to match_response_schema('analytics/cycle_analytics/stage', dir: 'ee')
+      end
+    end
 
-    expect(response).to have_gitlab_http_status(:not_found)
+    include_context 'when invalid stage parameters are given'
   end
 
-  it 'renders 403 when user has no reporter access' do
-    GroupMember.where(user: user).delete_all
-    group.add_guest(user)
+  describe 'PUT `update`' do
+    let(:stage) { create(:cycle_analytics_group_stage, parent: group) }
+    subject { put :update, params: params.merge(id: stage.id) }
 
-    subject
+    include_examples 'group permission check on the controller level'
 
-    expect(response).to have_gitlab_http_status(:forbidden)
-  end
+    context 'when valid parameters are given' do
+      before do
+        params.merge!({
+          name: 'my updated stage',
+          start_event_identifier: :merge_request_created,
+          end_event_identifier: :merge_request_merged
+        })
+      end
+
+      it 'succeeds' do
+        subject
 
-  it 'renders 403 when feature is not available for the group' do
-    stub_licensed_features(cycle_analytics_for_groups: false)
+        expect(response).to be_successful
+        expect(response).to match_response_schema('analytics/cycle_analytics/stage', dir: 'ee')
+      end
 
-    subject
+      it 'updates the name attribute' do
+        subject
 
-    expect(response).to have_gitlab_http_status(:forbidden)
+        stage.reload
+
+        expect(stage.name).to eq(params[:name])
+      end
+    end
+
+    include_context 'when invalid stage parameters are given'
   end
 
-  it 'renders 403 based on the response of the service object' do
-    expect_any_instance_of(Analytics::CycleAnalytics::Stages::ListService).to receive(:can?).and_return(false)
+  describe 'DELETE `destroy`' do
+    let(:stage) { create(:cycle_analytics_group_stage, parent: group) }
+
+    subject { delete :destroy, params: params }
+
+    before do
+      params[:id] = stage.id
+    end
+
+    include_examples 'group permission check on the controller level'
+
+    context 'when persisted stage id is passed' do
+      it 'succeeds' do
+        subject
+
+        expect(response).to be_successful
+      end
+
+      it 'deletes the record' do
+        subject
+
+        expect(group.reload.cycle_analytics_stages.find_by(id: stage.id)).to be_nil
+      end
+    end
+
+    context 'when default stage id is passed' do
+      before do
+        params[:id] = Gitlab::Analytics::CycleAnalytics::DefaultStages.names.first
+      end
 
-    subject
+      it 'fails with `forbidden` response' do
+        subject
 
-    expect(response).to have_gitlab_http_status(:forbidden)
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
   end
 end

ee/spec/fixtures/api/schemas/analytics/cycle_analytics/stage.json

@@ -21,5 +21,5 @@
       "type": "boolean"
     }
   },
-  "additionalProperties": false
+  "additionalProperties": true
 }

ee/spec/fixtures/api/schemas/analytics/cycle_analytics/validation_error.json

+{
+  "type": "object",
+  "properties": {
+    "message": { 
+      "type": "string" 
+    },
+    "errors": { 
+      "type": "object",
+      "additionalProperties" : {
+        "type" : "array",
+        "items": {
+          "type": "string"
+        }
+      }
+    }
+  },
+  "required": ["message", "errors"],
+  "additionalProperties": false
+}

ee/spec/services/analytics/cycle_analytics/stages/delete_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Analytics::CycleAnalytics::Stages::DeleteService do
+  let_it_be(:group, refind: true) { create(:group) }
+  let_it_be(:user, refind: true) { create(:user) }
+  let_it_be(:stage, refind: true) { create(:cycle_analytics_group_stage, group: group) }
+  let(:params) { { id: stage.id } }
+
+  subject { described_class.new(parent: group, params: params, current_user: user).execute }
+
+  before_all do
+    group.add_user(user, :reporter)
+  end
+
+  before do
+    stub_licensed_features(cycle_analytics_for_groups: true)
+  end
+
+  it_behaves_like 'permission check for cycle analytics stage services', :cycle_analytics_for_groups
+
+  context 'when persisted stage is given' do
+    it { expect(subject).to be_success }
+
+    it 'deletes the stage' do
+      subject
+
+      expect(group.cycle_analytics_stages.find_by(id: stage.id)).to be_nil
+    end
+  end
+
+  context 'disallows deletion when default stage is given' do
+    let_it_be(:stage, refind: true) { create(:cycle_analytics_group_stage, group: group, custom: false) }
+
+    it { expect(subject).not_to be_success }
+    it { expect(subject.http_status).to eq(:forbidden) }
+  end
+end

ee/spec/support/shared_examples/controllers/analytics/cycle_analytics/shared_stage_examples.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+shared_examples 'group permission check on the controller level' do
+  context 'when `group_id` is not provided' do
+    before do
+      params[:group_id] = nil
+    end
+
+    it 'renders `not_found` when group_id is not provided' do
+      subject
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+
+  context 'when `group_id` is not found' do
+    before do
+      params[:group_id] = 'missing_group'
+    end
+
+    it 'renders `not_found` when group is missing' do
+      subject
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+
+  context 'when feature flag is disabled' do
+    before do
+      stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => false)
+    end
+
+    it 'renders `not_found` response' do
+      subject
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+
+  context 'when user has no lower access level than `reporter`' do
+    before do
+      GroupMember.where(user: user).delete_all
+      group.add_guest(user)
+    end
+
+    it 'renders `forbidden` response' do
+      subject
+
+      expect(response).to have_gitlab_http_status(:forbidden)
+    end
+  end
+
+  context 'when feature is not available for the group' do
+    before do
+      stub_licensed_features(cycle_analytics_for_groups: false)
+    end
+
+    it 'renders `forbidden` response' do
+      subject
+
+      expect(response).to have_gitlab_http_status(:forbidden)
+    end
+  end
+end
+
+shared_context 'when invalid stage parameters are given' do
+  before do
+    params[:name] = ''
+  end
+
+  it 'renders the validation errors' do
+    subject
+
+    expect(response).to have_gitlab_http_status(:unprocessable_entity)
+    expect(response).to match_response_schema('analytics/cycle_analytics/validation_error', dir: 'ee')
+  end
+end