diff --git a/app/assets/javascripts/milestone.js b/app/assets/javascripts/milestone.js index 20d9fb825545893bd3688933c2d3aea075591a23..52e9b67c77da0f4753e52c278b0e87a5af1d483b 100644 --- a/app/assets/javascripts/milestone.js +++ b/app/assets/javascripts/milestone.js @@ -7,11 +7,6 @@ import { __ } from './locale'; export default class Milestone { constructor() { 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(); } @@ -23,12 +18,14 @@ export default class Milestone { this.loadTab($target); }); } - // eslint-disable-next-line class-methods-use-this + loadInitialTab() { - const $target = $(`.js-milestone-tabs a[href="${window.location.hash}"]`); + const $target = $(`.js-milestone-tabs a:not(.active)[href="${window.location.hash}"]`); if ($target.length) { $target.tab('show'); + } else { + this.loadTab($('.js-milestone-tabs a.active')); } } // eslint-disable-next-line class-methods-use-this diff --git a/app/controllers/concerns/milestone_actions.rb b/app/controllers/concerns/milestone_actions.rb index 29138e7b0143db932d7f257dd11a3e5374c18261..6470c75dfbd2f381d88a0e5ac2d777a41d265db7 100644 --- a/app/controllers/concerns/milestone_actions.rb +++ b/app/controllers/concerns/milestone_actions.rb @@ -3,13 +3,25 @@ module MilestoneActions extend ActiveSupport::Concern + def issues + respond_to do |format| + format.html { redirect_to milestone_redirect_path } + format.json do + render json: tabs_json("shared/milestones/_issues_tab", { + issues: @milestone.sorted_issues(current_user), # rubocop:disable Gitlab/ModuleWithInstanceVariables + show_project_name: Gitlab::Utils.to_boolean(params[:show_project_name]) + }) + end + end + end + 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.sorted_merge_requests(current_user), # rubocop:disable Gitlab/ModuleWithInstanceVariables - show_project_name: true + show_project_name: Gitlab::Utils.to_boolean(params[:show_project_name]) }) end end diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb index df3fb6b67c2a8c077f939b76746c21c195a2bef5..3f2894d378b66d7cbf4c09fa28dc4a916764fdb6 100644 --- a/app/controllers/groups/milestones_controller.rb +++ b/app/controllers/groups/milestones_controller.rb @@ -3,7 +3,7 @@ class Groups::MilestonesController < Groups::ApplicationController include MilestoneActions - before_action :milestone, only: [:edit, :show, :update, :merge_requests, :participants, :labels, :destroy] + before_action :milestone, only: [:edit, :show, :update, :issues, :merge_requests, :participants, :labels, :destroy] before_action :authorize_admin_milestones!, only: [:edit, :new, :create, :update, :destroy] before_action do push_frontend_feature_flag(:burnup_charts, @group) diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb index 16d63cc184f771b6f752b3ad3314406c32727cd4..8049a17068b1f9daba96c202f129f788438deb7b 100644 --- a/app/controllers/projects/milestones_controller.rb +++ b/app/controllers/projects/milestones_controller.rb @@ -5,7 +5,7 @@ class Projects::MilestonesController < Projects::ApplicationController include MilestoneActions before_action :check_issuables_available! - before_action :milestone, only: [:edit, :update, :destroy, :show, :merge_requests, :participants, :labels, :promote] + before_action :milestone, only: [:edit, :update, :destroy, :show, :issues, :merge_requests, :participants, :labels, :promote] before_action do push_frontend_feature_flag(:burnup_charts, @project) end @@ -14,7 +14,7 @@ class Projects::MilestonesController < Projects::ApplicationController before_action :authorize_read_milestone! # Allow admin milestone - before_action :authorize_admin_milestone!, except: [:index, :show, :merge_requests, :participants, :labels] + before_action :authorize_admin_milestone!, except: [:index, :show, :issues, :merge_requests, :participants, :labels] # Allow to promote milestone before_action :authorize_promote_milestone!, only: :promote diff --git a/app/helpers/timeboxes_helper.rb b/app/helpers/timeboxes_helper.rb index 34919f994ee6d396d2dde2286cb4703cb00cea5a..bbf8cf7dac382c7789ae101e21ca017a3ccb7d66 100644 --- a/app/helpers/timeboxes_helper.rb +++ b/app/helpers/timeboxes_helper.rb @@ -228,8 +228,8 @@ module TimeboxesHelper end alias_method :milestone_date_range, :timebox_date_range - def milestone_tab_path(milestone, tab) - url_for(action: tab, format: :json) + def milestone_tab_path(milestone, tab, params = {}) + url_for(params.merge(action: tab, format: :json)) end def update_milestone_path(milestone, params = {}) diff --git a/app/views/shared/milestones/_issuable.html.haml b/app/views/shared/milestones/_issuable.html.haml index f8bf3e7ad6ac03679f9288b56fa22ac14b5eb51d..a62ed009552bb3a65d69bb3ac458f10990c0a04a 100644 --- a/app/views/shared/milestones/_issuable.html.haml +++ b/app/views/shared/milestones/_issuable.html.haml @@ -10,8 +10,6 @@ %span - if show_project_name %strong #{project.name} · - - elsif show_full_project_name - %strong #{project.full_name} · - if issuable.is_a?(Issue) = confidential_icon(issuable) = link_to issuable.title, issuable_url_args, title: issuable.title diff --git a/app/views/shared/milestones/_issuables.html.haml b/app/views/shared/milestones/_issuables.html.haml index ee97f0172dae5189f9f2cbe1a7d76c1f164b9b58..9147e1c50e3e6441db0e96d3969afbcf18c3e967 100644 --- a/app/views/shared/milestones/_issuables.html.haml +++ b/app/views/shared/milestones/_issuables.html.haml @@ -15,4 +15,4 @@ = render partial: 'shared/milestones/issuable', collection: issuables, as: :issuable, - locals: { show_project_name: show_project_name, show_full_project_name: show_full_project_name } + locals: { show_project_name: show_project_name } diff --git a/app/views/shared/milestones/_issues_tab.html.haml b/app/views/shared/milestones/_issues_tab.html.haml index dc54eefbaa9519e635ce8edc5073d0787f473254..76ef636ec96504abaefae5b0d79804d9eae9a3d1 100644 --- a/app/views/shared/milestones/_issues_tab.html.haml +++ b/app/views/shared/milestones/_issues_tab.html.haml @@ -1,5 +1,4 @@ -- args = { show_project_name: local_assigns.fetch(:show_project_name, false), - show_full_project_name: local_assigns.fetch(:show_full_project_name, false) } +- args = { show_project_name: local_assigns.fetch(:show_project_name, false) } - if display_issues_count_warning?(@milestone) .flash-container diff --git a/app/views/shared/milestones/_merge_requests_tab.haml b/app/views/shared/milestones/_merge_requests_tab.haml index 0dbf2b27c8d812c07ea09ad7f87b53bd7c5144e0..a78440600ad1767b6df22dac3f36da2d7d3f6394 100644 --- a/app/views/shared/milestones/_merge_requests_tab.haml +++ b/app/views/shared/milestones/_merge_requests_tab.haml @@ -1,5 +1,4 @@ -- args = { show_project_name: local_assigns.fetch(:show_project_name, false), - show_full_project_name: local_assigns.fetch(:show_full_project_name, false) } +- args = { show_project_name: local_assigns.fetch(:show_project_name, false) } .row.gl-mt-3 .col-md-3 diff --git a/app/views/shared/milestones/_tabs.html.haml b/app/views/shared/milestones/_tabs.html.haml index 34f476241c658202fc6b8b393ea64b8dcb0c9333..33e634c3e7bb6502baab3da7307848bce531d3db 100644 --- a/app/views/shared/milestones/_tabs.html.haml +++ b/app/views/shared/milestones/_tabs.html.haml @@ -1,14 +1,16 @@ +- show_project_name = local_assigns.fetch(:show_project_name, false) + .scrolling-tabs-container.inner-page-scroll-tabs.is-smaller .fade-left= sprite_icon('chevron-lg-left', size: 12) .fade-right= sprite_icon('chevron-lg-right', size: 12) %ul.nav-links.scrolling-tabs.js-milestone-tabs.nav.nav-tabs %li.nav-item - = link_to '#tab-issues', class: 'nav-link active', data: { toggle: 'tab', show: '.tab-issues-buttons' } do + = link_to '#tab-issues', class: 'nav-link active', data: { toggle: 'tab', endpoint: milestone_tab_path(milestone, 'issues', show_project_name: show_project_name) } do = _('Issues') %span.badge.badge-pill= milestone.issues_visible_to_user(current_user).size - if milestone.merge_requests_enabled? %li.nav-item - = link_to '#tab-merge-requests', class: 'nav-link', data: { toggle: 'tab', endpoint: milestone_tab_path(milestone, 'merge_requests') } do + = link_to '#tab-merge-requests', class: 'nav-link', data: { toggle: 'tab', endpoint: milestone_tab_path(milestone, 'merge_requests', show_project_name: show_project_name) } do = _('Merge Requests') %span.badge.badge-pill= milestone.merge_requests_visible_to_user(current_user).size %li.nav-item @@ -20,20 +22,13 @@ = _('Labels') %span.badge.badge-pill= milestone.issue_labels_visible_by_user(current_user).count -- issues = milestone.sorted_issues(current_user) -- show_project_name = local_assigns.fetch(:show_project_name, false) -- show_full_project_name = local_assigns.fetch(:show_full_project_name, false) - .tab-content.milestone-content - .tab-pane.active#tab-issues{ data: { sort_endpoint: (sort_issues_project_milestone_path(@project, @milestone) if @project && current_user) } } - = render 'shared/milestones/issues_tab', issues: issues, show_project_name: show_project_name, show_full_project_name: show_full_project_name + .tab-pane.active#tab-issues + = render "shared/milestones/tab_loading" - if milestone.merge_requests_enabled? .tab-pane#tab-merge-requests - -# loaded async = render "shared/milestones/tab_loading" .tab-pane#tab-participants - -# loaded async = render "shared/milestones/tab_loading" .tab-pane#tab-labels - -# loaded async = render "shared/milestones/tab_loading" diff --git a/changelogs/unreleased/197227-load-milestone-issues-tab-async.yml b/changelogs/unreleased/197227-load-milestone-issues-tab-async.yml new file mode 100644 index 0000000000000000000000000000000000000000..b3f427ddbddbff54d62ff86d02de06c847818282 --- /dev/null +++ b/changelogs/unreleased/197227-load-milestone-issues-tab-async.yml @@ -0,0 +1,5 @@ +--- +title: Load issues tab in the milestone page asynchronously +merge_request: 42473 +author: +type: performance diff --git a/config/routes/group.rb b/config/routes/group.rb index e5bbfdf7548765c77f768e59315e91446ace8419..bf14afff318709179175ce269bdaf267e8b54e04 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -61,6 +61,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do resources :milestones, constraints: { id: %r{[^/]+} } do member do + get :issues get :merge_requests get :participants get :labels diff --git a/config/routes/project.rb b/config/routes/project.rb index 24b44646d9504cf2c04e7ddc3595f69f236cddf1..e0208e36a346ce1f3a096271cc554b8653d86983 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -161,8 +161,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do resources :milestones, constraints: { id: /\d+/ } do member do post :promote - put :sort_issues - put :sort_merge_requests + get :issues get :merge_requests get :participants get :labels diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb index 5c7b88a218a2c8c184aead9fc21ba9996d382174..2c85fe482e2bf0427f861de7ddb8b8182a2c9f9f 100644 --- a/spec/controllers/groups/milestones_controller_spec.rb +++ b/spec/controllers/groups/milestones_controller_spec.rb @@ -9,7 +9,6 @@ RSpec.describe Groups::MilestonesController do let(:user) { create(:user) } let(:title) { '肯定ä¸æ˜¯ä¸æ–‡çš„问题' } let(:milestone) { create(:milestone, project: project) } - let(:milestone_path) { group_milestone_path(group, milestone.safe_title, title: milestone.title) } let(:milestone_params) do { @@ -25,6 +24,12 @@ RSpec.describe Groups::MilestonesController do project.add_maintainer(user) end + it_behaves_like 'milestone tabs' do + let(:milestone) { create(:milestone, group: group) } + let(:milestone_path) { group_milestone_path(group, milestone.iid) } + let(:request_params) { { group_id: group, id: milestone.iid } } + end + describe '#index' do describe 'as HTML' do render_views diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index fa32d32f552090acbb4684feff5ab72ba6b9bf97..9e5d41b1075006787db17268227fa07b2d0c1f65 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -17,7 +17,9 @@ RSpec.describe Projects::MilestonesController do controller.instance_variable_set(:@project, project) end - it_behaves_like 'milestone tabs' + it_behaves_like 'milestone tabs' do + let(:request_params) { { namespace_id: project.namespace, project_id: project, id: milestone.iid } } + end describe "#show" do render_views diff --git a/spec/features/milestones/user_views_milestone_spec.rb b/spec/features/milestones/user_views_milestone_spec.rb index 420f8d49483f6e2f0185b8521e1b23ee9ae9c9de..11c6fa521d54f7617273295c1c0bc282b0de34fc 100644 --- a/spec/features/milestones/user_views_milestone_spec.rb +++ b/spec/features/milestones/user_views_milestone_spec.rb @@ -4,12 +4,16 @@ require 'spec_helper' RSpec.describe "User views milestone" do let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :repository, group: group) } let_it_be(:milestone) { create(:milestone, project: project) } let_it_be(:labels) { create_list(:label, 2, project: project) } - before do + before_all do project.add_developer(user) + end + + before do sign_in(user) end @@ -25,7 +29,7 @@ RSpec.describe "User views milestone" do expect { visit_milestone }.not_to exceed_query_limit(control) end - context 'limiting milestone issues' do + context 'issues list', :js do before_all do 2.times do create(:issue, milestone: milestone, project: project) @@ -34,6 +38,28 @@ RSpec.describe "User views milestone" do end end + context 'for a project milestone' do + it 'does not show the project name' do + visit(project_milestone_path(project, milestone)) + + wait_for_requests + + expect(page.find('#tab-issues')).not_to have_text(project.name) + end + end + + context 'for a group milestone' do + let(:group_milestone) { create(:milestone, group: group) } + + it 'shows the project name' do + create(:issue, project: project, milestone: group_milestone) + + visit(group_milestone_path(group, group_milestone)) + + expect(page.find('#tab-issues')).to have_text(project.name) + end + end + context 'when issues on milestone are over DISPLAY_ISSUES_LIMIT' do it "limits issues to display and shows warning" do stub_const('Milestoneish::DISPLAY_ISSUES_LIMIT', 3) @@ -56,6 +82,40 @@ RSpec.describe "User views milestone" do end end + context 'merge requests list', :js do + context 'for a project milestone' do + it 'does not show the project name' do + create(:merge_request, source_project: project, milestone: milestone) + + visit(project_milestone_path(project, milestone)) + + within('.js-milestone-tabs') do + click_link('Merge Requests') + end + + wait_for_requests + + expect(page.find('#tab-merge-requests')).not_to have_text(project.name) + end + end + + context 'for a group milestone' do + let(:group_milestone) { create(:milestone, group: group) } + + it 'shows the project name' do + create(:merge_request, source_project: project, milestone: group_milestone) + + visit(group_milestone_path(group, group_milestone)) + + within('.js-milestone-tabs') do + click_link('Merge Requests') + end + + expect(page.find('#tab-merge-requests')).to have_text(project.name) + end + end + end + private def visit_milestone diff --git a/spec/features/milestones/user_views_milestones_spec.rb b/spec/features/milestones/user_views_milestones_spec.rb index 3f6065771219a5e68a0448c323f42a77f212c016..f8b4b802a60625f7e21f19b7f9eaeb91a74b1cf0 100644 --- a/spec/features/milestones/user_views_milestones_spec.rb +++ b/spec/features/milestones/user_views_milestones_spec.rb @@ -21,7 +21,7 @@ RSpec.describe "User views milestones" do .and have_content("Merge Requests") end - context "with issues" do + context "with issues", :js do let_it_be(:issue) { create(:issue, project: project, milestone: milestone) } let_it_be(:closed_issue) { create(:closed_issue, project: project, milestone: milestone) } @@ -33,7 +33,6 @@ RSpec.describe "User views milestones" do .and have_selector("#tab-issues li.issuable-row", count: 2) .and have_content(issue.title) .and have_content(closed_issue.title) - .and have_selector("#tab-merge-requests") end end diff --git a/spec/support/shared_examples/controllers/milestone_tabs_shared_examples.rb b/spec/support/shared_examples/controllers/milestone_tabs_shared_examples.rb index 925c45005f07ffc6060000a0ffca78790694a820..2d35b1681ea754fa7ae51558565091f960a5150a 100644 --- a/spec/support/shared_examples/controllers/milestone_tabs_shared_examples.rb +++ b/spec/support/shared_examples/controllers/milestone_tabs_shared_examples.rb @@ -2,9 +2,28 @@ RSpec.shared_examples 'milestone tabs' do def go(path, extra_params = {}) - params = { namespace_id: project.namespace.to_param, project_id: project, id: milestone.iid } + get path, params: request_params.merge(extra_params) + end + + describe '#issues' do + context 'as html' do + before do + go(:issues, format: 'html') + end - get path, params: params.merge(extra_params) + it 'redirects to milestone#show' do + expect(response).to redirect_to(milestone_path) + end + end + + context 'as json' do + it 'renders the issues tab template to a string' do + go(:issues, format: 'json') + + expect(response).to render_template('shared/milestones/_issues_tab') + expect(json_response).to have_key('html') + end + end end describe '#merge_requests' do diff --git a/spec/views/shared/milestones/_issuables.html.haml_spec.rb b/spec/views/shared/milestones/_issuables.html.haml_spec.rb index 70ab6914580187b87a9ec793bd49dc004bb13b80..5eed2c96a4536b558766529d23ae532c114db511 100644 --- a/spec/views/shared/milestones/_issuables.html.haml_spec.rb +++ b/spec/views/shared/milestones/_issuables.html.haml_spec.rb @@ -6,8 +6,7 @@ RSpec.describe 'shared/milestones/_issuables.html.haml' do let(:issuables_size) { 100 } before do - allow(view).to receive_messages(title: nil, id: nil, show_project_name: nil, - show_full_project_name: nil, dom_class: '', + allow(view).to receive_messages(title: nil, id: nil, show_project_name: nil, dom_class: '', issuables: double(length: issuables_size).as_null_object) stub_template 'shared/milestones/_issuable.html.haml' => ''