From dc8770ad3de0bd8b8738724455f5a2bd76a8d262 Mon Sep 17 00:00:00 2001
From: Lee Tickett <ltickett@gitlab.com>
Date: Fri, 27 Sep 2024 07:25:33 +0000
Subject: [PATCH] Support CRM contacts and organizations in subgroups and out
of hierarchy
---
.../groups/application_controller.rb | 4 +-
.../groups/crm/contacts_controller.rb | 2 +-
.../groups/crm/organizations_controller.rb | 2 +-
app/finders/crm/contacts_finder.rb | 15 ++-----
app/models/customer_relations/contact.rb | 8 ++--
.../customer_relations/issue_contact.rb | 9 ++--
app/models/customer_relations/organization.rb | 8 ++--
app/models/group.rb | 21 ++++++++-
app/models/group/crm_settings.rb | 1 +
app/models/project.rb | 6 +++
app/policies/issue_policy.rb | 4 +-
.../issues/set_crm_contacts_service.rb | 2 +-
.../work_items/callbacks/crm_contacts.rb | 2 +-
lib/gitlab/quick_actions/issue_actions.rb | 2 +-
.../groups/menus/customer_relations_menu.rb | 4 +-
locale/gitlab.pot | 6 +--
spec/finders/crm/contacts_finder_spec.rb | 2 +-
.../models/customer_relations/contact_spec.rb | 43 +++++++++++++------
.../customer_relations/issue_contact_spec.rb | 14 +++++-
.../customer_relations/organization_spec.rb | 23 ++++++++--
spec/models/group/crm_settings_spec.rb | 3 +-
spec/models/group_spec.rb | 36 ++++++++++++++++
spec/models/project_spec.rb | 19 ++++++++
spec/policies/issue_policy_spec.rb | 26 +++++++++++
.../mutations/issues/set_crm_contacts_spec.rb | 23 ++++++++++
.../issue_sidebar_basic_entity_spec.rb | 10 ++---
26 files changed, 227 insertions(+), 68 deletions(-)
diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb
index 71b54527e8dab..2c38625fd1f76 100644
--- a/app/controllers/groups/application_controller.rb
+++ b/app/controllers/groups/application_controller.rb
@@ -77,8 +77,8 @@ def has_project_list?
false
end
- def validate_root_group!
- render_404 unless group.root?
+ def validate_crm_group!
+ render_404 unless group.crm_group?
end
def authorize_action!(action)
diff --git a/app/controllers/groups/crm/contacts_controller.rb b/app/controllers/groups/crm/contacts_controller.rb
index 5bc927911c103..a6c4e688d8a90 100644
--- a/app/controllers/groups/crm/contacts_controller.rb
+++ b/app/controllers/groups/crm/contacts_controller.rb
@@ -4,7 +4,7 @@ class Groups::Crm::ContactsController < Groups::ApplicationController
feature_category :team_planning
urgency :low
- before_action :validate_root_group!
+ before_action :validate_crm_group!
before_action :authorize_read_crm_contact!
def new
diff --git a/app/controllers/groups/crm/organizations_controller.rb b/app/controllers/groups/crm/organizations_controller.rb
index ef5ddcdbca6dc..8d532cb06a534 100644
--- a/app/controllers/groups/crm/organizations_controller.rb
+++ b/app/controllers/groups/crm/organizations_controller.rb
@@ -4,7 +4,7 @@ class Groups::Crm::OrganizationsController < Groups::ApplicationController
feature_category :team_planning
urgency :low
- before_action :validate_root_group!
+ before_action :validate_crm_group!
before_action :authorize_read_crm_organization!
def new
diff --git a/app/finders/crm/contacts_finder.rb b/app/finders/crm/contacts_finder.rb
index 58ec4cf8a4776..cae383ec21ff5 100644
--- a/app/finders/crm/contacts_finder.rb
+++ b/app/finders/crm/contacts_finder.rb
@@ -27,9 +27,10 @@ def initialize(current_user, params = {})
end
def execute
- return CustomerRelations::Contact.none unless root_group
+ group = params[:group]&.crm_group
+ return CustomerRelations::Contact.none unless group && can?(@current_user, :read_crm_contact, group)
- contacts = root_group.contacts
+ contacts = group.contacts
contacts = by_ids(contacts)
contacts = by_state(contacts)
contacts = by_search(contacts)
@@ -52,16 +53,6 @@ def sort_contacts(contacts)
end
end
- def root_group
- strong_memoize(:root_group) do
- group = params[:group]&.root_ancestor
-
- next unless can?(@current_user, :read_crm_contact, group)
-
- group
- end
- end
-
def by_search(contacts)
return contacts unless search?
diff --git a/app/models/customer_relations/contact.rb b/app/models/customer_relations/contact.rb
index ba6a98b13a309..f8971ef013c41 100644
--- a/app/models/customer_relations/contact.rb
+++ b/app/models/customer_relations/contact.rb
@@ -27,7 +27,7 @@ class CustomerRelations::Contact < ApplicationRecord
validates :description, length: { maximum: 1024 }
validates :email, uniqueness: { case_sensitive: false, scope: :group_id }
validate :validate_email_format
- validate :validate_root_group
+ validate :validate_crm_group
scope :order_scope_asc, ->(field) { order(arel_table[field].asc.nulls_last) }
scope :order_scope_desc, ->(field) { order(arel_table[field].desc.nulls_last) }
@@ -156,9 +156,9 @@ def validate_email_format
self.errors.add(:email, I18n.t(:invalid, scope: 'valid_email.validations.email')) unless ValidateEmail.valid?(self.email)
end
- def validate_root_group
- return if group&.root?
+ def validate_crm_group
+ return if group&.crm_group?
- self.errors.add(:base, _('contacts can only be added to root groups'))
+ self.errors.add(:base, _('contacts can only be added to root groups and groups configured as CRM targets'))
end
end
diff --git a/app/models/customer_relations/issue_contact.rb b/app/models/customer_relations/issue_contact.rb
index 70a30e583d534..aa1269d79b95f 100644
--- a/app/models/customer_relations/issue_contact.rb
+++ b/app/models/customer_relations/issue_contact.rb
@@ -6,7 +6,7 @@ class CustomerRelations::IssueContact < ApplicationRecord
belongs_to :issue, optional: false, inverse_of: :customer_relations_contacts
belongs_to :contact, optional: false, inverse_of: :issue_contacts
- validate :contact_belongs_to_root_group
+ validate :contact_belongs_to_crm_group
BATCH_DELETE_SIZE = 1_000
@@ -34,11 +34,10 @@ def self.delete_for_group(group)
private
- def contact_belongs_to_root_group
+ def contact_belongs_to_crm_group
return unless contact&.group_id
- return unless issue&.project&.namespace_id
- return if issue.project.root_ancestor&.id == contact.group_id
+ return if issue&.resource_parent&.crm_group&.id == contact.group_id
- errors.add(:base, _("The contact does not belong to the issue group's root ancestor"))
+ errors.add(:base, _("The contact does not belong to the issue group's CRM source group"))
end
end
diff --git a/app/models/customer_relations/organization.rb b/app/models/customer_relations/organization.rb
index bfd6ea8539ac7..8b0e31f729892 100644
--- a/app/models/customer_relations/organization.rb
+++ b/app/models/customer_relations/organization.rb
@@ -21,7 +21,7 @@ class CustomerRelations::Organization < ApplicationRecord
validates :name, uniqueness: { case_sensitive: false, scope: [:group_id] }
validates :name, length: { maximum: 255 }
validates :description, length: { maximum: 1024 }
- validate :validate_root_group
+ validate :validate_crm_group
scope :order_scope_asc, ->(field) { order(arel_table[field].asc.nulls_last) }
scope :order_scope_desc, ->(field) { order(arel_table[field].desc.nulls_last) }
@@ -90,9 +90,9 @@ def self.default_state_counts
end
end
- def validate_root_group
- return if group&.root?
+ def validate_crm_group
+ return if group&.crm_group?
- self.errors.add(:base, _('organizations can only be added to root groups'))
+ self.errors.add(:base, _('organizations can only be added to root groups and groups configured as CRM targets'))
end
end
diff --git a/app/models/group.rb b/app/models/group.rb
index dca2f0020ee10..e5766e5abaadf 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -20,6 +20,7 @@ class Group < Namespace
include ChronicDurationAttribute
include RunnerTokenExpirationInterval
include Importable
+ include IdInOrdered
extend ::Gitlab::Utils::Override
@@ -99,6 +100,9 @@ def of_ancestors_and_self
# AR defaults to nullify when trying to delete via has_many associations unless we set dependent: :delete_all
has_many :crm_organizations, class_name: 'CustomerRelations::Organization', inverse_of: :group, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
has_many :contacts, class_name: 'CustomerRelations::Contact', inverse_of: :group, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
+ has_one :crm_settings, class_name: 'Group::CrmSettings', inverse_of: :group
+ # Groups for which this is the source of CRM contacts/organizations
+ has_many :crm_targets, class_name: 'Group::CrmSettings', inverse_of: :source_group
has_many :cluster_groups, class_name: 'Clusters::Group'
has_many :clusters, through: :cluster_groups, class_name: 'Clusters::Cluster'
@@ -143,8 +147,6 @@ def of_ancestors_and_self
delegate :subgroup_runner_token_expiration_interval, :subgroup_runner_token_expiration_interval=, :subgroup_runner_token_expiration_interval_human_readable, :subgroup_runner_token_expiration_interval_human_readable=, to: :namespace_settings, allow_nil: true
delegate :project_runner_token_expiration_interval, :project_runner_token_expiration_interval=, :project_runner_token_expiration_interval_human_readable, :project_runner_token_expiration_interval_human_readable=, to: :namespace_settings, allow_nil: true
- has_one :crm_settings, class_name: 'Group::CrmSettings', inverse_of: :group
-
accepts_nested_attributes_for :variables, allow_destroy: true
accepts_nested_attributes_for :group_feature, update_only: true
@@ -1021,6 +1023,21 @@ def hook_attrs
}
end
+ def crm_group
+ Group.id_in_ordered(traversal_ids.reverse)
+ .joins(:crm_settings)
+ .where.not(crm_settings: { source_group_id: nil })
+ .first&.crm_settings&.source_group || root_ancestor
+ end
+ strong_memoize_attr :crm_group
+
+ def crm_group?
+ return true if root? && crm_settings&.source_group_id.nil?
+
+ crm_targets.present?
+ end
+ strong_memoize_attr :crm_group?
+
private
def feature_flag_enabled_for_self_or_ancestor?(feature_flag, type: :development)
diff --git a/app/models/group/crm_settings.rb b/app/models/group/crm_settings.rb
index 30fbe6ae07f7d..c0a8868770838 100644
--- a/app/models/group/crm_settings.rb
+++ b/app/models/group/crm_settings.rb
@@ -5,6 +5,7 @@ class Group::CrmSettings < ApplicationRecord
self.table_name = 'group_crm_settings'
belongs_to :group, -> { where(type: Group.sti_name) }, foreign_key: 'group_id'
+ belongs_to :source_group, -> { where(type: Group.sti_name) }, foreign_key: 'source_group_id', class_name: 'Group'
validates :group, presence: true
end
diff --git a/app/models/project.rb b/app/models/project.rb
index e955939218225..2bf7fa75cedc6 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -3343,6 +3343,12 @@ def crm_enabled?
group.crm_enabled?
end
+ def crm_group
+ return unless group
+
+ group.crm_group
+ end
+
def supports_lock_on_merge?
group&.supports_lock_on_merge? || ::Feature.enabled?(:enforce_locked_labels_on_merge, self, type: :ops)
end
diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb
index 81926cccc0639..0857947d5c9e3 100644
--- a/app/policies/issue_policy.rb
+++ b/app/policies/issue_policy.rb
@@ -18,10 +18,10 @@ def epics_license_available?
@user && (@user.admin? || can?(:reporter_access) || assignee_or_author?) # rubocop:disable Cop/UserAdmin
end
- desc "Project belongs to a group, crm is enabled and user can read contacts in the root group"
+ desc "Project belongs to a group, crm is enabled and user can read contacts in source group"
condition(:can_read_crm_contacts, scope: :subject) do
subject_container&.crm_enabled? &&
- (@user&.can?(:read_crm_contact, subject_container.root_ancestor) || @user&.support_bot?)
+ (@user&.can?(:read_crm_contact, subject_container.crm_group) || @user&.support_bot?)
end
desc "Issue is confidential"
diff --git a/app/services/issues/set_crm_contacts_service.rb b/app/services/issues/set_crm_contacts_service.rb
index 1a697678ac009..19901a2677602 100644
--- a/app/services/issues/set_crm_contacts_service.rb
+++ b/app/services/issues/set_crm_contacts_service.rb
@@ -52,7 +52,7 @@ def add
end
def add_by_email
- contact_ids = ::CustomerRelations::Contact.find_ids_by_emails(container.root_ancestor, emails(:add_emails))
+ contact_ids = ::CustomerRelations::Contact.find_ids_by_emails(container.crm_group, emails(:add_emails))
add_by_id(contact_ids)
end
diff --git a/app/services/work_items/callbacks/crm_contacts.rb b/app/services/work_items/callbacks/crm_contacts.rb
index a78e2fc1bd92c..fae6ccbdcc534 100644
--- a/app/services/work_items/callbacks/crm_contacts.rb
+++ b/app/services/work_items/callbacks/crm_contacts.rb
@@ -52,7 +52,7 @@ def feature_enabled?
end
def group
- @group ||= work_item.resource_parent.root_ancestor
+ @group ||= work_item.resource_parent.crm_group
end
def operation_mode_attribute
diff --git a/lib/gitlab/quick_actions/issue_actions.rb b/lib/gitlab/quick_actions/issue_actions.rb
index 674fa1872d161..22198b5909e32 100644
--- a/lib/gitlab/quick_actions/issue_actions.rb
+++ b/lib/gitlab/quick_actions/issue_actions.rb
@@ -299,7 +299,7 @@ module IssueActions
types Issue
condition do
current_user.can?(:set_issue_crm_contacts, quick_action_target) &&
- CustomerRelations::Contact.exists_for_group?(quick_action_target.resource_parent.root_ancestor)
+ CustomerRelations::Contact.exists_for_group?(quick_action_target.resource_parent.crm_group)
end
execution_message do
_('One or more contacts were successfully added.')
diff --git a/lib/sidebars/groups/menus/customer_relations_menu.rb b/lib/sidebars/groups/menus/customer_relations_menu.rb
index a0d89248d815b..41e8734aca4bd 100644
--- a/lib/sidebars/groups/menus/customer_relations_menu.rb
+++ b/lib/sidebars/groups/menus/customer_relations_menu.rb
@@ -23,9 +23,7 @@ def sprite_icon
override :render?
def render?
- return false unless context.group.root?
-
- can_read_contact?
+ context.group.crm_group?
end
override :serialize_as_menu_item_args
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index ecc0a5aea154c..50e1059376c96 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -54690,7 +54690,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 issue group's root ancestor"
+msgid "The contact does not belong to the issue group's CRM source group"
msgstr ""
msgid "The content for this wiki page failed to load. To fix this error, reload the page."
@@ -64595,7 +64595,7 @@ msgstr ""
msgid "conaninfo is too large. Maximum size is %{max_size} characters"
msgstr ""
-msgid "contacts can only be added to root groups"
+msgid "contacts can only be added to root groups and groups configured as CRM targets"
msgstr ""
msgid "container registry images"
@@ -65776,7 +65776,7 @@ msgstr ""
msgid "or sign in with"
msgstr ""
-msgid "organizations can only be added to root groups"
+msgid "organizations can only be added to root groups and groups configured as CRM targets"
msgstr ""
msgid "packages"
diff --git a/spec/finders/crm/contacts_finder_spec.rb b/spec/finders/crm/contacts_finder_spec.rb
index c54cd39534f20..01ca086e6fd35 100644
--- a/spec/finders/crm/contacts_finder_spec.rb
+++ b/spec/finders/crm/contacts_finder_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe Crm::ContactsFinder do
+RSpec.describe Crm::ContactsFinder, feature_category: :team_planning do
let_it_be(:user) { create(:user) }
describe '#execute' do
diff --git a/spec/models/customer_relations/contact_spec.rb b/spec/models/customer_relations/contact_spec.rb
index 6f124662b8e98..af8ac7b698db9 100644
--- a/spec/models/customer_relations/contact_spec.rb
+++ b/spec/models/customer_relations/contact_spec.rb
@@ -28,6 +28,35 @@
it { is_expected.to validate_uniqueness_of(:email).case_insensitive.scoped_to(:group_id) }
it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email
+
+ context 'when root group' do
+ subject { build(:contact, group: group) }
+
+ it { is_expected.to be_valid }
+
+ context 'with group.source_group_id' do
+ let(:crm_settings) { build(:crm_settings, source_group_id: group.id) }
+ let(:root_group) { build(:group, crm_settings: crm_settings) }
+
+ subject { build(:contact, group: root_group) }
+
+ it { is_expected.to be_invalid }
+ end
+ end
+
+ context 'when subgroup' do
+ subject { build(:contact, group: build(:group, parent: group)) }
+
+ it { is_expected.to be_invalid }
+
+ context 'with group.crm_targets' do
+ let(:target_group) { build(:group, crm_targets: [build(:crm_settings)], parent: group) }
+
+ subject { build(:contact, group: target_group) }
+
+ it { is_expected.to be_valid }
+ end
+ end
end
describe '.reference_prefix' do
@@ -42,20 +71,6 @@
it { expect(described_class.reference_postfix).to eq(']') }
end
- describe '#root_group' do
- context 'when root group' do
- subject { build(:contact, group: group) }
-
- it { is_expected.to be_valid }
- end
-
- context 'when subgroup' do
- subject { build(:contact, group: create(:group, parent: group)) }
-
- it { is_expected.to be_invalid }
- end
- end
-
describe '#before_validation' do
it 'strips leading and trailing whitespace' do
contact = described_class.new(first_name: ' First ', last_name: ' Last ', phone: ' 123456 ')
diff --git a/spec/models/customer_relations/issue_contact_spec.rb b/spec/models/customer_relations/issue_contact_spec.rb
index 221378d26b295..fb213b45c5c98 100644
--- a/spec/models/customer_relations/issue_contact_spec.rb
+++ b/spec/models/customer_relations/issue_contact_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe CustomerRelations::IssueContact do
+RSpec.describe CustomerRelations::IssueContact, feature_category: :team_planning do
let_it_be(:issue_contact, reload: true) { create(:issue_customer_relations_contact) }
let_it_be(:group) { create(:group) }
let_it_be(:subgroup) { create(:group, parent: group) }
@@ -61,6 +61,18 @@
expect(built).to be_valid
end
+
+ it 'succeeds when the contact belongs to the issue CRM group that is not an ancestor' do
+ new_contact = create(:contact)
+ new_group = create(:group)
+ create(:crm_settings, group: new_group, source_group: new_contact.group)
+ new_project = build_stubbed(:project, group: new_group)
+ new_issue = build_stubbed(:issue, project: new_project)
+
+ built = build(:issue_customer_relations_contact, issue: new_issue, contact: new_contact)
+
+ expect(built).to be_valid
+ end
end
describe '#self.find_contact_ids_by_emails' do
diff --git a/spec/models/customer_relations/organization_spec.rb b/spec/models/customer_relations/organization_spec.rb
index 8151bf18beda2..9896099df4c90 100644
--- a/spec/models/customer_relations/organization_spec.rb
+++ b/spec/models/customer_relations/organization_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe CustomerRelations::Organization, type: :model do
+RSpec.describe CustomerRelations::Organization, type: :model, feature_category: :team_planning do
let_it_be(:group) { create(:group) }
describe 'associations' do
@@ -17,19 +17,34 @@
it { is_expected.to validate_uniqueness_of(:name).case_insensitive.scoped_to([:group_id]) }
it { is_expected.to validate_length_of(:name).is_at_most(255) }
it { is_expected.to validate_length_of(:description).is_at_most(1024) }
- end
- describe '#root_group' do
context 'when root group' do
subject { build(:crm_organization, group: group) }
it { is_expected.to be_valid }
+
+ context 'with group.source_group_id' do
+ let(:crm_settings) { build(:crm_settings, source_group_id: group.id) }
+ let(:root_group) { build(:group, crm_settings: crm_settings) }
+
+ subject { build(:crm_organization, group: root_group) }
+
+ it { is_expected.to be_invalid }
+ end
end
context 'when subgroup' do
- subject { build(:crm_organization, group: create(:group, parent: group)) }
+ subject { build(:crm_organization, group: build(:group, parent: group)) }
it { is_expected.to be_invalid }
+
+ context 'with group.crm_targets' do
+ let(:target_group) { build(:group, crm_targets: [build(:crm_settings)], parent: group) }
+
+ subject { build(:crm_organization, group: target_group) }
+
+ it { is_expected.to be_valid }
+ end
end
end
diff --git a/spec/models/group/crm_settings_spec.rb b/spec/models/group/crm_settings_spec.rb
index 35fcdca638969..7ca1ab7b7be35 100644
--- a/spec/models/group/crm_settings_spec.rb
+++ b/spec/models/group/crm_settings_spec.rb
@@ -2,9 +2,10 @@
require 'spec_helper'
-RSpec.describe Group::CrmSettings do
+RSpec.describe Group::CrmSettings, feature_category: :team_planning do
describe 'associations' do
it { is_expected.to belong_to(:group) }
+ it { is_expected.to belong_to(:source_group).optional }
end
describe 'validations' do
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 219b8ade9b867..5df5e9c267101 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -55,6 +55,7 @@
it { is_expected.to have_many(:contacts).class_name('CustomerRelations::Contact') }
it { is_expected.to have_many(:crm_organizations).class_name('CustomerRelations::Organization') }
+ it { is_expected.to have_many(:crm_targets).class_name('Group::CrmSettings').inverse_of(:source_group) }
it { is_expected.to have_many(:protected_branches).inverse_of(:group).with_foreign_key(:namespace_id) }
it { is_expected.to have_one(:crm_settings) }
it { is_expected.to have_one(:group_feature) }
@@ -3975,4 +3976,39 @@ def define_cache_expectations(cache_key)
})
end
end
+
+ describe '#crm_group' do
+ let!(:crm_group) { create(:group) }
+ let!(:root_group) { create(:group) }
+ let!(:parent_group) { create(:group, parent: root_group) }
+ let!(:child_group) { create(:group, parent: parent_group) }
+
+ context 'when the group has a source_group_id' do
+ let!(:crm_settings) { create(:crm_settings, group: child_group, source_group: crm_group) }
+
+ it 'returns the source_group' do
+ expect(child_group.crm_group).to eq(crm_group)
+ end
+ end
+
+ context 'when the group does not have a source_group_id but is a root group' do
+ it 'returns the root group' do
+ expect(root_group.crm_group).to eq(root_group)
+ end
+ end
+
+ context 'when the group has no source_group_id and is not a root group' do
+ context 'when a parent group has a source_group_id' do
+ let!(:crm_settings) { create(:crm_settings, group: parent_group, source_group: crm_group) }
+
+ it 'traverses up the hierarchy and returns the first group with a source_group_id' do
+ expect(child_group.crm_group).to eq(crm_group)
+ end
+ end
+
+ it 'returns the root group if no groups in the hierarchy have a source_group_id' do
+ expect(child_group.crm_group).to eq(root_group)
+ end
+ end
+ end
end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 2ca12b72dbc8e..8db06583a706f 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -9665,4 +9665,23 @@ def create_build(new_pipeline = pipeline, name = 'test')
expect(described_class.by_any_overlap_with_traversal_ids(project_1.namespace_id)).to contain_exactly(project_1, project_2)
end
end
+
+ describe '#crm_group' do
+ context 'when project does not belong to group' do
+ let(:project) { build(:project) }
+
+ it 'returns nil' do
+ expect(project.crm_group).to be_nil
+ end
+ end
+
+ context 'when project belongs to a group' do
+ let(:group) { build(:group) }
+ let(:project) { build(:project, group: group) }
+
+ it 'returns the group.crm_group' do
+ expect(project.crm_group).to be(group.crm_group)
+ end
+ end
+ end
end
diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb
index f39fea7207e2b..fe5dd28117e74 100644
--- a/spec/policies/issue_policy_spec.rb
+++ b/spec/policies/issue_policy_spec.rb
@@ -585,6 +585,32 @@ def permissions(user, issue)
expect(policies).to be_disallowed(:set_issue_crm_contacts)
end
end
+
+ context 'when custom crm_group configured' do
+ let_it_be(:crm_settings) { create(:crm_settings, source_group: create(:group)) }
+ let_it_be(:subgroup) { create(:group, parent: create(:group), crm_settings: crm_settings) }
+ let_it_be(:project) { create(:project, group: subgroup) }
+
+ context 'when custom crm_group guest' do
+ it 'is disallowed' do
+ subgroup.parent.add_reporter(user)
+ crm_settings.source_group.add_guest(user)
+
+ expect(policies).to be_disallowed(:read_crm_contacts)
+ expect(policies).to be_disallowed(:set_issue_crm_contacts)
+ end
+ end
+
+ context 'when custom crm_group reporter' do
+ it 'is allowed' do
+ subgroup.parent.add_reporter(user)
+ crm_settings.source_group.add_reporter(user)
+
+ expect(policies).to be_allowed(:read_crm_contacts)
+ expect(policies).to be_allowed(:set_issue_crm_contacts)
+ end
+ end
+ end
end
context 'when user is an inherited member from the group' do
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
index 05e0c9975574a..3c38396e9640a 100644
--- a/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb
+++ b/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb
@@ -132,6 +132,7 @@ def expected_contacts(contacts)
let(:group2) { create(:group) }
let(:contact) { create(:contact, group: group2) }
let(:contact_ids) { [global_id_of(contact)] }
+ let(:initial_contacts) { [] }
before do
group2.add_reporter(user)
@@ -143,6 +144,28 @@ def expected_contacts(contacts)
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
+
+ context 'when that group is configured as the subgroup contact source' do
+ let!(:crm_settings) { create(:crm_settings, group: subgroup, source_group: group2) }
+
+ 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))
+ .to match_array(expected_contacts([contact]))
+ end
+ end
+
+ context 'when that group is configured as the root group contact source' do
+ let!(:crm_settings) { create(:crm_settings, group: group, source_group: group2) }
+
+ 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))
+ .to match_array(expected_contacts([contact]))
+ end
+ end
end
context 'when attempting to add more than 6' do
diff --git a/spec/serializers/issue_sidebar_basic_entity_spec.rb b/spec/serializers/issue_sidebar_basic_entity_spec.rb
index bd885afefe618..48bd10ee7abb6 100644
--- a/spec/serializers/issue_sidebar_basic_entity_spec.rb
+++ b/spec/serializers/issue_sidebar_basic_entity_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe IssueSidebarBasicEntity do
+RSpec.describe IssueSidebarBasicEntity, feature_category: :team_planning do
let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, :repository, group: group) }
let_it_be(:user) { create(:user, developer_of: project) }
@@ -68,7 +68,7 @@
describe 'show_crm_contacts' do
using RSpec::Parameterized::TableSyntax
- where(:is_reporter, :contacts_exist_for_group, :expected) do
+ where(:is_reporter, :contacts_exist_for_crm_group, :expected) do
false | false | false
false | true | false
true | false | false
@@ -77,10 +77,10 @@
with_them do
it 'sets proper boolean value for show_crm_contacts' do
- allow(CustomerRelations::Contact).to receive(:exists_for_group?).with(group).and_return(contacts_exist_for_group)
+ allow(CustomerRelations::Contact).to receive(:exists_for_group?).with(group).and_return(contacts_exist_for_crm_group)
if is_reporter
- project.root_ancestor.add_reporter(user)
+ project.crm_group.add_reporter(user)
end
expect(entity[:show_crm_contacts]).to be(expected)
@@ -95,7 +95,7 @@
subject(:entity) { serializer.represent(subgroup_issue, serializer: 'sidebar') }
before do
- subgroup_project.root_ancestor.add_reporter(user)
+ subgroup_project.crm_group.add_reporter(user)
end
context 'with crm enabled' do
--
GitLab