Load branches on new merge request page asynchronously

app/assets/javascripts/compare.js

@@ -1,86 +0,0 @@
-/* eslint-disable func-names, space-before-function-paren, wrap-iife, quotes, no-var, object-shorthand, consistent-return, no-unused-vars, comma-dangle, vars-on-top, prefer-template, max-len */
-
-import $ from 'jquery';
-import { localTimeAgo } from './lib/utils/datetime_utility';
-import axios from './lib/utils/axios_utils';
-
-export default class Compare {
-  constructor(opts) {
-    this.opts = opts;
-    this.source_loading = $(".js-source-loading");
-    this.target_loading = $(".js-target-loading");
-    $('.js-compare-dropdown').each((function(_this) {
-      return function(i, dropdown) {
-        var $dropdown;
-        $dropdown = $(dropdown);
-        return $dropdown.glDropdown({
-          selectable: true,
-          fieldName: $dropdown.data('fieldName'),
-          filterable: true,
-          id: function(obj, $el) {
-            return $el.data('id');
-          },
-          toggleLabel: function(obj, $el) {
-            return $el.text().trim();
-          },
-          clicked: function(e, el) {
-            if ($dropdown.is('.js-target-branch')) {
-              return _this.getTargetHtml();
-            } else if ($dropdown.is('.js-source-branch')) {
-              return _this.getSourceHtml();
-            } else if ($dropdown.is('.js-target-project')) {
-              return _this.getTargetProject();
-            }
-          }
-        });
-      };
-    })(this));
-    this.initialState();
-  }
-
-  initialState() {
-    this.getSourceHtml();
-    this.getTargetHtml();
-  }
-
-  getTargetProject() {
-    $('.mr_target_commit').empty();
-
-    return axios.get(this.opts.targetProjectUrl, {
-      params: {
-        target_project_id: $("input[name='merge_request[target_project_id]']").val(),
-      },
-    }).then(({ data }) => {
-      $('.js-target-branch-dropdown .dropdown-content').html(data);
-    });
-  }
-
-  getSourceHtml() {
-    return this.constructor.sendAjax(this.opts.sourceBranchUrl, this.source_loading, '.mr_source_commit', {
-      ref: $("input[name='merge_request[source_branch]']").val()
-    });
-  }
-
-  getTargetHtml() {
-    return this.constructor.sendAjax(this.opts.targetBranchUrl, this.target_loading, '.mr_target_commit', {
-      target_project_id: $("input[name='merge_request[target_project_id]']").val(),
-      ref: $("input[name='merge_request[target_branch]']").val()
-    });
-  }
-
-  static sendAjax(url, loading, target, params) {
-    const $target = $(target);
-
-    loading.show();
-    $target.empty();
-
-    return axios.get(url, {
-      params,
-    }).then(({ data }) => {
-      loading.hide();
-      $target.html(data);
-      const className = '.' + $target[0].className.replace(' ', '.');
-      localTimeAgo($('.js-timeago', className));
-    });
-  }
-}

app/assets/javascripts/compare_autocomplete.js

@@ -4,8 +4,9 @@ import $ from 'jquery';
 import { __ } from './locale';
 import axios from './lib/utils/axios_utils';
 import flash from './flash';
+import { capitalizeFirstCharacter } from './lib/utils/text_utility';
 
-export default function initCompareAutocomplete() {
+export default function initCompareAutocomplete(limitTo = null, clickHandler = () => {}) {
   $('.js-compare-dropdown').each(function() {
     var $dropdown, selected;
     $dropdown = $(this);
@@ -15,14 +16,27 @@ export default function initCompareAutocomplete() {
     const $filterInput = $('input[type="search"]', $dropdownContainer);
     $dropdown.glDropdown({
       data: function(term, callback) {
-        axios.get($dropdown.data('refsUrl'), {
+        const params = {
-          params: {
           ref: $dropdown.data('ref'),
           search: term,
-          },
+        };
-        }).then(({ data }) => {
+
+        if (limitTo) {
+          params.find = limitTo;
+        }
+
+        axios
+          .get($dropdown.data('refsUrl'), {
+            params,
+          })
+          .then(({ data }) => {
+            if (limitTo) {
+              callback(data[capitalizeFirstCharacter(limitTo)] || []);
+            } else {
               callback(data);
-        }).catch(() => flash(__('Error fetching refs')));
+            }
+          })
+          .catch(() => flash(__('Error fetching refs')));
       },
       selectable: true,
       filterable: true,
@@ -32,9 +46,15 @@ export default function initCompareAutocomplete() {
       renderRow: function(ref) {
         var link;
         if (ref.header != null) {
-          return $('<li />').addClass('dropdown-header').text(ref.header);
+          return $('<li />')
+            .addClass('dropdown-header')
+            .text(ref.header);
         } else {
-          link = $('<a />').attr('href', '#').addClass(ref === selected ? 'is-active' : '').text(ref).attr('data-ref', escape(ref));
+          link = $('<a />')
+            .attr('href', '#')
+            .addClass(ref === selected ? 'is-active' : '')
+            .text(ref)
+            .attr('data-ref', escape(ref));
           return $('<li />').append(link);
         }
       },
@@ -43,9 +63,10 @@ export default function initCompareAutocomplete() {
       },
       toggleLabel: function(obj, $el) {
         return $el.text().trim();
-      }
+      },
+      clicked: () => clickHandler($dropdown),
     });
-    $filterInput.on('keyup', (e) => {
+    $filterInput.on('keyup', e => {
       const keyCode = e.keyCode || e.which;
       if (keyCode !== 13) return;
       const text = $filterInput.val();
@@ -54,7 +75,7 @@ export default function initCompareAutocomplete() {
       $dropdownContainer.removeClass('open');
     });
 
-    $dropdownContainer.on('click', '.dropdown-content a', (e) => {
+    $dropdownContainer.on('click', '.dropdown-content a', e => {
       $dropdown.prop('title', e.target.text.replace(/_+?/g, '-'));
       if ($dropdown.hasClass('has-tooltip')) {
         $dropdown.tooltip('fixTitle');

app/assets/javascripts/pages/projects/compare/index.js

@@ -1,3 +1,3 @@
 import initCompareAutocomplete from '~/compare_autocomplete';
 
-document.addEventListener('DOMContentLoaded', initCompareAutocomplete);
+document.addEventListener('DOMContentLoaded', () => initCompareAutocomplete());

app/assets/javascripts/pages/projects/merge_requests/creations/new/compare.js

@@ -0,0 +1,60 @@
+import $ from 'jquery';
+import { localTimeAgo } from '~/lib/utils/datetime_utility';
+import axios from '~/lib/utils/axios_utils';
+import initCompareAutocomplete from '~/compare_autocomplete';
+import initTargetProjectDropdown from './target_project_dropdown';
+
+const updateCommitList = (url, $loadingIndicator, $commitList, params) => {
+  $loadingIndicator.show();
+  $commitList.empty();
+
+  return axios
+    .get(url, {
+      params,
+    })
+    .then(({ data }) => {
+      $loadingIndicator.hide();
+      $commitList.html(data);
+      localTimeAgo($('.js-timeago', $commitList));
+    });
+};
+
+export default mrNewCompareNode => {
+  const { sourceBranchUrl, targetBranchUrl } = mrNewCompareNode.dataset;
+  initTargetProjectDropdown();
+
+  const updateSourceBranchCommitList = () =>
+    updateCommitList(
+      sourceBranchUrl,
+      $(mrNewCompareNode).find('.js-source-loading'),
+      $(mrNewCompareNode).find('.mr_source_commit'),
+      {
+        ref: $(mrNewCompareNode)
+          .find("input[name='merge_request[source_branch]']")
+          .val(),
+      },
+    );
+  const updateTargetBranchCommitList = () =>
+    updateCommitList(
+      targetBranchUrl,
+      $(mrNewCompareNode).find('.js-target-loading'),
+      $(mrNewCompareNode).find('.mr_target_commit'),
+      {
+        target_project_id: $(mrNewCompareNode)
+          .find("input[name='merge_request[target_project_id]']")
+          .val(),
+        ref: $(mrNewCompareNode)
+          .find("input[name='merge_request[target_branch]']")
+          .val(),
+      },
+    );
+  initCompareAutocomplete('branches', $dropdown => {
+    if ($dropdown.is('.js-target-branch')) {
+      updateTargetBranchCommitList();
+    } else if ($dropdown.is('.js-source-branch')) {
+      updateSourceBranchCommitList();
+    }
+  });
+  updateSourceBranchCommitList();
+  updateTargetBranchCommitList();
+};

app/assets/javascripts/pages/projects/merge_requests/creations/new/index.js

@@ -1,18 +1,15 @@
-import Compare from '~/compare';
 import MergeRequest from '~/merge_request';
 import initPipelines from '~/commit/pipelines/pipelines_bundle';
+import initCompare from './compare';
 
 document.addEventListener('DOMContentLoaded', () => {
   const mrNewCompareNode = document.querySelector('.js-merge-request-new-compare');
   if (mrNewCompareNode) {
-    new Compare({ // eslint-disable-line no-new
+    initCompare(mrNewCompareNode);
-      targetProjectUrl: mrNewCompareNode.dataset.targetProjectUrl,
-      sourceBranchUrl: mrNewCompareNode.dataset.sourceBranchUrl,
-      targetBranchUrl: mrNewCompareNode.dataset.targetBranchUrl,
-    });
   } else {
     const mrNewSubmitNode = document.querySelector('.js-merge-request-new-submit');
-    new MergeRequest({ // eslint-disable-line no-new
+    // eslint-disable-next-line no-new
+    new MergeRequest({
       action: mrNewSubmitNode.dataset.mrSubmitAction,
     });
     initPipelines();

app/assets/javascripts/pages/projects/merge_requests/creations/new/target_project_dropdown.js

@@ -0,0 +1,22 @@
+import $ from 'jquery';
+
+export default () => {
+  const $targetProjectDropdown = $('.js-target-project');
+  $targetProjectDropdown.glDropdown({
+    selectable: true,
+    fieldName: $targetProjectDropdown.data('fieldName'),
+    filterable: true,
+    id(obj, $el) {
+      return $el.data('id');
+    },
+    toggleLabel(obj, $el) {
+      return $el.text().trim();
+    },
+    clicked({ $el }) {
+      $('.mr_target_commit').empty();
+      const $targetBranchDropdown = $('.js-target-branch');
+      $targetBranchDropdown.data('refsUrl', $el.data('refsUrl'));
+      $targetBranchDropdown.data('glDropdown').clearMenu();
+    },
+  });
+};

app/controllers/projects/merge_requests/creations_controller.rb

@@ -83,13 +83,6 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
     render layout: false
   end
 
-  def update_branches
-    @target_project = selected_target_project
-    @target_branches = @target_project ? @target_project.repository.branch_names : []
-
-    render layout: false
-  end
-
   private
 
   def build_merge_request

app/views/projects/merge_requests/creations/_new_compare.html.haml

@@ -3,7 +3,7 @@
 
 = form_for [@project.namespace.becomes(Namespace), @project, @merge_request], url: project_new_merge_request_path(@project), method: :get, html: { class: "merge-request-form form-inline js-requires-input" } do |f|
   .hide.alert.alert-danger.mr-compare-errors
-  .js-merge-request-new-compare.row{ 'data-target-project-url': project_new_merge_request_update_branches_path(@source_project), 'data-source-branch-url': project_new_merge_request_branch_from_path(@source_project), 'data-target-branch-url': project_new_merge_request_branch_to_path(@source_project) }
+  .js-merge-request-new-compare.row{ 'data-source-branch-url': project_new_merge_request_branch_from_path(@source_project), 'data-target-branch-url': project_new_merge_request_branch_to_path(@source_project) }
     .col-md-6
       .panel.panel-default.panel-new-merge-request
         .panel-heading
@@ -11,7 +11,7 @@
         .panel-body.clearfix
           .merge-request-select.dropdown
             = f.hidden_field :source_project_id
-            = dropdown_toggle @merge_request.source_project_path, { toggle: "dropdown", field_name: "#{f.object_name}[source_project_id]", disabled: @merge_request.persisted? }, { toggle_class: "js-compare-dropdown js-source-project" }
+            = dropdown_toggle @merge_request.source_project_path, { toggle: "dropdown", 'field-name': "#{f.object_name}[source_project_id]", disabled: @merge_request.persisted? }, { toggle_class: "js-compare-dropdown js-source-project" }
             .dropdown-menu.dropdown-menu-selectable.dropdown-source-project
               = dropdown_title("Select source project")
               = dropdown_filter("Search projects")
@@ -21,14 +21,12 @@
                   selected: f.object.source_project_id
           .merge-request-select.dropdown
             = f.hidden_field :source_branch
-            = dropdown_toggle f.object.source_branch || "Select source branch", { toggle: "dropdown", field_name: "#{f.object_name}[source_branch]" }, { toggle_class: "js-compare-dropdown js-source-branch git-revision-dropdown-toggle" }
+            = dropdown_toggle f.object.source_branch || _("Select source branch"), { toggle: "dropdown", 'field-name': "#{f.object_name}[source_branch]", 'refs-url': refs_project_path(@source_project), selected: f.object.source_branch }, { toggle_class: "js-compare-dropdown js-source-branch git-revision-dropdown-toggle" }
-            .dropdown-menu.dropdown-menu-selectable.dropdown-source-branch.git-revision-dropdown
+            .dropdown-menu.dropdown-menu-selectable.js-source-branch-dropdown.git-revision-dropdown
-              = dropdown_title("Select source branch")
+              = dropdown_title(_("Select source branch"))
-              = dropdown_filter("Search branches")
+              = dropdown_filter(_("Search branches"))
-              = dropdown_content do
+              = dropdown_content
-                = render 'projects/merge_requests/dropdowns/branch',
+              = dropdown_loading
-                  branches: @merge_request.source_branches,
-                  selected: f.object.source_branch
         .panel-footer
           .text-center= icon('spinner spin', class: 'js-source-loading')
           %ul.list-unstyled.mr_source_commit
@@ -41,7 +39,7 @@
           - projects = target_projects(@project)
           .merge-request-select.dropdown
             = f.hidden_field :target_project_id
-            = dropdown_toggle f.object.target_project.full_path, { toggle: "dropdown", field_name: "#{f.object_name}[target_project_id]", disabled: @merge_request.persisted? }, { toggle_class: "js-compare-dropdown js-target-project" }
+            = dropdown_toggle f.object.target_project.full_path, { toggle: "dropdown", 'field-name': "#{f.object_name}[target_project_id]", disabled: @merge_request.persisted? }, { toggle_class: "js-compare-dropdown js-target-project" }
             .dropdown-menu.dropdown-menu-selectable.dropdown-target-project
               = dropdown_title("Select target project")
               = dropdown_filter("Search projects")
@@ -51,14 +49,12 @@
                   selected: f.object.target_project_id
           .merge-request-select.dropdown
             = f.hidden_field :target_branch
-            = dropdown_toggle f.object.target_branch, { toggle: "dropdown", field_name: "#{f.object_name}[target_branch]" }, { toggle_class: "js-compare-dropdown js-target-branch git-revision-dropdown-toggle" }
+            = dropdown_toggle f.object.target_branch, { toggle: "dropdown", 'field-name': "#{f.object_name}[target_branch]", 'refs-url': refs_project_path(f.object.target_project), selected: f.object.target_branch }, { toggle_class: "js-compare-dropdown js-target-branch git-revision-dropdown-toggle" }
-            .dropdown-menu.dropdown-menu-selectable.dropdown-target-branch.js-target-branch-dropdown.git-revision-dropdown
+            .dropdown-menu.dropdown-menu-selectable.js-target-branch-dropdown.git-revision-dropdown
-              = dropdown_title("Select target branch")
+              = dropdown_title(_("Select target branch"))
-              = dropdown_filter("Search branches")
+              = dropdown_filter(_("Search branches"))
-              = dropdown_content do
+              = dropdown_content
-                = render 'projects/merge_requests/dropdowns/branch',
+              = dropdown_loading
-                  branches: @merge_request.target_branches,
-                  selected: f.object.target_branch
         .panel-footer
           .text-center= icon('spinner spin', class: "js-target-loading")
           %ul.list-unstyled.mr_target_commit

app/views/projects/merge_requests/dropdowns/_project.html.haml

@@ -1,5 +1,5 @@
 %ul
   - projects.each do |project|
     %li
-      %a{ href: "#", class: "#{('is-active' if selected == project.id)}", data: { id: project.id } }
+      %a{ href: "#", class: "#{('is-active' if selected == project.id)}", data: { id: project.id, 'refs-url': refs_project_path(project) } }
         = project.full_path

changelogs/unreleased/winh-new-mergerequest-branch-picker.yml

@@ -0,0 +1,5 @@
+---
+title: Load branches on new merge request page asynchronously
+merge_request: 18315
+author:
+type: changed

config/routes/project.rb

@@ -161,7 +161,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
           end
 
           get :diff_for_path
-          get :update_branches
           get :branch_from
           get :branch_to
         end

features/steps/project/forked_merge_requests.rb

@@ -53,7 +53,7 @@ class Spinach::Features::ProjectForkedMergeRequests < Spinach::FeatureSteps
 
     first('.js-source-branch').click
     wait_for_requests
-    first('.dropdown-source-branch .dropdown-content a', text: 'fix').click
+    first('.js-source-branch-dropdown .dropdown-content a', text: 'fix').click
 
     click_button "Compare branches and continue"
 

spec/controllers/projects/merge_requests/creations_controller_spec.rb

@@ -157,34 +157,4 @@ describe Projects::MergeRequests::CreationsController do
       expect(response).to have_gitlab_http_status(200)
     end
   end
-
-  describe 'GET #update_branches' do
-    before do
-      allow(Ability).to receive(:allowed?).and_call_original
-    end
-
-    it 'lists the branches of another fork if the user has access' do
-      expect(Ability).to receive(:allowed?).with(user, :read_project, project) { true }
-
-      get :update_branches,
-          namespace_id: fork_project.namespace,
-          project_id: fork_project,
-          target_project_id: project.id
-
-      expect(assigns(:target_branches)).not_to be_empty
-      expect(response).to have_gitlab_http_status(200)
-    end
-
-    it 'does not list branches when the user cannot read the project' do
-      expect(Ability).to receive(:allowed?).with(user, :read_project, project) { false }
-
-      get :update_branches,
-          namespace_id: fork_project.namespace,
-          project_id: fork_project,
-          target_project_id: project.id
-
-      expect(response).to have_gitlab_http_status(200)
-      expect(assigns(:target_branches)).to eq([])
-    end
-  end
 end

spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb

@@ -19,7 +19,7 @@ describe 'Merge request > User selects branches for new MR', :js do
     expect(page).to have_content('Target branch')
 
     first('.js-source-branch').click
-    find('.dropdown-source-branch .dropdown-content a', match: :first).click
+    find('.js-source-branch-dropdown .dropdown-content a', match: :first).click
 
     expect(page).to have_content "b83d6e3"
   end
@@ -35,22 +35,16 @@ describe 'Merge request > User selects branches for new MR', :js do
     expect(page).to have_content('Target branch')
 
     first('.js-target-branch').click
-    find('.dropdown-target-branch .dropdown-content a', text: 'v1.1.0', match: :first).click
+    find('.js-target-branch-dropdown .dropdown-content a', text: 'v1.1.0', match: :first).click
 
     expect(page).to have_content "b83d6e3"
   end
 
   it 'generates a diff for an orphaned branch' do
-    visit project_merge_requests_path(project)
+    visit project_new_merge_request_path(project)
-
-    page.within '.content' do
-      click_link 'New merge request'
-    end
-    expect(page).to have_content('Source branch')
-    expect(page).to have_content('Target branch')
 
     find('.js-source-branch', match: :first).click
-    find('.dropdown-source-branch .dropdown-content a', text: 'orphaned-branch', match: :first).click
+    find('.js-source-branch-dropdown .dropdown-content a', text: 'orphaned-branch', match: :first).click
 
     click_button "Compare branches"
     click_link "Changes"
@@ -71,19 +65,18 @@ describe 'Merge request > User selects branches for new MR', :js do
 
     first('.js-source-branch').click
 
-    input = find('.dropdown-source-branch .dropdown-input-field')
+    page.within '.js-source-branch-dropdown' do
+      input = find('.dropdown-input-field')
       input.click
       input.send_keys('orphaned-branch')
 
-    find('.dropdown-source-branch .dropdown-content li', match: :first)
+      expect(page).to have_css('.dropdown-content li', count: 1)
-    source_items = all('.dropdown-source-branch .dropdown-content li')
+    end
-
-    expect(source_items.count).to eq(1)
 
     first('.js-target-branch').click
 
-    find('.dropdown-target-branch .dropdown-content li', match: :first)
+    find('.js-target-branch-dropdown .dropdown-content li', match: :first)
-    target_items = all('.dropdown-target-branch .dropdown-content li')
+    target_items = all('.js-target-branch-dropdown .dropdown-content li')
 
     expect(target_items.count).to be > 1
   end
@@ -171,7 +164,6 @@ describe 'Merge request > User selects branches for new MR', :js do
 
       page.within('.merge-request') do
         click_link 'Pipelines'
-        wait_for_requests
 
         expect(page).to have_content "##{pipeline.id}"
       end

spec/support/shared_examples/features/creatable_merge_request_shared_examples.rb

@@ -79,7 +79,7 @@ RSpec.shared_examples 'a creatable merge request' do
     end
   end
 
-  it 'updates the branches when selecting a new target project' do
+  it 'updates the branches when selecting a new target project', :js do
     target_project_member = target_project.owner
     CreateBranchService.new(target_project, target_project_member)
     .execute('a-brand-new-branch-to-test', 'master')
@@ -92,7 +92,7 @@ RSpec.shared_examples 'a creatable merge request' do
 
     first('.js-target-branch').click
 
-    within('.dropdown-target-branch .dropdown-content') do
+    within('.js-target-branch-dropdown .dropdown-content') do
       expect(page).to have_content('a-brand-new-branch-to-test')
     end
   end