diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb index 71b54527e8dab94e7f64126fedda261ebbbf3fee..2c38625fd1f767822ccc356f330bc49aee9976ea 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 5bc927911c1031dd324035b59f11b66d401423e1..a6c4e688d8a90a86c6067b0fe86230abe9a604bc 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 ef5ddcdbca6dc4d73c64d140e1e9892621e3e887..8d532cb06a53484b429897642471ad66a1ec5756 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 58ec4cf8a4776d454e79afde9f8543bd9feac6f6..cae383ec21ff54ad73ae5e54b5cd326e71a984d6 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 ba6a98b13a3091251dd2ea5e43139faac372bb06..f8971ef013c417a3bdd6647f1380a0dcd490c577 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 70a30e583d534ce2e10c467dd1f55851737574a2..aa1269d79b95f1d56f14c4a7a5beb40feddc105b 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 bfd6ea8539ac7a9511c0e6450da22732da0722fb..8b0e31f7298927299650be8eb35a35766bac7112 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 dca2f0020ee10b42a1bb90a39886bcd43d6fd9bb..e5766e5abaadfc2f9ece299c6aff8bfabbbb2bac 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 30fbe6ae07f7d1286c69d62a16a8e5697a8b87b2..c0a8868770838296312a3bb88576a0f4fe74b9fd 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 e9559392182256e2ca5d31258ca6ff8b73e2e2f1..2bf7fa75cedc659a28148598343356ee796ec3bc 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 81926cccc0639d1643f4554642bd2d6f9cd19619..0857947d5c9e34d5cb159e3d43627a719192eaac 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 1a697678ac009476658d785fcb3a709a6091ad84..19901a2677602849e529ab073cb1642bc3ff00f5 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 a78e2fc1bd92c4a098b6a9bfc678837a401a2fe6..fae6ccbdcc5347d49834c75d37fd823d67c0cae7 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 674fa1872d161c9c5f95d659df51c1998c8127f2..22198b5909e3277f2beaad02a5f1cf2cb1f0b794 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 a0d89248d815b48f38deb33af2a7edf1848d9c55..41e8734aca4bd2d98ce4fb65c5626edea750b47d 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 ecc0a5aea154c4e86156265a1b1fdb1b4aaa00d0..50e1059376c96b9cf55b7b612fdd15f0e27e0da1 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 c54cd39534f2090b555f010485022a1cf022daff..01ca086e6fd3558e58a5da8be8d00ee41414bc82 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 6f124662b8e98e016f504dd44c103e484268c58c..af8ac7b698db9f89d989892d0a91b33e4bd3c877 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 221378d26b295fb2cda226f2e60ee7565616cbc5..fb213b45c5c982e468c8e7f149f33c3809bdf0d1 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 8151bf18beda2999cb2d28acab073830e8620883..9896099df4c90b9c72e208901dfe14dd7524a392 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 35fcdca638969e3f0e262a48c219055cc387de83..7ca1ab7b7be357436451c42fc496437d5a09b12a 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 219b8ade9b8671d9b467f74f0fa1370bd17f1259..5df5e9c2671012562885513c7688e304d4969db6 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 2ca12b72dbc8e692db9f4390415fa2ddee08ef47..8db06583a706f3b03994a98170b02a0e9466fb9c 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 f39fea7207e2b9be5ff41accc9d8b65592f0921b..fe5dd28117e74958d90cad809d0bb3af0e93387e 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 05e0c9975574a86d106e886406af8fefa566cf9e..3c38396e9640aa2fcc8eef0cbeb85585885ba579 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 bd885afefe61817e634184fa1902d9c5f1d52ddc..48bd10ee7abb60d8e15449bdf22a9e579caf0f26 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