Merge branch 'nfriend-add-release-asset-type-to-frontend' into 'master'

Add "Type" field to asset links on "Edit Release" page

See merge request gitlab-org/gitlab!31253

app/assets/javascripts/releases/components/asset_links_form.vue

@@ -8,12 +8,25 @@ import {
   GlIcon,
   GlTooltipDirective,
   GlFormInput,
+  GlFormSelect,
 } from '@gitlab/ui';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
+import { DEFAULT_ASSET_LINK_TYPE, ASSET_LINK_TYPE } from '../constants';
+import { s__ } from '~/locale';
 
 export default {
   name: 'AssetLinksForm',
-  components: { GlSprintf, GlLink, GlFormGroup, GlButton, GlIcon, GlFormInput },
+  components: {
+    GlSprintf,
+    GlLink,
+    GlFormGroup,
+    GlButton,
+    GlIcon,
+    GlFormInput,
+    GlFormSelect,
+  },
   directives: { GlTooltip: GlTooltipDirective },
+  mixins: [glFeatureFlagsMixin()],
   computed: {
     ...mapState('detail', ['release', 'releaseAssetsDocsPath']),
     ...mapGetters('detail', ['validationErrors']),
@@ -26,6 +39,7 @@ export default {
       'addEmptyAssetLink',
       'updateAssetLinkUrl',
       'updateAssetLinkName',
+      'updateAssetLinkType',
       'removeAssetLink',
     ]),
     onAddAnotherClicked() {
@@ -35,12 +49,6 @@ export default {
       this.removeAssetLink(linkId);
       this.ensureAtLeastOneLink();
     },
-    onUrlInput(linkIdToUpdate, newUrl) {
-      this.updateAssetLinkUrl({ linkIdToUpdate, newUrl });
-    },
-    onLinkTitleInput(linkIdToUpdate, newName) {
-      this.updateAssetLinkName({ linkIdToUpdate, newName });
-    },
     hasDuplicateUrl(link) {
       return Boolean(this.getLinkErrors(link).isDuplicate);
     },
@@ -73,6 +81,13 @@ export default {
       }
     },
   },
+  typeOptions: [
+    { value: ASSET_LINK_TYPE.IMAGE, text: s__('ReleaseAssetLinkType|Image') },
+    { value: ASSET_LINK_TYPE.PACKAGE, text: s__('ReleaseAssetLinkType|Package') },
+    { value: ASSET_LINK_TYPE.RUNBOOK, text: s__('ReleaseAssetLinkType|Runbook') },
+    { value: ASSET_LINK_TYPE.OTHER, text: s__('ReleaseAssetLinkType|Other') },
+  ],
+  defaultTypeOptionValue: DEFAULT_ASSET_LINK_TYPE,
 };
 </script>
 
@@ -123,7 +138,7 @@ export default {
           type="text"
           class="form-control"
           :state="isUrlValid(link)"
-          @change="onUrlInput(link.id, $event)"
+          @change="updateAssetLinkUrl({ linkIdToUpdate: link.id, newUrl: $event })"
         />
         <template #invalid-feedback>
           <span v-if="hasEmptyUrl(link)" class="invalid-feedback d-inline">
@@ -160,7 +175,7 @@ export default {
           type="text"
           class="form-control"
           :state="isNameValid(link)"
-          @change="onLinkTitleInput(link.id, $event)"
+          @change="updateAssetLinkName({ linkIdToUpdate: link.id, newName: $event })"
         />
         <template #invalid-feedback>
           <span v-if="hasEmptyName(link)" class="invalid-feedback d-inline">
@@ -169,6 +184,22 @@ export default {
         </template>
       </gl-form-group>
 
+      <gl-form-group
+        v-if="glFeatures.releaseAssetLinkType"
+        class="link-type-field col-auto"
+        :label="__('Type')"
+        :label-for="`asset-type-${index}`"
+      >
+        <gl-form-select
+          :id="`asset-type-${index}`"
+          ref="typeSelect"
+          :value="link.linkType || $options.defaultTypeOptionValue"
+          class="form-control pr-4"
+          :options="$options.typeOptions"
+          @change="updateAssetLinkType({ linkIdToUpdate: link.id, newType: $event })"
+        />
+      </gl-form-group>
+
       <div class="mb-5 mb-sm-3 mt-sm-4 col col-sm-auto">
         <gl-button
           v-gl-tooltip

app/assets/javascripts/releases/constants.js

 export const MAX_MILESTONES_TO_DISPLAY = 5;
 
 export const BACK_URL_PARAM = 'back_url';
+
+export const ASSET_LINK_TYPE = Object.freeze({
+  OTHER: 'other',
+  IMAGE: 'image',
+  PACKAGE: 'package',
+  RUNBOOK: 'runbook',
+});
+
+export const DEFAULT_ASSET_LINK_TYPE = ASSET_LINK_TYPE.OTHER;

app/assets/javascripts/releases/stores/modules/detail/actions.js

@@ -3,7 +3,10 @@ import api from '~/api';
 import createFlash from '~/flash';
 import { s__ } from '~/locale';
 import { redirectTo } from '~/lib/utils/url_utility';
-import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
+import {
+  convertObjectPropsToCamelCase,
+  convertObjectPropsToSnakeCase,
+} from '~/lib/utils/common_utils';
 
 export const requestRelease = ({ commit }) => commit(types.REQUEST_RELEASE);
 export const receiveReleaseSuccess = ({ commit }, data) =>
@@ -91,7 +94,11 @@ export const updateRelease = ({ dispatch, state, getters }) => {
         // Create a new link for each link in the form
         return Promise.all(
           getters.releaseLinksToCreate.map(l =>
-            api.createReleaseLink(state.projectId, release.tagName, l),
+            api.createReleaseLink(
+              state.projectId,
+              release.tagName,
+              convertObjectPropsToSnakeCase(l, { deep: true }),
+            ),
           ),
         );
       })
@@ -118,6 +125,10 @@ export const updateAssetLinkName = ({ commit }, { linkIdToUpdate, newName }) =>
   commit(types.UPDATE_ASSET_LINK_NAME, { linkIdToUpdate, newName });
 };
 
+export const updateAssetLinkType = ({ commit }, { linkIdToUpdate, newType }) => {
+  commit(types.UPDATE_ASSET_LINK_TYPE, { linkIdToUpdate, newType });
+};
+
 export const removeAssetLink = ({ commit }, linkIdToRemove) => {
   commit(types.REMOVE_ASSET_LINK, linkIdToRemove);
 };

app/assets/javascripts/releases/stores/modules/detail/mutation_types.js

@@ -13,4 +13,5 @@ export const RECEIVE_UPDATE_RELEASE_ERROR = 'RECEIVE_UPDATE_RELEASE_ERROR';
 export const ADD_EMPTY_ASSET_LINK = 'ADD_EMPTY_ASSET_LINK';
 export const UPDATE_ASSET_LINK_URL = 'UPDATE_ASSET_LINK_URL';
 export const UPDATE_ASSET_LINK_NAME = 'UPDATE_ASSET_LINK_NAME';
+export const UPDATE_ASSET_LINK_TYPE = 'UPDATE_ASSET_LINK_TYPE';
 export const REMOVE_ASSET_LINK = 'REMOVE_ASSET_LINK';

app/assets/javascripts/releases/stores/modules/detail/mutations.js

 import * as types from './mutation_types';
 import { uniqueId, cloneDeep } from 'lodash';
+import { DEFAULT_ASSET_LINK_TYPE } from '../../../constants';
 
 const findReleaseLink = (release, id) => {
   return release.assets.links.find(l => l.id === id);
@@ -49,6 +50,7 @@ export default {
       id: uniqueId('new-link-'),
       url: '',
       name: '',
+      linkType: DEFAULT_ASSET_LINK_TYPE,
     });
   },
 
@@ -62,6 +64,11 @@ export default {
     linkToUpdate.name = newName;
   },
 
+  [types.UPDATE_ASSET_LINK_TYPE](state, { linkIdToUpdate, newType }) {
+    const linkToUpdate = findReleaseLink(state.release, linkIdToUpdate);
+    linkToUpdate.linkType = newType;
+  },
+
   [types.REMOVE_ASSET_LINK](state, linkIdToRemove) {
     state.release.assets.links = state.release.assets.links.filter(l => l.id !== linkIdToRemove);
   },

app/controllers/projects/releases_controller.rb

@@ -10,6 +10,7 @@ class Projects::ReleasesController < Projects::ApplicationController
     push_frontend_feature_flag(:release_evidence_collection, project, default_enabled: true)
     push_frontend_feature_flag(:release_show_page, project, default_enabled: true)
     push_frontend_feature_flag(:release_asset_link_editing, project, default_enabled: true)
+    push_frontend_feature_flag(:release_asset_link_type, project, default_enabled: false)
   end
   before_action :authorize_update_release!, only: %i[edit update]
 

locale/gitlab.pot

@@ -17957,6 +17957,18 @@ msgstr ""
 msgid "Release title"
 msgstr ""
 
+msgid "ReleaseAssetLinkType|Image"
+msgstr ""
+
+msgid "ReleaseAssetLinkType|Other"
+msgstr ""
+
+msgid "ReleaseAssetLinkType|Package"
+msgstr ""
+
+msgid "ReleaseAssetLinkType|Runbook"
+msgstr ""
+
 msgid "Releases"
 msgstr ""
 

spec/frontend/releases/components/asset_links_form_spec.js

@@ -3,6 +3,7 @@ import { mount, createLocalVue } from '@vue/test-utils';
 import AssetLinksForm from '~/releases/components/asset_links_form.vue';
 import { release as originalRelease } from '../mock_data';
 import * as commonUtils from '~/lib/utils/common_utils';
+import { ASSET_LINK_TYPE, DEFAULT_ASSET_LINK_TYPE } from '~/releases/constants';
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -24,6 +25,7 @@ describe('Release edit component', () => {
       addEmptyAssetLink: jest.fn(),
       updateAssetLinkUrl: jest.fn(),
       updateAssetLinkName: jest.fn(),
+      updateAssetLinkType: jest.fn(),
       removeAssetLink: jest.fn().mockImplementation((_context, linkId) => {
         state.release.assets.links = state.release.assets.links.filter(l => l.id !== linkId);
       }),
@@ -51,6 +53,11 @@ describe('Release edit component', () => {
     wrapper = mount(AssetLinksForm, {
       localVue,
       store,
+      provide: {
+        glFeatures: {
+          releaseAssetLinkType: true,
+        },
+      },
     });
   };
 
@@ -103,7 +110,7 @@ describe('Release edit component', () => {
       );
     });
 
-    it('calls the "updateAssetLinName" store method when text is entered into the "Link title" input field', () => {
+    it('calls the "updateAssetLinkName" store method when text is entered into the "Link title" input field', () => {
       const linkIdToUpdate = release.assets.links[0].id;
       const newName = 'updated name';
 
@@ -121,6 +128,31 @@ describe('Release edit component', () => {
         undefined,
       );
     });
+
+    it('calls the "updateAssetLinkType" store method when an option is selected from the "Type" dropdown', () => {
+      const linkIdToUpdate = release.assets.links[0].id;
+      const newType = ASSET_LINK_TYPE.RUNBOOK;
+
+      expect(actions.updateAssetLinkType).not.toHaveBeenCalled();
+
+      wrapper.find({ ref: 'typeSelect' }).vm.$emit('change', newType);
+
+      expect(actions.updateAssetLinkType).toHaveBeenCalledTimes(1);
+      expect(actions.updateAssetLinkType).toHaveBeenCalledWith(
+        expect.anything(),
+        {
+          linkIdToUpdate,
+          newType,
+        },
+        undefined,
+      );
+    });
+
+    it('selects the default asset type if no type was provided by the backend', () => {
+      const selected = wrapper.find({ ref: 'typeSelect' }).element.value;
+
+      expect(selected).toBe(DEFAULT_ASSET_LINK_TYPE);
+    });
   });
 
   describe('validation', () => {

spec/frontend/releases/stores/modules/detail/actions_spec.js

@@ -10,6 +10,7 @@ import createFlash from '~/flash';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import { redirectTo } from '~/lib/utils/url_utility';
 import api from '~/api';
+import { ASSET_LINK_TYPE } from '~/releases/constants';
 
 jest.mock('~/flash', () => jest.fn());
 
@@ -130,6 +131,54 @@ describe('Release detail actions', () => {
     });
   });
 
+  describe('updateAssetLinkUrl', () => {
+    it(`commits ${types.UPDATE_ASSET_LINK_URL} with the updated link URL`, () => {
+      const params = {
+        linkIdToUpdate: 2,
+        newUrl: 'https://example.com/updated',
+      };
+
+      return testAction(actions.updateAssetLinkUrl, params, state, [
+        { type: types.UPDATE_ASSET_LINK_URL, payload: params },
+      ]);
+    });
+  });
+
+  describe('updateAssetLinkName', () => {
+    it(`commits ${types.UPDATE_ASSET_LINK_NAME} with the updated link name`, () => {
+      const params = {
+        linkIdToUpdate: 2,
+        newName: 'Updated link name',
+      };
+
+      return testAction(actions.updateAssetLinkName, params, state, [
+        { type: types.UPDATE_ASSET_LINK_NAME, payload: params },
+      ]);
+    });
+  });
+
+  describe('updateAssetLinkType', () => {
+    it(`commits ${types.UPDATE_ASSET_LINK_TYPE} with the updated link type`, () => {
+      const params = {
+        linkIdToUpdate: 2,
+        newType: ASSET_LINK_TYPE.RUNBOOK,
+      };
+
+      return testAction(actions.updateAssetLinkType, params, state, [
+        { type: types.UPDATE_ASSET_LINK_TYPE, payload: params },
+      ]);
+    });
+  });
+
+  describe('removeAssetLink', () => {
+    it(`commits ${types.REMOVE_ASSET_LINK} with the ID of the asset link to remove`, () => {
+      const idToRemove = 2;
+      return testAction(actions.removeAssetLink, idToRemove, state, [
+        { type: types.REMOVE_ASSET_LINK, payload: idToRemove },
+      ]);
+    });
+  });
+
   describe('updateReleaseMilestones', () => {
     it(`commits ${types.UPDATE_RELEASE_MILESTONES} with the updated release milestones`, () => {
       const newReleaseMilestones = ['v0.0', 'v0.1'];

spec/frontend/releases/stores/modules/detail/mutations_spec.js

@@ -3,6 +3,7 @@ import mutations from '~/releases/stores/modules/detail/mutations';
 import * as types from '~/releases/stores/modules/detail/mutation_types';
 import { release as originalRelease } from '../../../mock_data';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
+import { ASSET_LINK_TYPE, DEFAULT_ASSET_LINK_TYPE } from '~/releases/constants';
 
 describe('Release detail mutations', () => {
   let state;
@@ -24,7 +25,7 @@ describe('Release detail mutations', () => {
     it('set state.isFetchingRelease to true', () => {
       mutations[types.REQUEST_RELEASE](state);
 
-      expect(state.isFetchingRelease).toEqual(true);
+      expect(state.isFetchingRelease).toBe(true);
     });
   });
 
@@ -32,9 +33,9 @@ describe('Release detail mutations', () => {
     it('handles a successful response from the server', () => {
       mutations[types.RECEIVE_RELEASE_SUCCESS](state, release);
 
-      expect(state.fetchError).toEqual(undefined);
+      expect(state.fetchError).toBeUndefined();
 
-      expect(state.isFetchingRelease).toEqual(false);
+      expect(state.isFetchingRelease).toBe(false);
 
       expect(state.release).toEqual(release);
 
@@ -47,7 +48,7 @@ describe('Release detail mutations', () => {
       const error = { message: 'An error occurred!' };
       mutations[types.RECEIVE_RELEASE_ERROR](state, error);
 
-      expect(state.isFetchingRelease).toEqual(false);
+      expect(state.isFetchingRelease).toBe(false);
 
       expect(state.release).toBeUndefined();
 
@@ -61,7 +62,7 @@ describe('Release detail mutations', () => {
       const newTitle = 'The new release title';
       mutations[types.UPDATE_RELEASE_TITLE](state, newTitle);
 
-      expect(state.release.name).toEqual(newTitle);
+      expect(state.release.name).toBe(newTitle);
     });
   });
 
@@ -71,7 +72,7 @@ describe('Release detail mutations', () => {
       const newNotes = 'The new release notes';
       mutations[types.UPDATE_RELEASE_NOTES](state, newNotes);
 
-      expect(state.release.description).toEqual(newNotes);
+      expect(state.release.description).toBe(newNotes);
     });
   });
 
@@ -79,7 +80,7 @@ describe('Release detail mutations', () => {
     it('set state.isUpdatingRelease to true', () => {
       mutations[types.REQUEST_UPDATE_RELEASE](state);
 
-      expect(state.isUpdatingRelease).toEqual(true);
+      expect(state.isUpdatingRelease).toBe(true);
     });
   });
 
@@ -87,9 +88,9 @@ describe('Release detail mutations', () => {
     it('handles a successful response from the server', () => {
       mutations[types.RECEIVE_UPDATE_RELEASE_SUCCESS](state, release);
 
-      expect(state.updateError).toEqual(undefined);
+      expect(state.updateError).toBeUndefined();
 
-      expect(state.isUpdatingRelease).toEqual(false);
+      expect(state.isUpdatingRelease).toBe(false);
     });
   });
 
@@ -98,7 +99,7 @@ describe('Release detail mutations', () => {
       const error = { message: 'An error occurred!' };
       mutations[types.RECEIVE_UPDATE_RELEASE_ERROR](state, error);
 
-      expect(state.isUpdatingRelease).toEqual(false);
+      expect(state.isUpdatingRelease).toBe(false);
 
       expect(state.updateError).toEqual(error);
     });
@@ -118,6 +119,7 @@ describe('Release detail mutations', () => {
           id: expect.stringMatching(/^new-link-/),
           url: '',
           name: '',
+          linkType: DEFAULT_ASSET_LINK_TYPE,
         },
       ]);
     });
@@ -134,7 +136,7 @@ describe('Release detail mutations', () => {
         newUrl,
       });
 
-      expect(state.release.assets.links[0].url).toEqual(newUrl);
+      expect(state.release.assets.links[0].url).toBe(newUrl);
     });
   });
 
@@ -149,7 +151,22 @@ describe('Release detail mutations', () => {
         newName,
       });
 
-      expect(state.release.assets.links[0].name).toEqual(newName);
+      expect(state.release.assets.links[0].name).toBe(newName);
+    });
+  });
+
+  describe(`${types.UPDATE_ASSET_LINK_TYPE}`, () => {
+    it('updates an asset link with a new type', () => {
+      state.release = release;
+
+      const newType = ASSET_LINK_TYPE.RUNBOOK;
+
+      mutations[types.UPDATE_ASSET_LINK_TYPE](state, {
+        linkIdToUpdate: state.release.assets.links[0].id,
+        newType,
+      });
+
+      expect(state.release.assets.links[0].linkType).toBe(newType);
     });
   });