Merge branch '323982-remove-enable-value-stream-analytics-horizontal-navigation-ff' into 'master'

Remove `value_stream_analytics_path_navigation` feature flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!60449

doc/user/group/value_stream_analytics/index.md

@@ -189,9 +189,7 @@ GitLab allows users to create multiple value streams, hide default stages and cr
 ### Stage path
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/210315) in GitLab 13.0.
-> - It's [deployed behind a feature flag](../../feature_flags.md), enabled by default.
-> - It's enabled on GitLab.com.
-> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](../../../administration/feature_flags.md). **(FREE SELF)**
+> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/323982) in GitLab 13.12.
 
 ![Value stream path navigation](img/vsa_path_nav_v13_11.png "Value stream path navigation")
 
@@ -213,14 +211,6 @@ Hovering over a stage item displays a popover with the following information:
 - Start event description for the given stage
 - End event description
 
-Horizontal path navigation is enabled by default. If you have a self-managed instance, an
-administrator can [open a Rails console](../../../administration/troubleshooting/navigating_gitlab_via_rails_console.md)
-and disable it with the following command:
-
-```ruby
-Feature.disable(:value_stream_analytics_path_navigation)
-```
-
 ### Adding a stage
 
 In the following example we're creating a new stage that measures and tracks issues from creation

ee/app/assets/javascripts/analytics/cycle_analytics/components/base.vue

@@ -7,13 +7,10 @@ import ProjectsDropdownFilter from '../../shared/components/projects_dropdown_fi
 import { DATE_RANGE_LIMIT } from '../../shared/constants';
 import { toYmd } from '../../shared/utils';
 import { PROJECTS_PER_PAGE, OVERVIEW_STAGE_ID } from '../constants';
-import CustomStageForm from './custom_stage_form.vue';
 import DurationChart from './duration_chart.vue';
 import FilterBar from './filter_bar.vue';
 import Metrics from './metrics.vue';
 import PathNavigation from './path_navigation.vue';
-import StageTable from './stage_table.vue';
-import StageTableNav from './stage_table_nav.vue';
 import StageTableNew from './stage_table_new.vue';
 import TypeOfWorkCharts from './type_of_work_charts.vue';
 import ValueStreamSelect from './value_stream_select.vue';
@@ -25,10 +22,7 @@ export default {
     DurationChart,
     GlEmptyState,
     ProjectsDropdownFilter,
-    StageTable,
     TypeOfWorkCharts,
-    CustomStageForm,
-    StageTableNav,
     StageTableNew,
     PathNavigation,
     FilterBar,
@@ -89,31 +83,14 @@ export default {
       return !this.currentGroup && !this.isLoading;
     },
     shouldDisplayFilters() {
-      return !this.errorCode;
+      return !this.errorCode && !this.hasNoAccessError;
     },
     shouldDisplayDurationChart() {
-      return (
-        !this.featureFlags.hasPathNavigation ||
-        (this.featureFlags.hasDurationChart &&
-          this.isOverviewStageSelected &&
-          !this.hasNoAccessError)
-      );
-    },
-    shouldDisplayTypeOfWorkCharts() {
-      return (
-        !this.featureFlags.hasPathNavigation ||
-        (this.isOverviewStageSelected && !this.hasNoAccessError)
-      );
+      return this.featureFlags.hasDurationChart;
     },
     selectedStageReady() {
       return !this.hasNoAccessError && this.selectedStage;
     },
-    shouldDisplayPathNavigation() {
-      return this.featureFlags.hasPathNavigation && this.selectedStageReady;
-    },
-    shouldDisplayVerticalNavigation() {
-      return !this.featureFlags.hasPathNavigation && this.selectedStageReady;
-    },
     shouldDisplayCreateMultipleValueStreams() {
       return Boolean(!this.shouldRenderEmptyState && !this.isLoadingValueStreams);
     },
@@ -122,12 +99,6 @@ export default {
     },
     query() {
       const selectedProjectIds = this.selectedProjectIds?.length ? this.selectedProjectIds : null;
-      const stageParams = this.featureFlags.hasPathNavigation
-        ? {
-            sort: (!this.isOverviewStageSelected && this.pagination?.sort) || null,
-            direction: (!this.isOverviewStageSelected && this.pagination?.direction) || null,
-          }
-        : {};
 
       return {
         value_stream_id: this.selectedValueStream?.id || null,
@@ -135,7 +106,8 @@ export default {
         created_after: toYmd(this.startDate),
         created_before: toYmd(this.endDate),
         stage_id: (!this.isOverviewStageSelected && this.selectedStage?.id) || null, // the `overview` stage is always the default, so dont persist the id if its selected
-        ...stageParams,
+        sort: (!this.isOverviewStageSelected && this.pagination?.sort) || null,
+        direction: (!this.isOverviewStageSelected && this.pagination?.direction) || null,
       };
     },
     stageCount() {
@@ -224,7 +196,7 @@ export default {
     />
     <div v-if="!shouldRenderEmptyState" class="gl-max-w-full">
       <path-navigation
-        v-if="shouldDisplayPathNavigation"
+        v-if="selectedStageReady"
         :key="`path_navigation_key_${pathNavigationData.length}`"
         class="js-path-navigation gl-w-full gl-pb-2"
         :loading="isLoading"
@@ -276,14 +248,17 @@ export default {
         "
       />
       <template v-else>
-        <metrics
-          v-if="!featureFlags.hasPathNavigation || isOverviewStageSelected"
-          :group-path="currentGroupPath"
-          :request-params="cycleAnalyticsRequestParams"
+        <template v-if="isOverviewStageSelected">
+          <metrics :group-path="currentGroupPath" :request-params="cycleAnalyticsRequestParams" />
+          <duration-chart
+            v-if="shouldDisplayDurationChart"
+            class="gl-mt-3"
+            :stages="activeStages"
           />
-        <template v-if="featureFlags.hasPathNavigation">
+          <type-of-work-charts />
+        </template>
         <stage-table-new
-            v-if="!isLoading && !isOverviewStageSelected"
+          v-else
           :is-loading="isLoading || isLoadingStage"
           :stage-events="currentStageEvents"
           :selected-stage="selectedStage"
@@ -292,49 +267,8 @@ export default {
           :pagination="pagination"
           @handleUpdatePagination="onHandleUpdatePagination"
         />
+        <url-sync v-if="selectedStageReady" :query="query" />
       </template>
-        <stage-table
-          v-else
-          :key="stageCount"
-          class="js-stage-table"
-          :current-stage="selectedStage"
-          :is-loading="isLoading"
-          :is-loading-stage="isLoadingStage"
-          :is-empty-stage="isEmptyStage"
-          :custom-stage-form-active="customStageFormActive"
-          :current-stage-events="currentStageEvents"
-          :no-data-svg-path="noDataSvgPath"
-          :empty-state-message="selectedStageError"
-          :has-path-navigation="featureFlags.hasPathNavigation"
-        >
-          <template v-if="shouldDisplayVerticalNavigation" #nav>
-            <stage-table-nav
-              :current-stage="selectedStage"
-              :stages="activeStages"
-              :medians="medians"
-              :is-creating-custom-stage="isCreatingCustomStage"
-              :custom-ordering="enableCustomOrdering"
-              @reorderStage="onStageReorder"
-              @selectStage="onStageSelect"
-              @editStage="onShowEditStageForm"
-              @showAddStageForm="onShowAddStageForm"
-              @hideStage="onUpdateCustomStage"
-              @removeStage="onRemoveStage"
-            />
-          </template>
-          <template v-if="customStageFormActive" #content>
-            <custom-stage-form
-              :events="formEvents"
-              @createStage="onCreateCustomStage"
-              @updateStage="onUpdateCustomStage"
-              @clearErrors="$emit('clear-form-errors')"
-            />
-          </template>
-        </stage-table>
-        <url-sync :query="query" />
-      </template>
-      <duration-chart v-if="shouldDisplayDurationChart" class="gl-mt-3" :stages="activeStages" />
-      <type-of-work-charts v-if="shouldDisplayTypeOfWorkCharts" />
     </div>
   </div>
 </template>

ee/app/assets/javascripts/analytics/cycle_analytics/index.js

@@ -19,10 +19,7 @@ export default () => {
   const { emptyStateSvgPath, noDataSvgPath, noAccessSvgPath } = el.dataset;
   const initialData = buildCycleAnalyticsInitialData(el.dataset);
   const store = createStore();
-  const {
-    cycleAnalyticsScatterplotEnabled: hasDurationChart = false,
-    valueStreamAnalyticsPathNavigation: hasPathNavigation = false,
-  } = gon?.features;
+  const { cycleAnalyticsScatterplotEnabled: hasDurationChart = false } = gon?.features;
 
   const {
     author_username = null,
@@ -39,7 +36,7 @@ export default () => {
     selectedMilestone: milestone_title,
     selectedAssigneeList: assignee_username,
     selectedLabelList: label_name,
-    featureFlags: { hasDurationChart, hasPathNavigation },
+    featureFlags: { hasDurationChart },
     pagination: { sort: sort?.value || null, direction: direction?.value || null },
   });
 

ee/app/assets/javascripts/analytics/cycle_analytics/store/actions.js

@@ -172,41 +172,12 @@ export const receiveGroupStagesError = ({ commit }, error) => {
   });
 };
 
-export const setDefaultSelectedStage = ({ state: { featureFlags }, dispatch, getters }) => {
-  const { activeStages = [] } = getters;
+export const setDefaultSelectedStage = ({ dispatch }) =>
+  dispatch('setSelectedStage', OVERVIEW_STAGE_CONFIG);
 
-  if (featureFlags?.hasPathNavigation) {
-    return dispatch('setSelectedStage', OVERVIEW_STAGE_CONFIG);
-  }
-
-  if (activeStages?.length) {
-    const [firstActiveStage] = activeStages;
-    return Promise.all([
-      dispatch('setSelectedStage', firstActiveStage),
-      dispatch('fetchStageData', firstActiveStage.slug),
-    ]);
-  }
-
-  createFlash({
-    message: __('There was an error while fetching value stream analytics data.'),
-  });
-
-  return Promise.resolve();
-};
-
-export const receiveGroupStagesSuccess = (
-  { state: { featureFlags }, commit, dispatch },
-  stages,
-) => {
+export const receiveGroupStagesSuccess = ({ commit }, stages) =>
   commit(types.RECEIVE_GROUP_STAGES_SUCCESS, stages);
 
-  if (!featureFlags?.hasPathNavigation) {
-    return dispatch('setDefaultSelectedStage');
-  }
-
-  return Promise.resolve();
-};
-
 export const fetchGroupStagesAndEvents = ({ dispatch, getters }) => {
   const {
     currentValueStreamId: valueStreamId,

ee/app/assets/javascripts/analytics/cycle_analytics/store/mutations.js

@@ -51,17 +51,7 @@ export default {
     state.medians = {};
   },
   [types.RECEIVE_STAGE_MEDIANS_SUCCESS](state, medians = []) {
-    if (state?.featureFlags?.hasPathNavigation) {
     state.medians = formatMedianValuesWithOverview(medians);
-    } else {
-      state.medians = medians.reduce(
-        (acc, { id, value, error = null }) => ({
-          ...acc,
-          [id]: { value, error },
-        }),
-        {},
-      );
-    }
   },
   [types.RECEIVE_STAGE_MEDIANS_ERROR](state) {
     state.medians = {};

ee/app/controllers/groups/analytics/cycle_analytics_controller.rb

@@ -14,7 +14,6 @@ class Groups::Analytics::CycleAnalyticsController < Groups::Analytics::Applicati
 
   before_action do
     push_frontend_feature_flag(:cycle_analytics_scatterplot_enabled, default_enabled: true)
-    push_frontend_feature_flag(:value_stream_analytics_path_navigation, @group, default_enabled: :yaml)
     render_403 unless can?(current_user, :read_group_cycle_analytics, @group)
   end
 

ee/changelogs/unreleased/323982-remove-enable-value-stream-analytics-horizontal-navigation-ff.yml

+---
+title: Remove the value_stream_analytics_path_navigation feature flag
+merge_request: 60449
+author:
+type: changed

ee/config/feature_flags/development/value_stream_analytics_path_navigation.yml

----
-name: value_stream_analytics_path_navigation
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/31069
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/323982
-milestone: '13.0'
-type: development
-group: group::optimize
-default_enabled: true

ee/spec/features/groups/analytics/cycle_analytics/charts_spec.rb

@@ -42,11 +42,8 @@ RSpec.describe 'Value stream analytics charts', :js do
 
   context 'Duration chart' do
     duration_stage_selector = '.js-dropdown-stages'
-    stage_nav_selector = '.stage-nav'
-    stage_table_selector = '.js-stage-table'
 
     let(:duration_chart_dropdown) { page.find(duration_stage_selector) }
-    let(:first_default_stage) { page.find('.stage-nav-item-cell', text: 'Issue').ancestor('.stage-nav-item') }
     let(:custom_value_stream_name) { "New created value stream" }
 
     let_it_be(:translated_default_stage_names) do
@@ -93,34 +90,6 @@ RSpec.describe 'Value stream analytics charts', :js do
 
       expect(duration_chart_stages).not_to include(first_stage_name)
     end
-
-    context 'With the path navigation feature flag disabled' do
-      let(:nav) { page.find(stage_nav_selector) }
-
-      before do
-        stub_feature_flags(value_stream_analytics_path_navigation: false)
-        select_group(group, stage_table_selector)
-      end
-
-      it_behaves_like 'has all the default stages'
-
-      context 'hidden stage' do
-        before do
-          toggle_more_options(first_default_stage)
-
-          click_button(_('Hide stage'))
-        end
-
-        it 'will not appear in the duration chart dropdown' do
-          # wait for the stage list to laod
-          expect(nav).to have_content(s_('CycleAnalyticsStage|Plan'))
-
-          toggle_duration_chart_dropdown
-
-          expect(duration_chart_stages).not_to include(s_('CycleAnalyticsStage|Issue'))
-        end
-      end
-    end
   end
 
   describe 'Tasks by type chart', :js do

ee/spec/features/groups/analytics/cycle_analytics/cycle_analytics_spec.rb

@@ -17,21 +17,6 @@ RSpec.describe 'Group value stream analytics' do
   it 'pushes frontend feature flags' do
     visit group_analytics_cycle_analytics_path(group)
 
-    expect(page).to have_pushed_frontend_feature_flags(
-      cycleAnalyticsScatterplotEnabled: true,
-      valueStreamAnalyticsPathNavigation: true
-    )
-  end
-
-  context 'when `value_stream_analytics_path_navigation` is disabled for a group' do
-    before do
-      stub_feature_flags(value_stream_analytics_path_navigation: false, thing: group)
-    end
-
-    it 'pushes disabled feature flag to the frontend' do
-      visit group_analytics_cycle_analytics_path(group)
-
-      expect(page).to have_pushed_frontend_feature_flags(valueStreamAnalyticsPathNavigation: false)
-    end
+    expect(page).to have_pushed_frontend_feature_flags(cycleAnalyticsScatterplotEnabled: true)
   end
 end

ee/spec/features/groups/analytics/cycle_analytics/filters_and_data_spec.rb

@@ -172,44 +172,6 @@ RSpec.describe 'Group value stream analytics filters and data', :js do
     end
   end
 
-  context 'with path navigation feature flag disabled' do
-    before do
-      stub_feature_flags(value_stream_analytics_path_navigation: false)
-
-      select_group(group, '.js-stage-table')
-    end
-
-    it 'does not show the path navigation' do
-      expect(page).not_to have_selector(path_nav_selector)
-    end
-
-    it 'shows the vertical stage navigation' do
-      expect(page).to have_selector(stage_nav_selector, visible: true)
-    end
-
-    it 'displays the default list of stages' do
-      stage_nav = page.find(stage_nav_selector)
-
-      %w[Issue Plan Code Test Review Staging].each do |item|
-        string_id = "CycleAnalytics|#{item}"
-        expect(stage_nav).to have_content(s_(string_id))
-      end
-    end
-
-    it 'each stage will have median values', :sidekiq_might_not_need_inline do
-      stage_medians = page.all('.stage-nav .stage-median').collect(&:text)
-
-      expect(stage_medians).to eq(["Not enough data"] * 6)
-    end
-
-    it 'displays the stage table headers' do
-      expect(page).to have_selector('.stage-header', visible: true)
-      expect(page).to have_selector('.median-header', visible: true)
-      expect(page).to have_selector('.event-header', visible: true)
-      expect(page).to have_selector('.total-time-header', visible: true)
-    end
-  end
-
   context 'without valid query parameters set' do
     context 'with created_after date > created_before date' do
       before do

ee/spec/frontend/analytics/cycle_analytics/store/actions_spec.js

@@ -394,17 +394,6 @@ describe('Value Stream Analytics actions', () => {
   });
 
   describe('receiveGroupStagesSuccess', () => {
-    describe('when the `hasPathNavigation` feature flag is enabled', () => {
-      beforeEach(() => {
-        state = {
-          ...state,
-          featureFlags: {
-            ...state.featureFlags,
-            hasPathNavigation: true,
-          },
-        };
-      });
-
     it(`commits the ${types.RECEIVE_GROUP_STAGES_SUCCESS} mutation'`, () => {
       return testAction(
         actions.receiveGroupStagesSuccess,
@@ -421,50 +410,7 @@ describe('Value Stream Analytics actions', () => {
     });
   });
 
-    describe('when the `hasPathNavigation` feature flag is disabled', () => {
-      beforeEach(() => {
-        state = {
-          ...state,
-          featureFlags: {
-            ...state.featureFlags,
-            hasPathNavigation: false,
-          },
-        };
-      });
-
-      it(`commits the ${types.RECEIVE_GROUP_STAGES_SUCCESS} mutation and dispatches 'setDefaultSelectedStage`, () => {
-        return testAction(
-          actions.receiveGroupStagesSuccess,
-          { ...customizableStagesAndEvents.stages },
-          state,
-          [
-            {
-              type: types.RECEIVE_GROUP_STAGES_SUCCESS,
-              payload: { ...customizableStagesAndEvents.stages },
-            },
-          ],
-          [{ type: 'setDefaultSelectedStage' }],
-        );
-      });
-    });
-  });
-
   describe('setDefaultSelectedStage', () => {
-    describe('when the `hasPathNavigation` feature flag is enabled', () => {
-      beforeEach(() => {
-        state = {
-          ...state,
-          featureFlags: {
-            ...state.featureFlags,
-            hasPathNavigation: true,
-          },
-        };
-      });
-
-      afterEach(() => {
-        mock.restore();
-      });
-
     it("dispatches the 'setSelectedStage' with the overview stage", () => {
       return testAction(
         actions.setDefaultSelectedStage,
@@ -476,61 +422,6 @@ describe('Value Stream Analytics actions', () => {
     });
   });
 
-    describe('when the `hasPathNavigation` feature flag is disabled', () => {
-      beforeEach(() => {
-        state = {
-          ...state,
-          featureFlags: {
-            ...state.featureFlags,
-            hasPathNavigation: false,
-          },
-        };
-      });
-
-      afterEach(() => {
-        mock.restore();
-      });
-
-      it("dispatches the 'fetchStageData' action", () => {
-        return testAction(
-          actions.setDefaultSelectedStage,
-          null,
-          state,
-          [],
-          [
-            { type: 'setSelectedStage', payload: selectedStage },
-            { type: 'fetchStageData', payload: selectedStageSlug },
-          ],
-        );
-      });
-
-      it.each`
-        data
-        ${[]}
-        ${null}
-      `('with $data will flash an error', ({ data }) => {
-        actions.setDefaultSelectedStage(
-          { state, getters: { activeStages: data }, dispatch: () => {} },
-          {},
-        );
-        expect(createFlash).toHaveBeenCalledWith({ message: flashErrorMessage });
-      });
-
-      it('will select the first active stage', () => {
-        return testAction(
-          actions.setDefaultSelectedStage,
-          null,
-          state,
-          [],
-          [
-            { type: 'setSelectedStage', payload: stages[1] },
-            { type: 'fetchStageData', payload: stages[1].slug },
-          ],
-        );
-      });
-    });
-  });
-
   describe('updateStage', () => {
     const stageId = 'cool-stage';
     const payload = { hidden: true };

ee/spec/frontend/analytics/cycle_analytics/store/mutations_spec.js

@@ -177,26 +177,8 @@ describe('Value Stream Analytics mutations', () => {
   });
 
   describe(`${types.RECEIVE_STAGE_MEDIANS_SUCCESS}`, () => {
-    it('sets each id as a key in the median object with the corresponding value and error', () => {
-      const stateWithData = {
-        medians: {},
-      };
-
-      mutations[types.RECEIVE_STAGE_MEDIANS_SUCCESS](stateWithData, [
-        { id: 1, value: 20 },
-        { id: 2, value: 10 },
-      ]);
-
-      expect(stateWithData.medians).toEqual({
-        1: { value: 20, error: null },
-        2: { value: 10, error: null },
-      });
-    });
-
-    describe('with hasPathNavigation set to true', () => {
     beforeEach(() => {
       state = {
-          featureFlags: { hasPathNavigation: true },
         medians: {},
       };
 
@@ -214,7 +196,6 @@ describe('Value Stream Analytics mutations', () => {
       expect(state.medians).toMatchObject({ overview: '5d' });
     });
   });
-  });
 
   describe(`${types.INITIALIZE_VSA}`, () => {
     const initialData = {

locale/gitlab.pot

@@ -9748,9 +9748,6 @@ msgstr ""
 msgid "CustomCycleAnalytics|End event label"
 msgstr ""
 
-msgid "CustomCycleAnalytics|New stage"
-msgstr ""
-
 msgid "CustomCycleAnalytics|Stage name already exists"
 msgstr ""
 
@@ -26710,9 +26707,6 @@ msgstr ""
 msgid "Reconfigure"
 msgstr ""
 
-msgid "Recover hidden stage"
-msgstr ""
-
 msgid "Recovering projects"
 msgstr ""