Commit 227c3221 authored by Lee Tickett's avatar Lee Tickett Committed by Vitali Tatarintev

Support CRM contacts only in root groups

To reduce complexity, it has been decided to only support
CRM contacts and organizations in root groups for now.

This MR ensures the CRM feature can only be enabled for
root groups, and updates validation to only check root
groups rather than the entire hierarchy.

Changelog: fixed
parent 4e1397b1
...@@ -17,10 +17,6 @@ module IssuableActions ...@@ -17,10 +17,6 @@ module IssuableActions
def show def show
respond_to do |format| respond_to do |format|
format.html do format.html do
@show_crm_contacts = issuable.is_a?(Issue) && # rubocop:disable Gitlab/ModuleWithInstanceVariables
can?(current_user, :read_crm_contact, issuable.project.group) &&
CustomerRelations::Contact.exists_for_group?(issuable.project.group)
@issuable_sidebar = serializer.represent(issuable, serializer: 'sidebar') # rubocop:disable Gitlab/ModuleWithInstanceVariables @issuable_sidebar = serializer.represent(issuable, serializer: 'sidebar') # rubocop:disable Gitlab/ModuleWithInstanceVariables
render 'show' render 'show'
end end
......
...@@ -82,6 +82,10 @@ class Groups::ApplicationController < ApplicationController ...@@ -82,6 +82,10 @@ class Groups::ApplicationController < ApplicationController
def has_project_list? def has_project_list?
false false
end end
def validate_root_group!
render_404 unless group.root?
end
end end
Groups::ApplicationController.prepend_mod_with('Groups::ApplicationController') Groups::ApplicationController.prepend_mod_with('Groups::ApplicationController')
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
class Groups::Crm::ContactsController < Groups::ApplicationController class Groups::Crm::ContactsController < Groups::ApplicationController
feature_category :team_planning feature_category :team_planning
before_action :validate_root_group!
before_action :authorize_read_crm_contact! before_action :authorize_read_crm_contact!
def new def new
......
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
class Groups::Crm::OrganizationsController < Groups::ApplicationController class Groups::Crm::OrganizationsController < Groups::ApplicationController
feature_category :team_planning feature_category :team_planning
before_action :validate_root_group!
before_action :authorize_read_crm_organization! before_action :authorize_read_crm_organization!
def new def new
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
module Groups module Groups
module CrmSettingsHelper module CrmSettingsHelper
def crm_feature_flag_enabled?(group) def crm_feature_available?(group)
Feature.enabled?(:customer_relations, group) Feature.enabled?(:customer_relations, group)
end end
end end
......
...@@ -23,8 +23,9 @@ class CustomerRelations::Contact < ApplicationRecord ...@@ -23,8 +23,9 @@ class CustomerRelations::Contact < ApplicationRecord
validates :last_name, presence: true, length: { maximum: 255 } validates :last_name, presence: true, length: { maximum: 255 }
validates :email, length: { maximum: 255 } validates :email, length: { maximum: 255 }
validates :description, length: { maximum: 1024 } validates :description, length: { maximum: 1024 }
validates :email, uniqueness: { scope: :group_id }
validate :validate_email_format validate :validate_email_format
validate :unique_email_for_group_hierarchy validate :validate_root_group
def self.reference_prefix def self.reference_prefix
'[contact:' '[contact:'
...@@ -41,14 +42,13 @@ class CustomerRelations::Contact < ApplicationRecord ...@@ -41,14 +42,13 @@ class CustomerRelations::Contact < ApplicationRecord
def self.find_ids_by_emails(group, emails) def self.find_ids_by_emails(group, emails)
raise ArgumentError, "Cannot lookup more than #{MAX_PLUCK} emails" if emails.length > MAX_PLUCK raise ArgumentError, "Cannot lookup more than #{MAX_PLUCK} emails" if emails.length > MAX_PLUCK
where(group_id: group.self_and_ancestor_ids, email: emails) where(group: group, email: emails).pluck(:id)
.pluck(:id)
end end
def self.exists_for_group?(group) def self.exists_for_group?(group)
return false unless group return false unless group
exists?(group_id: group.self_and_ancestor_ids) exists?(group: group)
end end
private private
...@@ -59,13 +59,9 @@ class CustomerRelations::Contact < ApplicationRecord ...@@ -59,13 +59,9 @@ class CustomerRelations::Contact < ApplicationRecord
self.errors.add(:email, I18n.t(:invalid, scope: 'valid_email.validations.email')) unless ValidateEmail.valid?(self.email) self.errors.add(:email, I18n.t(:invalid, scope: 'valid_email.validations.email')) unless ValidateEmail.valid?(self.email)
end end
def unique_email_for_group_hierarchy def validate_root_group
return unless group return if group&.root?
return unless email
duplicate_email_exists = CustomerRelations::Contact self.errors.add(:base, _('contacts can only be added to root groups'))
.where(group_id: group.self_and_hierarchy.pluck(:id), email: email)
.where.not(id: id).exists?
self.errors.add(:email, _('contact with same email already exists in group hierarchy')) if duplicate_email_exists
end end
end end
...@@ -6,7 +6,7 @@ class CustomerRelations::IssueContact < ApplicationRecord ...@@ -6,7 +6,7 @@ class CustomerRelations::IssueContact < ApplicationRecord
belongs_to :issue, optional: false, inverse_of: :customer_relations_contacts belongs_to :issue, optional: false, inverse_of: :customer_relations_contacts
belongs_to :contact, optional: false, inverse_of: :issue_contacts belongs_to :contact, optional: false, inverse_of: :issue_contacts
validate :contact_belongs_to_issue_group_or_ancestor validate :contact_belongs_to_root_group
def self.find_contact_ids_by_emails(issue_id, emails) def self.find_contact_ids_by_emails(issue_id, emails)
raise ArgumentError, "Cannot lookup more than #{MAX_PLUCK} emails" if emails.length > MAX_PLUCK raise ArgumentError, "Cannot lookup more than #{MAX_PLUCK} emails" if emails.length > MAX_PLUCK
...@@ -24,11 +24,11 @@ class CustomerRelations::IssueContact < ApplicationRecord ...@@ -24,11 +24,11 @@ class CustomerRelations::IssueContact < ApplicationRecord
private private
def contact_belongs_to_issue_group_or_ancestor def contact_belongs_to_root_group
return unless contact&.group_id return unless contact&.group_id
return unless issue&.project&.namespace_id return unless issue&.project&.namespace_id
return if issue.project.group&.self_and_ancestor_ids&.include?(contact.group_id) return if issue.project.root_ancestor&.id == contact.group_id
errors.add(:base, _('The contact does not belong to the issue group or an ancestor')) errors.add(:base, _("The contact does not belong to the issue group's root ancestor"))
end end
end end
...@@ -19,9 +19,18 @@ class CustomerRelations::Organization < ApplicationRecord ...@@ -19,9 +19,18 @@ class CustomerRelations::Organization < ApplicationRecord
validates :name, uniqueness: { case_sensitive: false, scope: [:group_id] } validates :name, uniqueness: { case_sensitive: false, scope: [:group_id] }
validates :name, length: { maximum: 255 } validates :name, length: { maximum: 255 }
validates :description, length: { maximum: 1024 } validates :description, length: { maximum: 1024 }
validate :validate_root_group
def self.find_by_name(group_id, name) def self.find_by_name(group_id, name)
where(group: group_id) where(group: group_id)
.where('LOWER(name) = LOWER(?)', name) .where('LOWER(name) = LOWER(?)', name)
end end
private
def validate_root_group
return if group&.root?
self.errors.add(:base, _('organizations can only be added to root groups'))
end
end end
...@@ -13,7 +13,7 @@ class IssuePolicy < IssuablePolicy ...@@ -13,7 +13,7 @@ class IssuePolicy < IssuablePolicy
end end
desc "User can read contacts belonging to the issue group" 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) } condition(:can_read_crm_contacts, scope: :subject) { @user.can?(:read_crm_contact, @subject.project.root_ancestor) }
desc "Issue is confidential" desc "Issue is confidential"
condition(:confidential, scope: :subject) { @subject.confidential? } condition(:confidential, scope: :subject) { @subject.confidential? }
......
...@@ -10,6 +10,11 @@ class IssueSidebarBasicEntity < IssuableSidebarBasicEntity ...@@ -10,6 +10,11 @@ class IssueSidebarBasicEntity < IssuableSidebarBasicEntity
can?(current_user, :update_escalation_status, issue.project) can?(current_user, :update_escalation_status, issue.project)
end end
end end
expose :show_crm_contacts do |issuable|
current_user&.can?(:read_crm_contact, issuable.project.root_ancestor) &&
CustomerRelations::Contact.exists_for_group?(issuable.project.root_ancestor)
end
end end
IssueSidebarBasicEntity.prepend_mod_with('IssueSidebarBasicEntity') IssueSidebarBasicEntity.prepend_mod_with('IssueSidebarBasicEntity')
...@@ -52,7 +52,7 @@ module Issues ...@@ -52,7 +52,7 @@ module Issues
end end
def add_by_email def add_by_email
contact_ids = ::CustomerRelations::Contact.find_ids_by_emails(project_group, emails(:add_emails)) contact_ids = ::CustomerRelations::Contact.find_ids_by_emails(project_group.root_ancestor, emails(:add_emails))
add_by_id(contact_ids) add_by_id(contact_ids)
end end
......
...@@ -43,7 +43,7 @@ ...@@ -43,7 +43,7 @@
= render_if_exists 'groups/personal_access_token_expiration_policy', f: f, group: @group = render_if_exists 'groups/personal_access_token_expiration_policy', f: f, group: @group
= render 'groups/settings/membership', f: f, group: @group = render 'groups/settings/membership', f: f, group: @group
- if crm_feature_flag_enabled?(@group) - if crm_feature_available?(@group)
%h5= _('Customer relations') %h5= _('Customer relations')
.form-group.gl-mb-3 .form-group.gl-mb-3
= f.gitlab_ui_checkbox_component :crm_enabled, = f.gitlab_ui_checkbox_component :crm_enabled,
......
...@@ -41,7 +41,7 @@ ...@@ -41,7 +41,7 @@
.block{ class: 'gl-pt-0! gl-collapse-empty', data: { qa_selector: 'iteration_container', testid: 'iteration_container' } }< .block{ class: 'gl-pt-0! gl-collapse-empty', data: { qa_selector: 'iteration_container', testid: 'iteration_container' } }<
= render_if_exists 'shared/issuable/iteration_select', can_edit: can_edit_issuable.to_s, group_path: @project.group.full_path, project_path: issuable_sidebar[:project_full_path], issue_iid: issuable_sidebar[:iid], issuable_type: issuable_type = render_if_exists 'shared/issuable/iteration_select', can_edit: can_edit_issuable.to_s, group_path: @project.group.full_path, project_path: issuable_sidebar[:project_full_path], issue_iid: issuable_sidebar[:iid], issuable_type: issuable_type
- if @show_crm_contacts - if issuable_sidebar[:show_crm_contacts]
.block.contact .block.contact
#js-issue-crm-contacts{ data: { issue_id: issuable_sidebar[:id] } } #js-issue-crm-contacts{ data: { issue_id: issuable_sidebar[:id] } }
......
...@@ -6,15 +6,18 @@ info: To determine the technical writer assigned to the Stage/Group associated w ...@@ -6,15 +6,18 @@ info: To determine the technical writer assigned to the Stage/Group associated w
# Customer relations management (CRM) **(FREE)** # Customer relations management (CRM) **(FREE)**
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/2256) in GitLab 14.6 [with a flag](../../administration/feature_flags.md) named `customer_relations`. Disabled by default.
FLAG: FLAG:
On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the feature flag](../../administration/feature_flags.md) named `customer_relations`. On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the feature flag](../../administration/feature_flags.md) named `customer_relations`.
On GitLab.com, this feature is not available. On GitLab.com, this feature is not available.
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/2256) in GitLab 14.6 [with a flag](../../administration/feature_flags.md) named `customer_relations`. Disabled by default.
> - In GitLab 14.8 and later, you can [create contacts and organizations only in root groups](https://gitlab.com/gitlab-org/gitlab/-/issues/350634).
With customer relations management (CRM) you can create a record of contacts With customer relations management (CRM) you can create a record of contacts
(individuals) and organizations (companies) and relate them to issues. (individuals) and organizations (companies) and relate them to issues.
Contacts and organizations can only be created for root groups.
You can use contacts and organizations to tie work to customers for billing and reporting purposes. You can use contacts and organizations to tie work to customers for billing and reporting purposes.
To read more about what is planned for the future, see [issue 2256](https://gitlab.com/gitlab-org/gitlab/-/issues/2256). To read more about what is planned for the future, see [issue 2256](https://gitlab.com/gitlab-org/gitlab/-/issues/2256).
......
...@@ -203,6 +203,18 @@ RSpec.describe 'Group navbar' do ...@@ -203,6 +203,18 @@ RSpec.describe 'Group navbar' do
it_behaves_like 'verified navigation bar' it_behaves_like 'verified navigation bar'
end end
context 'when customer relations feature and flag is enabled but subgroup' do
let(:group) { create(:group, :crm_enabled, parent: create(:group)) }
before do
stub_feature_flags(customer_relations: true)
visit group_path(group)
end
it_behaves_like 'verified navigation bar'
end
end end
context 'when iterations are available' do context 'when iterations are available' do
......
...@@ -291,7 +291,7 @@ module Gitlab ...@@ -291,7 +291,7 @@ module Gitlab
types Issue types Issue
condition do condition do
current_user.can?(:set_issue_crm_contacts, quick_action_target) && current_user.can?(:set_issue_crm_contacts, quick_action_target) &&
CustomerRelations::Contact.exists_for_group?(quick_action_target.project.group) CustomerRelations::Contact.exists_for_group?(quick_action_target.project.root_ancestor)
end end
execution_message do execution_message do
_('One or more contacts were successfully added.') _('One or more contacts were successfully added.')
...@@ -306,7 +306,7 @@ module Gitlab ...@@ -306,7 +306,7 @@ module Gitlab
types Issue types Issue
condition do condition do
current_user.can?(:set_issue_crm_contacts, quick_action_target) && current_user.can?(:set_issue_crm_contacts, quick_action_target) &&
CustomerRelations::Contact.exists_for_group?(quick_action_target.project.group) CustomerRelations::Contact.exists_for_group?(quick_action_target.project.root_ancestor)
end end
execution_message do execution_message do
_('One or more contacts were successfully removed.') _('One or more contacts were successfully removed.')
......
...@@ -24,6 +24,8 @@ module Sidebars ...@@ -24,6 +24,8 @@ module Sidebars
override :render? override :render?
def render? def render?
return false unless context.group.root?
can_read_contact? || can_read_organization? can_read_contact? || can_read_organization?
end end
......
...@@ -36526,7 +36526,7 @@ msgstr "" ...@@ -36526,7 +36526,7 @@ msgstr ""
msgid "The connection will time out after %{timeout}. For repositories that take longer, use a clone/push combination." msgid "The connection will time out after %{timeout}. For repositories that take longer, use a clone/push combination."
msgstr "" msgstr ""
msgid "The contact does not belong to the issue group or an ancestor" msgid "The contact does not belong to the issue group's root ancestor"
msgstr "" msgstr ""
msgid "The content editor may change the markdown formatting style of the document, which may not match your original markdown style." msgid "The content editor may change the markdown formatting style of the document, which may not match your original markdown style."
...@@ -43315,7 +43315,7 @@ msgstr "" ...@@ -43315,7 +43315,7 @@ msgstr ""
msgid "compliance violation has already been recorded" msgid "compliance violation has already been recorded"
msgstr "" msgstr ""
msgid "contact with same email already exists in group hierarchy" msgid "contacts can only be added to root groups"
msgstr "" msgstr ""
msgid "container registry images" msgid "container registry images"
...@@ -44225,6 +44225,9 @@ msgstr "" ...@@ -44225,6 +44225,9 @@ msgstr ""
msgid "or" msgid "or"
msgstr "" msgstr ""
msgid "organizations can only be added to root groups"
msgstr ""
msgid "other card matches" msgid "other card matches"
msgstr "" msgstr ""
......
...@@ -21,7 +21,7 @@ FactoryBot.define do ...@@ -21,7 +21,7 @@ FactoryBot.define do
trait :for_issue do trait :for_issue do
issue { raise ArgumentError, '`issue` is manadatory' } issue { raise ArgumentError, '`issue` is manadatory' }
contact { association(:contact, group: issue.project.group) } contact { association(:contact, group: issue.project.root_ancestor) }
end end
end end
end end
...@@ -59,6 +59,18 @@ RSpec.describe 'Group navbar' do ...@@ -59,6 +59,18 @@ RSpec.describe 'Group navbar' do
it_behaves_like 'verified navigation bar' it_behaves_like 'verified navigation bar'
end end
context 'when customer_relations feature and flag is enabled but subgroup' do
let(:group) { create(:group, :crm_enabled, parent: create(:group)) }
before do
stub_feature_flags(customer_relations: true)
visit group_path(group)
end
it_behaves_like 'verified navigation bar'
end
context 'when dependency proxy is available' do context 'when dependency proxy is available' do
before do before do
stub_config(dependency_proxy: { enabled: true }) stub_config(dependency_proxy: { enabled: true })
......
...@@ -3,13 +3,16 @@ ...@@ -3,13 +3,16 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Groups::CrmSettingsHelper do RSpec.describe Groups::CrmSettingsHelper do
let_it_be(:group) { create(:group) } let_it_be(:root_group) { create(:group) }
describe '#crm_feature_flag_enabled?' do describe '#crm_feature_available?' do
subject do subject do
helper.crm_feature_flag_enabled?(group) helper.crm_feature_available?(group)
end end
context 'in root group' do
let(:group) { root_group }
context 'when feature flag is enabled' do context 'when feature flag is enabled' do
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
end end
...@@ -22,4 +25,23 @@ RSpec.describe Groups::CrmSettingsHelper do ...@@ -22,4 +25,23 @@ RSpec.describe Groups::CrmSettingsHelper do
it { is_expected.to be_falsy } it { is_expected.to be_falsy }
end end
end end
context 'in subgroup' do
let_it_be(:subgroup) { create(:group, parent: root_group) }
let(:group) { subgroup }
context 'when feature flag is enabled' do
it { is_expected.to be_truthy }
end
context 'when feature flag is disabled' do
before do
stub_feature_flags(customer_relations: false)
end
it { is_expected.to be_falsy }
end
end
end
end end
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe CustomerRelations::Contact, type: :model do RSpec.describe CustomerRelations::Contact, type: :model do
let_it_be(:group) { create(:group) }
describe 'associations' do describe 'associations' do
it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:group) }
it { is_expected.to belong_to(:organization).optional } it { is_expected.to belong_to(:organization).optional }
...@@ -23,6 +25,8 @@ RSpec.describe CustomerRelations::Contact, type: :model do ...@@ -23,6 +25,8 @@ RSpec.describe CustomerRelations::Contact, type: :model do
it { is_expected.to validate_length_of(:email).is_at_most(255) } it { is_expected.to validate_length_of(:email).is_at_most(255) }
it { is_expected.to validate_length_of(:description).is_at_most(1024) } it { is_expected.to validate_length_of(:description).is_at_most(1024) }
it { is_expected.to validate_uniqueness_of(:email).scoped_to(:group_id) }
it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email
end end
...@@ -38,33 +42,15 @@ RSpec.describe CustomerRelations::Contact, type: :model do ...@@ -38,33 +42,15 @@ RSpec.describe CustomerRelations::Contact, type: :model do
it { expect(described_class.reference_postfix).to eq(']') } it { expect(described_class.reference_postfix).to eq(']') }
end end
describe '#unique_email_for_group_hierarchy' do describe '#root_group' do
let_it_be(:parent) { create(:group) } context 'when root group' do
let_it_be(:group) { create(:group, parent: parent) }
let_it_be(:subgroup) { create(:group, parent: group) }
let_it_be(:existing_contact) { create(:contact, group: group) }
context 'with unique email for group hierarchy' do
subject { build(:contact, group: group) } subject { build(:contact, group: group) }
it { is_expected.to be_valid } it { is_expected.to be_valid }
end end
context 'with duplicate email in group' do context 'when subgroup' do
subject { build(:contact, email: existing_contact.email, group: group) } subject { build(:contact, group: create(:group, parent: group)) }
it { is_expected.to be_invalid }
end
context 'with duplicate email in parent group' do
subject { build(:contact, email: existing_contact.email, group: subgroup) }
it { is_expected.to be_invalid }
end
context 'with duplicate email in subgroup' do
subject { build(:contact, email: existing_contact.email, group: parent) }
it { is_expected.to be_invalid } it { is_expected.to be_invalid }
end end
...@@ -82,7 +68,6 @@ RSpec.describe CustomerRelations::Contact, type: :model do ...@@ -82,7 +68,6 @@ RSpec.describe CustomerRelations::Contact, type: :model do
end end
describe '#self.find_ids_by_emails' do describe '#self.find_ids_by_emails' do
let_it_be(:group) { create(:group) }
let_it_be(:group_contacts) { create_list(:contact, 2, group: group) } let_it_be(:group_contacts) { create_list(:contact, 2, group: group) }
let_it_be(:other_contacts) { create_list(:contact, 2) } let_it_be(:other_contacts) { create_list(:contact, 2) }
...@@ -92,13 +77,6 @@ RSpec.describe CustomerRelations::Contact, type: :model do ...@@ -92,13 +77,6 @@ RSpec.describe CustomerRelations::Contact, type: :model do
expect(contact_ids).to match_array(group_contacts.pluck(:id)) expect(contact_ids).to match_array(group_contacts.pluck(:id))
end end
it 'returns ids of contacts from parent group' do
subgroup = create(:group, parent: group)
contact_ids = described_class.find_ids_by_emails(subgroup, group_contacts.pluck(:email))
expect(contact_ids).to match_array(group_contacts.pluck(:id))
end
it 'does not return ids of contacts from other groups' do it 'does not return ids of contacts from other groups' do
contact_ids = described_class.find_ids_by_emails(group, other_contacts.pluck(:email)) contact_ids = described_class.find_ids_by_emails(group, other_contacts.pluck(:email))
...@@ -112,28 +90,17 @@ RSpec.describe CustomerRelations::Contact, type: :model do ...@@ -112,28 +90,17 @@ RSpec.describe CustomerRelations::Contact, type: :model do
end end
describe '#self.exists_for_group?' do describe '#self.exists_for_group?' do
let(:group) { create(:group) } context 'with no contacts in group' do
let(:subgroup) { create(:group, parent: group) }
context 'with no contacts in group or parent' do
it 'returns false' do it 'returns false' do
expect(described_class.exists_for_group?(subgroup)).to be_falsey expect(described_class.exists_for_group?(group)).to be_falsey
end end
end end
context 'with contacts in group' do context 'with contacts in group' do
it 'returns true' do
create(:contact, group: subgroup)
expect(described_class.exists_for_group?(subgroup)).to be_truthy
end
end
context 'with contacts in parent' do
it 'returns true' do it 'returns true' do
create(:contact, group: group) create(:contact, group: group)
expect(described_class.exists_for_group?(subgroup)).to be_truthy expect(described_class.exists_for_group?(group)).to be_truthy
end end
end end
end end
......
...@@ -6,7 +6,8 @@ RSpec.describe CustomerRelations::IssueContact do ...@@ -6,7 +6,8 @@ RSpec.describe CustomerRelations::IssueContact do
let_it_be(:issue_contact, reload: true) { create(:issue_customer_relations_contact) } let_it_be(:issue_contact, reload: true) { create(:issue_customer_relations_contact) }
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:subgroup) { create(:group, parent: group) } let_it_be(:subgroup) { create(:group, parent: group) }
let_it_be(:project) { create(:project, group: subgroup) } let_it_be(:project) { create(:project, group: group) }
let_it_be(:subgroup_project) { create(:project, group: subgroup) }
let_it_be(:issue) { create(:issue, project: project) } let_it_be(:issue) { create(:issue, project: project) }
subject { issue_contact } subject { issue_contact }
...@@ -27,33 +28,36 @@ RSpec.describe CustomerRelations::IssueContact do ...@@ -27,33 +28,36 @@ RSpec.describe CustomerRelations::IssueContact do
let(:for_issue) { build(:issue_customer_relations_contact, :for_issue, issue: issue) } let(:for_issue) { build(:issue_customer_relations_contact, :for_issue, issue: issue) }
let(:for_contact) { build(:issue_customer_relations_contact, :for_contact, contact: contact) } let(:for_contact) { build(:issue_customer_relations_contact, :for_contact, contact: contact) }
context 'for root groups' do
it 'uses objects from the same group', :aggregate_failures do it 'uses objects from the same group', :aggregate_failures do
expect(stubbed.contact.group).to eq(stubbed.issue.project.group) expect(stubbed.contact.group).to eq(stubbed.issue.project.group)
expect(built.contact.group).to eq(built.issue.project.group) expect(built.contact.group).to eq(built.issue.project.group)
expect(created.contact.group).to eq(created.issue.project.group) expect(created.contact.group).to eq(created.issue.project.group)
end end
end
it 'builds using the same group', :aggregate_failures do context 'for subgroups' do
expect(for_issue.contact.group).to eq(subgroup) it 'builds using the root ancestor' do
expect(for_contact.issue.project.group).to eq(group) expect(for_issue.contact.group).to eq(group)
end
end end
end end
describe 'validation' do describe 'validation' do
it 'fails when the contact group does not belong to the issue group or ancestors' do it 'fails when the contact group is unrelated to the issue group' do
built = build(:issue_customer_relations_contact, issue: create(:issue), contact: create(:contact)) built = build(:issue_customer_relations_contact, issue: create(:issue), contact: create(:contact))
expect(built).not_to be_valid expect(built).not_to be_valid
end end
it 'succeeds when the contact group is the same as the issue group' do it 'succeeds when the contact belongs to a root group and is the same as the issue group' do
built = build(:issue_customer_relations_contact, issue: create(:issue, project: project), contact: create(:contact, group: subgroup)) built = build(:issue_customer_relations_contact, issue: create(:issue, project: project), contact: create(:contact, group: group))
expect(built).to be_valid expect(built).to be_valid
end end
it 'succeeds when the contact group is an ancestor of the issue group' do it 'succeeds when the contact belongs to a root group and it is an ancestor of the issue group' do
built = build(:issue_customer_relations_contact, issue: create(:issue, project: project), contact: create(:contact, group: group)) built = build(:issue_customer_relations_contact, issue: create(:issue, project: subgroup_project), contact: create(:contact, group: group))
expect(built).to be_valid expect(built).to be_valid
end end
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe CustomerRelations::Organization, type: :model do RSpec.describe CustomerRelations::Organization, type: :model do
let_it_be(:group) { create(:group) }
describe 'associations' do describe 'associations' do
it { is_expected.to belong_to(:group).with_foreign_key('group_id') } it { is_expected.to belong_to(:group).with_foreign_key('group_id') }
end end
...@@ -17,6 +19,20 @@ RSpec.describe CustomerRelations::Organization, type: :model do ...@@ -17,6 +19,20 @@ RSpec.describe CustomerRelations::Organization, type: :model do
it { is_expected.to validate_length_of(:description).is_at_most(1024) } it { is_expected.to validate_length_of(:description).is_at_most(1024) }
end end
describe '#root_group' do
context 'when root group' do
subject { build(:organization, group: group) }
it { is_expected.to be_valid }
end
context 'when subgroup' do
subject { build(:organization, group: create(:group, parent: group)) }
it { is_expected.to be_invalid }
end
end
describe '#name' do describe '#name' do
it 'strips name' do it 'strips name' do
organization = described_class.new(name: ' GitLab ') organization = described_class.new(name: ' GitLab ')
...@@ -27,7 +43,6 @@ RSpec.describe CustomerRelations::Organization, type: :model do ...@@ -27,7 +43,6 @@ RSpec.describe CustomerRelations::Organization, type: :model do
end end
describe '#find_by_name' do describe '#find_by_name' do
let!(:group) { create(:group) }
let!(:organiztion1) { create(:organization, group: group, name: 'Test') } let!(:organiztion1) { create(:organization, group: group, name: 'Test') }
let!(:organiztion2) { create(:organization, group: create(:group), name: 'Test') } let!(:organiztion2) { create(:organization, group: create(:group), name: 'Test') }
......
...@@ -2883,7 +2883,14 @@ RSpec.describe Group do ...@@ -2883,7 +2883,14 @@ RSpec.describe Group do
expect(group.crm_enabled?).to be_truthy expect(group.crm_enabled?).to be_truthy
end end
it 'returns true where crm_settings.state is enabled for subgroup' do
subgroup = create(:group, :crm_enabled, parent: group)
expect(subgroup.crm_enabled?).to be_truthy
end end
end
describe '.get_ids_by_ids_or_paths' do describe '.get_ids_by_ids_or_paths' do
let(:group_path) { 'group_path' } let(:group_path) { 'group_path' }
let!(:group) { create(:group, path: group_path) } let!(:group) { create(:group, path: group_path) }
......
...@@ -1167,7 +1167,6 @@ RSpec.describe Issue do ...@@ -1167,7 +1167,6 @@ RSpec.describe Issue do
end end
describe '#check_for_spam?' do describe '#check_for_spam?' do
using RSpec::Parameterized::TableSyntax
let_it_be(:support_bot) { ::User.support_bot } let_it_be(:support_bot) { ::User.support_bot }
where(:support_bot?, :visibility_level, :confidential, :new_attributes, :check_for_spam?) do where(:support_bot?, :visibility_level, :confidential, :new_attributes, :check_for_spam?) do
......
...@@ -396,4 +396,36 @@ RSpec.describe IssuePolicy do ...@@ -396,4 +396,36 @@ RSpec.describe IssuePolicy do
expect(policies).to be_allowed(:read_issue_iid) expect(policies).to be_allowed(:read_issue_iid)
end end
end end
describe 'set_issue_crm_contacts' do
let(:user) { create(:user) }
let(:subgroup) { create(:group, :crm_enabled, parent: create(:group, :crm_enabled)) }
let(:project) { create(:project, group: subgroup) }
let(:issue) { create(:issue, project: project) }
let(:policies) { described_class.new(user, issue) }
context 'when project reporter' do
it 'is disallowed' do
project.add_reporter(user)
expect(policies).to be_disallowed(:set_issue_crm_contacts)
end
end
context 'when subgroup reporter' do
it 'is allowed' do
subgroup.add_reporter(user)
expect(policies).to be_disallowed(:set_issue_crm_contacts)
end
end
context 'when root group reporter' do
it 'is allowed' do
subgroup.parent.add_reporter(user)
expect(policies).to be_allowed(:set_issue_crm_contacts)
end
end
end
end end
...@@ -9,12 +9,10 @@ RSpec.describe 'Setting issues crm contacts' do ...@@ -9,12 +9,10 @@ RSpec.describe 'Setting issues crm contacts' do
let_it_be(:group) { create(:group, :crm_enabled) } let_it_be(:group) { create(:group, :crm_enabled) }
let_it_be(:subgroup) { create(:group, :crm_enabled, parent: group) } let_it_be(:subgroup) { create(:group, :crm_enabled, parent: group) }
let_it_be(:project) { create(:project, group: subgroup) } let_it_be(:project) { create(:project, group: subgroup) }
let_it_be(:group_contacts) { create_list(:contact, 4, group: group) } let_it_be(:contacts) { create_list(:contact, 4, group: group) }
let_it_be(:subgroup_contacts) { create_list(:contact, 4, group: subgroup) }
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: project) }
let(:operation_mode) { Types::MutationOperationModeEnum.default_mode } let(:operation_mode) { Types::MutationOperationModeEnum.default_mode }
let(:contacts) { subgroup_contacts }
let(:initial_contacts) { contacts[0..1] } let(:initial_contacts) { contacts[0..1] }
let(:mutation_contacts) { contacts[1..2] } let(:mutation_contacts) { contacts[1..2] }
let(:contact_ids) { contact_global_ids(mutation_contacts) } let(:contact_ids) { contact_global_ids(mutation_contacts) }
...@@ -116,15 +114,7 @@ RSpec.describe 'Setting issues crm contacts' do ...@@ -116,15 +114,7 @@ RSpec.describe 'Setting issues crm contacts' do
end end
end end
context 'with issue group contacts' do
let(:contacts) { subgroup_contacts }
it_behaves_like 'successful mutation' it_behaves_like 'successful mutation'
end
context 'with issue ancestor group contacts' do
it_behaves_like 'successful mutation'
end
context 'when the contact does not exist' do context 'when the contact does not exist' do
let(:contact_ids) { ["gid://gitlab/CustomerRelations::Contact/#{non_existing_record_id}"] } let(:contact_ids) { ["gid://gitlab/CustomerRelations::Contact/#{non_existing_record_id}"] }
......
...@@ -49,6 +49,12 @@ RSpec.describe Groups::Crm::ContactsController do ...@@ -49,6 +49,12 @@ RSpec.describe Groups::Crm::ContactsController do
it_behaves_like 'response with 404 status' it_behaves_like 'response with 404 status'
end end
context 'when subgroup' do
let(:group) { create(:group, :private, :crm_enabled, parent: create(:group)) }
it_behaves_like 'response with 404 status'
end
end end
context 'with unauthorized user' do context 'with unauthorized user' do
......
...@@ -49,6 +49,12 @@ RSpec.describe Groups::Crm::OrganizationsController do ...@@ -49,6 +49,12 @@ RSpec.describe Groups::Crm::OrganizationsController do
it_behaves_like 'response with 404 status' it_behaves_like 'response with 404 status'
end end
context 'when subgroup' do
let(:group) { create(:group, :private, :crm_enabled, parent: create(:group)) }
it_behaves_like 'response with 404 status'
end
end end
context 'with unauthorized user' do context 'with unauthorized user' do
......
...@@ -3,9 +3,10 @@ ...@@ -3,9 +3,10 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe IssueSidebarBasicEntity do RSpec.describe IssueSidebarBasicEntity do
let_it_be(:project) { create(:project, :repository) } let_it_be(:group) { create(:group, :crm_enabled) }
let_it_be(:project) { create(:project, :repository, group: group) }
let_it_be(:user) { create(:user, developer_projects: [project]) } let_it_be(:user) { create(:user, developer_projects: [project]) }
let_it_be(:issue) { create(:issue, project: project, assignees: [user]) } let_it_be_with_reload(:issue) { create(:issue, project: project, assignees: [user]) }
let(:serializer) { IssueSerializer.new(current_user: user, project: project) } let(:serializer) { IssueSerializer.new(current_user: user, project: project) }
...@@ -71,4 +72,27 @@ RSpec.describe IssueSidebarBasicEntity do ...@@ -71,4 +72,27 @@ RSpec.describe IssueSidebarBasicEntity do
end end
end end
end end
describe 'show_crm_contacts' do
using RSpec::Parameterized::TableSyntax
where(:is_reporter, :contacts_exist_for_group, :expected) do
false | false | false
false | true | false
true | false | false
true | true | true
end
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)
if is_reporter
project.root_ancestor.add_reporter(user)
end
expect(entity[:show_crm_contacts]).to be(expected)
end
end
end
end end
...@@ -5,7 +5,7 @@ require 'spec_helper' ...@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe Issues::SetCrmContactsService do RSpec.describe Issues::SetCrmContactsService do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group, :crm_enabled) } let_it_be(:group) { create(:group, :crm_enabled) }
let_it_be(:project) { create(:project, group: group) } let_it_be(:project) { create(:project, group: create(:group, parent: group)) }
let_it_be(:contacts) { create_list(:contact, 4, group: group) } let_it_be(:contacts) { create_list(:contact, 4, group: group) }
let_it_be(:issue, reload: true) { create(:issue, project: project) } let_it_be(:issue, reload: true) { create(:issue, project: project) }
let_it_be(:issue_contact_1) do let_it_be(:issue_contact_1) do
......
...@@ -203,7 +203,7 @@ RSpec.shared_context 'group navbar structure' do ...@@ -203,7 +203,7 @@ RSpec.shared_context 'group navbar structure' do
nav_sub_items: [] nav_sub_items: []
}, },
{ {
nav_item: _('Group information'), nav_item: group.root? ? _('Group information') : _('Subgroup information'),
nav_sub_items: [ nav_sub_items: [
_('Activity'), _('Activity'),
_('Labels'), _('Labels'),
......
...@@ -11,7 +11,7 @@ RSpec.describe 'shared/issuable/_sidebar.html.haml' do ...@@ -11,7 +11,7 @@ RSpec.describe 'shared/issuable/_sidebar.html.haml' do
end end
context 'project in a group' do context 'project in a group' do
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group, :crm_enabled) }
let_it_be(:project) { create(:project, group: group) } let_it_be(:project) { create(:project, group: group) }
let_it_be(:issue) { create(:issue, project: project) } let_it_be(:issue) { create(:issue, project: project) }
let_it_be(:incident) { create(:incident, project: project) } let_it_be(:incident) { create(:incident, project: project) }
...@@ -35,5 +35,34 @@ RSpec.describe 'shared/issuable/_sidebar.html.haml' do ...@@ -35,5 +35,34 @@ RSpec.describe 'shared/issuable/_sidebar.html.haml' do
expect(rendered).not_to have_css('[data-testid="escalation_status_container"]') expect(rendered).not_to have_css('[data-testid="escalation_status_container"]')
end end
end end
context 'crm contacts widget' do
let(:issuable) { issue }
context 'without permission' do
it 'is expected not to be shown' do
create(:contact, group: group)
expect(rendered).not_to have_css('#js-issue-crm-contacts')
end
end
context 'without contacts' do
it 'is expected not to be shown' do
group.add_developer(user)
expect(rendered).not_to have_css('#js-issue-crm-contacts')
end
end
context 'with permission and contacts' do
it 'is expected to be shown' do
create(:contact, group: group)
group.add_developer(user)
expect(rendered).to have_css('#js-issue-crm-contacts')
end
end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment