Merge branch 'winh-new-branch-url-encode' into 'master'

Fix branch name encoding for dropdown on issue page

Closes #47472

See merge request gitlab-org/gitlab-ce!19634

app/assets/javascripts/create_merge_request_dropdown.js

@@ -66,8 +66,14 @@ export default class CreateMergeRequestDropdown {
   }
 
   bindEvents() {
-    this.createMergeRequestButton.addEventListener('click', this.onClickCreateMergeRequestButton.bind(this));
-    this.createTargetButton.addEventListener('click', this.onClickCreateMergeRequestButton.bind(this));
+    this.createMergeRequestButton.addEventListener(
+      'click',
+      this.onClickCreateMergeRequestButton.bind(this),
+    );
+    this.createTargetButton.addEventListener(
+      'click',
+      this.onClickCreateMergeRequestButton.bind(this),
+    );
     this.branchInput.addEventListener('keyup', this.onChangeInput.bind(this));
     this.dropdownToggle.addEventListener('click', this.onClickSetFocusOnBranchNameInput.bind(this));
     this.refInput.addEventListener('keyup', this.onChangeInput.bind(this));
@@ -77,7 +83,8 @@ export default class CreateMergeRequestDropdown {
   checkAbilityToCreateBranch() {
     this.setUnavailableButtonState();
 
-    axios.get(this.canCreatePath)
+    axios
+      .get(this.canCreatePath)
       .then(({ data }) => {
         this.setUnavailableButtonState(false);
 
@@ -105,7 +112,8 @@ export default class CreateMergeRequestDropdown {
   createBranch() {
     this.isCreatingBranch = true;
 
-    return axios.post(this.createBranchPath)
+    return axios
+      .post(this.createBranchPath)
       .then(({ data }) => {
         this.branchCreated = true;
         window.location.href = data.url;
@@ -116,7 +124,8 @@ export default class CreateMergeRequestDropdown {
   createMergeRequest() {
     this.isCreatingMergeRequest = true;
 
-    return axios.post(this.createMrPath)
+    return axios
+      .post(this.createMrPath)
       .then(({ data }) => {
         this.mergeRequestCreated = true;
         window.location.href = data.url;
@@ -195,7 +204,8 @@ export default class CreateMergeRequestDropdown {
   getRef(ref, target = 'all') {
     if (!ref) return false;
 
-    return axios.get(this.refsPath + ref)
+    return axios
+      .get(`${this.refsPath}${encodeURIComponent(ref)}`)
       .then(({ data }) => {
         const branches = data[Object.keys(data)[0]];
         const tags = data[Object.keys(data)[1]];
@@ -204,7 +214,8 @@ export default class CreateMergeRequestDropdown {
         if (target === 'branch') {
           result = CreateMergeRequestDropdown.findByValue(branches, ref);
         } else {
-          result = CreateMergeRequestDropdown.findByValue(branches, ref, true) ||
+          result =
+            CreateMergeRequestDropdown.findByValue(branches, ref, true) ||
             CreateMergeRequestDropdown.findByValue(tags, ref, true);
           this.suggestedRef = result;
         }
@@ -255,11 +266,13 @@ export default class CreateMergeRequestDropdown {
   }
 
   isBusy() {
-    return this.isCreatingMergeRequest ||
+    return (
+      this.isCreatingMergeRequest ||
       this.mergeRequestCreated ||
       this.isCreatingBranch ||
       this.branchCreated ||
-      this.isGettingRef;
+      this.isGettingRef
+    );
   }
 
   onChangeInput(event) {
@@ -271,7 +284,8 @@ export default class CreateMergeRequestDropdown {
       value = this.branchInput.value;
     } else if (event.target === this.refInput) {
       target = 'ref';
-      value = event.target.value.slice(0, event.target.selectionStart) +
+      value =
+        event.target.value.slice(0, event.target.selectionStart) +
         event.target.value.slice(event.target.selectionEnd);
     } else {
       return false;
@@ -396,7 +410,8 @@ export default class CreateMergeRequestDropdown {
 
   showNotAvailableMessage(target) {
     const { input, message } = this.getTargetData(target);
-    const text = target === 'branch' ? __('Branch is already taken') : __('Source is not available');
+    const text =
+      target === 'branch' ? __('Branch is already taken') : __('Source is not available');
 
     this.removeMessage(target);
     input.classList.add('gl-field-error-outline');
@@ -459,11 +474,15 @@ export default class CreateMergeRequestDropdown {
   // target - 'branch' or 'ref'
   // ref - string - the new value to use as branch or ref
   updateCreatePaths(target, ref) {
-    const pathReplacement = `$1${ref}`;
+    const pathReplacement = `$1${encodeURIComponent(ref)}`;
 
-    this.createBranchPath = this.createBranchPath.replace(this.regexps[target].createBranchPath,
-      pathReplacement);
-    this.createMrPath = this.createMrPath.replace(this.regexps[target].createMrPath,
-      pathReplacement);
+    this.createBranchPath = this.createBranchPath.replace(
+      this.regexps[target].createBranchPath,
+      pathReplacement,
+    );
+    this.createMrPath = this.createMrPath.replace(
+      this.regexps[target].createMrPath,
+      pathReplacement,
+    );
   }
 }

changelogs/unreleased/winh-new-branch-url-encode.yml

@@ -0,0 +1,5 @@
+---
+title: Fix branch name encoding for dropdown on issue page
+merge_request: 19634
+author:
+type: fixed

spec/javascripts/create_merge_request_dropdown_spec.js

@@ -0,0 +1,67 @@
+import axios from '~/lib/utils/axios_utils';
+import MockAdapter from 'axios-mock-adapter';
+import CreateMergeRequestDropdown from '~/create_merge_request_dropdown';
+import { TEST_HOST } from 'spec/test_constants';
+
+describe('CreateMergeRequestDropdown', () => {
+  let axiosMock;
+  let dropdown;
+
+  beforeEach(() => {
+    axiosMock = new MockAdapter(axios);
+
+    setFixtures(`
+      <div id="dummy-wrapper-element">
+        <div class="available"></div>
+        <div class="unavailable">
+          <div class="fa"></div>
+          <div class="text"></div>
+        </div>
+        <div class="js-ref"></div>
+        <div class="js-create-merge-request"></div>
+        <div class="js-create-target"></div>
+        <div class="js-dropdown-toggle"></div>
+      </div>
+    `);
+
+    const dummyElement = document.getElementById('dummy-wrapper-element');
+    dropdown = new CreateMergeRequestDropdown(dummyElement);
+    dropdown.refsPath = `${TEST_HOST}/dummy/refs?search=`;
+  });
+
+  afterEach(() => {
+    axiosMock.restore();
+  });
+
+  describe('getRef', () => {
+    it('escapes branch names correctly', done => {
+      const endpoint = `${dropdown.refsPath}contains%23hash`;
+      spyOn(axios, 'get').and.callThrough();
+      axiosMock.onGet(endpoint).replyOnce({});
+
+      dropdown
+        .getRef('contains#hash')
+        .then(() => {
+          expect(axios.get).toHaveBeenCalledWith(endpoint);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
+
+  describe('updateCreatePaths', () => {
+    it('escapes branch names correctly', () => {
+      dropdown.createBranchPath = `${TEST_HOST}/branches?branch_name=some-branch&issue=42`;
+      dropdown.createMrPath = `${TEST_HOST}/create_merge_request?branch_name=some-branch&ref=master`;
+
+      dropdown.updateCreatePaths('branch', 'contains#hash');
+
+      expect(dropdown.createBranchPath).toBe(
+        `${TEST_HOST}/branches?branch_name=contains%23hash&issue=42`,
+      );
+      expect(dropdown.createMrPath).toBe(
+        `${TEST_HOST}/create_merge_request?branch_name=contains%23hash&ref=master`,
+      );
+    });
+  });
+});