diff options
| -rw-r--r-- | app/finders/group_descendants_finder.rb | 6 | ||||
| -rw-r--r-- | app/models/concerns/group_descendant.rb | 15 | ||||
| -rw-r--r-- | changelogs/unreleased/bvl-shared-groups-on-group-page.yml | 5 | ||||
| -rw-r--r-- | spec/finders/group_descendants_finder_spec.rb | 9 | ||||
| -rw-r--r-- | spec/models/concerns/group_descendant_spec.rb | 17 |
5 files changed, 37 insertions, 15 deletions
diff --git a/app/finders/group_descendants_finder.rb b/app/finders/group_descendants_finder.rb index e72fd8eb3a5..051ea108e06 100644 --- a/app/finders/group_descendants_finder.rb +++ b/app/finders/group_descendants_finder.rb @@ -134,10 +134,8 @@ class GroupDescendantsFinder end def direct_child_projects - GroupProjectsFinder.new(group: parent_group, - current_user: current_user, - options: { only_owned: true }, - params: params).execute + GroupProjectsFinder.new(group: parent_group, current_user: current_user, params: params) + .execute end # Finds all projects nested under `parent_group` or any of its descendant diff --git a/app/models/concerns/group_descendant.rb b/app/models/concerns/group_descendant.rb index 01957da0bf3..261ace57a17 100644 --- a/app/models/concerns/group_descendant.rb +++ b/app/models/concerns/group_descendant.rb @@ -37,7 +37,20 @@ module GroupDescendant parent ||= preloaded.detect { |possible_parent| possible_parent.is_a?(Group) && possible_parent.id == child.parent_id } if parent.nil? && !child.parent_id.nil? - raise ArgumentError.new('parent was not preloaded') + parent = child.parent + + exception = ArgumentError.new <<~MSG + parent: [GroupDescendant: #{parent.inspect}] was not preloaded for [#{child.inspect}]") + This error is not user facing, but causes a +1 query. + MSG + extras = { + parent: parent, + child: child, + preloaded: preloaded.map(&:full_path) + } + issue_url = 'https://gitlab.com/gitlab-org/gitlab-ce/issues/40785' + + Gitlab::Sentry.track_exception(exception, issue_url: issue_url, extra: extras) end if parent.nil? && hierarchy_top.present? diff --git a/changelogs/unreleased/bvl-shared-groups-on-group-page.yml b/changelogs/unreleased/bvl-shared-groups-on-group-page.yml new file mode 100644 index 00000000000..6c0703fd138 --- /dev/null +++ b/changelogs/unreleased/bvl-shared-groups-on-group-page.yml @@ -0,0 +1,5 @@ +--- +title: Show shared projects on group page +merge_request: 18390 +author: +type: fixed diff --git a/spec/finders/group_descendants_finder_spec.rb b/spec/finders/group_descendants_finder_spec.rb index 375bcc9087e..796d40cb625 100644 --- a/spec/finders/group_descendants_finder_spec.rb +++ b/spec/finders/group_descendants_finder_spec.rb @@ -35,15 +35,6 @@ describe GroupDescendantsFinder do expect(finder.execute).to contain_exactly(project) end - it 'does not include projects shared with the group' do - project = create(:project, namespace: group) - other_project = create(:project) - other_project.project_group_links.create(group: group, - group_access: ProjectGroupLink::MASTER) - - expect(finder.execute).to contain_exactly(project) - end - context 'when archived is `true`' do let(:params) { { archived: 'true' } } diff --git a/spec/models/concerns/group_descendant_spec.rb b/spec/models/concerns/group_descendant_spec.rb index c163fb01a81..28352d8c961 100644 --- a/spec/models/concerns/group_descendant_spec.rb +++ b/spec/models/concerns/group_descendant_spec.rb @@ -79,9 +79,24 @@ describe GroupDescendant, :nested_groups do expect(described_class.build_hierarchy(groups)).to eq(expected_hierarchy) end + it 'tracks the exception when a parent was not preloaded' do + expect(Gitlab::Sentry).to receive(:track_exception).and_call_original + + expect { GroupDescendant.build_hierarchy([subsub_group]) }.to raise_error(ArgumentError) + end + + it 'recovers if a parent was not reloaded by querying for the parent' do + expected_hierarchy = { parent => { subgroup => subsub_group } } + + # this does not raise in production, so stubbing it here. + allow(Gitlab::Sentry).to receive(:track_exception) + + expect(GroupDescendant.build_hierarchy([subsub_group])).to eq(expected_hierarchy) + end + it 'raises an error if not all elements were preloaded' do expect { described_class.build_hierarchy([subsub_group]) } - .to raise_error('parent was not preloaded') + .to raise_error(/was not preloaded/) end end end |