diff options
author | Winnie Hellmann <winnie@gitlab.com> | 2018-05-02 22:23:48 +0000 |
---|---|---|
committer | Clement Ho <clemmakesapps@gmail.com> | 2018-05-02 22:23:48 +0000 |
commit | 244f511eece07e906ec82bebf3ccb2f69a15ea92 (patch) | |
tree | 05fbe11c6e843c5444b2802bd1ec3d5e5f97ec39 | |
parent | 8bf030722dee9904e9b29bfd14c0099be9179b92 (diff) | |
download | gitlab-ce-244f511eece07e906ec82bebf3ccb2f69a15ea92.tar.gz |
Load branches on new merge request page asynchronously
15 files changed, 158 insertions, 189 deletions
diff --git a/app/assets/javascripts/compare.js b/app/assets/javascripts/compare.js deleted file mode 100644 index 303a5bf4a53..00000000000 --- a/app/assets/javascripts/compare.js +++ /dev/null @@ -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)); - }); - } -} diff --git a/app/assets/javascripts/compare_autocomplete.js b/app/assets/javascripts/compare_autocomplete.js index 260c91cac24..9c88466e576 100644 --- a/app/assets/javascripts/compare_autocomplete.js +++ b/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'), { - params: { - ref: $dropdown.data('ref'), - search: term, - }, - }).then(({ data }) => { - callback(data); - }).catch(() => flash(__('Error fetching refs'))); + const params = { + ref: $dropdown.data('ref'), + search: term, + }; + + 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'))); }, 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'); diff --git a/app/assets/javascripts/pages/projects/compare/index.js b/app/assets/javascripts/pages/projects/compare/index.js index d1c78bd61db..768da8fb236 100644 --- a/app/assets/javascripts/pages/projects/compare/index.js +++ b/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()); diff --git a/app/assets/javascripts/pages/projects/merge_requests/creations/new/compare.js b/app/assets/javascripts/pages/projects/merge_requests/creations/new/compare.js new file mode 100644 index 00000000000..46f3f55a400 --- /dev/null +++ b/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(); +}; diff --git a/app/assets/javascripts/pages/projects/merge_requests/creations/new/index.js b/app/assets/javascripts/pages/projects/merge_requests/creations/new/index.js index 6c9afddefac..01a0b4870c1 100644 --- a/app/assets/javascripts/pages/projects/merge_requests/creations/new/index.js +++ b/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 - targetProjectUrl: mrNewCompareNode.dataset.targetProjectUrl, - sourceBranchUrl: mrNewCompareNode.dataset.sourceBranchUrl, - targetBranchUrl: mrNewCompareNode.dataset.targetBranchUrl, - }); + initCompare(mrNewCompareNode); } 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(); diff --git a/app/assets/javascripts/pages/projects/merge_requests/creations/new/target_project_dropdown.js b/app/assets/javascripts/pages/projects/merge_requests/creations/new/target_project_dropdown.js new file mode 100644 index 00000000000..b72fe6681df --- /dev/null +++ b/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(); + }, + }); +}; diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index 4a377fefc62..81129456ad8 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/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 diff --git a/app/views/projects/merge_requests/creations/_new_compare.html.haml b/app/views/projects/merge_requests/creations/_new_compare.html.haml index f81db9b4e28..4e10511411f 100644 --- a/app/views/projects/merge_requests/creations/_new_compare.html.haml +++ b/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-menu.dropdown-menu-selectable.dropdown-source-branch.git-revision-dropdown - = dropdown_title("Select source branch") - = dropdown_filter("Search branches") - = dropdown_content do - = render 'projects/merge_requests/dropdowns/branch', - branches: @merge_request.source_branches, - selected: f.object.source_branch + = 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.js-source-branch-dropdown.git-revision-dropdown + = dropdown_title(_("Select source branch")) + = dropdown_filter(_("Search branches")) + = dropdown_content + = dropdown_loading .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-menu.dropdown-menu-selectable.dropdown-target-branch.js-target-branch-dropdown.git-revision-dropdown - = dropdown_title("Select target branch") - = dropdown_filter("Search branches") - = dropdown_content do - = render 'projects/merge_requests/dropdowns/branch', - branches: @merge_request.target_branches, - selected: f.object.target_branch + = 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.js-target-branch-dropdown.git-revision-dropdown + = dropdown_title(_("Select target branch")) + = dropdown_filter(_("Search branches")) + = dropdown_content + = dropdown_loading .panel-footer .text-center= icon('spinner spin', class: "js-target-loading") %ul.list-unstyled.mr_target_commit diff --git a/app/views/projects/merge_requests/dropdowns/_project.html.haml b/app/views/projects/merge_requests/dropdowns/_project.html.haml index aaf1ab00eeb..b3cf3c1d369 100644 --- a/app/views/projects/merge_requests/dropdowns/_project.html.haml +++ b/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 diff --git a/changelogs/unreleased/winh-new-mergerequest-branch-picker.yml b/changelogs/unreleased/winh-new-mergerequest-branch-picker.yml new file mode 100644 index 00000000000..401ecd09ef2 --- /dev/null +++ b/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 diff --git a/config/routes/project.rb b/config/routes/project.rb index 2a1bcb8cde2..0d24c5a5d4f 100644 --- a/config/routes/project.rb +++ b/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 diff --git a/features/steps/project/forked_merge_requests.rb b/features/steps/project/forked_merge_requests.rb index fd51ee1a316..82b931b2246 100644 --- a/features/steps/project/forked_merge_requests.rb +++ b/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" diff --git a/spec/controllers/projects/merge_requests/creations_controller_spec.rb b/spec/controllers/projects/merge_requests/creations_controller_spec.rb index 24310b847e8..00d76f3c39a 100644 --- a/spec/controllers/projects/merge_requests/creations_controller_spec.rb +++ b/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 diff --git a/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb b/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb index dbca279569a..42c279af117 100644 --- a/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb +++ b/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) - - page.within '.content' do - click_link 'New merge request' - end - expect(page).to have_content('Source branch') - expect(page).to have_content('Target branch') + visit project_new_merge_request_path(project) 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') - input.click - input.send_keys('orphaned-branch') + 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) - source_items = all('.dropdown-source-branch .dropdown-content li') - - expect(source_items.count).to eq(1) + expect(page).to have_css('.dropdown-content li', count: 1) + end first('.js-target-branch').click - find('.dropdown-target-branch .dropdown-content li', match: :first) - target_items = all('.dropdown-target-branch .dropdown-content li') + find('.js-target-branch-dropdown .dropdown-content li', match: :first) + 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 diff --git a/spec/support/shared_examples/features/creatable_merge_request_shared_examples.rb b/spec/support/shared_examples/features/creatable_merge_request_shared_examples.rb index 5b0b609f7f2..5a569d233bc 100644 --- a/spec/support/shared_examples/features/creatable_merge_request_shared_examples.rb +++ b/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 |