Commit 498d3236 authored by Sean McGivern's avatar Sean McGivern

Merge branch '30106-group-issues' into 'master'

View all issues of all subgroups on the group issue page

Closes #30106 and #39388

See merge request gitlab-org/gitlab-ce!16706
parents 76b9f1a4 7f0ebeff
...@@ -94,6 +94,7 @@ module IssuableCollections ...@@ -94,6 +94,7 @@ module IssuableCollections
@filter_params[:project_id] = @project.id @filter_params[:project_id] = @project.id
elsif @group elsif @group
@filter_params[:group_id] = @group.id @filter_params[:group_id] = @group.id
@filter_params[:include_subgroups] = true
else else
# TODO: this filter ignore issues/mr created in public or # TODO: this filter ignore issues/mr created in public or
# internal repos where you are not a member. Enable this filter # internal repos where you are not a member. Enable this filter
......
...@@ -87,9 +87,18 @@ class GroupProjectsFinder < ProjectsFinder ...@@ -87,9 +87,18 @@ class GroupProjectsFinder < ProjectsFinder
options.fetch(:only_shared, false) options.fetch(:only_shared, false)
end end
# subgroups are supported only for owned projects not for shared
def include_subgroups?
options.fetch(:include_subgroups, false)
end
def owned_projects def owned_projects
if include_subgroups?
Project.where(namespace_id: group.self_and_descendants.select(:id))
else
group.projects group.projects
end end
end
def shared_projects def shared_projects
group.shared_projects group.shared_projects
......
...@@ -43,6 +43,7 @@ class IssuableFinder ...@@ -43,6 +43,7 @@ class IssuableFinder
search search
sort sort
state state
include_subgroups
].freeze ].freeze
ARRAY_PARAMS = { label_name: [], iids: [], assignee_username: [] }.freeze ARRAY_PARAMS = { label_name: [], iids: [], assignee_username: [] }.freeze
...@@ -148,7 +149,8 @@ class IssuableFinder ...@@ -148,7 +149,8 @@ class IssuableFinder
if current_user && params[:authorized_only].presence && !current_user_related? if current_user && params[:authorized_only].presence && !current_user_related?
current_user.authorized_projects current_user.authorized_projects
elsif group elsif group
GroupProjectsFinder.new(group: group, current_user: current_user).execute finder_options = { include_subgroups: params[:include_subgroups], only_owned: true }
GroupProjectsFinder.new(group: group, current_user: current_user, options: finder_options).execute
else else
ProjectsFinder.new(current_user: current_user, project_ids_relation: item_project_ids(items)).execute ProjectsFinder.new(current_user: current_user, project_ids_relation: item_project_ids(items)).execute
end end
......
---
title: Include subgroup issues and merge requests on the group page
merge_request:
author:
type: changed
...@@ -3,6 +3,7 @@ require 'spec_helper' ...@@ -3,6 +3,7 @@ require 'spec_helper'
feature 'Group issues page' do feature 'Group issues page' do
include FilteredSearchHelpers include FilteredSearchHelpers
context 'with shared examples' do
let(:path) { issues_group_path(group) } let(:path) { issues_group_path(group) }
let(:issuable) { create(:issue, project: project, title: "this is my created issuable")} let(:issuable) { create(:issue, project: project, title: "this is my created issuable")}
...@@ -39,4 +40,24 @@ feature 'Group issues page' do ...@@ -39,4 +40,24 @@ feature 'Group issues page' do
expect(find('#js-dropdown-assignee .filter-dropdown')).not_to have_content(user2.name) expect(find('#js-dropdown-assignee .filter-dropdown')).not_to have_content(user2.name)
end end
end end
end
context 'issues list', :nested_groups do
let(:group) { create(:group)}
let(:subgroup) { create(:group, parent: group) }
let(:project) { create(:project, :public, group: group)}
let(:subgroup_project) { create(:project, :public, group: subgroup)}
let!(:issue) { create(:issue, project: project, title: 'root group issue') }
let!(:subgroup_issue) { create(:issue, project: subgroup_project, title: 'subgroup issue') }
it 'returns all group and subgroup issues' do
visit issues_group_path(group)
page.within('.issuable-list') do
expect(page).to have_selector('li.issue', count: 2)
expect(page).to have_content('root group issue')
expect(page).to have_content('subgroup issue')
end
end
end
end end
...@@ -2,6 +2,7 @@ require 'spec_helper' ...@@ -2,6 +2,7 @@ require 'spec_helper'
describe GroupProjectsFinder do describe GroupProjectsFinder do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:subgroup) { create(:group, parent: group) }
let(:current_user) { create(:user) } let(:current_user) { create(:user) }
let(:options) { {} } let(:options) { {} }
...@@ -12,6 +13,8 @@ describe GroupProjectsFinder do ...@@ -12,6 +13,8 @@ describe GroupProjectsFinder do
let!(:shared_project_1) { create(:project, :public, path: '3') } let!(:shared_project_1) { create(:project, :public, path: '3') }
let!(:shared_project_2) { create(:project, :private, path: '4') } let!(:shared_project_2) { create(:project, :private, path: '4') }
let!(:shared_project_3) { create(:project, :internal, path: '5') } let!(:shared_project_3) { create(:project, :internal, path: '5') }
let!(:subgroup_project) { create(:project, :public, path: '6', group: subgroup) }
let!(:subgroup_private_project) { create(:project, :private, path: '7', group: subgroup) }
before do before do
shared_project_1.project_group_links.create(group_access: Gitlab::Access::MASTER, group: group) shared_project_1.project_group_links.create(group_access: Gitlab::Access::MASTER, group: group)
...@@ -35,13 +38,33 @@ describe GroupProjectsFinder do ...@@ -35,13 +38,33 @@ describe GroupProjectsFinder do
context "only owned" do context "only owned" do
let(:options) { { only_owned: true } } let(:options) { { only_owned: true } }
context 'with subgroups projects', :nested_groups do
before do
options[:include_subgroups] = true
end
it { is_expected.to match_array([private_project, public_project, subgroup_project, subgroup_private_project]) }
end
context 'without subgroups projects' do
it { is_expected.to match_array([private_project, public_project]) } it { is_expected.to match_array([private_project, public_project]) }
end end
end
context "all" do context "all" do
context 'with subgroups projects', :nested_groups do
before do
options[:include_subgroups] = true
end
it { is_expected.to match_array([shared_project_3, shared_project_2, shared_project_1, private_project, public_project, subgroup_project, subgroup_private_project]) }
end
context 'without subgroups projects' do
it { is_expected.to match_array([shared_project_3, shared_project_2, shared_project_1, private_project, public_project]) } it { is_expected.to match_array([shared_project_3, shared_project_2, shared_project_1, private_project, public_project]) }
end end
end end
end
describe 'without group member current_user' do describe 'without group member current_user' do
before do before do
...@@ -71,24 +94,55 @@ describe GroupProjectsFinder do ...@@ -71,24 +94,55 @@ describe GroupProjectsFinder do
context "without external user" do context "without external user" do
before do before do
private_project.add_master(current_user) private_project.add_master(current_user)
subgroup_private_project.add_master(current_user)
end end
context 'with subgroups projects', :nested_groups do
before do
options[:include_subgroups] = true
end
it { is_expected.to match_array([private_project, public_project, subgroup_project, subgroup_private_project]) }
end
context 'without subgroups projects' do
it { is_expected.to match_array([private_project, public_project]) } it { is_expected.to match_array([private_project, public_project]) }
end end
end
context "with external user" do context "with external user" do
before do before do
current_user.update_attributes(external: true) current_user.update_attributes(external: true)
end end
context 'with subgroups projects', :nested_groups do
before do
options[:include_subgroups] = true
end
it { is_expected.to match_array([public_project, subgroup_project]) }
end
context 'without subgroups projects' do
it { is_expected.to eq([public_project]) } it { is_expected.to eq([public_project]) }
end end
end end
end
context "all" do context "all" do
context 'with subgroups projects', :nested_groups do
before do
options[:include_subgroups] = true
end
it { is_expected.to match_array([shared_project_3, shared_project_2, shared_project_1, public_project, subgroup_project]) }
end
context 'without subgroups projects' do
it { is_expected.to match_array([shared_project_3, shared_project_2, shared_project_1, public_project]) } it { is_expected.to match_array([shared_project_3, shared_project_2, shared_project_1, public_project]) }
end end
end end
end
describe "no user" do describe "no user" do
context "only shared" do context "only shared" do
...@@ -100,7 +154,17 @@ describe GroupProjectsFinder do ...@@ -100,7 +154,17 @@ describe GroupProjectsFinder do
context "only owned" do context "only owned" do
let(:options) { { only_owned: true } } let(:options) { { only_owned: true } }
context 'with subgroups projects', :nested_groups do
before do
options[:include_subgroups] = true
end
it { is_expected.to match_array([public_project, subgroup_project]) }
end
context 'without subgroups projects' do
it { is_expected.to eq([public_project]) } it { is_expected.to eq([public_project]) }
end end
end end
end
end end
...@@ -3,13 +3,17 @@ require 'spec_helper' ...@@ -3,13 +3,17 @@ require 'spec_helper'
describe IssuesFinder do describe IssuesFinder do
set(:user) { create(:user) } set(:user) { create(:user) }
set(:user2) { create(:user) } set(:user2) { create(:user) }
set(:project1) { create(:project) } set(:group) { create(:group) }
set(:subgroup) { create(:group, parent: group) }
set(:project1) { create(:project, group: group) }
set(:project2) { create(:project) } set(:project2) { create(:project) }
set(:project3) { create(:project, group: subgroup) }
set(:milestone) { create(:milestone, project: project1) } set(:milestone) { create(:milestone, project: project1) }
set(:label) { create(:label, project: project2) } set(:label) { create(:label, project: project2) }
set(:issue1) { create(:issue, author: user, assignees: [user], project: project1, milestone: milestone, title: 'gitlab', created_at: 1.week.ago) } set(:issue1) { create(:issue, author: user, assignees: [user], project: project1, milestone: milestone, title: 'gitlab', created_at: 1.week.ago) }
set(:issue2) { create(:issue, author: user, assignees: [user], project: project2, description: 'gitlab') } set(:issue2) { create(:issue, author: user, assignees: [user], project: project2, description: 'gitlab') }
set(:issue3) { create(:issue, author: user2, assignees: [user2], project: project2, title: 'tanuki', description: 'tanuki', created_at: 1.week.from_now) } set(:issue3) { create(:issue, author: user2, assignees: [user2], project: project2, title: 'tanuki', description: 'tanuki', created_at: 1.week.from_now) }
set(:issue4) { create(:issue, project: project3) }
set(:award_emoji1) { create(:award_emoji, name: 'thumbsup', user: user, awardable: issue1) } set(:award_emoji1) { create(:award_emoji, name: 'thumbsup', user: user, awardable: issue1) }
set(:award_emoji2) { create(:award_emoji, name: 'thumbsup', user: user2, awardable: issue2) } set(:award_emoji2) { create(:award_emoji, name: 'thumbsup', user: user2, awardable: issue2) }
set(:award_emoji3) { create(:award_emoji, name: 'thumbsdown', user: user, awardable: issue3) } set(:award_emoji3) { create(:award_emoji, name: 'thumbsdown', user: user, awardable: issue3) }
...@@ -25,10 +29,12 @@ describe IssuesFinder do ...@@ -25,10 +29,12 @@ describe IssuesFinder do
project1.add_master(user) project1.add_master(user)
project2.add_developer(user) project2.add_developer(user)
project2.add_developer(user2) project2.add_developer(user2)
project3.add_developer(user)
issue1 issue1
issue2 issue2
issue3 issue3
issue4
award_emoji1 award_emoji1
award_emoji2 award_emoji2
...@@ -39,7 +45,7 @@ describe IssuesFinder do ...@@ -39,7 +45,7 @@ describe IssuesFinder do
let(:scope) { 'all' } let(:scope) { 'all' }
it 'returns all issues' do it 'returns all issues' do
expect(issues).to contain_exactly(issue1, issue2, issue3) expect(issues).to contain_exactly(issue1, issue2, issue3, issue4)
end end
context 'filtering by assignee ID' do context 'filtering by assignee ID' do
...@@ -50,6 +56,26 @@ describe IssuesFinder do ...@@ -50,6 +56,26 @@ describe IssuesFinder do
end end
end end
context 'filtering by group_id' do
let(:params) { { group_id: group.id } }
context 'when include_subgroup param not set' do
it 'returns all group issues' do
expect(issues).to contain_exactly(issue1)
end
end
context 'when include_subgroup param is true', :nested_groups do
before do
params[:include_subgroups] = true
end
it 'returns all group and subgroup issues' do
expect(issues).to contain_exactly(issue1, issue4)
end
end
end
context 'filtering by author ID' do context 'filtering by author ID' do
let(:params) { { author_id: user2.id } } let(:params) { { author_id: user2.id } }
...@@ -87,7 +113,7 @@ describe IssuesFinder do ...@@ -87,7 +113,7 @@ describe IssuesFinder do
let(:params) { { milestone_title: Milestone::None.title } } let(:params) { { milestone_title: Milestone::None.title } }
it 'returns issues with no milestone' do it 'returns issues with no milestone' do
expect(issues).to contain_exactly(issue2, issue3) expect(issues).to contain_exactly(issue2, issue3, issue4)
end end
end end
...@@ -185,7 +211,7 @@ describe IssuesFinder do ...@@ -185,7 +211,7 @@ describe IssuesFinder do
let(:params) { { label_name: Label::None.title } } let(:params) { { label_name: Label::None.title } }
it 'returns issues with no labels' do it 'returns issues with no labels' do
expect(issues).to contain_exactly(issue1, issue3) expect(issues).to contain_exactly(issue1, issue3, issue4)
end end
end end
...@@ -210,7 +236,7 @@ describe IssuesFinder do ...@@ -210,7 +236,7 @@ describe IssuesFinder do
let(:params) { { state: 'opened' } } let(:params) { { state: 'opened' } }
it 'returns only opened issues' do it 'returns only opened issues' do
expect(issues).to contain_exactly(issue1, issue2, issue3) expect(issues).to contain_exactly(issue1, issue2, issue3, issue4)
end end
end end
...@@ -226,7 +252,7 @@ describe IssuesFinder do ...@@ -226,7 +252,7 @@ describe IssuesFinder do
let(:params) { { state: 'all' } } let(:params) { { state: 'all' } }
it 'returns all issues' do it 'returns all issues' do
expect(issues).to contain_exactly(issue1, issue2, issue3, closed_issue) expect(issues).to contain_exactly(issue1, issue2, issue3, closed_issue, issue4)
end end
end end
...@@ -234,7 +260,7 @@ describe IssuesFinder do ...@@ -234,7 +260,7 @@ describe IssuesFinder do
let(:params) { { state: 'invalid_state' } } let(:params) { { state: 'invalid_state' } }
it 'returns all issues' do it 'returns all issues' do
expect(issues).to contain_exactly(issue1, issue2, issue3, closed_issue) expect(issues).to contain_exactly(issue1, issue2, issue3, closed_issue, issue4)
end end
end end
end end
...@@ -338,7 +364,7 @@ describe IssuesFinder do ...@@ -338,7 +364,7 @@ describe IssuesFinder do
end end
it "doesn't return issues if feature disabled" do it "doesn't return issues if feature disabled" do
[project1, project2].each do |project| [project1, project2, project3].each do |project|
project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED) project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED)
end end
...@@ -351,7 +377,7 @@ describe IssuesFinder do ...@@ -351,7 +377,7 @@ describe IssuesFinder do
it 'returns the number of rows for the default state' do it 'returns the number of rows for the default state' do
finder = described_class.new(user) finder = described_class.new(user)
expect(finder.row_count).to eq(3) expect(finder.row_count).to eq(4)
end end
it 'returns the number of rows for a given state' do it 'returns the number of rows for a given state' do
......
...@@ -6,31 +6,36 @@ describe MergeRequestsFinder do ...@@ -6,31 +6,36 @@ describe MergeRequestsFinder do
let(:user) { create :user } let(:user) { create :user }
let(:user2) { create :user } let(:user2) { create :user }
let(:project1) { create(:project, :public) } let(:group) { create(:group) }
let(:subgroup) { create(:group, parent: group) }
let(:project1) { create(:project, :public, group: group) }
let(:project2) { fork_project(project1, user) } let(:project2) { fork_project(project1, user) }
let(:project3) do let(:project3) do
p = fork_project(project1, user) p = fork_project(project1, user)
p.update!(archived: true) p.update!(archived: true)
p p
end end
let(:project4) { create(:project, :public, group: subgroup) }
let!(:merge_request1) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1) } let!(:merge_request1) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1) }
let!(:merge_request2) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1, state: 'closed') } let!(:merge_request2) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1, state: 'closed') }
let!(:merge_request3) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project2) } let!(:merge_request3) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project2) }
let!(:merge_request4) { create(:merge_request, :simple, author: user, source_project: project3, target_project: project3) } let!(:merge_request4) { create(:merge_request, :simple, author: user, source_project: project3, target_project: project3) }
let!(:merge_request5) { create(:merge_request, :simple, author: user, source_project: project4, target_project: project4) }
before do before do
project1.add_master(user) project1.add_master(user)
project2.add_developer(user) project2.add_developer(user)
project3.add_developer(user) project3.add_developer(user)
project2.add_developer(user2) project2.add_developer(user2)
project4.add_developer(user)
end end
describe "#execute" do describe "#execute" do
it 'filters by scope' do it 'filters by scope' do
params = { scope: 'authored', state: 'opened' } params = { scope: 'authored', state: 'opened' }
merge_requests = described_class.new(user, params).execute merge_requests = described_class.new(user, params).execute
expect(merge_requests.size).to eq(3) expect(merge_requests.size).to eq(4)
end end
it 'filters by project' do it 'filters by project' do
...@@ -39,10 +44,26 @@ describe MergeRequestsFinder do ...@@ -39,10 +44,26 @@ describe MergeRequestsFinder do
expect(merge_requests.size).to eq(1) expect(merge_requests.size).to eq(1)
end end
it 'filters by group' do
params = { group_id: group.id }
merge_requests = described_class.new(user, params).execute
expect(merge_requests.size).to eq(2)
end
it 'filters by group including subgroups', :nested_groups do
params = { group_id: group.id, include_subgroups: true }
merge_requests = described_class.new(user, params).execute
expect(merge_requests.size).to eq(3)
end
it 'filters by non_archived' do it 'filters by non_archived' do
params = { non_archived: true } params = { non_archived: true }
merge_requests = described_class.new(user, params).execute merge_requests = described_class.new(user, params).execute
expect(merge_requests.size).to eq(3) expect(merge_requests.size).to eq(4)
end end
it 'filters by iid' do it 'filters by iid' do
...@@ -73,14 +94,14 @@ describe MergeRequestsFinder do ...@@ -73,14 +94,14 @@ describe MergeRequestsFinder do
end end
context 'with created_after and created_before params' do context 'with created_after and created_before params' do
let(:project4) { create(:project, forked_from_project: project1) } let(:new_project) { create(:project, forked_from_project: project1) }
let!(:new_merge_request) do let!(:new_merge_request) do
create(:merge_request, create(:merge_request,
:simple, :simple,
author: user, author: user,
created_at: 1.week.from_now, created_at: 1.week.from_now,
source_project: project4, source_project: new_project,
target_project: project1) target_project: project1)
end end
...@@ -89,12 +110,12 @@ describe MergeRequestsFinder do ...@@ -89,12 +110,12 @@ describe MergeRequestsFinder do
:simple, :simple,
author: user, author: user,
created_at: 1.week.ago, created_at: 1.week.ago,
source_project: project4, source_project: new_project,
target_project: project4) target_project: new_project)
end end
before do before do
project4.add_master(user) new_project.add_master(user)
end end
it 'filters by created_after' do it 'filters by created_after' do
...@@ -106,7 +127,7 @@ describe MergeRequestsFinder do ...@@ -106,7 +127,7 @@ describe MergeRequestsFinder do
end end
it 'filters by created_before' do it 'filters by created_before' do
params = { project_id: project4.id, created_before: old_merge_request.created_at + 1.second } params = { project_id: new_project.id, created_before: old_merge_request.created_at + 1.second }
merge_requests = described_class.new(user, params).execute merge_requests = described_class.new(user, params).execute
...@@ -119,7 +140,7 @@ describe MergeRequestsFinder do ...@@ -119,7 +140,7 @@ describe MergeRequestsFinder do
it 'returns the number of rows for the default state' do it 'returns the number of rows for the default state' do
finder = described_class.new(user) finder = described_class.new(user)
expect(finder.row_count).to eq(3) expect(finder.row_count).to eq(4)
end end
it 'returns the number of rows for a given state' do it 'returns the number of rows for a given state' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment