diff --git a/app/assets/javascripts/milestone.js b/app/assets/javascripts/milestone.js index 38c673e890782b89cc376cd21a9d34c81389281d..cd2bd97616065002bd57be25f6cfe74e5d430071 100644 --- a/app/assets/javascripts/milestone.js +++ b/app/assets/javascripts/milestone.js @@ -21,7 +21,7 @@ Milestone.sortIssues = function(data) { var sort_issues_url; - sort_issues_url = location.href + "/sort_issues"; + sort_issues_url = location.pathname + "/sort_issues"; return $.ajax({ type: "PUT", url: sort_issues_url, @@ -38,7 +38,7 @@ Milestone.sortMergeRequests = function(data) { var sort_mr_url; - sort_mr_url = location.href + "/sort_merge_requests"; + sort_mr_url = location.pathname + "/sort_merge_requests"; return $.ajax({ type: "PUT", url: sort_mr_url, @@ -81,10 +81,14 @@ }; function Milestone() { - var oldMouseStart; this.bindIssuesSorting(); - this.bindMergeRequestSorting(); this.bindTabsSwitching(); + + // Load merge request tab if it is active + // merge request tab is active based on different conditions in the backend + this.loadTab($('.js-milestone-tabs .active a')); + + this.loadInitialTab(); } Milestone.prototype.bindIssuesSorting = function() { @@ -100,13 +104,11 @@ }; Milestone.prototype.bindTabsSwitching = function() { - return $('a[data-toggle="tab"]').on('show.bs.tab', function(e) { - var currentTabClass, previousTabClass; - currentTabClass = $(e.target).data('show'); - previousTabClass = $(e.relatedTarget).data('show'); - $(previousTabClass).hide(); - $(currentTabClass).removeClass('hidden'); - return $(currentTabClass).show(); + return $('a[data-toggle="tab"]').on('show.bs.tab', (e) => { + const $target = $(e.target); + + location.hash = $target.attr('href'); + this.loadTab($target); }); }; @@ -169,6 +171,35 @@ }); }; + Milestone.prototype.loadInitialTab = function() { + const $target = $(`.js-milestone-tabs a[href="${location.hash}"]`); + + if ($target.length) { + $target.tab('show'); + } + }; + + Milestone.prototype.loadTab = function($target) { + const endpoint = $target.data('endpoint'); + const tabElId = $target.attr('href'); + + if (endpoint && !$target.hasClass('is-loaded')) { + $.ajax({ + url: endpoint, + dataType: 'JSON', + }) + .fail(() => new Flash('Error loading milestone tab')) + .done((data) => { + $(tabElId).html(data.html); + $target.addClass('is-loaded'); + + if (tabElId === '#tab-merge-requests') { + this.bindMergeRequestSorting(); + } + }); + } + }; + return Milestone; })(); }).call(window); diff --git a/app/controllers/concerns/milestone_actions.rb b/app/controllers/concerns/milestone_actions.rb new file mode 100644 index 0000000000000000000000000000000000000000..3e2a0fe4f8bd76f9f3434e34a2361976633e39c1 --- /dev/null +++ b/app/controllers/concerns/milestone_actions.rb @@ -0,0 +1,53 @@ +module MilestoneActions + extend ActiveSupport::Concern + + def merge_requests + respond_to do |format| + format.html { redirect_to milestone_redirect_path } + format.json do + render json: tabs_json("shared/milestones/_merge_requests_tab", { + merge_requests: @milestone.merge_requests, + show_project_name: true + }) + end + end + end + + def participants + respond_to do |format| + format.html { redirect_to milestone_redirect_path } + format.json do + render json: tabs_json("shared/milestones/_participants_tab", { + users: @milestone.participants + }) + end + end + end + + def labels + respond_to do |format| + format.html { redirect_to milestone_redirect_path } + format.json do + render json: tabs_json("shared/milestones/_labels_tab", { + labels: @milestone.labels + }) + end + end + end + + private + + def tabs_json(partial, data = {}) + { + html: view_to_html_string(partial, data) + } + end + + def milestone_redirect_path + if @project + namespace_project_milestone_path(@project.namespace, @project, @milestone) + else + group_milestone_path(@group, @milestone.safe_title, title: @milestone.title) + end + end +end diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb index 4310259620151e1f61f66f74e9a4af34b9dec675..e52fa7660447321b35156941802c5720d288519e 100644 --- a/app/controllers/groups/milestones_controller.rb +++ b/app/controllers/groups/milestones_controller.rb @@ -1,6 +1,8 @@ class Groups::MilestonesController < Groups::ApplicationController + include MilestoneActions + before_action :group_projects - before_action :milestone, only: [:show, :update] + before_action :milestone, only: [:show, :update, :merge_requests, :participants, :labels] before_action :authorize_admin_milestones!, only: [:new, :create, :update] def index diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb index 6d376dbb89767ef155372bcc1bb2be80c6869712..0c9a85946f8fd642821d874b242391323451f817 100644 --- a/app/controllers/projects/milestones_controller.rb +++ b/app/controllers/projects/milestones_controller.rb @@ -1,12 +1,14 @@ class Projects::MilestonesController < Projects::ApplicationController + include MilestoneActions + before_action :module_enabled - before_action :milestone, only: [:edit, :update, :destroy, :show, :sort_issues, :sort_merge_requests] + before_action :milestone, only: [:edit, :update, :destroy, :show, :sort_issues, :sort_merge_requests, :merge_requests, :participants, :labels] # Allow read any milestone before_action :authorize_read_milestone! # Allow admin milestone - before_action :authorize_admin_milestone!, except: [:index, :show] + before_action :authorize_admin_milestone!, except: [:index, :show, :merge_requests, :participants, :labels] respond_to :html diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb index 93a06cabd02092ce5b2d592c9ba9dd0fb8bcd92f..dea284c5a4aff59d85eecaaeb54e4f7e339b63a4 100644 --- a/app/helpers/milestones_helper.rb +++ b/app/helpers/milestones_helper.rb @@ -132,4 +132,28 @@ def data_warning_for(burndown) content_tag(:div, message.html_safe, id: "data-warning", class: "settings-message prepend-top-20") end end + + def milestone_merge_request_tab_path(milestone) + if @project + merge_requests_namespace_project_milestone_path(@project.namespace, @project, milestone, format: :json) + elsif @group + merge_requests_group_milestone_path(@group, milestone.safe_title, title: milestone.title, format: :json) + end + end + + def milestone_participants_tab_path(milestone) + if @project + participants_namespace_project_milestone_path(@project.namespace, @project, milestone, format: :json) + elsif @group + participants_group_milestone_path(@group, milestone.safe_title, title: milestone.title, format: :json) + end + end + + def milestone_labels_tab_path(milestone) + if @project + labels_namespace_project_milestone_path(@project.namespace, @project, milestone, format: :json) + elsif @group + labels_group_milestone_path(@group, milestone.safe_title, title: milestone.title, format: :json) + end + end end diff --git a/app/views/shared/milestones/_tab_loading.html.haml b/app/views/shared/milestones/_tab_loading.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..68458c2d0aad24eb8d2f9c153b4e4116165b2607 --- /dev/null +++ b/app/views/shared/milestones/_tab_loading.html.haml @@ -0,0 +1,2 @@ +.text-center.prepend-top-default + = icon('spin spinner 2x', 'aria-hidden': 'true', 'aria-label': 'Loading tab content') diff --git a/app/views/shared/milestones/_tabs.html.haml b/app/views/shared/milestones/_tabs.html.haml index 9a4502873efe81697caf13992428749a0dff1818..6717d59f033f06013165c612e2d04ee0959c8888 100644 --- a/app/views/shared/milestones/_tabs.html.haml +++ b/app/views/shared/milestones/_tabs.html.haml @@ -1,27 +1,27 @@ .scrolling-tabs-container.inner-page-scroll-tabs.is-smaller .fade-left= icon('angle-left') .fade-right= icon('angle-right') - %ul.nav-links.scrolling-tabs + %ul.nav-links.scrolling-tabs.js-milestone-tabs - if milestone.is_a?(GlobalMilestone) || can?(current_user, :read_issue, @project) %li.active = link_to '#tab-issues', 'data-toggle' => 'tab', 'data-show' => '.tab-issues-buttons' do Issues %span.badge= milestone.issues_visible_to_user(current_user).size %li - = link_to '#tab-merge-requests', 'data-toggle' => 'tab', 'data-show' => '.tab-merge-requests-buttons' do + = link_to '#tab-merge-requests', 'data-toggle' => 'tab', 'data-endpoint': milestone_merge_request_tab_path(milestone) do Merge Requests %span.badge= milestone.merge_requests.size - else %li.active - = link_to '#tab-merge-requests', 'data-toggle' => 'tab', 'data-show' => '.tab-merge-requests-buttons' do + = link_to '#tab-merge-requests', 'data-toggle' => 'tab', 'data-endpoint': milestone_merge_request_tab_path(milestone) do Merge Requests %span.badge= milestone.merge_requests.size %li - = link_to '#tab-participants', 'data-toggle' => 'tab' do + = link_to '#tab-participants', 'data-toggle' => 'tab', 'data-endpoint': milestone_participants_tab_path(milestone) do Participants %span.badge= milestone.participants.count %li - = link_to '#tab-labels', 'data-toggle' => 'tab' do + = link_to '#tab-labels', 'data-toggle' => 'tab', 'data-endpoint': milestone_labels_tab_path(milestone) do Labels %span.badge= milestone.labels.count @@ -33,11 +33,15 @@ .tab-pane.active#tab-issues = render 'shared/milestones/issues_tab', issues: milestone.issues_visible_to_user(current_user).include_associations, show_project_name: show_project_name, show_full_project_name: show_full_project_name .tab-pane#tab-merge-requests - = render 'shared/milestones/merge_requests_tab', merge_requests: milestone.merge_requests, show_project_name: show_project_name, show_full_project_name: show_full_project_name + -# loaded async + = render "shared/milestones/tab_loading" - else .tab-pane.active#tab-merge-requests - = render 'shared/milestones/merge_requests_tab', merge_requests: milestone.merge_requests, show_project_name: show_project_name, show_full_project_name: show_full_project_name + -# loaded async + = render "shared/milestones/tab_loading" .tab-pane#tab-participants - = render 'shared/milestones/participants_tab', users: milestone.participants + -# loaded async + = render "shared/milestones/tab_loading" .tab-pane#tab-labels - = render 'shared/milestones/labels_tab', labels: milestone.labels + -# loaded async + = render "shared/milestones/tab_loading" diff --git a/changelogs/unreleased/async-milestone-tabs.yml b/changelogs/unreleased/async-milestone-tabs.yml new file mode 100644 index 0000000000000000000000000000000000000000..c199a95610c883e0ad54e66b26337152b603aff1 --- /dev/null +++ b/changelogs/unreleased/async-milestone-tabs.yml @@ -0,0 +1,4 @@ +--- +title: Load milestone tabs asynchronously to increase initial load performance +merge_request: +author: diff --git a/config/routes/group.rb b/config/routes/group.rb index c961ca9d1967de1ee996c14739199485bbab7886..d2d3991af946231d8ba377e892a1b1a82c71c7ca 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -25,7 +25,13 @@ end resource :avatar, only: [:destroy] - resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create] + resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create] do + member do + get :merge_requests + get :participants + get :labels + end + end ## EE-specific resource :notification_setting, only: [:update] diff --git a/config/routes/project.rb b/config/routes/project.rb index 6ed483d866bfaee9de6a542d35072be9921cf9a8..6ff12bfff5e6cd07cf2a48481faac95ecf52582e 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -240,6 +240,9 @@ member do put :sort_issues put :sort_merge_requests + get :merge_requests + get :participants + get :labels end end diff --git a/features/group/milestones.feature b/features/group/milestones.feature index d6c05df9840ed4f68ccb3488fe0ef9c5dcc98a7e..1c1539b3e126247d3c5fc44fcaf21473b52e3a7b 100644 --- a/features/group/milestones.feature +++ b/features/group/milestones.feature @@ -38,6 +38,7 @@ Feature: Group Milestones And I should see the "feature" label And I should see the project name in the Issue row + @javascript Scenario: I should see the Labels tab Given Group has projects with milestones When I visit group "Owned" page diff --git a/features/project/milestone.feature b/features/project/milestone.feature index 713f0f3b97965e8651ec63fefbc61a18998a1806..5e7b211fa27644c8941fc3d3fbbb23f0321170f0 100644 --- a/features/project/milestone.feature +++ b/features/project/milestone.feature @@ -7,14 +7,6 @@ Feature: Project Milestone And milestone has issue "Bugfix1" with labels: "bug", "feature" And milestone has issue "Bugfix2" with labels: "bug", "enhancement" - - @javascript - Scenario: Listing issues from issues tab - Given I visit project "Shop" milestones page - And I click link "v2.2" - Then I should see the labels "bug", "enhancement" and "feature" - And I should see the "bug" label listed only once - @javascript Scenario: Listing labels from labels tab Given I visit project "Shop" milestones page diff --git a/features/steps/group/milestones.rb b/features/steps/group/milestones.rb index f8f5e3f23822fb321a050826163cc63bf92f16a7..49fcd6f1201430668dde67e12f6b0f7c58281398 100644 --- a/features/steps/group/milestones.rb +++ b/features/steps/group/milestones.rb @@ -1,4 +1,5 @@ class Spinach::Features::GroupMilestones < Spinach::FeatureSteps + include WaitForAjax include SharedAuthentication include SharedPaths include SharedGroup @@ -90,6 +91,8 @@ class Spinach::Features::GroupMilestones < Spinach::FeatureSteps end step 'I should see the list of labels' do + wait_for_ajax + page.within('#tab-labels') do expect(page).to have_content 'bug' expect(page).to have_content 'feature' diff --git a/features/steps/project/project_milestone.rb b/features/steps/project/project_milestone.rb index 1864b3a2b52df776e0b96f994c3f8029fb376cb8..dc1190b7eea913648a37e9cf64afb65dfdf548a0 100644 --- a/features/steps/project/project_milestone.rb +++ b/features/steps/project/project_milestone.rb @@ -2,6 +2,7 @@ class Spinach::Features::ProjectMilestone < Spinach::FeatureSteps include SharedAuthentication include SharedProject include SharedPaths + include WaitForAjax step 'milestone has issue "Bugfix1" with labels: "bug", "feature"' do project = Project.find_by(name: "Shop") @@ -34,6 +35,8 @@ class Spinach::Features::ProjectMilestone < Spinach::FeatureSteps end step 'I should see the labels "bug", "enhancement" and "feature"' do + wait_for_ajax + page.within('#tab-issues') do expect(page).to have_content 'bug' expect(page).to have_content 'enhancement' diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb index 6e4b5f78e334773ace89d943e7a5449cdb7c3df3..7cf2996ffd0932dadb868b15b2fcf6adf0f5e7af 100644 --- a/spec/controllers/groups/milestones_controller_spec.rb +++ b/spec/controllers/groups/milestones_controller_spec.rb @@ -6,6 +6,16 @@ let(:project2) { create(:empty_project, group: group) } let(:user) { create(:user) } let(:title) { '肯定不是中文的问题' } + let(:milestone) do + project_milestone = create(:milestone, project: project) + + GroupMilestone.build( + group, + [project], + project_milestone.title + ) + end + let(:milestone_path) { group_milestone_path(group, milestone.safe_title, title: milestone.title) } before do sign_in(user) @@ -14,6 +24,8 @@ controller.instance_variable_set(:@group, group) end + it_behaves_like 'milestone tabs' + describe "#create" do it "creates group milestone with Chinese title" do post :create, diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index 14207bf6b7af6690e4bee1d6dc320f817b5d90f6..ba6f5dea71da8d137093e6a3ba90c692ebf5df26 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -6,6 +6,7 @@ let(:milestone) { create(:milestone, project: project) } let(:issue) { create(:issue, project: project, milestone: milestone) } let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) } + let(:milestone_path) { namespace_project_milestone_path } before do sign_in(user) @@ -13,6 +14,22 @@ controller.instance_variable_set(:@project, project) end + it_behaves_like 'milestone tabs' + + describe "#show" do + render_views + + def view_milestone + get :show, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid + end + + it 'shows milestone page' do + view_milestone + + expect(response).to have_http_status(200) + end + end + describe "#destroy" do it "removes milestone" do expect(issue.milestone_id).to eq(milestone.id) diff --git a/spec/features/milestones/milestones_spec.rb b/spec/features/milestones/milestones_spec.rb index 2fa3e72ab08afcbf8c4aa970cba1118216964dcb..7062a09f2f6a174b14284ed316d542b3654a4d00 100644 --- a/spec/features/milestones/milestones_spec.rb +++ b/spec/features/milestones/milestones_spec.rb @@ -87,6 +87,9 @@ def create_and_drag_merge_request(params = {}) visit namespace_project_milestone_path(project.namespace, project, milestone) page.find("a[href='#tab-merge-requests']").click + + wait_for_ajax + scroll_into_view('.milestone-content') drag_to(selector: '.merge_requests-sortable-list', list_to_index: 1) diff --git a/spec/support/milestone_tabs_examples.rb b/spec/support/milestone_tabs_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..c48ecb1672e0cb8c9aa243bf70541bf2a981ef46 --- /dev/null +++ b/spec/support/milestone_tabs_examples.rb @@ -0,0 +1,67 @@ +shared_examples 'milestone tabs' do + def go(path, extra_params = {}) + params = if milestone.is_a?(GlobalMilestone) + { group_id: group.id, id: milestone.safe_title, title: milestone.title } + else + { namespace_id: project.namespace.to_param, project_id: project, id: milestone.iid } + end + + get path, params.merge(extra_params) + end + describe '#merge_requests' do + context 'as html' do + before { go(:merge_requests, format: 'html') } + + it 'redirects to milestone#show' do + expect(response).to redirect_to(milestone_path) + end + end + + context 'as json' do + before { go(:merge_requests, format: 'json') } + + it 'renders the merge requests tab template to a string' do + expect(response).to render_template('shared/milestones/_merge_requests_tab') + expect(json_response).to have_key('html') + end + end + end + + describe '#participants' do + context 'as html' do + before { go(:participants, format: 'html') } + + it 'redirects to milestone#show' do + expect(response).to redirect_to(milestone_path) + end + end + + context 'as json' do + before { go(:participants, format: 'json') } + + it 'renders the participants tab template to a string' do + expect(response).to render_template('shared/milestones/_participants_tab') + expect(json_response).to have_key('html') + end + end + end + + describe '#labels' do + context 'as html' do + before { go(:labels, format: 'html') } + + it 'redirects to milestone#show' do + expect(response).to redirect_to(milestone_path) + end + end + + context 'as json' do + before { go(:labels, format: 'json') } + + it 'renders the labels tab template to a string' do + expect(response).to render_template('shared/milestones/_labels_tab') + expect(json_response).to have_key('html') + end + end + end +end