Commit c60e640c authored by Peter Leitzen's avatar Peter Leitzen

Merge branch '35526-associate-stages-with-value-stream' into 'master'

Enforce presence of Value Stream when creating a stage

See merge request gitlab-org/gitlab!36661
parents 0169a9d3 29b2bd85
# frozen_string_literal: true
class AddDefaultValueStreamToGroupsWithGroupStages < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
class Group < ActiveRecord::Base
def self.find_sti_class(typename)
if typename == 'Group'
Group
else
super
end
end
self.table_name = 'namespaces'
has_many :group_value_streams
has_many :group_stages
end
class GroupValueStream < ActiveRecord::Base
self.table_name = 'analytics_cycle_analytics_group_value_streams'
has_many :group_stages
belongs_to :group
end
class GroupStage < ActiveRecord::Base
self.table_name = 'analytics_cycle_analytics_group_stages'
belongs_to :group_value_stream
end
def up
Group.where(type: 'Group').joins(:group_stages).distinct.find_each do |group|
Group.transaction do
group_value_stream = group.group_value_streams.first_or_create!(name: 'default')
group.group_stages.update_all(group_value_stream_id: group_value_stream.id)
end
end
change_column_null :analytics_cycle_analytics_group_stages, :group_value_stream_id, false
end
def down
change_column_null :analytics_cycle_analytics_group_stages, :group_value_stream_id, true
GroupValueStream.where(name: 'default').includes(:group_stages).find_each do |value_stream|
GroupValueStream.transaction do
value_stream.group_stages.update_all(group_value_stream_id: nil)
value_stream.destroy!
end
end
end
end
# frozen_string_literal: true
class ValidateForeignKeyOnCycleAnalyticsGroupStages < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
# same as in db/migrate/20200701064756_add_not_valid_foreign_key_to_cycle_analytics_group_stages.rb
CONSTRAINT_NAME = 'fk_analytics_cycle_analytics_group_stages_group_value_stream_id'
def up
validate_foreign_key :analytics_cycle_analytics_group_stages, :group_value_stream_id, name: CONSTRAINT_NAME
end
def down
remove_foreign_key_if_exists :analytics_cycle_analytics_group_stages, column: :group_value_stream_id, name: CONSTRAINT_NAME
add_foreign_key :analytics_cycle_analytics_group_stages, :analytics_cycle_analytics_group_value_streams,
column: :group_value_stream_id, name: CONSTRAINT_NAME, on_delete: :cascade, validate: false
end
end
...@@ -8801,7 +8801,7 @@ CREATE TABLE public.analytics_cycle_analytics_group_stages ( ...@@ -8801,7 +8801,7 @@ CREATE TABLE public.analytics_cycle_analytics_group_stages (
hidden boolean DEFAULT false NOT NULL, hidden boolean DEFAULT false NOT NULL,
custom boolean DEFAULT true NOT NULL, custom boolean DEFAULT true NOT NULL,
name character varying(255) NOT NULL, name character varying(255) NOT NULL,
group_value_stream_id bigint group_value_stream_id bigint NOT NULL
); );
CREATE SEQUENCE public.analytics_cycle_analytics_group_stages_id_seq CREATE SEQUENCE public.analytics_cycle_analytics_group_stages_id_seq
...@@ -21263,7 +21263,7 @@ ALTER TABLE ONLY public.merge_request_metrics ...@@ -21263,7 +21263,7 @@ ALTER TABLE ONLY public.merge_request_metrics
ADD CONSTRAINT fk_ae440388cc FOREIGN KEY (latest_closed_by_id) REFERENCES public.users(id) ON DELETE SET NULL; ADD CONSTRAINT fk_ae440388cc FOREIGN KEY (latest_closed_by_id) REFERENCES public.users(id) ON DELETE SET NULL;
ALTER TABLE ONLY public.analytics_cycle_analytics_group_stages ALTER TABLE ONLY public.analytics_cycle_analytics_group_stages
ADD CONSTRAINT fk_analytics_cycle_analytics_group_stages_group_value_stream_id FOREIGN KEY (group_value_stream_id) REFERENCES public.analytics_cycle_analytics_group_value_streams(id) ON DELETE CASCADE NOT VALID; ADD CONSTRAINT fk_analytics_cycle_analytics_group_stages_group_value_stream_id FOREIGN KEY (group_value_stream_id) REFERENCES public.analytics_cycle_analytics_group_value_streams(id) ON DELETE CASCADE;
ALTER TABLE ONLY public.fork_network_members ALTER TABLE ONLY public.fork_network_members
ADD CONSTRAINT fk_b01280dae4 FOREIGN KEY (forked_from_project_id) REFERENCES public.projects(id) ON DELETE SET NULL; ADD CONSTRAINT fk_b01280dae4 FOREIGN KEY (forked_from_project_id) REFERENCES public.projects(id) ON DELETE SET NULL;
...@@ -23722,6 +23722,8 @@ COPY "schema_migrations" (version) FROM STDIN; ...@@ -23722,6 +23722,8 @@ COPY "schema_migrations" (version) FROM STDIN;
20200630091656 20200630091656
20200630110826 20200630110826
20200701064756 20200701064756
20200701070435
20200701091253
20200701093859 20200701093859
20200701205710 20200701205710
20200702123805 20200702123805
......
...@@ -9,6 +9,7 @@ module Analytics ...@@ -9,6 +9,7 @@ module Analytics
check_feature_flag Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG check_feature_flag Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG
before_action :load_group before_action :load_group
before_action :load_value_stream
before_action :validate_params, only: %i[median records duration_chart] before_action :validate_params, only: %i[median records duration_chart]
def index def index
...@@ -79,7 +80,7 @@ module Analytics ...@@ -79,7 +80,7 @@ module Analytics
end end
def list_service def list_service
::Analytics::CycleAnalytics::Stages::ListService.new(parent: @group, current_user: current_user) ::Analytics::CycleAnalytics::Stages::ListService.new(parent: @group, current_user: current_user, params: list_params)
end end
def create_service def create_service
...@@ -108,17 +109,27 @@ module Analytics ...@@ -108,17 +109,27 @@ module Analytics
super.merge({ group: @group }) super.merge({ group: @group })
end end
def list_params
{ value_stream: @value_stream }
end
def update_params def update_params
params.permit(:name, :start_event_identifier, :end_event_identifier, :id, :move_after_id, :move_before_id, :hidden, :start_event_label_id, :end_event_label_id) params.permit(:name, :start_event_identifier, :end_event_identifier, :id, :move_after_id, :move_before_id, :hidden, :start_event_label_id, :end_event_label_id).merge(list_params)
end end
def create_params def create_params
params.permit(:name, :start_event_identifier, :end_event_identifier, :start_event_label_id, :end_event_label_id) params.permit(:name, :start_event_identifier, :end_event_identifier, :start_event_label_id, :end_event_label_id).merge(list_params)
end end
def delete_params def delete_params
params.permit(:id) params.permit(:id)
end end
def load_value_stream
if params[:value_stream_id]
@value_stream = @group.value_streams.find(params[:value_stream_id])
end
end
end end
end end
end end
...@@ -12,6 +12,8 @@ module Analytics ...@@ -12,6 +12,8 @@ module Analytics
alias_attribute :parent, :group alias_attribute :parent, :group
alias_attribute :parent_id, :group_id alias_attribute :parent_id, :group_id
scope :by_value_stream, -> (value_stream) { where(group_value_stream_id: value_stream.id) }
def self.relative_positioning_query_base(stage) def self.relative_positioning_query_base(stage)
where(group_id: stage.group_id) where(group_id: stage.group_id)
end end
......
...@@ -6,9 +6,12 @@ module Analytics ...@@ -6,9 +6,12 @@ module Analytics
class BaseService class BaseService
include Gitlab::Allowable include Gitlab::Allowable
DEFAULT_VALUE_STREAM_NAME = 'default'
def initialize(parent:, current_user:, params: {}) def initialize(parent:, current_user:, params: {})
@parent = parent @parent = parent
@current_user = current_user @current_user = current_user
@params = params
end end
def execute def execute
...@@ -47,10 +50,14 @@ module Analytics ...@@ -47,10 +50,14 @@ module Analytics
end end
def build_default_stages def build_default_stages
Gitlab::Analytics::CycleAnalytics::DefaultStages.all.map do |params| Gitlab::Analytics::CycleAnalytics::DefaultStages.all.map do |stage_params|
parent.cycle_analytics_stages.build(params) parent.cycle_analytics_stages.build(stage_params.merge(value_stream: value_stream))
end end
end end
def value_stream
@value_stream ||= params[:value_stream] || parent.value_streams.safe_find_or_create_by!(name: DEFAULT_VALUE_STREAM_NAME)
end
end end
end end
end end
......
...@@ -16,6 +16,7 @@ module Analytics ...@@ -16,6 +16,7 @@ module Analytics
parent.class.transaction do parent.class.transaction do
persist_default_stages! persist_default_stages!
stage.value_stream ||= value_stream
stage.save! stage.save!
end end
......
...@@ -17,7 +17,9 @@ module Analytics ...@@ -17,7 +17,9 @@ module Analytics
end end
def persisted_stages def persisted_stages
parent.cycle_analytics_stages.for_list scope = parent.cycle_analytics_stages
scope = scope.by_value_stream(params[:value_stream]) if params[:value_stream]
scope.for_list
end end
end end
end end
......
---
title: Enforce presence of Value Stream when creating a stage
merge_request: 36661
author:
type: added
...@@ -37,7 +37,15 @@ constraints(::Constraints::GroupUrlConstrainer.new) do ...@@ -37,7 +37,15 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
get :records get :records
end end
end end
resources :value_streams, only: [:index, :create] resources :value_streams, only: [:index, :create] do
resources :stages, only: [:index, :create, :update, :destroy] do
member do
get :duration_chart
get :median
get :records
end
end
end
resource :summary, controller: :summary, only: :show resource :summary, controller: :summary, only: :show
get '/time_summary' => 'summary#time_summary' get '/time_summary' => 'summary#time_summary'
end end
......
...@@ -5,231 +5,19 @@ require 'spec_helper' ...@@ -5,231 +5,19 @@ require 'spec_helper'
RSpec.describe Groups::Analytics::CycleAnalytics::StagesController do RSpec.describe Groups::Analytics::CycleAnalytics::StagesController do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:group, refind: true) { create(:group) } let_it_be(:group, refind: true) { create(:group) }
let(:params) { { group_id: group } }
before do context 'when params have only group_id' do
stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => true) let(:params) { { group_id: group } }
stub_licensed_features(cycle_analytics_for_groups: true) let(:parent) { group }
group.add_reporter(user) it_behaves_like 'cycle analytics stages controller'
sign_in(user)
end end
describe 'GET #index' do context 'when params have group_id and value_stream_id' do
subject { get :index, params: params } let_it_be(:value_stream) { create(:cycle_analytics_group_value_stream, group: group) }
let(:params) { { group_id: group, value_stream_id: value_stream.id } }
let(:parent) { group }
it 'succeeds' do it_behaves_like 'cycle analytics stages controller'
subject
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('analytics/cycle_analytics/stages', dir: 'ee')
end
it 'returns correct start events' do
subject
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'] }
expect(response_start_events).to eq(start_events)
end
it 'does not include internal events' do
subject
response_event_names = json_response['events'].map { |s| s['name'] }
event_names = Gitlab::Analytics::CycleAnalytics::StageEvents.events
internal_events = Gitlab::Analytics::CycleAnalytics::StageEvents.internal_events
expected_event_names = (event_names - internal_events).map(&:name)
expect(response_event_names).to eq(expected_event_names.sort)
end
it 'succeeds for subgroups' do
subgroup = create(:group, parent: group)
params[:group_id] = subgroup.full_path
subject
expect(response).to have_gitlab_http_status(:ok)
end
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)
subject
expect(response).to have_gitlab_http_status(:forbidden)
end
include_examples 'group permission check on the controller level'
end
describe 'POST #create' do
subject { post :create, params: params }
include_examples 'group permission check on the controller level'
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 'creates the stage' do
subject
expect(response).to have_gitlab_http_status(:created)
expect(response).to match_response_schema('analytics/cycle_analytics/stage', dir: 'ee')
end
end
include_context 'when invalid stage parameters are given'
end
describe 'PUT #update' do
let(:stage) { create(:cycle_analytics_group_stage, parent: group, relative_position: 15) }
subject { put :update, params: params.merge(id: stage.id) }
include_examples 'group permission check on the controller level'
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
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('analytics/cycle_analytics/stage', dir: 'ee')
end
it 'updates the name attribute' do
subject
stage.reload
expect(stage.name).to eq(params[:name])
end
context 'hidden attribute' do
before do
params[:hidden] = true
end
it 'updates the hidden attribute' do
subject
stage.reload
expect(stage.hidden).to eq(true)
end
end
context 'when positioning parameter is given' do
before do
params[:move_before_id] = create(:cycle_analytics_group_stage, parent: group, relative_position: 10).id
end
it 'moves the stage before the last place' do
subject
before_last = group.cycle_analytics_stages.ordered[-2]
expect(before_last.id).to eq(stage.id)
end
end
end
include_context 'when invalid stage parameters are given'
end
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 have_gitlab_http_status(:ok)
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
it 'fails with `forbidden` response' do
subject
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
describe 'data endpoints' do
let(:stage) { create(:cycle_analytics_group_stage, parent: group) }
before do
params[:id] = stage.id
end
describe 'GET #median' do
subject { get :median, params: params }
it 'matches the response schema' do
subject
expect(response).to match_response_schema('analytics/cycle_analytics/median', dir: 'ee')
end
include_examples 'cycle analytics data endpoint examples'
end
describe 'GET #records' do
subject { get :records, params: params }
include_examples 'cycle analytics data endpoint examples'
include_examples 'group permission check on the controller level'
end
describe 'GET #duration_chart' do
subject { get :duration_chart, params: params }
it 'matches the response schema' do
fake_result = [double(MergeRequest, duration_in_seconds: 10, finished_at: Time.current)]
expect_any_instance_of(Gitlab::Analytics::CycleAnalytics::DataForDurationChart).to receive(:load).and_return(fake_result)
subject
expect(response).to match_response_schema('analytics/cycle_analytics/duration_chart', dir: 'ee')
end
include_examples 'cycle analytics data endpoint examples'
include_examples 'group permission check on the controller level'
end
end end
end end
...@@ -5,6 +5,8 @@ FactoryBot.define do ...@@ -5,6 +5,8 @@ FactoryBot.define do
sequence(:name) { |n| "Stage ##{n}" } sequence(:name) { |n| "Stage ##{n}" }
start_event_identifier { Gitlab::Analytics::CycleAnalytics::StageEvents::MergeRequestCreated.identifier } start_event_identifier { Gitlab::Analytics::CycleAnalytics::StageEvents::MergeRequestCreated.identifier }
end_event_identifier { Gitlab::Analytics::CycleAnalytics::StageEvents::MergeRequestMerged.identifier } end_event_identifier { Gitlab::Analytics::CycleAnalytics::StageEvents::MergeRequestMerged.identifier }
group group
value_stream { FactoryBot.build(:cycle_analytics_group_value_stream, group: group) }
end end
end end
...@@ -4,7 +4,8 @@ require 'spec_helper' ...@@ -4,7 +4,8 @@ require 'spec_helper'
RSpec.describe 'Analytics (JavaScript fixtures)', :sidekiq_inline do RSpec.describe 'Analytics (JavaScript fixtures)', :sidekiq_inline do
include JavaScriptFixturesHelpers include JavaScriptFixturesHelpers
let(:group) { create(:group)} let(:group) { create(:group) }
let(:value_stream) { create(:cycle_analytics_group_value_stream, group: group) }
let(:project) { create(:project, :repository, namespace: group) } let(:project) { create(:project, :repository, namespace: group) }
let(:user) { create(:user, :admin) } let(:user) { create(:user, :admin) }
let(:milestone) { create(:milestone, project: project) } let(:milestone) { create(:milestone, project: project) }
...@@ -134,7 +135,7 @@ RSpec.describe 'Analytics (JavaScript fixtures)', :sidekiq_inline do ...@@ -134,7 +135,7 @@ RSpec.describe 'Analytics (JavaScript fixtures)', :sidekiq_inline do
# Persist the default stages # Persist the default stages
Gitlab::Analytics::CycleAnalytics::DefaultStages.all.map do |params| Gitlab::Analytics::CycleAnalytics::DefaultStages.all.map do |params|
group.cycle_analytics_stages.build(params).save! group.cycle_analytics_stages.build(params.merge(value_stream: value_stream)).save!
end end
create_label_based_cycle_analytics_stage create_label_based_cycle_analytics_stage
......
...@@ -4,8 +4,17 @@ require 'spec_helper' ...@@ -4,8 +4,17 @@ require 'spec_helper'
RSpec.describe Analytics::CycleAnalytics::Stages::CreateService do RSpec.describe Analytics::CycleAnalytics::Stages::CreateService do
let_it_be(:group, refind: true) { create(:group) } let_it_be(:group, refind: true) { create(:group) }
let_it_be(:value_stream, refind: true) { create(:cycle_analytics_group_value_stream, group: group) }
let_it_be(:user, refind: true) { create(:user) } let_it_be(:user, refind: true) { create(:user) }
let(:params) { { name: 'my stage', start_event_identifier: :merge_request_created, end_event_identifier: :merge_request_merged } }
let(:params) do
{
name: 'my stage',
value_stream: value_stream,
start_event_identifier: :merge_request_created,
end_event_identifier: :merge_request_merged
}
end
before_all do before_all do
group.add_user(user, :reporter) group.add_user(user, :reporter)
...@@ -96,7 +105,8 @@ RSpec.describe Analytics::CycleAnalytics::Stages::CreateService do ...@@ -96,7 +105,8 @@ RSpec.describe Analytics::CycleAnalytics::Stages::CreateService do
start_event_identifier: :issue_label_added, start_event_identifier: :issue_label_added,
end_event_identifier: :issue_label_removed, end_event_identifier: :issue_label_removed,
start_event_label_id: label.id, start_event_label_id: label.id,
end_event_label_id: label.id end_event_label_id: label.id,
value_stream: value_stream
} }
end end
...@@ -111,4 +121,17 @@ RSpec.describe Analytics::CycleAnalytics::Stages::CreateService do ...@@ -111,4 +121,17 @@ RSpec.describe Analytics::CycleAnalytics::Stages::CreateService do
expect(stage.end_event_label).to eq(label) expect(stage.end_event_label).to eq(label)
end end
end end
context 'when `value_stream` is not provided' do
before do
params.delete(:value_stream)
end
let(:stage) { subject.payload[:stage] }
it 'creates a `default` value stream object' do
expect(stage).to be_persisted
expect(stage.value_stream.name).to eq(Analytics::CycleAnalytics::Stages::BaseService::DEFAULT_VALUE_STREAM_NAME)
end
end
end end
...@@ -4,8 +4,9 @@ require 'spec_helper' ...@@ -4,8 +4,9 @@ require 'spec_helper'
RSpec.describe Analytics::CycleAnalytics::Stages::DeleteService do RSpec.describe Analytics::CycleAnalytics::Stages::DeleteService do
let_it_be(:group, refind: true) { create(:group) } let_it_be(:group, refind: true) { create(:group) }
let_it_be(:value_stream, refind: true) { create(:cycle_analytics_group_value_stream, group: group) }
let_it_be(:user, refind: true) { create(:user) } let_it_be(:user, refind: true) { create(:user) }
let_it_be(:stage, refind: true) { create(:cycle_analytics_group_stage, group: group) } let_it_be(:stage, refind: true) { create(:cycle_analytics_group_stage, group: group, value_stream: value_stream) }
let(:params) { { id: stage.id } } let(:params) { { id: stage.id } }
subject { described_class.new(parent: group, params: params, current_user: user).execute } subject { described_class.new(parent: group, params: params, current_user: user).execute }
...@@ -31,7 +32,7 @@ RSpec.describe Analytics::CycleAnalytics::Stages::DeleteService do ...@@ -31,7 +32,7 @@ RSpec.describe Analytics::CycleAnalytics::Stages::DeleteService do
end end
context 'disallows deletion when default stage is given' do context 'disallows deletion when default stage is given' do
let_it_be(:stage, refind: true) { create(:cycle_analytics_group_stage, group: group, custom: false) } let_it_be(:stage, refind: true) { create(:cycle_analytics_group_stage, group: group, custom: false, value_stream: value_stream) }
it { expect(subject).not_to be_success } it { expect(subject).not_to be_success }
it { expect(subject.http_status).to eq(:forbidden) } it { expect(subject.http_status).to eq(:forbidden) }
......
...@@ -4,6 +4,7 @@ require 'spec_helper' ...@@ -4,6 +4,7 @@ require 'spec_helper'
RSpec.describe Analytics::CycleAnalytics::Stages::ListService do RSpec.describe Analytics::CycleAnalytics::Stages::ListService do
let_it_be(:group, refind: true) { create(:group) } let_it_be(:group, refind: true) { create(:group) }
let_it_be(:value_stream, refind: true) { create(:cycle_analytics_group_value_stream, group: group) }
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let(:stages) { subject.payload[:stages] } let(:stages) { subject.payload[:stages] }
...@@ -28,9 +29,9 @@ RSpec.describe Analytics::CycleAnalytics::Stages::ListService do ...@@ -28,9 +29,9 @@ RSpec.describe Analytics::CycleAnalytics::Stages::ListService do
end end
context 'when there are persisted stages' do context 'when there are persisted stages' do
let_it_be(:stage1) { create(:cycle_analytics_group_stage, parent: group, relative_position: 2) } let_it_be(:stage1) { create(:cycle_analytics_group_stage, parent: group, relative_position: 2, value_stream: value_stream) }
let_it_be(:stage2) { create(:cycle_analytics_group_stage, parent: group, relative_position: 3) } let_it_be(:stage2) { create(:cycle_analytics_group_stage, parent: group, relative_position: 3, value_stream: value_stream) }
let_it_be(:stage3) { create(:cycle_analytics_group_stage, parent: group, relative_position: 1) } let_it_be(:stage3) { create(:cycle_analytics_group_stage, parent: group, relative_position: 1, value_stream: value_stream) }
it 'returns the persisted stages in order' do it 'returns the persisted stages in order' do
expect(stages).to eq([stage3, stage1, stage2]) expect(stages).to eq([stage3, stage1, stage2])
......
...@@ -4,9 +4,10 @@ require 'spec_helper' ...@@ -4,9 +4,10 @@ require 'spec_helper'
RSpec.describe Analytics::CycleAnalytics::Stages::UpdateService do RSpec.describe Analytics::CycleAnalytics::Stages::UpdateService do
let_it_be(:group, refind: true) { create(:group) } let_it_be(:group, refind: true) { create(:group) }
let_it_be(:value_stream, refind: true) { create(:cycle_analytics_group_value_stream, group: group) }
let_it_be(:user, refind: true) { create(:user) } let_it_be(:user, refind: true) { create(:user) }
let(:default_stages) { Gitlab::Analytics::CycleAnalytics::DefaultStages.all } let(:default_stages) { Gitlab::Analytics::CycleAnalytics::DefaultStages.all }
let(:params) { {} } let(:params) { { value_stream: value_stream } }
let(:persisted_stages) { group.reload.cycle_analytics_stages.ordered } let(:persisted_stages) { group.reload.cycle_analytics_stages.ordered }
subject { described_class.new(parent: group, params: params, current_user: user).execute } subject { described_class.new(parent: group, params: params, current_user: user).execute }
...@@ -23,7 +24,7 @@ RSpec.describe Analytics::CycleAnalytics::Stages::UpdateService do ...@@ -23,7 +24,7 @@ RSpec.describe Analytics::CycleAnalytics::Stages::UpdateService do
context 'when updating a default stage' do context 'when updating a default stage' do
let(:stage) { Analytics::CycleAnalytics::GroupStage.new(default_stages.first.merge(group: group)) } let(:stage) { Analytics::CycleAnalytics::GroupStage.new(default_stages.first.merge(group: group)) }
let(:params) { { id: stage.name, hidden: true } } let(:params) { { id: stage.name, hidden: true, value_stream: value_stream } }
let(:updated_stage) { subject.payload[:stage] } let(:updated_stage) { subject.payload[:stage] }
context 'when hiding a default stage' do context 'when hiding a default stage' do
...@@ -96,8 +97,8 @@ RSpec.describe Analytics::CycleAnalytics::Stages::UpdateService do ...@@ -96,8 +97,8 @@ RSpec.describe Analytics::CycleAnalytics::Stages::UpdateService do
end end
context 'when updating a custom stage' do context 'when updating a custom stage' do
let_it_be(:stage) { create(:cycle_analytics_group_stage, group: group) } let_it_be(:stage) { create(:cycle_analytics_group_stage, group: group, value_stream: value_stream) }
let(:params) { { id: stage.id, name: 'my new stage name' } } let(:params) { { id: stage.id, name: 'my new stage name', value_stream: value_stream } }
it { expect(subject).to be_success } it { expect(subject).to be_success }
it { expect(subject.http_status).to eq(:ok) } it { expect(subject.http_status).to eq(:ok) }
...@@ -115,13 +116,13 @@ RSpec.describe Analytics::CycleAnalytics::Stages::UpdateService do ...@@ -115,13 +116,13 @@ RSpec.describe Analytics::CycleAnalytics::Stages::UpdateService do
end end
context 'when positioning a stage' do context 'when positioning a stage' do
let!(:first_stage) { create(:cycle_analytics_group_stage, group: group, relative_position: 10) } let!(:first_stage) { create(:cycle_analytics_group_stage, group: group, value_stream: value_stream, relative_position: 10) }
let!(:middle_stage) { create(:cycle_analytics_group_stage, group: group, relative_position: 11) } let!(:middle_stage) { create(:cycle_analytics_group_stage, group: group, value_stream: value_stream, relative_position: 11) }
let!(:last_stage) { create(:cycle_analytics_group_stage, group: group, relative_position: 12) } let!(:last_stage) { create(:cycle_analytics_group_stage, group: group, value_stream: value_stream, relative_position: 12) }
context 'when there are stages without position' do context 'when there are stages without position' do
let!(:unpositioned_stage1) { create(:cycle_analytics_group_stage, group: group) } let!(:unpositioned_stage1) { create(:cycle_analytics_group_stage, group: group, value_stream: value_stream) }
let!(:unpositioned_stage2) { create(:cycle_analytics_group_stage, group: group) } let!(:unpositioned_stage2) { create(:cycle_analytics_group_stage, group: group, value_stream: value_stream) }
before do before do
params[:id] = first_stage.id params[:id] = first_stage.id
...@@ -182,7 +183,7 @@ RSpec.describe Analytics::CycleAnalytics::Stages::UpdateService do ...@@ -182,7 +183,7 @@ RSpec.describe Analytics::CycleAnalytics::Stages::UpdateService do
context 'when `move_before_id` points to a stage within a different group' do context 'when `move_before_id` points to a stage within a different group' do
before do before do
params[:id] = last_stage.id params[:id] = last_stage.id
params[:move_before_id] = create(:cycle_analytics_group_stage, group: create(:group)).id params[:move_before_id] = create(:cycle_analytics_group_stage, group: create(:group), value_stream: value_stream).id
end end
it { expect { subject }.to raise_error(ActiveRecord::RecordNotFound) } it { expect { subject }.to raise_error(ActiveRecord::RecordNotFound) }
......
# frozen_string_literal: true # frozen_string_literal: true
RSpec.shared_examples 'cycle analytics stages controller' do
before do
stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => true)
stub_licensed_features(cycle_analytics_for_groups: true)
group.add_reporter(user)
sign_in(user)
end
describe 'GET #index' do
subject { get :index, params: params }
it 'succeeds' do
subject
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('analytics/cycle_analytics/stages', dir: 'ee')
end
it 'returns correct start events' do
subject
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'] }
expect(response_start_events).to eq(start_events)
end
it 'does not include internal events' do
subject
response_event_names = json_response['events'].map { |s| s['name'] }
event_names = Gitlab::Analytics::CycleAnalytics::StageEvents.events
internal_events = Gitlab::Analytics::CycleAnalytics::StageEvents.internal_events
expected_event_names = (event_names - internal_events).map(&:name)
expect(response_event_names).to eq(expected_event_names.sort)
end
it 'succeeds for subgroups' do
subgroup = create(:group, parent: group)
params[:group_id] = subgroup.full_path
params[:value_stream_id] = create(:cycle_analytics_group_value_stream, group: subgroup).id
subject
expect(response).to have_gitlab_http_status(:ok)
end
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)
subject
expect(response).to have_gitlab_http_status(:forbidden)
end
include_examples 'group permission check on the controller level'
end
describe 'POST #create' do
subject { post :create, params: params }
include_examples 'group permission check on the controller level'
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 'creates the stage' do
subject
expect(response).to have_gitlab_http_status(:created)
expect(response).to match_response_schema('analytics/cycle_analytics/stage', dir: 'ee')
end
end
include_context 'when invalid stage parameters are given'
end
describe 'PUT #update' do
let(:stage) { create(:cycle_analytics_group_stage, parent: parent, relative_position: 15) }
subject { put :update, params: params.merge(id: stage.id) }
include_examples 'group permission check on the controller level'
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
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('analytics/cycle_analytics/stage', dir: 'ee')
end
it 'updates the name attribute' do
subject
stage.reload
expect(stage.name).to eq(params[:name])
end
context 'hidden attribute' do
before do
params[:hidden] = true
end
it 'updates the hidden attribute' do
subject
stage.reload
expect(stage.hidden).to eq(true)
end
end
context 'when positioning parameter is given' do
before do
params[:move_before_id] = create(:cycle_analytics_group_stage, parent: group, relative_position: 10).id
end
it 'moves the stage before the last place' do
subject
before_last = group.cycle_analytics_stages.ordered[-2]
expect(before_last.id).to eq(stage.id)
end
end
end
include_context 'when invalid stage parameters are given'
end
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 have_gitlab_http_status(:ok)
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
it 'fails with `forbidden` response' do
subject
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
describe 'data endpoints' do
let(:stage) { create(:cycle_analytics_group_stage, parent: group) }
before do
params[:id] = stage.id
end
describe 'GET #median' do
subject { get :median, params: params }
it 'matches the response schema' do
subject
expect(response).to match_response_schema('analytics/cycle_analytics/median', dir: 'ee')
end
include_examples 'cycle analytics data endpoint examples'
end
describe 'GET #records' do
subject { get :records, params: params }
include_examples 'cycle analytics data endpoint examples'
include_examples 'group permission check on the controller level'
end
describe 'GET #duration_chart' do
subject { get :duration_chart, params: params }
it 'matches the response schema' do
fake_result = [double(MergeRequest, duration_in_seconds: 10, finished_at: Time.current)]
expect_any_instance_of(Gitlab::Analytics::CycleAnalytics::DataForDurationChart).to receive(:load).and_return(fake_result)
subject
expect(response).to match_response_schema('analytics/cycle_analytics/duration_chart', dir: 'ee')
end
include_examples 'cycle analytics data endpoint examples'
include_examples 'group permission check on the controller level'
end
end
end
RSpec.shared_examples 'group permission check on the controller level' do RSpec.shared_examples 'group permission check on the controller level' do
context 'when `group_id` is not found' do context 'when `group_id` is not found' do
before do before do
......
# frozen_string_literal: true
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20200701070435_add_default_value_stream_to_groups_with_group_stages.rb')
RSpec.describe AddDefaultValueStreamToGroupsWithGroupStages, schema: 20200624142207 do
let(:groups) { table(:namespaces) }
let(:group_stages) { table(:analytics_cycle_analytics_group_stages) }
let(:value_streams) { table(:analytics_cycle_analytics_group_value_streams) }
let!(:group) { groups.create!(name: 'test', path: 'path', type: 'Group') }
let!(:group_stage) { group_stages.create!(name: 'test', group_id: group.id, start_event_identifier: 1, end_event_identifier: 2) }
describe '#up' do
it 'creates default value stream record for the group' do
migrate!
group_value_streams = value_streams.where(group_id: group.id)
expect(group_value_streams.size).to eq(1)
value_stream = group_value_streams.first
expect(value_stream.name).to eq('default')
end
it 'migrates existing stages to the default value stream' do
migrate!
group_stage.reload
value_stream = value_streams.find_by(group_id: group.id, name: 'default')
expect(group_stage.group_value_stream_id).to eq(value_stream.id)
end
end
describe '#down' do
it 'sets the group_value_stream_id to nil' do
described_class.new.down
group_stage.reload
expect(group_stage.group_value_stream_id).to be_nil
end
end
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