From 6b0c5e6ccbc849ecc65a8fe3a704b06407e54f39 Mon Sep 17 00:00:00 2001
From: Lee Tickett <lee@tickett.net>
Date: Mon, 8 Nov 2021 11:28:20 +0000
Subject: [PATCH] Add issues set crm contacts service and graphql mutation
Inroduce a new graphql mutation for setting/adding/removing customer
relations contacts to/from issues (and the required service to
support it)
Changelog: added
---
.../mutations/issues/set_crm_contacts.rb | 48 ++++++
app/graphql/types/mutation_type.rb | 1 +
.../customer_relations/issue_contact.rb | 2 +-
app/policies/issue_policy.rb | 7 +
.../issues/set_crm_contacts_service.rb | 90 ++++++++++
doc/api/graphql/reference/index.md | 22 +++
locale/gitlab.pot | 2 +-
.../mutations/issues/set_crm_contacts_spec.rb | 161 +++++++++++++++++
.../issues/set_crm_contacts_service_spec.rb | 162 ++++++++++++++++++
9 files changed, 493 insertions(+), 2 deletions(-)
create mode 100644 app/graphql/mutations/issues/set_crm_contacts.rb
create mode 100644 app/services/issues/set_crm_contacts_service.rb
create mode 100644 spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb
create mode 100644 spec/services/issues/set_crm_contacts_service_spec.rb
diff --git a/app/graphql/mutations/issues/set_crm_contacts.rb b/app/graphql/mutations/issues/set_crm_contacts.rb
new file mode 100644
index 0000000000000..7a9e6237eaa18
--- /dev/null
+++ b/app/graphql/mutations/issues/set_crm_contacts.rb
@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+module Mutations
+ module Issues
+ class SetCrmContacts < Base
+ graphql_name 'IssueSetCrmContacts'
+
+ argument :crm_contact_ids,
+ [::Types::GlobalIDType[::CustomerRelations::Contact]],
+ required: true,
+ description: 'Customer relations contact IDs to set. Replaces existing contacts by default.'
+
+ argument :operation_mode,
+ Types::MutationOperationModeEnum,
+ required: false,
+ description: 'Changes the operation mode. Defaults to REPLACE.'
+
+ def resolve(project_path:, iid:, crm_contact_ids:, operation_mode: Types::MutationOperationModeEnum.enum[:replace])
+ issue = authorized_find!(project_path: project_path, iid: iid)
+ project = issue.project
+ raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless Feature.enabled?(:customer_relations, project.group, default_enabled: :yaml)
+
+ crm_contact_ids = crm_contact_ids.compact.map do |crm_contact_id|
+ raise Gitlab::Graphql::Errors::ArgumentError, "Contact #{crm_contact_id} is invalid." unless crm_contact_id.respond_to?(:model_id)
+
+ crm_contact_id.model_id.to_i
+ end
+
+ attribute_name = case operation_mode
+ when Types::MutationOperationModeEnum.enum[:append]
+ :add_crm_contact_ids
+ when Types::MutationOperationModeEnum.enum[:remove]
+ :remove_crm_contact_ids
+ else
+ :crm_contact_ids
+ end
+
+ response = ::Issues::SetCrmContactsService.new(project: project, current_user: current_user, params: { attribute_name => crm_contact_ids })
+ .execute(issue)
+
+ {
+ issue: issue,
+ errors: response.errors
+ }
+ end
+ end
+ end
+end
diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb
index 4104774c1dee1..7be995a40b217 100644
--- a/app/graphql/types/mutation_type.rb
+++ b/app/graphql/types/mutation_type.rb
@@ -49,6 +49,7 @@ class MutationType < BaseObject
mount_mutation Mutations::Environments::CanaryIngress::Update
mount_mutation Mutations::Issues::Create
mount_mutation Mutations::Issues::SetAssignees
+ mount_mutation Mutations::Issues::SetCrmContacts
mount_mutation Mutations::Issues::SetConfidential
mount_mutation Mutations::Issues::SetLocked
mount_mutation Mutations::Issues::SetDueDate
diff --git a/app/models/customer_relations/issue_contact.rb b/app/models/customer_relations/issue_contact.rb
index ba73b685906ca..98faf8d664450 100644
--- a/app/models/customer_relations/issue_contact.rb
+++ b/app/models/customer_relations/issue_contact.rb
@@ -15,6 +15,6 @@ def contact_belongs_to_issue_group
return unless issue&.project&.namespace_id
return if contact.group_id == issue.project.namespace_id
- errors.add(:base, _('The contact does not belong to the same group as the issue.'))
+ errors.add(:base, _('The contact does not belong to the same group as the issue'))
end
end
diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb
index 575e532c615d6..c9c13b296436c 100644
--- a/app/policies/issue_policy.rb
+++ b/app/policies/issue_policy.rb
@@ -12,6 +12,9 @@ class IssuePolicy < IssuablePolicy
@user && IssueCollection.new([@subject]).visible_to(@user).any?
end
+ desc "User can read contacts belonging to the issue group"
+ condition(:can_read_crm_contacts, scope: :subject) { @user.can?(:read_crm_contact, @subject.project.group) }
+
desc "Issue is confidential"
condition(:confidential, scope: :subject) { @subject.confidential? }
@@ -77,6 +80,10 @@ class IssuePolicy < IssuablePolicy
rule { ~persisted & can?(:create_issue) }.policy do
enable :set_confidentiality
end
+
+ rule { can?(:set_issue_metadata) & can_read_crm_contacts }.policy do
+ enable :set_issue_crm_contacts
+ end
end
IssuePolicy.prepend_mod_with('IssuePolicy')
diff --git a/app/services/issues/set_crm_contacts_service.rb b/app/services/issues/set_crm_contacts_service.rb
new file mode 100644
index 0000000000000..13fe30b5ac870
--- /dev/null
+++ b/app/services/issues/set_crm_contacts_service.rb
@@ -0,0 +1,90 @@
+# frozen_string_literal: true
+
+module Issues
+ class SetCrmContactsService < ::BaseProjectService
+ attr_accessor :issue, :errors
+
+ MAX_ADDITIONAL_CONTACTS = 6
+
+ def execute(issue)
+ @issue = issue
+ @errors = []
+
+ return error_no_permissions unless allowed?
+ return error_invalid_params unless valid_params?
+
+ determine_changes if params[:crm_contact_ids]
+
+ return error_too_many if too_many?
+
+ add_contacts if params[:add_crm_contact_ids]
+ remove_contacts if params[:remove_crm_contact_ids]
+
+ if issue.valid?
+ ServiceResponse.success(payload: issue)
+ else
+ # The default error isn't very helpful: "Issue customer relations contacts is invalid"
+ issue.errors.delete(:issue_customer_relations_contacts)
+ issue.errors.add(:issue_customer_relations_contacts, errors.to_sentence)
+ ServiceResponse.error(payload: issue, message: issue.errors.full_messages)
+ end
+ end
+
+ private
+
+ def determine_changes
+ existing_contact_ids = issue.issue_customer_relations_contacts.map(&:contact_id)
+ params[:add_crm_contact_ids] = params[:crm_contact_ids] - existing_contact_ids
+ params[:remove_crm_contact_ids] = existing_contact_ids - params[:crm_contact_ids]
+ end
+
+ def add_contacts
+ params[:add_crm_contact_ids].uniq.each do |contact_id|
+ issue_contact = issue.issue_customer_relations_contacts.create(contact_id: contact_id)
+
+ unless issue_contact.persisted?
+ # The validation ensures that the id exists and the user has permission
+ errors << "#{contact_id}: The resource that you are attempting to access does not exist or you don't have permission to perform this action"
+ end
+ end
+ end
+
+ def remove_contacts
+ issue.issue_customer_relations_contacts
+ .where(contact_id: params[:remove_crm_contact_ids]) # rubocop: disable CodeReuse/ActiveRecord
+ .delete_all
+ end
+
+ def allowed?
+ current_user&.can?(:set_issue_crm_contacts, issue)
+ end
+
+ def valid_params?
+ set_present? ^ add_or_remove_present?
+ end
+
+ def set_present?
+ params[:crm_contact_ids].present?
+ end
+
+ def add_or_remove_present?
+ params[:add_crm_contact_ids].present? || params[:remove_crm_contact_ids].present?
+ end
+
+ def too_many?
+ params[:add_crm_contact_ids] && params[:add_crm_contact_ids].length > MAX_ADDITIONAL_CONTACTS
+ end
+
+ def error_no_permissions
+ ServiceResponse.error(message: ['You have insufficient permissions to set customer relations contacts for this issue'])
+ end
+
+ def error_invalid_params
+ ServiceResponse.error(message: ['You cannot combine crm_contact_ids with add_crm_contact_ids or remove_crm_contact_ids'])
+ end
+
+ def error_too_many
+ ServiceResponse.error(payload: issue, message: ["You can only add up to #{MAX_ADDITIONAL_CONTACTS} contacts at one time"])
+ end
+ end
+end
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 01f1a98cd2af1..c47b1151f6a16 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -2820,6 +2820,28 @@ Input type: `IssueSetConfidentialInput`
| <a id="mutationissuesetconfidentialerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
| <a id="mutationissuesetconfidentialissue"></a>`issue` | [`Issue`](#issue) | Issue after mutation. |
+### `Mutation.issueSetCrmContacts`
+
+Input type: `IssueSetCrmContactsInput`
+
+#### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationissuesetcrmcontactsclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationissuesetcrmcontactscrmcontactids"></a>`crmContactIds` | [`[CustomerRelationsContactID!]!`](#customerrelationscontactid) | Customer relations contact IDs to set. Replaces existing contacts by default. |
+| <a id="mutationissuesetcrmcontactsiid"></a>`iid` | [`String!`](#string) | IID of the issue to mutate. |
+| <a id="mutationissuesetcrmcontactsoperationmode"></a>`operationMode` | [`MutationOperationMode`](#mutationoperationmode) | Changes the operation mode. Defaults to REPLACE. |
+| <a id="mutationissuesetcrmcontactsprojectpath"></a>`projectPath` | [`ID!`](#id) | Project the issue to mutate is in. |
+
+#### Fields
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationissuesetcrmcontactsclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationissuesetcrmcontactserrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
+| <a id="mutationissuesetcrmcontactsissue"></a>`issue` | [`Issue`](#issue) | Issue after mutation. |
+
### `Mutation.issueSetDueDate`
Input type: `IssueSetDueDateInput`
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index ca8befc9ba475..c3f5beeaa5d17 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -34136,7 +34136,7 @@ msgstr ""
msgid "The connection will time out after %{timeout}. For repositories that take longer, use a clone/push combination."
msgstr ""
-msgid "The contact does not belong to the same group as the issue."
+msgid "The contact does not belong to the same group as the issue"
msgstr ""
msgid "The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository."
diff --git a/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb b/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb
new file mode 100644
index 0000000000000..3da702c55d7bb
--- /dev/null
+++ b/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb
@@ -0,0 +1,161 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Setting issues crm contacts' do
+ include GraphqlHelpers
+
+ let_it_be(:user) { create(:user) }
+ let_it_be(:group) { create(:group) }
+ let_it_be(:project) { create(:project, group: group) }
+ let_it_be(:contacts) { create_list(:contact, 4, group: group) }
+
+ let(:issue) { create(:issue, project: project) }
+ let(:operation_mode) { Types::MutationOperationModeEnum.default_mode }
+ let(:crm_contact_ids) { [global_id_of(contacts[1]), global_id_of(contacts[2])] }
+ let(:does_not_exist_or_no_permission) { "The resource that you are attempting to access does not exist or you don't have permission to perform this action" }
+
+ let(:mutation) do
+ variables = {
+ project_path: issue.project.full_path,
+ iid: issue.iid.to_s,
+ operation_mode: operation_mode,
+ crm_contact_ids: crm_contact_ids
+ }
+
+ graphql_mutation(:issue_set_crm_contacts, variables,
+ <<-QL.strip_heredoc
+ clientMutationId
+ errors
+ issue {
+ customerRelationsContacts {
+ nodes {
+ id
+ }
+ }
+ }
+ QL
+ )
+ end
+
+ def mutation_response
+ graphql_mutation_response(:issue_set_crm_contacts)
+ end
+
+ before do
+ create(:issue_customer_relations_contact, issue: issue, contact: contacts[0])
+ create(:issue_customer_relations_contact, issue: issue, contact: contacts[1])
+ end
+
+ context 'when the user has no permission' do
+ it 'returns expected error' do
+ error = Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR
+ post_graphql_mutation(mutation, current_user: user)
+
+ expect(graphql_errors).to include(a_hash_including('message' => error))
+ end
+ end
+
+ context 'when the user has permission' do
+ before do
+ group.add_reporter(user)
+ end
+
+ context 'when the feature is disabled' do
+ before do
+ stub_feature_flags(customer_relations: false)
+ end
+
+ it 'raises expected error' do
+ post_graphql_mutation(mutation, current_user: user)
+
+ expect(graphql_errors).to include(a_hash_including('message' => 'Feature disabled'))
+ end
+ end
+
+ context 'replace' do
+ it 'updates the issue with correct contacts' do
+ post_graphql_mutation(mutation, current_user: user)
+
+ expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes, :id))
+ .to match_array([global_id_of(contacts[1]), global_id_of(contacts[2])])
+ end
+ end
+
+ context 'append' do
+ let(:crm_contact_ids) { [global_id_of(contacts[3])] }
+ let(:operation_mode) { Types::MutationOperationModeEnum.enum[:append] }
+
+ it 'updates the issue with correct contacts' do
+ post_graphql_mutation(mutation, current_user: user)
+
+ expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes, :id))
+ .to match_array([global_id_of(contacts[0]), global_id_of(contacts[1]), global_id_of(contacts[3])])
+ end
+ end
+
+ context 'remove' do
+ let(:crm_contact_ids) { [global_id_of(contacts[0])] }
+ let(:operation_mode) { Types::MutationOperationModeEnum.enum[:remove] }
+
+ it 'updates the issue with correct contacts' do
+ post_graphql_mutation(mutation, current_user: user)
+
+ expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes, :id))
+ .to match_array([global_id_of(contacts[1])])
+ end
+ end
+
+ context 'when the contact does not exist' do
+ let(:crm_contact_ids) { ["gid://gitlab/CustomerRelations::Contact/#{non_existing_record_id}"] }
+
+ it 'returns expected error' do
+ post_graphql_mutation(mutation, current_user: user)
+
+ expect(graphql_data_at(:issue_set_crm_contacts, :errors))
+ .to match_array(["Issue customer relations contacts #{non_existing_record_id}: #{does_not_exist_or_no_permission}"])
+ end
+ end
+
+ context 'when the contact belongs to a different group' do
+ let(:group2) { create(:group) }
+ let(:contact) { create(:contact, group: group2) }
+ let(:crm_contact_ids) { [global_id_of(contact)] }
+
+ before do
+ group2.add_reporter(user)
+ end
+
+ it 'returns expected error' do
+ post_graphql_mutation(mutation, current_user: user)
+
+ expect(graphql_data_at(:issue_set_crm_contacts, :errors))
+ .to match_array(["Issue customer relations contacts #{contact.id}: #{does_not_exist_or_no_permission}"])
+ end
+ end
+
+ context 'when attempting to add more than 6' do
+ let(:operation_mode) { Types::MutationOperationModeEnum.enum[:append] }
+ let(:gid) { global_id_of(contacts[0]) }
+ let(:crm_contact_ids) { [gid, gid, gid, gid, gid, gid, gid] }
+
+ it 'returns expected error' do
+ post_graphql_mutation(mutation, current_user: user)
+
+ expect(graphql_data_at(:issue_set_crm_contacts, :errors))
+ .to match_array(["You can only add up to 6 contacts at one time"])
+ end
+ end
+
+ context 'when trying to remove non-existent contact' do
+ let(:operation_mode) { Types::MutationOperationModeEnum.enum[:remove] }
+ let(:crm_contact_ids) { ["gid://gitlab/CustomerRelations::Contact/#{non_existing_record_id}"] }
+
+ it 'raises expected error' do
+ post_graphql_mutation(mutation, current_user: user)
+
+ expect(graphql_data_at(:issue_set_crm_contacts, :errors)).to be_empty
+ end
+ end
+ end
+end
diff --git a/spec/services/issues/set_crm_contacts_service_spec.rb b/spec/services/issues/set_crm_contacts_service_spec.rb
new file mode 100644
index 0000000000000..65b22fe3b3577
--- /dev/null
+++ b/spec/services/issues/set_crm_contacts_service_spec.rb
@@ -0,0 +1,162 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Issues::SetCrmContactsService do
+ let_it_be(:user) { create(:user) }
+ let_it_be(:group) { create(:group) }
+ let_it_be(:project) { create(:project, group: group) }
+ let_it_be(:contacts) { create_list(:contact, 4, group: group) }
+
+ let(:issue) { create(:issue, project: project) }
+ let(:does_not_exist_or_no_permission) { "The resource that you are attempting to access does not exist or you don't have permission to perform this action" }
+
+ before do
+ create(:issue_customer_relations_contact, issue: issue, contact: contacts[0])
+ create(:issue_customer_relations_contact, issue: issue, contact: contacts[1])
+ end
+
+ subject(:set_crm_contacts) do
+ described_class.new(project: project, current_user: user, params: params).execute(issue)
+ end
+
+ describe '#execute' do
+ context 'when the user has no permission' do
+ let(:params) { { crm_contact_ids: [contacts[1].id, contacts[2].id] } }
+
+ it 'returns expected error response' do
+ response = set_crm_contacts
+
+ expect(response).to be_error
+ expect(response.message).to match_array(['You have insufficient permissions to set customer relations contacts for this issue'])
+ end
+ end
+
+ context 'when user has permission' do
+ before do
+ group.add_reporter(user)
+ end
+
+ context 'when the contact does not exist' do
+ let(:params) { { crm_contact_ids: [non_existing_record_id] } }
+
+ it 'returns expected error response' do
+ response = set_crm_contacts
+
+ expect(response).to be_error
+ expect(response.message).to match_array(["Issue customer relations contacts #{non_existing_record_id}: #{does_not_exist_or_no_permission}"])
+ end
+ end
+
+ context 'when the contact belongs to a different group' do
+ let(:group2) { create(:group) }
+ let(:contact) { create(:contact, group: group2) }
+ let(:params) { { crm_contact_ids: [contact.id] } }
+
+ before do
+ group2.add_reporter(user)
+ end
+
+ it 'returns expected error response' do
+ response = set_crm_contacts
+
+ expect(response).to be_error
+ expect(response.message).to match_array(["Issue customer relations contacts #{contact.id}: #{does_not_exist_or_no_permission}"])
+ end
+ end
+
+ context 'replace' do
+ let(:params) { { crm_contact_ids: [contacts[1].id, contacts[2].id] } }
+
+ it 'updates the issue with correct contacts' do
+ response = set_crm_contacts
+
+ expect(response).to be_success
+ expect(issue.customer_relations_contacts).to match_array([contacts[1], contacts[2]])
+ end
+ end
+
+ context 'add' do
+ let(:params) { { add_crm_contact_ids: [contacts[3].id] } }
+
+ it 'updates the issue with correct contacts' do
+ response = set_crm_contacts
+
+ expect(response).to be_success
+ expect(issue.customer_relations_contacts).to match_array([contacts[0], contacts[1], contacts[3]])
+ end
+ end
+
+ context 'remove' do
+ let(:params) { { remove_crm_contact_ids: [contacts[0].id] } }
+
+ it 'updates the issue with correct contacts' do
+ response = set_crm_contacts
+
+ expect(response).to be_success
+ expect(issue.customer_relations_contacts).to match_array([contacts[1]])
+ end
+ end
+
+ context 'when attempting to add more than 6' do
+ let(:id) { contacts[0].id }
+ let(:params) { { add_crm_contact_ids: [id, id, id, id, id, id, id] } }
+
+ it 'returns expected error message' do
+ response = set_crm_contacts
+
+ expect(response).to be_error
+ expect(response.message).to match_array(['You can only add up to 6 contacts at one time'])
+ end
+ end
+
+ context 'when trying to remove non-existent contact' do
+ let(:params) { { remove_crm_contact_ids: [non_existing_record_id] } }
+
+ it 'returns expected error message' do
+ response = set_crm_contacts
+
+ expect(response).to be_success
+ expect(response.message).to be_nil
+ end
+ end
+
+ context 'when combining params' do
+ let(:error_invalid_params) { 'You cannot combine crm_contact_ids with add_crm_contact_ids or remove_crm_contact_ids' }
+
+ context 'add and remove' do
+ let(:params) { { remove_crm_contact_ids: [contacts[1].id], add_crm_contact_ids: [contacts[3].id] } }
+
+ it 'updates the issue with correct contacts' do
+ response = set_crm_contacts
+
+ expect(response).to be_success
+ expect(issue.customer_relations_contacts).to match_array([contacts[0], contacts[3]])
+ end
+ end
+
+ context 'replace and remove' do
+ let(:params) { { crm_contact_ids: [contacts[3].id], remove_crm_contact_ids: [contacts[0].id] } }
+
+ it 'returns expected error response' do
+ response = set_crm_contacts
+
+ expect(response).to be_error
+ expect(response.message).to match_array([error_invalid_params])
+ end
+ end
+
+ context 'replace and add' do
+ let(:params) { { crm_contact_ids: [contacts[3].id], add_crm_contact_ids: [contacts[1].id] } }
+
+ it 'returns expected error response' do
+ response = set_crm_contacts
+
+ expect(response).to be_error
+ expect(response.message).to match_array([error_invalid_params])
+ end
+ end
+ end
+ end
+ end
+end
--
GitLab