From d548f543a1906b6b29b89d767475cbe26964ab1d Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Wed, 7 Feb 2024 16:42:10 +0000
Subject: [PATCH] Added hierarchy finder for saved replies
This allows for the GraphQL query to fetch all saved replies
for the current group and all ancestor groups.
https://gitlab.com/gitlab-org/gitlab/-/issues/440817
---
doc/api/graphql/reference/index.md | 21 +++++++++-
ee/app/finders/groups/saved_replies_finder.rb | 24 +++++++++++
ee/app/graphql/ee/types/group_type.rb | 5 +--
.../groups/saved_replies_resolver.rb | 27 ++++++++++++
ee/app/models/groups/saved_reply.rb | 2 +
.../groups/saved_replies_finder_spec.rb | 42 +++++++++++++++++++
ee/spec/models/groups/saved_reply_spec.rb | 16 +++++++
.../api/graphql/groups/saved_replies_spec.rb | 29 +++++++++----
8 files changed, 155 insertions(+), 11 deletions(-)
create mode 100644 ee/app/finders/groups/saved_replies_finder.rb
create mode 100644 ee/app/graphql/resolvers/groups/saved_replies_resolver.rb
create mode 100644 ee/spec/finders/groups/saved_replies_finder_spec.rb
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index b4c29f41922d..26e9162c4adb 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -20008,7 +20008,6 @@ GPG signature for a signed commit.
| <a id="grouprequestaccessenabled"></a>`requestAccessEnabled` | [`Boolean`](#boolean) | Indicates if users can request access to namespace. |
| <a id="grouprequiretwofactorauthentication"></a>`requireTwoFactorAuthentication` | [`Boolean`](#boolean) | Indicates if all users in this group are required to set up two-factor authentication. |
| <a id="grouprootstoragestatistics"></a>`rootStorageStatistics` | [`RootStorageStatistics`](#rootstoragestatistics) | Aggregated storage statistics of the namespace. Only available for root namespaces. |
-| <a id="groupsavedreplies"></a>`savedReplies` **{warning-solid}** | [`GroupSavedReplyConnection`](#groupsavedreplyconnection) | **Introduced** in 16.10. **Status**: Experiment. Saved replies available to the group. Available only when feature flag `group_saved_replies_flag` is enabled. This field can only be resolved for one group in any single request. |
| <a id="groupsecuritypolicyproject"></a>`securityPolicyProject` | [`Project`](#project) | Security policy project assigned to the namespace. |
| <a id="groupsharewithgrouplock"></a>`shareWithGroupLock` | [`Boolean`](#boolean) | Indicates if sharing a project with another group within this group is prevented. |
| <a id="groupsharedrunnerssetting"></a>`sharedRunnersSetting` | [`SharedRunnersSetting`](#sharedrunnerssetting) | Shared runners availability for the namespace and its descendants. |
@@ -20892,6 +20891,26 @@ four standard [pagination arguments](#pagination-arguments):
| <a id="grouprunnersupgradestatus"></a>`upgradeStatus` | [`CiRunnerUpgradeStatus`](#cirunnerupgradestatus) | Filter by upgrade status. |
| <a id="grouprunnersversionprefix"></a>`versionPrefix` **{warning-solid}** | [`String`](#string) | **Introduced** in 16.6. **Status**: Experiment. Filter runners by version. Runners that contain runner managers with the version at the start of the search term are returned. For example, the search term '14.' returns runner managers with versions '14.11.1' and '14.2.3'. |
+##### `Group.savedReplies`
+
+Saved replies available to the group. Available only when feature flag `group_saved_replies_flag` is enabled. This field can only be resolved for one group in any single request.
+
+NOTE:
+**Introduced** in 16.10.
+**Status**: Experiment.
+
+Returns [`GroupSavedReplyConnection`](#groupsavedreplyconnection).
+
+This field returns a [connection](#connections). It accepts the
+four standard [pagination arguments](#pagination-arguments):
+`before: String`, `after: String`, `first: Int`, and `last: Int`.
+
+###### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="groupsavedrepliesincludeancestorgroups"></a>`includeAncestorGroups` | [`Boolean`](#boolean) | Include saved replies from parent groups. |
+
##### `Group.savedReply`
Saved reply in the group. Available only when feature flag `group_saved_replies_flag` is enabled. This field can only be resolved for one group in any single request.
diff --git a/ee/app/finders/groups/saved_replies_finder.rb b/ee/app/finders/groups/saved_replies_finder.rb
new file mode 100644
index 000000000000..2a007eaf6f4b
--- /dev/null
+++ b/ee/app/finders/groups/saved_replies_finder.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module Groups
+ class SavedRepliesFinder < Base
+ include FinderWithGroupHierarchy
+ include Gitlab::Utils::StrongMemoize
+
+ def initialize(group, params = {})
+ @group = group
+ @params = params
+ @skip_authorization = true
+ end
+
+ def execute
+ return group.saved_replies unless params[:include_ancestor_groups]
+
+ ::Groups::SavedReply.for_groups(group_ids_for(group))
+ end
+
+ private
+
+ attr_reader :group, :params, :skip_authorization
+ end
+end
diff --git a/ee/app/graphql/ee/types/group_type.rb b/ee/app/graphql/ee/types/group_type.rb
index 3649f4ce5203..4b8cd3766d16 100644
--- a/ee/app/graphql/ee/types/group_type.rb
+++ b/ee/app/graphql/ee/types/group_type.rb
@@ -217,12 +217,11 @@ module GroupType
field :saved_replies,
::Types::Groups::SavedReplyType.connection_type,
null: true,
+ resolver: ::Resolvers::Groups::SavedRepliesResolver,
description: 'Saved replies available to the group. Available only when feature flag ' \
'`group_saved_replies_flag` is enabled. This field can only be resolved ' \
'for one group in any single request.',
- alpha: { milestone: '16.10' } do
- extension ::Gitlab::Graphql::Limit::FieldCallCount, limit: 1
- end
+ alpha: { milestone: '16.10' }
field :saved_reply,
resolver: ::Resolvers::Groups::SavedReplyResolver,
diff --git a/ee/app/graphql/resolvers/groups/saved_replies_resolver.rb b/ee/app/graphql/resolvers/groups/saved_replies_resolver.rb
new file mode 100644
index 000000000000..115dcd500759
--- /dev/null
+++ b/ee/app/graphql/resolvers/groups/saved_replies_resolver.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module Resolvers
+ module Groups
+ class SavedRepliesResolver < BaseResolver
+ include Gitlab::Graphql::Authorize::AuthorizeResource
+
+ extension ::Gitlab::Graphql::Limit::FieldCallCount, limit: 1
+
+ authorizes_object!
+
+ authorize :read_saved_replies
+
+ argument :include_ancestor_groups,
+ GraphQL::Types::Boolean,
+ required: false,
+ default_value: false,
+ description: 'Include saved replies from parent groups.'
+
+ type ::Types::Groups::SavedReplyType, null: true
+
+ def resolve(**args)
+ ::Groups::SavedRepliesFinder.new(object, args).execute
+ end
+ end
+ end
+end
diff --git a/ee/app/models/groups/saved_reply.rb b/ee/app/models/groups/saved_reply.rb
index f771bcce5e4f..9ec2fe30b75e 100644
--- a/ee/app/models/groups/saved_reply.rb
+++ b/ee/app/models/groups/saved_reply.rb
@@ -10,5 +10,7 @@ def self.namespace_foreign_key
include SavedReplyConcern
belongs_to :group
+
+ scope :for_groups, ->(group_ids) { where(group_id: group_ids) }
end
end
diff --git a/ee/spec/finders/groups/saved_replies_finder_spec.rb b/ee/spec/finders/groups/saved_replies_finder_spec.rb
new file mode 100644
index 000000000000..79197a24f0f5
--- /dev/null
+++ b/ee/spec/finders/groups/saved_replies_finder_spec.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Groups::SavedRepliesFinder, feature_category: :code_review_workflow do
+ describe '#execute' do
+ let(:include_ancestor_groups) { false }
+
+ subject(:execute) do
+ described_class.new(group, { include_ancestor_groups: include_ancestor_groups }).execute
+ end
+
+ context 'when inside a group' do
+ let_it_be(:group) { create(:group) }
+ let_it_be(:saved_reply) { create(:group_saved_reply, group: group) }
+
+ it { expect(execute).to contain_exactly(saved_reply) }
+
+ context 'when include_ancestor_groups is true' do
+ let(:include_ancestor_groups) { true }
+
+ it { expect(execute).to contain_exactly(saved_reply) }
+ end
+ end
+
+ context 'when inside a subgroup' do
+ let_it_be(:parent) { create(:group) }
+ let_it_be(:group) { create(:group, parent: parent) }
+ let_it_be(:saved_reply) { create(:group_saved_reply, group: parent) }
+
+ context 'when include_ancestor_groups is true' do
+ let(:include_ancestor_groups) { true }
+
+ it { expect(execute).to contain_exactly(saved_reply) }
+ end
+
+ context 'when include_ancestor_groups is false' do
+ it { expect(execute).to be_empty }
+ end
+ end
+ end
+end
diff --git a/ee/spec/models/groups/saved_reply_spec.rb b/ee/spec/models/groups/saved_reply_spec.rb
index a0683e75a293..c127f801258b 100644
--- a/ee/spec/models/groups/saved_reply_spec.rb
+++ b/ee/spec/models/groups/saved_reply_spec.rb
@@ -13,4 +13,20 @@
it { is_expected.to validate_length_of(:name).is_at_most(255) }
it { is_expected.to validate_length_of(:content).is_at_most(10000) }
end
+
+ describe '#for_groups' do
+ let_it_be(:group) { create(:group) }
+ let_it_be(:saved_reply) { create(:group_saved_reply, group: group) }
+
+ it { expect(described_class.for_groups([group.id])).to eq([saved_reply]) }
+
+ context 'with subgroup' do
+ let_it_be(:subgroup) { create(:group, parent: group) }
+ let_it_be(:subgroup_saved_reply) { create(:group_saved_reply, group: subgroup) }
+
+ it do
+ expect(described_class.for_groups([subgroup.id, group.id])).to match_array([saved_reply, subgroup_saved_reply])
+ end
+ end
+ end
end
diff --git a/ee/spec/requests/api/graphql/groups/saved_replies_spec.rb b/ee/spec/requests/api/graphql/groups/saved_replies_spec.rb
index 8d6f37bc609c..8e6efd8624f2 100644
--- a/ee/spec/requests/api/graphql/groups/saved_replies_spec.rb
+++ b/ee/spec/requests/api/graphql/groups/saved_replies_spec.rb
@@ -8,13 +8,15 @@
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:saved_reply) { create(:group_saved_reply, group: group) }
+ let(:include_ancestor_groups) { false }
+ let(:group_path) { group.full_path }
let(:query) do
<<~QUERY
- query groupSavedReplies($groupPath: ID!) {
+ query groupSavedReplies($groupPath: ID! $includeAncestorGroups: Boolean) {
group(fullPath: $groupPath) {
id
- savedReplies {
+ savedReplies(includeAncestorGroups: $includeAncestorGroups) {
nodes {
id
name
@@ -31,7 +33,8 @@
query,
current_user: user,
variables: {
- groupPath: group.full_path
+ groupPath: group_path,
+ includeAncestorGroups: include_ancestor_groups
}
)
end
@@ -45,10 +48,10 @@
stub_licensed_features(group_saved_replies: false)
end
- it 'returns empty array' do
+ it 'returns nil' do
post_query
- expect(saved_reply_graphl_response).to be_empty
+ expect(saved_reply_graphl_response).to be_nil
end
end
@@ -58,10 +61,10 @@
stub_licensed_features(group_saved_replies: true)
end
- it 'returns empty array' do
+ it 'returns nil' do
post_query
- expect(saved_reply_graphl_response).to be_empty
+ expect(saved_reply_graphl_response).to be_nil
end
end
@@ -75,6 +78,18 @@
expect(saved_reply_graphl_response).to contain_exactly(a_graphql_entity_for(saved_reply, :name, :content))
end
+
+ context 'when group path is a sub-group' do
+ let_it_be(:subgroup) { create(:group, parent: group) }
+ let(:group_path) { subgroup.full_path }
+ let(:include_ancestor_groups) { true }
+
+ it 'includes saved replies from ancestor groups' do
+ post_query
+
+ expect(saved_reply_graphl_response).to contain_exactly(a_graphql_entity_for(saved_reply, :name, :content))
+ end
+ end
end
def saved_reply_graphl_response
--
GitLab