Minor review feedback

Fix capitalisation of Value Streams

Minor spec updates to make use of
components instead of data-testids

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

@@ -13,6 +13,7 @@ import {
 import { mapState, mapActions } from 'vuex';
 import { sprintf, __ } from '~/locale';
 import { debounce } from 'lodash';
+import { DATA_REFETCH_DELAY } from '../../shared/constants';
 
 const ERRORS = {
   MIN_LENGTH: __('Name is required'),
@@ -67,10 +68,10 @@ export default {
       return Boolean(this.data.length);
     },
     selectedValueStreamName() {
-      return this?.selectedValueStream?.name || '';
+      return this.selectedValueStream?.name || '';
     },
     selectedValueStreamId() {
-      return this?.selectedValueStream?.id || null;
+      return this.selectedValueStream?.id || null;
     },
   },
   mounted() {
@@ -95,9 +96,9 @@ export default {
     onHandleInput: debounce(function debouncedValidation() {
       const { name } = this;
       this.errors = validate({ name });
-    }, 250),
+    }, DATA_REFETCH_DELAY),
     isSelected(id) {
-      return this.selectedValueStreamId && this.selectedValueStreamId === id;
+      return Boolean(this.selectedValueStreamId && this.selectedValueStreamId === id);
     },
     onSelect(id) {
       this.setSelectedValueStream(id);
@@ -107,12 +108,7 @@ export default {
 </script>
 <template>
   <gl-form>
-    <gl-dropdown
-      v-if="hasValueStreams"
-      data-testid="select-value-stream"
-      :text="selectedValueStreamName"
-      right
-    >
+    <gl-dropdown v-if="hasValueStreams" :text="selectedValueStreamName" right>
       <gl-dropdown-item
         v-for="{ id, name: streamName } in data"
         :key="id"
@@ -123,22 +119,18 @@ export default {
       >
       <gl-dropdown-divider />
       <gl-dropdown-item v-gl-modal-directive="'create-value-stream-modal'" @click="onHandleInput">{{
-        __('Create new value stream')
+        __('Create new Value Stream')
       }}</gl-dropdown-item>
     </gl-dropdown>
-    <gl-button
-      v-else
-      v-gl-modal-directive="'create-value-stream-modal'"
-      data-testid="create-value-stream"
-      @click="onHandleInput"
-      >{{ __('Create new value stream') }}</gl-button
-    >
+    <gl-button v-else v-gl-modal-directive="'create-value-stream-modal'" @click="onHandleInput">{{
+      __('Create new Value Stream')
+    }}</gl-button>
     <gl-modal
       ref="modal"
       modal-id="create-value-stream-modal"
       :title="__('Value Stream Name')"
       :action-primary="{
-        text: __('Create value stream'),
+        text: __('Create Value Stream'),
         attributes: [
           { variant: 'success' },
           {
@@ -160,7 +152,7 @@ export default {
           id="create-value-stream-name"
           v-model.trim="name"
           name="create-value-stream-name"
-          :placeholder="__('Example: My value stream')"
+          :placeholder="__('Example: My Value Stream')"
           :state="isValid"
           required
           @input="onHandleInput"

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

@@ -1019,10 +1019,10 @@ RSpec.describe 'Group Value Stream Analytics', :js do
     end
 
     it 'can create a value stream' do
-      page.find_button(_('Create new value stream')).click
+      page.find_button(_('Create new Value Stream')).click
 
       fill_in 'create-value-stream-name', with: custom_value_stream_name
-      page.find_button(_('Create value stream')).click
+      page.find_button(_('Create Value Stream')).click
 
       expect(page).to have_text(_("'%{name}' Value Stream created") % { name: custom_value_stream_name })
     end

ee/spec/frontend/analytics/cycle_analytics/components/value_stream_select_spec.js

 import Vuex from 'vuex';
 import { shallowMount, createLocalVue } from '@vue/test-utils';
-import { GlModal } from '@gitlab/ui';
+import { GlButton, GlModal, GlNewDropdown as GlDropdown } from '@gitlab/ui';
 import ValueStreamSelect from 'ee/analytics/cycle_analytics/components/value_stream_select.vue';
 import { valueStreams } from '../mock_data';
 import { findDropdownItemText } from '../helpers';
@@ -49,10 +49,9 @@ describe('ValueStreamSelect', () => {
   const findModal = () => wrapper.find(GlModal);
   const submitButtonDisabledState = () => findModal().props('actionPrimary').attributes[1].disabled;
   const submitForm = () => findModal().vm.$emit('primary', mockEvent);
-  const findSelectValueStreamDropdown = () => wrapper.find('[data-testid="select-value-stream"]');
+  const findSelectValueStreamDropdown = () => wrapper.find(GlDropdown);
   const findSelectValueStreamDropdownOptions = _wrapper => findDropdownItemText(_wrapper);
-
-  const findCreateValueStreamButton = () => wrapper.find('[data-testid="create-value-stream"]');
+  const findCreateValueStreamButton = () => wrapper.find(GlButton);
 
   beforeEach(() => {
     wrapper = createComponent({
@@ -66,18 +65,20 @@ describe('ValueStreamSelect', () => {
     wrapper.destroy();
   });
 
-  it('does not display the create value stream button', () => {
-    expect(findCreateValueStreamButton().exists()).toBe(false);
-  });
+  describe('with value streams available', () => {
+    it('does not display the create value stream button', () => {
+      expect(findCreateValueStreamButton().exists()).toBe(false);
+    });
 
-  it('displays the select value stream dropdown', () => {
-    expect(findSelectValueStreamDropdown().exists()).toBe(true);
-  });
+    it('displays the select value stream dropdown', () => {
+      expect(findSelectValueStreamDropdown().exists()).toBe(true);
+    });
 
-  it('renders each value stream including a create button', () => {
-    const opts = findSelectValueStreamDropdownOptions(wrapper);
-    [...valueStreams.map(v => v.name), 'Create new value stream'].forEach(vs => {
-      expect(opts).toContain(vs);
+    it('renders each value stream including a create button', () => {
+      const opts = findSelectValueStreamDropdownOptions(wrapper);
+      [...valueStreams.map(v => v.name), 'Create new Value Stream'].forEach(vs => {
+        expect(opts).toContain(vs);
+      });
     });
   });
 

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

@@ -21,6 +21,7 @@ const error = new Error(`Request failed with status code ${httpStatusCodes.NOT_F
 const flashErrorMessage = 'There was an error while fetching value stream analytics data.';
 const [selectedStage] = stages;
 const selectedStageSlug = selectedStage.slug;
+const [selectedValueStream] = valueStreams;
 
 const stageEndpoint = ({ stageId }) =>
   `/groups/${selectedGroup.fullPath}/-/analytics/value_stream_analytics/stages/${stageId}`;
@@ -135,7 +136,7 @@ describe('Cycle analytics actions', () => {
     beforeEach(() => {
       state = { ...state, selectedGroup };
       mock = new MockAdapter(axios);
-      mock.onGet(endpoints.stageData).reply(200, { events: [] });
+      mock.onGet(endpoints.stageData).reply(httpStatusCodes.OK, { events: [] });
     });
 
     it('dispatches receiveStageDataSuccess with received data on success', () => {
@@ -419,7 +420,7 @@ describe('Cycle analytics actions', () => {
     const payload = { hidden: true };
 
     beforeEach(() => {
-      mock.onPut(stageEndpoint({ stageId }), payload).replyOnce(200, payload);
+      mock.onPut(stageEndpoint({ stageId }), payload).replyOnce(httpStatusCodes.OK, payload);
       state = { selectedGroup };
     });
 
@@ -565,7 +566,7 @@ describe('Cycle analytics actions', () => {
     const stageId = 'cool-stage';
 
     beforeEach(() => {
-      mock.onDelete(stageEndpoint({ stageId })).replyOnce(200);
+      mock.onDelete(stageEndpoint({ stageId })).replyOnce(httpStatusCodes.OK);
       state = { selectedGroup };
     });
 
@@ -617,7 +618,7 @@ describe('Cycle analytics actions', () => {
     const stageId = 'cool-stage';
 
     beforeEach(() => {
-      mock.onDelete(stageEndpoint({ stageId })).replyOnce(200);
+      mock.onDelete(stageEndpoint({ stageId })).replyOnce(httpStatusCodes.OK);
       state = { selectedGroup };
     });
 
@@ -650,7 +651,7 @@ describe('Cycle analytics actions', () => {
     beforeEach(() => {
       state = { ...state, stages: [{ slug: selectedStageSlug }], selectedGroup };
       mock = new MockAdapter(axios);
-      mock.onGet(endpoints.stageMedian).reply(200, { events: [] });
+      mock.onGet(endpoints.stageMedian).reply(httpStatusCodes.OK, { events: [] });
       mockDispatch = jest.fn();
     });
 
@@ -923,7 +924,7 @@ describe('Cycle analytics actions', () => {
         },
       };
       mock = new MockAdapter(axios);
-      mock.onGet(endpoints.valueStreamData).reply(200, { stages: [], events: [] });
+      mock.onGet(endpoints.valueStreamData).reply(httpStatusCodes.OK, { stages: [], events: [] });
     });
 
     it(`commits ${types.REQUEST_VALUE_STREAMS} and dispatches receiveValueStreamsSuccess with received data on success`, () => {
@@ -978,7 +979,7 @@ describe('Cycle analytics actions', () => {
               payload: valueStreams,
             },
           ],
-          [{ type: 'setSelectedValueStream', payload: 1 }],
+          [{ type: 'setSelectedValueStream', payload: selectedValueStream.id }],
         );
       });
     });

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

@@ -63,22 +63,34 @@ describe('Cycle analytics mutations', () => {
     ${types.SET_SELECTED_PROJECTS}             | ${selectedProjects}           | ${{ selectedProjects }}
     ${types.SET_DATE_RANGE}                    | ${{ startDate, endDate }}     | ${{ startDate, endDate }}
     ${types.SET_SELECTED_STAGE}                | ${{ id: 'first-stage' }}      | ${{ selectedStage: { id: 'first-stage' } }}
-    ${types.SET_SELECTED_VALUE_STREAM}         | ${valueStreams[1].id}         | ${{ selectedValueStream: valueStreams[1] }}
     ${types.RECEIVE_CREATE_VALUE_STREAM_ERROR} | ${{ name: ['is required'] }}  | ${{ createValueStreamErrors: { name: ['is required'] }, isCreatingValueStream: false }}
     ${types.RECEIVE_VALUE_STREAMS_SUCCESS}     | ${valueStreams}               | ${{ valueStreams, isLoadingValueStreams: false }}
+    ${types.SET_SELECTED_VALUE_STREAM}         | ${valueStreams[1].id}         | ${{ selectedValueStream: {} }}
   `(
     '$mutation with payload $payload will update state with $expectedState',
     ({ mutation, payload, expectedState }) => {
-      state = {
-        valueStreams,
-        selectedGroup: { fullPath: 'rad-stage' },
-      };
+      state = { selectedGroup: { fullPath: 'rad-stage' } };
       mutations[mutation](state, payload);
 
       expect(state).toMatchObject(expectedState);
     },
   );
 
+  describe('with value streams available', () => {
+    it.each`
+      mutation                           | payload               | expectedState
+      ${types.SET_SELECTED_VALUE_STREAM} | ${valueStreams[1].id} | ${{ selectedValueStream: valueStreams[1] }}
+      ${types.SET_SELECTED_VALUE_STREAM} | ${'fake-id'}          | ${{ selectedValueStream: {} }}
+    `(
+      '$mutation with payload $payload will update state with $expectedState',
+      ({ mutation, payload, expectedState }) => {
+        state = { valueStreams };
+        mutations[mutation](state, payload);
+        expect(state).toMatchObject(expectedState);
+      },
+    );
+  });
+
   describe(`${types.RECEIVE_CYCLE_ANALYTICS_DATA_SUCCESS}`, () => {
     it('will set isLoading=false and errorCode=null', () => {
       mutations[types.RECEIVE_CYCLE_ANALYTICS_DATA_SUCCESS](state, {

ee/spec/frontend/api_spec.js

@@ -339,7 +339,7 @@ describe('Api', () => {
         const response = [{ name: 'value stream 1', id: 1 }];
 
         const expectedUrl = `${dummyCycleAnalyticsUrlRoot}/-/analytics/value_stream_analytics/value_streams`;
-        mock.onGet(expectedUrl).reply(200, response);
+        mock.onGet(expectedUrl).reply(httpStatus.OK, response);
 
         Api.cycleAnalyticsValueStreams(groupId)
           .then(responseObj =>
@@ -360,7 +360,7 @@ describe('Api', () => {
           name: 'cool-value-stream-stage',
         };
         const expectedUrl = `${dummyCycleAnalyticsUrlRoot}/-/analytics/value_stream_analytics/value_streams`;
-        mock.onPost(expectedUrl).reply(200, response);
+        mock.onPost(expectedUrl).reply(httpStatus.OK, response);
 
         Api.cycleAnalyticsCreateValueStream(groupId, customValueStream)
           .then(({ data, config: { data: reqData, url } }) => {

locale/gitlab.pot

@@ -6860,6 +6860,9 @@ msgstr ""
 msgid "Create Project"
 msgstr ""
 
+msgid "Create Value Stream"
+msgstr ""
+
 msgid "Create a GitLab account first, and then connect it to your %{label} account."
 msgstr ""
 
@@ -6956,6 +6959,9 @@ msgstr ""
 msgid "Create new"
 msgstr ""
 
+msgid "Create new Value Stream"
+msgstr ""
+
 msgid "Create new board"
 msgstr ""
 
@@ -6977,9 +6983,6 @@ msgstr ""
 msgid "Create new label"
 msgstr ""
 
-msgid "Create new value stream"
-msgstr ""
-
 msgid "Create new..."
 msgstr ""
 
@@ -6995,9 +6998,6 @@ msgstr ""
 msgid "Create snippet"
 msgstr ""
 
-msgid "Create value stream"
-msgstr ""
-
 msgid "Create wildcard: %{searchTerm}"
 msgstr ""
 
@@ -9709,7 +9709,7 @@ msgstr ""
 msgid "Example: @sub\\.company\\.com$"
 msgstr ""
 
-msgid "Example: My value stream"
+msgid "Example: My Value Stream"
 msgstr ""
 
 msgid "Example: Usage = single query. (Requested) / (Capacity) = multiple queries combined into a formula."