From 29c3f5e7e685f18156efd00aa88e5d204694ceae Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 27 Feb 2018 11:23:50 +0000 Subject: Merge branch '43510-merge-requests-and-issues-don-t-show-for-all-subgroups' into 'master' Resolve "Merge requests and issues don't show for all subgroups" Closes #43510 See merge request gitlab-org/gitlab-ce!17312 --- app/controllers/groups/application_controller.rb | 4 - app/controllers/groups_controller.rb | 1 - app/helpers/groups_helper.rb | 18 ++- app/views/groups/issues.html.haml | 7 +- app/views/groups/merge_requests.html.haml | 2 +- app/views/layouts/nav/sidebar/_group.html.haml | 5 +- ...sts-and-issues-don-t-show-for-all-subgroups.yml | 6 + spec/features/groups/empty_states_spec.rb | 124 ++++++++++++++------- 8 files changed, 108 insertions(+), 59 deletions(-) create mode 100644 changelogs/unreleased/43510-merge-requests-and-issues-don-t-show-for-all-subgroups.yml diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb index 96ce686c989..43abed7a934 100644 --- a/app/controllers/groups/application_controller.rb +++ b/app/controllers/groups/application_controller.rb @@ -16,10 +16,6 @@ class Groups::ApplicationController < ApplicationController @projects ||= GroupProjectsFinder.new(group: group, current_user: current_user).execute end - def group_merge_requests - @group_merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id).execute - end - def authorize_admin_group! unless can?(current_user, :admin_group, group) return render_404 diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 7d129c5dece..4c8afebb028 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -14,7 +14,6 @@ class GroupsController < Groups::ApplicationController before_action :authorize_create_group!, only: [:new] before_action :group_projects, only: [:projects, :activity, :issues, :merge_requests] - before_action :group_merge_requests, only: [:merge_requests] before_action :event_filter, only: [:activity] before_action :user_actions, only: [:show, :subgroups] diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 23de3590b93..c4558c6150b 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -11,6 +11,20 @@ module GroupsHelper can?(current_user, :change_share_with_group_lock, group) end + def group_issues_count(state:) + IssuesFinder + .new(current_user, group_id: @group.id, state: state, non_archived: true, include_subgroups: true) + .execute + .count + end + + def group_merge_requests_count(state:) + MergeRequestsFinder + .new(current_user, group_id: @group.id, state: state, non_archived: true, include_subgroups: true) + .execute + .count + end + def group_icon(group, options = {}) img_path = group_icon_url(group, options) image_tag img_path, options @@ -69,10 +83,6 @@ module GroupsHelper end end - def group_issues(group) - IssuesFinder.new(current_user, group_id: group.id).execute - end - def remove_group_message(group) _("You are going to remove %{group_name}. Removed groups CANNOT be restored! Are you ABSOLUTELY sure?") % { group_name: group.name } diff --git a/app/views/groups/issues.html.haml b/app/views/groups/issues.html.haml index 00909982d59..70c830915d0 100644 --- a/app/views/groups/issues.html.haml +++ b/app/views/groups/issues.html.haml @@ -1,5 +1,4 @@ - page_title "Issues" -- group_issues_exists = group_issues(@group).exists? = content_for :meta_tags do = auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{@group.name} issues") @@ -7,7 +6,9 @@ = webpack_bundle_tag 'common_vue' = webpack_bundle_tag 'filtered_search' -- if group_issues_exists +- if group_issues_count(state: 'all').zero? + = render 'shared/empty_states/issues', project_select_button: true +- else .top-area = render 'shared/issuable/nav', type: :issues .nav-controls @@ -20,5 +21,3 @@ = render 'shared/issuable/search_bar', type: :issues = render 'shared/issues' -- else - = render 'shared/empty_states/issues', project_select_button: true diff --git a/app/views/groups/merge_requests.html.haml b/app/views/groups/merge_requests.html.haml index 694292aa7c1..4326c60bd80 100644 --- a/app/views/groups/merge_requests.html.haml +++ b/app/views/groups/merge_requests.html.haml @@ -4,7 +4,7 @@ = webpack_bundle_tag 'common_vue' = webpack_bundle_tag 'filtered_search' -- if @group_merge_requests.empty? +- if group_merge_requests_count(state: 'all').zero? = render 'shared/empty_states/merge_requests', project_select_button: true - else .top-area diff --git a/app/views/layouts/nav/sidebar/_group.html.haml b/app/views/layouts/nav/sidebar/_group.html.haml index 09a43a2cac5..56e6d108fd6 100644 --- a/app/views/layouts/nav/sidebar/_group.html.haml +++ b/app/views/layouts/nav/sidebar/_group.html.haml @@ -1,5 +1,6 @@ -- issues_count = IssuesFinder.new(current_user, group_id: @group.id, state: 'opened').execute.count -- merge_requests_count = MergeRequestsFinder.new(current_user, group_id: @group.id, state: 'opened', non_archived: true).execute.count +- issues_count = group_issues_count(state: 'opened') +- merge_requests_count = group_merge_requests_count(state: 'opened') +- issues_sub_menu_items = ['groups#issues', 'labels#index', 'milestones#index'] .nav-sidebar{ class: ("sidebar-collapsed-desktop" if collapsed_sidebar?) } .nav-sidebar-inner-scroll diff --git a/changelogs/unreleased/43510-merge-requests-and-issues-don-t-show-for-all-subgroups.yml b/changelogs/unreleased/43510-merge-requests-and-issues-don-t-show-for-all-subgroups.yml new file mode 100644 index 00000000000..e163c04f430 --- /dev/null +++ b/changelogs/unreleased/43510-merge-requests-and-issues-don-t-show-for-all-subgroups.yml @@ -0,0 +1,6 @@ +--- +title: Ensure group issues and merge requests pages show results from subgroups when + there are no results from the current group +merge_request: 17312 +author: +type: fixed diff --git a/spec/features/groups/empty_states_spec.rb b/spec/features/groups/empty_states_spec.rb index 243e8536168..04217fec06c 100644 --- a/spec/features/groups/empty_states_spec.rb +++ b/spec/features/groups/empty_states_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'Groups Merge Requests Empty States' do +feature 'Group empty states' do let(:group) { create(:group) } let(:user) { create(:group_member, :developer, user: create(:user), group: group ).user } @@ -8,62 +8,100 @@ feature 'Groups Merge Requests Empty States' do sign_in(user) end - context 'group has a project' do - let(:project) { create(:project, namespace: group) } + [:issue, :merge_request].each do |issuable| + issuable_name = issuable.to_s.humanize.downcase + project_relation = issuable == :issue ? :project : :source_project - before do - project.add_master(user) - end + context "for #{issuable_name}s" do + let(:path) { public_send(:"#{issuable}s_group_path", group) } - context 'the project has a merge request' do - before do - create(:merge_request, source_project: project) + context 'group has a project' do + let(:project) { create(:project, namespace: group) } - visit merge_requests_group_path(group) - end + before do + project.add_master(user) + end - it 'should not display an empty state' do - expect(page).not_to have_selector('.empty-state') - end - end + context "the project has #{issuable_name}s" do + before do + create(issuable, project_relation => project) - context 'the project has no merge requests', :js do - before do - visit merge_requests_group_path(group) - end + visit path + end - it 'should display an empty state' do - expect(page).to have_selector('.empty-state') - end + it 'does not display an empty state' do + expect(page).not_to have_selector('.empty-state') + end + end + + context "the project has no #{issuable_name}s", :js do + before do + visit path + end + + it 'displays an empty state' do + expect(page).to have_selector('.empty-state') + end + + it "shows a new #{issuable_name} button" do + within '.empty-state' do + expect(page).to have_content("create #{issuable_name}") + end + end + + it "the new #{issuable_name} button opens a project dropdown" do + within '.empty-state' do + find('.new-project-item-select-button').click + end - it 'should show a new merge request button' do - within '.empty-state' do - expect(page).to have_content('create merge request') + expect(page).to have_selector('.ajax-project-dropdown') + end end end - it 'the new merge request button opens a project dropdown' do - within '.empty-state' do - find('.new-project-item-select-button').click - end + context 'group without a project' do + context 'group has a subgroup', :nested_groups do + let(:subgroup) { create(:group, parent: group) } + let(:subgroup_project) { create(:project, namespace: subgroup) } - expect(page).to have_selector('.ajax-project-dropdown') - end - end - end + context "the project has #{issuable_name}s" do + before do + create(issuable, project_relation => subgroup_project) - context 'group without a project' do - before do - visit merge_requests_group_path(group) - end + visit path + end - it 'should display an empty state' do - expect(page).to have_selector('.empty-state') - end + it 'does not display an empty state' do + expect(page).not_to have_selector('.empty-state') + end + end - it 'should not show a new merge request button' do - within '.empty-state' do - expect(page).not_to have_link('create merge request') + context "the project has no #{issuable_name}s" do + before do + visit path + end + + it 'displays an empty state' do + expect(page).to have_selector('.empty-state') + end + end + end + + context 'group has no subgroups' do + before do + visit path + end + + it 'displays an empty state' do + expect(page).to have_selector('.empty-state') + end + + it "shows a new #{issuable_name} button" do + within '.empty-state' do + expect(page).not_to have_link("create #{issuable_name}") + end + end + end end end end -- cgit v1.2.1