Merge branch 'send-full-ref-when-running-manual-pipeline' into 'master'

Send full ref when triggering manual pipeline

See merge request gitlab-org/gitlab!48142

app/assets/javascripts/pipeline_new/components/pipeline_new_form.vue

@@ -12,10 +12,12 @@ import {
   GlLink,
   GlDropdown,
   GlDropdownItem,
+  GlDropdownSectionHeader,
   GlSearchBoxByType,
   GlSprintf,
   GlLoadingIcon,
 } from '@gitlab/ui';
+import * as Sentry from '~/sentry/wrapper';
 import { s__, __, n__ } from '~/locale';
 import axios from '~/lib/utils/axios_utils';
 import { redirectTo } from '~/lib/utils/url_utility';
@@ -46,6 +48,7 @@ export default {
     GlLink,
     GlDropdown,
     GlDropdownItem,
+    GlDropdownSectionHeader,
     GlSearchBoxByType,
     GlSprintf,
     GlLoadingIcon,
@@ -59,11 +62,19 @@ export default {
       type: String,
       required: true,
     },
+    defaultBranch: {
+      type: String,
+      required: true,
+    },
     projectId: {
       type: String,
       required: true,
     },
-    refs: {
+    branches: {
+      type: Array,
+      required: true,
+    },
+    tags: {
       type: Array,
       required: true,
     },
@@ -94,7 +105,9 @@ export default {
   data() {
     return {
       searchTerm: '',
-      refValue: this.refParam,
+      refValue: {
+        shortName: this.refParam,
+      },
       form: {},
       error: null,
       warnings: [],
@@ -104,9 +117,21 @@ export default {
     };
   },
   computed: {
-    filteredRefs() {
-      const lowerCasedSearchTerm = this.searchTerm.toLowerCase();
-      return this.refs.filter(ref => ref.toLowerCase().includes(lowerCasedSearchTerm));
+    lowerCasedSearchTerm() {
+      return this.searchTerm.toLowerCase();
+    },
+    filteredBranches() {
+      return this.branches.filter(branch =>
+        branch.shortName.toLowerCase().includes(this.lowerCasedSearchTerm),
+      );
+    },
+    filteredTags() {
+      return this.tags.filter(tag =>
+        tag.shortName.toLowerCase().includes(this.lowerCasedSearchTerm),
+      );
+    },
+    hasTags() {
+      return this.tags.length > 0;
     },
     overMaxWarningsLimit() {
       return this.totalWarnings > this.maxWarnings;
@@ -120,14 +145,27 @@ export default {
     shouldShowWarning() {
       return this.warnings.length > 0 && !this.isWarningDismissed;
     },
+    refShortName() {
+      return this.refValue.shortName;
+    },
+    refFullName() {
+      return this.refValue.fullName;
+    },
     variables() {
-      return this.form[this.refValue]?.variables ?? [];
+      return this.form[this.refFullName]?.variables ?? [];
     },
     descriptions() {
-      return this.form[this.refValue]?.descriptions ?? {};
+      return this.form[this.refFullName]?.descriptions ?? {};
     },
   },
   created() {
+    // this is needed until we add support for ref type in url query strings
+    // ensure default branch is called with full ref on load
+    // https://gitlab.com/gitlab-org/gitlab/-/issues/287815
+    if (this.refValue.shortName === this.defaultBranch) {
+      this.refValue.fullName = `refs/heads/${this.refValue.shortName}`;
+    }
+
     this.setRefSelected(this.refValue);
   },
   methods: {
@@ -170,19 +208,19 @@ export default {
     setRefSelected(refValue) {
       this.refValue = refValue;
 
-      if (!this.form[refValue]) {
-        this.fetchConfigVariables(refValue)
+      if (!this.form[this.refFullName]) {
+        this.fetchConfigVariables(this.refFullName || this.refShortName)
           .then(({ descriptions, params }) => {
-            Vue.set(this.form, refValue, {
+            Vue.set(this.form, this.refFullName, {
               variables: [],
               descriptions,
             });
 
             // Add default variables from yml
-            this.setVariableParams(refValue, VARIABLE_TYPE, params);
+            this.setVariableParams(this.refFullName, VARIABLE_TYPE, params);
           })
           .catch(() => {
-            Vue.set(this.form, refValue, {
+            Vue.set(this.form, this.refFullName, {
               variables: [],
               descriptions: {},
             });
@@ -190,20 +228,19 @@ export default {
           .finally(() => {
             // Add/update variables, e.g. from query string
             if (this.variableParams) {
-              this.setVariableParams(refValue, VARIABLE_TYPE, this.variableParams);
+              this.setVariableParams(this.refFullName, VARIABLE_TYPE, this.variableParams);
             }
             if (this.fileParams) {
-              this.setVariableParams(refValue, FILE_TYPE, this.fileParams);
+              this.setVariableParams(this.refFullName, FILE_TYPE, this.fileParams);
             }
 
             // Adds empty var at the end of the form
-            this.addEmptyVariable(refValue);
+            this.addEmptyVariable(this.refFullName);
           });
       }
     },
-
     isSelected(ref) {
-      return ref === this.refValue;
+      return ref.fullName === this.refValue.fullName;
     },
     removeVariable(index) {
       this.variables.splice(index, 1);
@@ -211,7 +248,6 @@ export default {
     canRemove(index) {
       return index < this.variables.length - 1;
     },
-
     fetchConfigVariables(refValue) {
       if (!gon?.features?.newPipelineFormPrefilledVars) {
         return Promise.resolve({ params: {}, descriptions: {} });
@@ -251,9 +287,11 @@ export default {
 
           return { params, descriptions };
         })
-        .catch(() => {
+        .catch(error => {
           this.isLoading = false;
 
+          Sentry.captureException(error);
+
           return { params: {}, descriptions: {} };
         });
     },
@@ -268,7 +306,9 @@ export default {
 
       return axios
         .post(this.pipelinesPath, {
-          ref: this.refValue,
+          // send shortName as fall back for query params
+          // https://gitlab.com/gitlab-org/gitlab/-/issues/287815
+          ref: this.refValue.fullName || this.refShortName,
           variables_attributes: filteredVariables,
         })
         .then(({ data }) => {
@@ -326,20 +366,29 @@ export default {
       </details>
     </gl-alert>
     <gl-form-group :label="s__('Pipeline|Run for')">
-      <gl-dropdown :text="refValue" block>
-        <gl-search-box-by-type
-          v-model.trim="searchTerm"
-          :placeholder="__('Search branches and tags')"
-        />
+      <gl-dropdown :text="refShortName" block>
+        <gl-search-box-by-type v-model.trim="searchTerm" :placeholder="__('Search refs')" />
+        <gl-dropdown-section-header>{{ __('Branches') }}</gl-dropdown-section-header>
+        <gl-dropdown-item
+          v-for="branch in filteredBranches"
+          :key="branch.fullName"
+          class="gl-font-monospace"
+          is-check-item
+          :is-checked="isSelected(branch)"
+          @click="setRefSelected(branch)"
+        >
+          {{ branch.shortName }}
+        </gl-dropdown-item>
+        <gl-dropdown-section-header v-if="hasTags">{{ __('Tags') }}</gl-dropdown-section-header>
         <gl-dropdown-item
-          v-for="(ref, index) in filteredRefs"
-          :key="index"
+          v-for="tag in filteredTags"
+          :key="tag.fullName"
           class="gl-font-monospace"
           is-check-item
-          :is-checked="isSelected(ref)"
-          @click="setRefSelected(ref)"
+          :is-checked="isSelected(tag)"
+          @click="setRefSelected(tag)"
         >
-          {{ ref }}
+          {{ tag.shortName }}
         </gl-dropdown-item>
       </gl-dropdown>
 
@@ -372,7 +421,7 @@ export default {
             :placeholder="s__('CiVariables|Input variable key')"
             :class="$options.formElementClasses"
             data-testid="pipeline-form-ci-variable-key"
-            @change="addEmptyVariable(refValue)"
+            @change="addEmptyVariable(refFullName)"
           />
           <gl-form-input
             v-model="variable.value"

app/assets/javascripts/pipeline_new/constants.js

 export const VARIABLE_TYPE = 'env_var';
 export const FILE_TYPE = 'file';
 export const CONFIG_VARIABLES_TIMEOUT = 5000;
+export const BRANCH_REF_TYPE = 'branch';
+export const TAG_REF_TYPE = 'tag';

app/assets/javascripts/pipeline_new/index.js

 import Vue from 'vue';
 import PipelineNewForm from './components/pipeline_new_form.vue';
+import formatRefs from './utils/format_refs';
 
 export default () => {
   const el = document.getElementById('js-new-pipeline');
@@ -7,17 +8,20 @@ export default () => {
     projectId,
     pipelinesPath,
     configVariablesPath,
+    defaultBranch,
     refParam,
     varParam,
     fileParam,
-    refNames,
+    branchRefs,
+    tagRefs,
     settingsLink,
     maxWarnings,
   } = el?.dataset;
 
   const variableParams = JSON.parse(varParam);
   const fileParams = JSON.parse(fileParam);
-  const refs = JSON.parse(refNames);
+  const branches = formatRefs(JSON.parse(branchRefs), 'branch');
+  const tags = formatRefs(JSON.parse(tagRefs), 'tag');
 
   return new Vue({
     el,
@@ -27,10 +31,12 @@ export default () => {
           projectId,
           pipelinesPath,
           configVariablesPath,
+          defaultBranch,
           refParam,
           variableParams,
           fileParams,
-          refs,
+          branches,
+          tags,
           settingsLink,
           maxWarnings: Number(maxWarnings),
         },

app/assets/javascripts/pipeline_new/utils/format_refs.js

+import { BRANCH_REF_TYPE, TAG_REF_TYPE } from '../constants';
+
+export default (refs, type) => {
+  let fullName;
+
+  return refs.map(ref => {
+    if (type === BRANCH_REF_TYPE) {
+      fullName = `refs/heads/${ref}`;
+    } else if (type === TAG_REF_TYPE) {
+      fullName = `refs/tags/${ref}`;
+    }
+
+    return {
+      shortName: ref,
+      fullName,
+    };
+  });
+};

app/views/projects/pipelines/new.html.haml

@@ -10,10 +10,12 @@
   #js-new-pipeline{ data: { project_id: @project.id,
     pipelines_path: project_pipelines_path(@project),
     config_variables_path: config_variables_namespace_project_pipelines_path(@project.namespace, @project),
+    default_branch: @project.default_branch,
     ref_param: params[:ref] || @project.default_branch,
     var_param: params[:var].to_json,
     file_param: params[:file_var].to_json,
-    ref_names: @project.repository.ref_names.to_json.html_safe,
+    branch_refs: @project.repository.branch_names.to_json.html_safe,
+    tag_refs: @project.repository.tag_names.to_json.html_safe,
     settings_link: project_settings_ci_cd_path(@project),
     max_warnings: ::Gitlab::Ci::Warnings::MAX_LIMIT } }
 

changelogs/unreleased/send-full-ref-when-running-manual-pipeline.yml

+---
+title: Manually trigger pipelines correctly when branches and tags have the same name. Separate tags and branches in trigger pipeline form.
+merge_request: 48142
+author:
+type: fixed

locale/gitlab.pot

@@ -23919,6 +23919,9 @@ msgstr ""
 msgid "Search projects..."
 msgstr ""
 
+msgid "Search refs"
+msgstr ""
+
 msgid "Search requirements"
 msgstr ""
 

spec/frontend/pipeline_new/components/pipeline_new_form_spec.js

@@ -5,7 +5,14 @@ import waitForPromises from 'helpers/wait_for_promises';
 import httpStatusCodes from '~/lib/utils/http_status';
 import axios from '~/lib/utils/axios_utils';
 import PipelineNewForm from '~/pipeline_new/components/pipeline_new_form.vue';
-import { mockRefs, mockParams, mockPostParams, mockProjectId, mockError } from '../mock_data';
+import {
+  mockBranches,
+  mockTags,
+  mockParams,
+  mockPostParams,
+  mockProjectId,
+  mockError,
+} from '../mock_data';
 import { redirectTo } from '~/lib/utils/url_utility';
 
 jest.mock('~/lib/utils/url_utility', () => ({
@@ -37,6 +44,10 @@ describe('Pipeline New Form', () => {
   const findWarnings = () => wrapper.findAll('[data-testid="run-pipeline-warning"]');
   const findLoadingIcon = () => wrapper.find(GlLoadingIcon);
   const getExpectedPostParams = () => JSON.parse(mock.history.post[0].data);
+  const changeRef = i =>
+    findDropdownItems()
+      .at(i)
+      .vm.$emit('click');
 
   const createComponent = (term = '', props = {}, method = shallowMount) => {
     wrapper = method(PipelineNewForm, {
@@ -44,7 +55,8 @@ describe('Pipeline New Form', () => {
         projectId: mockProjectId,
         pipelinesPath,
         configVariablesPath,
-        refs: mockRefs,
+        branches: mockBranches,
+        tags: mockTags,
         defaultBranch: 'master',
         settingsLink: '',
         maxWarnings: 25,
@@ -76,8 +88,11 @@ describe('Pipeline New Form', () => {
     });
 
     it('displays dropdown with all branches and tags', () => {
+      const refLength = mockBranches.length + mockTags.length;
+
       createComponent();
-      expect(findDropdownItems()).toHaveLength(mockRefs.length);
+
+      expect(findDropdownItems()).toHaveLength(refLength);
     });
 
     it('when user enters search term the list is filtered', () => {
@@ -130,15 +145,6 @@ describe('Pipeline New Form', () => {
       expect(findVariableRows()).toHaveLength(2);
     });
 
-    it('creates a pipeline on submit', async () => {
-      findForm().vm.$emit('submit', dummySubmitEvent);
-
-      await waitForPromises();
-
-      expect(getExpectedPostParams()).toEqual(mockPostParams);
-      expect(redirectTo).toHaveBeenCalledWith(`${pipelinesPath}/${postResponse.id}`);
-    });
-
     it('creates blank variable on input change event', async () => {
       const input = findKeyInputs().at(2);
       input.element.value = 'test_var_2';
@@ -150,45 +156,81 @@ describe('Pipeline New Form', () => {
       expect(findKeyInputs().at(3).element.value).toBe('');
       expect(findValueInputs().at(3).element.value).toBe('');
     });
+  });
 
-    describe('when the form has been modified', () => {
-      const selectRef = i =>
-        findDropdownItems()
-          .at(i)
-          .vm.$emit('click');
+  describe('Pipeline creation', () => {
+    beforeEach(async () => {
+      mock.onPost(pipelinesPath).reply(httpStatusCodes.OK, postResponse);
 
-      beforeEach(async () => {
-        const input = findKeyInputs().at(0);
-        input.element.value = 'test_var_2';
-        input.trigger('change');
+      await waitForPromises();
+    });
+    it('creates pipeline with full ref and variables', async () => {
+      createComponent();
 
-        findRemoveIcons()
-          .at(1)
-          .trigger('click');
+      changeRef(0);
 
-        await wrapper.vm.$nextTick();
-      });
+      findForm().vm.$emit('submit', dummySubmitEvent);
 
-      it('form values are restored when the ref changes', async () => {
-        expect(findVariableRows()).toHaveLength(2);
+      await waitForPromises();
 
-        selectRef(1);
-        await waitForPromises();
+      expect(getExpectedPostParams().ref).toEqual(wrapper.vm.$data.refValue.fullName);
+      expect(redirectTo).toHaveBeenCalledWith(`${pipelinesPath}/${postResponse.id}`);
+    });
+    it('creates a pipeline with short ref and variables', async () => {
+      // query params are used
+      createComponent('', mockParams);
 
-        expect(findVariableRows()).toHaveLength(3);
-        expect(findKeyInputs().at(0).element.value).toBe('test_var');
-      });
+      await waitForPromises();
 
-      it('form values are restored again when the ref is reverted', async () => {
-        selectRef(1);
-        await waitForPromises();
+      findForm().vm.$emit('submit', dummySubmitEvent);
 
-        selectRef(2);
-        await waitForPromises();
+      await waitForPromises();
 
-        expect(findVariableRows()).toHaveLength(2);
-        expect(findKeyInputs().at(0).element.value).toBe('test_var_2');
-      });
+      expect(getExpectedPostParams()).toEqual(mockPostParams);
+      expect(redirectTo).toHaveBeenCalledWith(`${pipelinesPath}/${postResponse.id}`);
+    });
+  });
+
+  describe('When the ref has been changed', () => {
+    beforeEach(async () => {
+      createComponent('', {}, mount);
+
+      await waitForPromises();
+    });
+    it('variables persist between ref changes', async () => {
+      changeRef(0); // change to master
+
+      await waitForPromises();
+
+      const masterInput = findKeyInputs().at(0);
+      masterInput.element.value = 'build_var';
+      masterInput.trigger('change');
+
+      await wrapper.vm.$nextTick();
+
+      changeRef(1); // change to branch-1
+
+      await waitForPromises();
+
+      const branchOneInput = findKeyInputs().at(0);
+      branchOneInput.element.value = 'deploy_var';
+      branchOneInput.trigger('change');
+
+      await wrapper.vm.$nextTick();
+
+      changeRef(0); // change back to master
+
+      await waitForPromises();
+
+      expect(findKeyInputs().at(0).element.value).toBe('build_var');
+      expect(findVariableRows().length).toBe(2);
+
+      changeRef(1); // change back to branch-1
+
+      await waitForPromises();
+
+      expect(findKeyInputs().at(0).element.value).toBe('deploy_var');
+      expect(findVariableRows().length).toBe(2);
     });
   });
 
@@ -321,6 +363,7 @@ describe('Pipeline New Form', () => {
 
     it('shows the correct warning title', () => {
       const { length } = mockError.warnings;
+
       expect(findWarningAlertSummary().attributes('message')).toBe(`${length} warnings found:`);
     });
 

spec/frontend/pipeline_new/mock_data.js

-export const mockRefs = ['master', 'branch-1', 'tag-1'];
+export const mockBranches = [
+  { shortName: 'master', fullName: 'refs/heads/master' },
+  { shortName: 'branch-1', fullName: 'refs/heads/branch-1' },
+  { shortName: 'branch-2', fullName: 'refs/heads/branch-2' },
+];
+
+export const mockTags = [
+  { shortName: '1.0.0', fullName: 'refs/tags/1.0.0' },
+  { shortName: '1.1.0', fullName: 'refs/tags/1.1.0' },
+  { shortName: '1.2.0', fullName: 'refs/tags/1.2.0' },
+];
 
 export const mockParams = {
   refParam: 'tag-1',
@@ -31,3 +41,7 @@ export const mockError = {
   ],
   total_warnings: 7,
 };
+
+export const mockBranchRefs = ['master', 'dev', 'release'];
+
+export const mockTagRefs = ['1.0.0', '1.1.0', '1.2.0'];

spec/frontend/pipeline_new/utils/format_refs_spec.js

+import formatRefs from '~/pipeline_new/utils/format_refs';
+import { BRANCH_REF_TYPE, TAG_REF_TYPE } from '~/pipeline_new/constants';
+import { mockBranchRefs, mockTagRefs } from '../mock_data';
+
+describe('Format refs util', () => {
+  it('formats branch ref correctly', () => {
+    expect(formatRefs(mockBranchRefs, BRANCH_REF_TYPE)).toEqual([
+      { fullName: 'refs/heads/master', shortName: 'master' },
+      { fullName: 'refs/heads/dev', shortName: 'dev' },
+      { fullName: 'refs/heads/release', shortName: 'release' },
+    ]);
+  });
+
+  it('formats tag ref correctly', () => {
+    expect(formatRefs(mockTagRefs, TAG_REF_TYPE)).toEqual([
+      { fullName: 'refs/tags/1.0.0', shortName: '1.0.0' },
+      { fullName: 'refs/tags/1.1.0', shortName: '1.1.0' },
+      { fullName: 'refs/tags/1.2.0', shortName: '1.2.0' },
+    ]);
+  });
+});