From 7b6340d74cb605e010252a2d92c245576fd041b3 Mon Sep 17 00:00:00 2001 From: Lee Tickett <ltickett@gitlab.com> Date: Sun, 8 Jan 2023 21:32:42 +0000 Subject: [PATCH 1/6] Enable CRM by default for new groups Changelog: changed --- app/models/group.rb | 1 + ...0108205644_change_group_crm_settings_enabled_default.rb | 7 +++++++ db/schema_migrations/20230108205644 | 1 + db/structure.sql | 2 +- doc/user/crm/index.md | 6 ++++-- 5 files changed, 14 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20230108205644_change_group_crm_settings_enabled_default.rb create mode 100644 db/schema_migrations/20230108205644 diff --git a/app/models/group.rb b/app/models/group.rb index a43815781f2c7c0b..c4c8df6582023e5a 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -172,6 +172,7 @@ def of_ancestors_and_self after_create :post_create_hook after_create -> { create_or_load_association(:group_feature) } + after_create -> { create_or_load_association(:crm_settings) } after_update :path_changed_hook, if: :saved_change_to_path? after_destroy :post_destroy_hook after_commit :update_two_factor_requirement diff --git a/db/migrate/20230108205644_change_group_crm_settings_enabled_default.rb b/db/migrate/20230108205644_change_group_crm_settings_enabled_default.rb new file mode 100644 index 0000000000000000..57bf3e6191aa32a7 --- /dev/null +++ b/db/migrate/20230108205644_change_group_crm_settings_enabled_default.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class ChangeGroupCrmSettingsEnabledDefault < Gitlab::Database::Migration[2.1] + def change + change_column_default(:group_crm_settings, :enabled, from: false, to: true) + end +end diff --git a/db/schema_migrations/20230108205644 b/db/schema_migrations/20230108205644 new file mode 100644 index 0000000000000000..a11c47e5fb446211 --- /dev/null +++ b/db/schema_migrations/20230108205644 @@ -0,0 +1 @@ +2513a093d556b741729fc6e6015f78d5f309e4deae25f7ba379e04553e7d71f2 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 1ebd38f5bffb5689..fb50e103c85e2083 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -17710,7 +17710,7 @@ CREATE TABLE group_crm_settings ( group_id bigint NOT NULL, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, - enabled boolean DEFAULT false NOT NULL + enabled boolean DEFAULT true NOT NULL ); CREATE SEQUENCE group_crm_settings_group_id_seq diff --git a/doc/user/crm/index.md b/doc/user/crm/index.md index d552b540c89d899c..b664db479ffc0d5e 100644 --- a/doc/user/crm/index.md +++ b/doc/user/crm/index.md @@ -34,9 +34,11 @@ For more information about what is planned for the future, see [issue 2256](http ## Enable customer relations management (CRM) -Customer relations management features must be enabled at the group level. If your +> [Enabled by default for new groups](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108378) from GitLab 15.8. + +Customer relations management features are enabled at the group level. If your group also contains subgroups, and you want to use CRM features in the subgroup, -you must enable CRM features for the subgroup. +CRM features must also be enabled for the subgroup. To enable customer relations management in a group or subgroup: -- GitLab From 31199916ac51a4ff59051ae39c5e6f068c59db2c Mon Sep 17 00:00:00 2001 From: Lee Tickett <ltickett@gitlab.com> Date: Sun, 8 Jan 2023 23:19:34 +0000 Subject: [PATCH 2/6] Default crm_enabled when no crm_settings --- app/models/group.rb | 3 +- ee/spec/features/groups/navbar_spec.rb | 4 +-- ee/spec/policies/group_policy_spec.rb | 2 +- spec/factories/groups.rb | 4 +-- .../groups/crm/contacts/create_spec.rb | 2 +- spec/features/groups/navbar_spec.rb | 4 +-- spec/features/issues/gfm_autocomplete_spec.rb | 2 +- spec/finders/crm/contacts_finder_spec.rb | 6 ++-- spec/finders/crm/organizations_finder_spec.rb | 8 ++--- .../contacts/create_spec.rb | 2 +- .../contacts/update_spec.rb | 2 +- .../organizations/create_spec.rb | 2 +- .../organizations/update_spec.rb | 2 +- .../crm/contact_state_counts_resolver_spec.rb | 2 +- .../resolvers/crm/contacts_resolver_spec.rb | 2 +- ...organization_state_counts_resolver_spec.rb | 2 +- .../crm/organizations_resolver_spec.rb | 2 +- .../handler/service_desk_handler_spec.rb | 2 +- .../contact_state_counts_spec.rb | 2 +- spec/models/group_spec.rb | 6 ++-- spec/policies/group_policy_spec.rb | 32 +++++++++---------- spec/policies/issue_policy_spec.rb | 4 +-- .../requests/api/graphql/crm/contacts_spec.rb | 2 +- .../mutations/issues/set_crm_contacts_spec.rb | 6 ++-- .../groups/crm/contacts_controller_spec.rb | 6 ++-- .../crm/organizations_controller_spec.rb | 6 ++-- .../issue_sidebar_basic_entity_spec.rb | 6 ++-- .../contacts/create_service_spec.rb | 4 +-- .../contacts/update_service_spec.rb | 4 +-- .../organizations/create_service_spec.rb | 2 +- .../organizations/update_service_spec.rb | 4 +-- spec/services/groups/transfer_service_spec.rb | 2 +- spec/services/issues/create_service_spec.rb | 2 +- .../issues/set_crm_contacts_service_spec.rb | 6 ++-- spec/services/issues/update_service_spec.rb | 2 +- .../projects/autocomplete_service_spec.rb | 2 +- .../quick_actions/interpret_service_spec.rb | 2 +- .../policies/group_policy_shared_context.rb | 2 +- .../issuable/_sidebar.html.haml_spec.rb | 2 +- 39 files changed, 78 insertions(+), 79 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index c4c8df6582023e5a..de0b5ee71be1e568 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -172,7 +172,6 @@ def of_ancestors_and_self after_create :post_create_hook after_create -> { create_or_load_association(:group_feature) } - after_create -> { create_or_load_association(:crm_settings) } after_update :path_changed_hook, if: :saved_change_to_path? after_destroy :post_destroy_hook after_commit :update_two_factor_requirement @@ -855,7 +854,7 @@ def group_feature end def crm_enabled? - crm_settings&.enabled? + crm_settings.nil? || crm_settings.enabled? end def shared_with_group_links_visible_to_user(user) diff --git a/ee/spec/features/groups/navbar_spec.rb b/ee/spec/features/groups/navbar_spec.rb index d2c5b9035de20d2d..3fd2a397472f06d1 100644 --- a/ee/spec/features/groups/navbar_spec.rb +++ b/ee/spec/features/groups/navbar_spec.rb @@ -122,7 +122,7 @@ end context 'when customer relations feature is enabled' do - let(:group) { create(:group, :crm_enabled) } + let(:group) { create(:group) } before do insert_customer_relations_nav(_('Iterations')) @@ -134,7 +134,7 @@ end context 'when customer relations feature enabled but subgroup' do - let(:group) { create(:group, :crm_enabled, parent: create(:group)) } + let(:group) { create(:group, parent: create(:group)) } before do visit group_path(group) diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index c3481efc77505479..14d1ce8304b5f587 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -2541,7 +2541,7 @@ def set_access_level(access_level) end context 'with a public group' do - let_it_be(:public_group) { create(:group, :public, :crm_enabled) } + let_it_be(:public_group) { create(:group, :public) } subject { described_class.new(user, public_group) } diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb index 2f55c3ab567a24b7..e92e4768b8b9d494 100644 --- a/spec/factories/groups.rb +++ b/spec/factories/groups.rb @@ -113,9 +113,9 @@ def create_graph(parent: nil, children: 4, depth: 4) end end - trait :crm_enabled do + trait :crm_disabled do after(:create) do |group| - create(:crm_settings, group: group, enabled: true) + create(:crm_settings, group: group, enabled: false) end end diff --git a/spec/features/groups/crm/contacts/create_spec.rb b/spec/features/groups/crm/contacts/create_spec.rb index aa05ef82a8bc90ab..b0e10477018806ff 100644 --- a/spec/features/groups/crm/contacts/create_spec.rb +++ b/spec/features/groups/crm/contacts/create_spec.rb @@ -4,7 +4,7 @@ RSpec.describe 'Create a CRM contact', :js, feature_category: :service_desk do let(:user) { create(:user) } - let(:group) { create(:group, :crm_enabled) } + let(:group) { create(:group) } let!(:crm_organization) { create(:crm_organization, group: group, name: 'GitLab') } before do diff --git a/spec/features/groups/navbar_spec.rb b/spec/features/groups/navbar_spec.rb index e9cdacf82557d48b..c72fbf4fd3fd3fde 100644 --- a/spec/features/groups/navbar_spec.rb +++ b/spec/features/groups/navbar_spec.rb @@ -43,7 +43,7 @@ end context 'when customer_relations feature is enabled' do - let(:group) { create(:group, :crm_enabled) } + let(:group) { create(:group) } before do if Gitlab.ee? @@ -59,7 +59,7 @@ end context 'when customer_relations feature is enabled but subgroup' do - let(:group) { create(:group, :crm_enabled, parent: create(:group)) } + let(:group) { create(:group, parent: create(:group)) } before do visit group_path(group) diff --git a/spec/features/issues/gfm_autocomplete_spec.rb b/spec/features/issues/gfm_autocomplete_spec.rb index 58c84d26fea84100..409153a7725e4cc0 100644 --- a/spec/features/issues/gfm_autocomplete_spec.rb +++ b/spec/features/issues/gfm_autocomplete_spec.rb @@ -8,7 +8,7 @@ let_it_be(:user) { create(:user, name: '💃speciąl someone💃', username: 'someone.special') } let_it_be(:user2) { create(:user, name: 'Marge Simpson', username: 'msimpson') } - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } let_it_be(:issue) { create(:issue, project: project, assignees: [user]) } let_it_be(:label) { create(:label, project: project, title: 'special+') } diff --git a/spec/finders/crm/contacts_finder_spec.rb b/spec/finders/crm/contacts_finder_spec.rb index d0339ce2b18c7b81..b2dd8569a9b79fbd 100644 --- a/spec/finders/crm/contacts_finder_spec.rb +++ b/spec/finders/crm/contacts_finder_spec.rb @@ -9,7 +9,7 @@ subject { described_class.new(user, group: group).execute } context 'when customer relations feature is enabled for the group' do - let_it_be(:root_group) { create(:group, :crm_enabled) } + let_it_be(:root_group) { create(:group) } let_it_be(:group) { create(:group, parent: root_group) } let_it_be(:contact_1) { create(:contact, group: root_group) } @@ -58,7 +58,7 @@ end context 'with search informations' do - let_it_be(:search_test_group) { create(:group, :crm_enabled) } + let_it_be(:search_test_group) { create(:group) } let_it_be(:search_test_a) do create( @@ -189,7 +189,7 @@ end describe '.counts_by_state' do - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let_it_be(:active_contacts) { create_list(:contact, 3, group: group, state: :active) } let_it_be(:inactive_contacts) { create_list(:contact, 2, group: group, state: :inactive) } diff --git a/spec/finders/crm/organizations_finder_spec.rb b/spec/finders/crm/organizations_finder_spec.rb index bc174f927a7f866e..6d955ae0d7db2ec9 100644 --- a/spec/finders/crm/organizations_finder_spec.rb +++ b/spec/finders/crm/organizations_finder_spec.rb @@ -9,7 +9,7 @@ subject { described_class.new(user, group: group).execute } context 'when customer relations feature is enabled for the group' do - let_it_be(:root_group) { create(:group, :crm_enabled) } + let_it_be(:root_group) { create(:group) } let_it_be(:group) { create(:group, parent: root_group) } let_it_be(:crm_organization_1) { create(:crm_organization, group: root_group) } @@ -58,7 +58,7 @@ end context 'with search informations' do - let_it_be(:search_test_group) { create(:group, :crm_enabled) } + let_it_be(:search_test_group) { create(:group) } let_it_be(:search_test_a) do create( @@ -130,7 +130,7 @@ end context 'when sorting' do - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let_it_be(:sort_test_a) do create( @@ -185,7 +185,7 @@ end describe '.counts_by_state' do - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let_it_be(:active_crm_organizations) { create_list(:crm_organization, 3, group: group, state: :active) } let_it_be(:inactive_crm_organizations) { create_list(:crm_organization, 2, group: group, state: :inactive) } diff --git a/spec/graphql/mutations/customer_relations/contacts/create_spec.rb b/spec/graphql/mutations/customer_relations/contacts/create_spec.rb index 3ee898c2079ad7db..0117e025a0c18bd2 100644 --- a/spec/graphql/mutations/customer_relations/contacts/create_spec.rb +++ b/spec/graphql/mutations/customer_relations/contacts/create_spec.rb @@ -7,7 +7,7 @@ let_it_be(:user) { create(:user) } - let(:group) { create(:group, :crm_enabled) } + let(:group) { create(:group) } let(:not_found_or_does_not_belong) { 'The specified organization was not found or does not belong to this group' } let(:valid_params) do attributes_for(:contact, diff --git a/spec/graphql/mutations/customer_relations/contacts/update_spec.rb b/spec/graphql/mutations/customer_relations/contacts/update_spec.rb index 421bb4f1b067b6b9..2ea13dbeb3d5cf7d 100644 --- a/spec/graphql/mutations/customer_relations/contacts/update_spec.rb +++ b/spec/graphql/mutations/customer_relations/contacts/update_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Mutations::CustomerRelations::Contacts::Update do let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let(:first_name) { 'Lionel' } let(:last_name) { 'Smith' } diff --git a/spec/graphql/mutations/customer_relations/organizations/create_spec.rb b/spec/graphql/mutations/customer_relations/organizations/create_spec.rb index cf1ff2d56537d289..f9aef7264993d62a 100644 --- a/spec/graphql/mutations/customer_relations/organizations/create_spec.rb +++ b/spec/graphql/mutations/customer_relations/organizations/create_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Create do let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let(:valid_params) do attributes_for(:crm_organization, diff --git a/spec/graphql/mutations/customer_relations/organizations/update_spec.rb b/spec/graphql/mutations/customer_relations/organizations/update_spec.rb index 2fad320b4972761c..c62d685924148624 100644 --- a/spec/graphql/mutations/customer_relations/organizations/update_spec.rb +++ b/spec/graphql/mutations/customer_relations/organizations/update_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Update do let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let(:name) { 'GitLab' } let(:default_rate) { 1000.to_f } diff --git a/spec/graphql/resolvers/crm/contact_state_counts_resolver_spec.rb b/spec/graphql/resolvers/crm/contact_state_counts_resolver_spec.rb index 0128ec792b363807..607855ef41cad00c 100644 --- a/spec/graphql/resolvers/crm/contact_state_counts_resolver_spec.rb +++ b/spec/graphql/resolvers/crm/contact_state_counts_resolver_spec.rb @@ -6,7 +6,7 @@ include GraphqlHelpers let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } before_all do create(:contact, group: group, email: "x@test.com") diff --git a/spec/graphql/resolvers/crm/contacts_resolver_spec.rb b/spec/graphql/resolvers/crm/contacts_resolver_spec.rb index 1a53f42633fe62ed..f7ca034842bab83d 100644 --- a/spec/graphql/resolvers/crm/contacts_resolver_spec.rb +++ b/spec/graphql/resolvers/crm/contacts_resolver_spec.rb @@ -6,7 +6,7 @@ include GraphqlHelpers let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let_it_be(:contact_a) do create( diff --git a/spec/graphql/resolvers/crm/organization_state_counts_resolver_spec.rb b/spec/graphql/resolvers/crm/organization_state_counts_resolver_spec.rb index f6c2040b7d04d364..eb704a538d043b68 100644 --- a/spec/graphql/resolvers/crm/organization_state_counts_resolver_spec.rb +++ b/spec/graphql/resolvers/crm/organization_state_counts_resolver_spec.rb @@ -6,7 +6,7 @@ include GraphqlHelpers let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } before_all do create(:crm_organization, group: group, name: "ABC Corp") diff --git a/spec/graphql/resolvers/crm/organizations_resolver_spec.rb b/spec/graphql/resolvers/crm/organizations_resolver_spec.rb index edc1986799a71ad6..f3685014b61d9889 100644 --- a/spec/graphql/resolvers/crm/organizations_resolver_spec.rb +++ b/spec/graphql/resolvers/crm/organizations_resolver_spec.rb @@ -6,7 +6,7 @@ include GraphqlHelpers let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let_it_be(:crm_organization_a) do create( diff --git a/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb b/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb index 9d484198cc0ce70e..806c01a993e3bf2d 100644 --- a/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb @@ -17,7 +17,7 @@ let(:message_id) { 'CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com' } let(:issue_email_participants_count) { 1 } - let_it_be(:group) { create(:group, :private, :crm_enabled, name: "email") } + let_it_be(:group) { create(:group, :private, name: "email") } let(:expected_subject) { "The message subject! @all" } let(:expected_description) do diff --git a/spec/models/customer_relations/contact_state_counts_spec.rb b/spec/models/customer_relations/contact_state_counts_spec.rb index a19f6f0848951b9a..de9da7cf88dc32cb 100644 --- a/spec/models/customer_relations/contact_state_counts_spec.rb +++ b/spec/models/customer_relations/contact_state_counts_spec.rb @@ -4,7 +4,7 @@ RSpec.describe CustomerRelations::ContactStateCounts do let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let(:counter) { described_class.new(user, group, params) } let(:params) { {} } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index ce2c9ced47f036a8..697235339435fb18 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -3260,8 +3260,8 @@ def define_cache_expectations(cache_key) end describe '#crm_enabled?' do - it 'returns false where no crm_settings exist' do - expect(group.crm_enabled?).to be_falsey + it 'returns true where no crm_settings exist' do + expect(group.crm_enabled?).to be_truthy end it 'returns false where crm_settings.state is disabled' do @@ -3277,7 +3277,7 @@ def define_cache_expectations(cache_key) end it 'returns true where crm_settings.state is enabled for subgroup' do - subgroup = create(:group, :crm_enabled, parent: group) + subgroup = create(:group, parent: group) expect(subgroup.crm_enabled?).to be_truthy end diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 4632fcca12a7f1fc..150d7c75aac1b7bc 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -8,7 +8,7 @@ using RSpec::Parameterized::TableSyntax context 'public group with no user' do - let(:group) { create(:group, :public, :crm_enabled) } + let(:group) { create(:group, :public) } let(:current_user) { nil } specify do @@ -23,7 +23,7 @@ end context 'public group with user who is not a member' do - let(:group) { create(:group, :public, :crm_enabled) } + let(:group) { create(:group, :public) } let(:current_user) { create(:user) } specify do @@ -38,7 +38,7 @@ end context 'private group that has been invited to a public project and with no user' do - let(:project) { create(:project, :public, group: create(:group, :crm_enabled)) } + let(:project) { create(:project, :public, group: create(:group)) } let(:current_user) { nil } before do @@ -53,7 +53,7 @@ end context 'private group that has been invited to a public project and with a foreign user' do - let(:project) { create(:project, :public, group: create(:group, :crm_enabled)) } + let(:project) { create(:project, :public, group: create(:group)) } let(:current_user) { create(:user) } before do @@ -78,7 +78,7 @@ it { expect_allowed(*(public_permissions - [:read_counts])) } context 'in subgroups' do - let(:subgroup) { create(:group, :private, :crm_enabled, parent: group) } + let(:subgroup) { create(:group, :private, parent: group) } let(:project) { create(:project, namespace: subgroup) } it { expect_allowed(*(public_permissions - [:read_counts])) } @@ -274,7 +274,7 @@ describe 'private nested group use the highest access level from the group and inherited permissions' do let_it_be(:nested_group) do - create(:group, :private, :owner_subgroup_creation_only, :crm_enabled, parent: group) + create(:group, :private, :owner_subgroup_creation_only, parent: group) end before_all do @@ -378,7 +378,7 @@ let(:current_user) { owner } context 'when the group share_with_group_lock is enabled' do - let(:group) { create(:group, :crm_enabled, share_with_group_lock: true, parent: parent) } + let(:group) { create(:group, share_with_group_lock: true, parent: parent) } before do group.add_owner(owner) @@ -386,10 +386,10 @@ context 'when the parent group share_with_group_lock is enabled' do context 'when the group has a grandparent' do - let(:parent) { create(:group, :crm_enabled, share_with_group_lock: true, parent: grandparent) } + let(:parent) { create(:group, share_with_group_lock: true, parent: grandparent) } context 'when the grandparent share_with_group_lock is enabled' do - let(:grandparent) { create(:group, :crm_enabled, share_with_group_lock: true) } + let(:grandparent) { create(:group, share_with_group_lock: true) } context 'when the current_user owns the parent' do before do @@ -415,7 +415,7 @@ end context 'when the grandparent share_with_group_lock is disabled' do - let(:grandparent) { create(:group, :crm_enabled) } + let(:grandparent) { create(:group) } context 'when the current_user owns the parent' do before do @@ -432,7 +432,7 @@ end context 'when the group does not have a grandparent' do - let(:parent) { create(:group, :crm_enabled, share_with_group_lock: true) } + let(:parent) { create(:group, share_with_group_lock: true) } context 'when the current_user owns the parent' do before do @@ -449,7 +449,7 @@ end context 'when the parent group share_with_group_lock is disabled' do - let(:parent) { create(:group, :crm_enabled) } + let(:parent) { create(:group) } it { expect_allowed(:change_share_with_group_lock) } end @@ -905,14 +905,14 @@ end it_behaves_like 'clusterable policies' do - let(:clusterable) { create(:group, :crm_enabled) } + let(:clusterable) { create(:group) } let(:cluster) do create(:cluster, :provided_by_gcp, :group, groups: [clusterable]) end end describe 'update_max_artifacts_size' do - let(:group) { create(:group, :public, :crm_enabled) } + let(:group) { create(:group, :public) } context 'when no user' do let(:current_user) { nil } @@ -942,7 +942,7 @@ end describe 'design activity' do - let_it_be(:group) { create(:group, :public, :crm_enabled) } + let_it_be(:group) { create(:group, :public) } let(:current_user) { nil } @@ -1266,7 +1266,7 @@ it { expect_disallowed(:read_label) } context 'when group hierarchy has a project with service desk enabled' do - let_it_be(:subgroup) { create(:group, :private, :crm_enabled, parent: group) } + let_it_be(:subgroup) { create(:group, :private, parent: group) } let_it_be(:project) { create(:project, group: subgroup, service_desk_enabled: true) } it { expect_allowed(:read_label) } diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb index 94b05e50ef695b67..6bcc42df41860909 100644 --- a/spec/policies/issue_policy_spec.rb +++ b/spec/policies/issue_policy_spec.rb @@ -489,7 +489,7 @@ def permissions(user, issue) describe 'crm permissions' do let(:user) { create(:user) } - let(:subgroup) { create(:group, :crm_enabled, parent: create(:group, :crm_enabled)) } + let(:subgroup) { create(:group, parent: create(:group)) } let(:project) { create(:project, group: subgroup) } let(:issue) { create(:issue, project: project) } let(:policies) { described_class.new(user, issue) } @@ -522,7 +522,7 @@ def permissions(user, issue) end context 'when crm disabled on subgroup' do - let(:subgroup) { create(:group, parent: create(:group, :crm_enabled)) } + let(:subgroup) { create(:group, :crm_disabled, parent: create(:group)) } it 'is disallowed' do subgroup.parent.add_reporter(user) diff --git a/spec/requests/api/graphql/crm/contacts_spec.rb b/spec/requests/api/graphql/crm/contacts_spec.rb index 3ae19de63edcf4ec..b6785491388f1199 100644 --- a/spec/requests/api/graphql/crm/contacts_spec.rb +++ b/spec/requests/api/graphql/crm/contacts_spec.rb @@ -6,7 +6,7 @@ include GraphqlHelpers let_it_be(:current_user) { create(:user) } - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let_it_be(:contact_a) do create( 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 cdab267162ece4ad..a62a79180af41357 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 @@ -6,8 +6,8 @@ include GraphqlHelpers let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group, :crm_enabled) } - let_it_be(:subgroup) { create(:group, :crm_enabled, parent: group) } + let_it_be(:group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: group) } let_it_be(:project) { create(:project, group: subgroup) } let_it_be(:contacts) { create_list(:contact, 4, group: group) } @@ -129,7 +129,7 @@ def expected_contacts(contacts) end context 'when the contact belongs to a different group' do - let(:group2) { create(:group, :crm_enabled) } + let(:group2) { create(:group) } let(:contact) { create(:contact, group: group2) } let(:contact_ids) { [global_id_of(contact)] } diff --git a/spec/requests/groups/crm/contacts_controller_spec.rb b/spec/requests/groups/crm/contacts_controller_spec.rb index 4916ce60108c4144..cb16561a921befc6 100644 --- a/spec/requests/groups/crm/contacts_controller_spec.rb +++ b/spec/requests/groups/crm/contacts_controller_spec.rb @@ -24,7 +24,7 @@ shared_examples 'ok response with index template if authorized' do context 'private group' do - let(:group) { create(:group, :private, :crm_enabled) } + let(:group) { create(:group, :private) } context 'with authorized user' do before do @@ -43,7 +43,7 @@ end context 'when subgroup' do - let(:group) { create(:group, :private, :crm_enabled, parent: create(:group)) } + let(:group) { create(:group, :private, parent: create(:group)) } it_behaves_like 'response with 404 status' end @@ -68,7 +68,7 @@ end context 'public group' do - let(:group) { create(:group, :public, :crm_enabled) } + let(:group) { create(:group, :public) } context 'with anonymous user' do it_behaves_like 'response with 404 status' diff --git a/spec/requests/groups/crm/organizations_controller_spec.rb b/spec/requests/groups/crm/organizations_controller_spec.rb index 3e7e9a8e87842909..079fc6b894aaf7ee 100644 --- a/spec/requests/groups/crm/organizations_controller_spec.rb +++ b/spec/requests/groups/crm/organizations_controller_spec.rb @@ -24,7 +24,7 @@ shared_examples 'ok response with index template if authorized' do context 'private group' do - let(:group) { create(:group, :private, :crm_enabled) } + let(:group) { create(:group, :private) } context 'with authorized user' do before do @@ -43,7 +43,7 @@ end context 'when subgroup' do - let(:group) { create(:group, :private, :crm_enabled, parent: create(:group)) } + let(:group) { create(:group, :private, parent: create(:group)) } it_behaves_like 'response with 404 status' end @@ -68,7 +68,7 @@ end context 'public group' do - let(:group) { create(:group, :public, :crm_enabled) } + let(:group) { create(:group, :public) } context 'with anonymous user' do it_behaves_like 'response with 404 status' diff --git a/spec/serializers/issue_sidebar_basic_entity_spec.rb b/spec/serializers/issue_sidebar_basic_entity_spec.rb index ef80bcd5eb1409f6..a7e7730516ac2abb 100644 --- a/spec/serializers/issue_sidebar_basic_entity_spec.rb +++ b/spec/serializers/issue_sidebar_basic_entity_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe IssueSidebarBasicEntity do - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :repository, group: group) } let_it_be(:user) { create(:user, developer_projects: [project]) } let_it_be_with_reload(:issue) { create(:issue, project: project, assignees: [user]) } @@ -99,7 +99,7 @@ end context 'with crm enabled' do - let(:subgroup) { create(:group, :crm_enabled, parent: group) } + let(:subgroup) { create(:group, parent: group) } it 'is true' do allow(CustomerRelations::Contact).to receive(:exists_for_group?).with(group).and_return(true) @@ -109,7 +109,7 @@ end context 'with crm disabled' do - let(:subgroup) { create(:group, parent: group) } + let(:subgroup) { create(:group, :crm_disabled, parent: group) } it 'is false' do allow(CustomerRelations::Contact).to receive(:exists_for_group?).with(group).and_return(true) diff --git a/spec/services/customer_relations/contacts/create_service_spec.rb b/spec/services/customer_relations/contacts/create_service_spec.rb index 91aa51385e75af9c..ac973f4a14bebe97 100644 --- a/spec/services/customer_relations/contacts/create_service_spec.rb +++ b/spec/services/customer_relations/contacts/create_service_spec.rb @@ -12,7 +12,7 @@ subject(:response) { described_class.new(group: group, current_user: user, params: params).execute } context 'when user does not have permission' do - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } before_all do group.add_reporter(user) @@ -25,7 +25,7 @@ end context 'when user has permission' do - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } before_all do group.add_developer(user) diff --git a/spec/services/customer_relations/contacts/update_service_spec.rb b/spec/services/customer_relations/contacts/update_service_spec.rb index 105b5bad5f7f3b25..3dc1ddc5ab6ff0ee 100644 --- a/spec/services/customer_relations/contacts/update_service_spec.rb +++ b/spec/services/customer_relations/contacts/update_service_spec.rb @@ -11,7 +11,7 @@ describe '#execute' do context 'when the user has no permission' do - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let(:params) { { first_name: 'Gary' } } @@ -24,7 +24,7 @@ end context 'when user has permission' do - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } before_all do group.add_developer(user) diff --git a/spec/services/customer_relations/organizations/create_service_spec.rb b/spec/services/customer_relations/organizations/create_service_spec.rb index 8748fe447636fbce..0c313f7f92554045 100644 --- a/spec/services/customer_relations/organizations/create_service_spec.rb +++ b/spec/services/customer_relations/organizations/create_service_spec.rb @@ -6,7 +6,7 @@ describe '#execute' do let_it_be(:user) { create(:user) } - let(:group) { create(:group, :crm_enabled) } + let(:group) { create(:group) } let(:params) { attributes_for(:crm_organization, group: group) } subject(:response) { described_class.new(group: group, current_user: user, params: params).execute } diff --git a/spec/services/customer_relations/organizations/update_service_spec.rb b/spec/services/customer_relations/organizations/update_service_spec.rb index f11b99b101e4f787..0d8c661340026682 100644 --- a/spec/services/customer_relations/organizations/update_service_spec.rb +++ b/spec/services/customer_relations/organizations/update_service_spec.rb @@ -11,7 +11,7 @@ describe '#execute' do context 'when the user has no permission' do - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let(:params) { { name: 'GitLab' } } @@ -24,7 +24,7 @@ end context 'when user has permission' do - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } before_all do group.add_developer(user) diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb index ce67c4c51fe85f41..8a866b632bd67eaa 100644 --- a/spec/services/groups/transfer_service_spec.rb +++ b/spec/services/groups/transfer_service_spec.rb @@ -17,7 +17,7 @@ end let_it_be(:user) { create(:user) } - let_it_be(:new_parent_group) { create(:group, :public, :crm_enabled) } + let_it_be(:new_parent_group) { create(:group, :public) } let!(:group_member) { create(:group_member, :owner, group: group, user: user) } let(:transfer_service) { described_class.new(group, user) } diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 7cd2cd8f564b8f14..2e6625679521239c 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -5,7 +5,7 @@ RSpec.describe Issues::CreateService, feature_category: :team_planning do include AfterNextHelpers - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let_it_be_with_reload(:project) { create(:project, :public, group: group) } let_it_be(:user) { create(:user) } diff --git a/spec/services/issues/set_crm_contacts_service_spec.rb b/spec/services/issues/set_crm_contacts_service_spec.rb index 7d709bbd9c8e0f61..5ff16dbeda1b6dfb 100644 --- a/spec/services/issues/set_crm_contacts_service_spec.rb +++ b/spec/services/issues/set_crm_contacts_service_spec.rb @@ -4,8 +4,8 @@ RSpec.describe Issues::SetCrmContactsService, feature_category: :team_planning do let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group, :crm_enabled) } - let_it_be(:project) { create(:project, group: create(:group, :crm_enabled, parent: group)) } + let_it_be(:group) { create(: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(:issue, reload: true) { create(:issue, project: project) } let_it_be(:issue_contact_1) do @@ -60,7 +60,7 @@ context 'but the crm setting is disabled' do let(:params) { { replace_ids: [contacts[1].id, contacts[2].id] } } - let(:subgroup_with_crm_disabled) { create(:group, parent: group) } + let(:subgroup_with_crm_disabled) { create(:group, :crm_disabled, parent: group) } let(:project_with_crm_disabled) { create(:project, group: subgroup_with_crm_disabled) } let(:issue_with_crm_disabled) { create(:issue, project: project_with_crm_disabled) } diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index e8bcdc2c44bdd3ac..f6cbacaf760cbb63 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -7,7 +7,7 @@ let_it_be(:user2) { create(:user) } let_it_be(:user3) { create(:user) } let_it_be(:guest) { create(:user) } - let_it_be(:group) { create(:group, :public, :crm_enabled) } + let_it_be(:group) { create(:group, :public) } let_it_be(:project, reload: true) { create(:project, :repository, group: group) } let_it_be(:label) { create(:label, title: 'a', project: project) } let_it_be(:label2) { create(:label, title: 'b', project: project) } diff --git a/spec/services/projects/autocomplete_service_spec.rb b/spec/services/projects/autocomplete_service_spec.rb index 4bfbb1e3dd2a9668..0ec6362761e0fb6c 100644 --- a/spec/services/projects/autocomplete_service_spec.rb +++ b/spec/services/projects/autocomplete_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Projects::AutocompleteService, feature_category: :groups_and_projects do - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :public, group: group) } let_it_be(:owner) { create(:user) } let_it_be(:issue) { create(:issue, project: project, title: 'Issue 1') } diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index 0e16e5b6d27e239e..fb81bfd469b88620 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -5,7 +5,7 @@ RSpec.describe QuickActions::InterpretService, feature_category: :team_planning do include AfterNextHelpers - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let_it_be(:public_project) { create(:project, :public, group: group) } let_it_be(:repository_project) { create(:project, :repository) } let_it_be(:project) { public_project } diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index 094a78aa45f1b8c4..509bc413b8d4c161 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb @@ -8,7 +8,7 @@ let_it_be(:owner) { create(:user) } let_it_be(:admin) { create(:admin) } let_it_be(:non_group_member) { create(:user) } - let_it_be(:group, refind: true) { create(:group, :private, :owner_subgroup_creation_only, :crm_enabled) } + let_it_be(:group, refind: true) { create(:group, :private, :owner_subgroup_creation_only) } let(:public_permissions) do %i[ diff --git a/spec/views/shared/issuable/_sidebar.html.haml_spec.rb b/spec/views/shared/issuable/_sidebar.html.haml_spec.rb index 31f79c250735063b..a8307cc536e64888 100644 --- a/spec/views/shared/issuable/_sidebar.html.haml_spec.rb +++ b/spec/views/shared/issuable/_sidebar.html.haml_spec.rb @@ -11,7 +11,7 @@ end context 'project in a group' do - let_it_be(:group) { create(:group, :crm_enabled) } + let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } let_it_be(:issue) { create(:issue, project: project) } let_it_be(:incident) { create(:incident, project: project) } -- GitLab From e265319af6fc7df55d31b6d1530184ff869860cf Mon Sep 17 00:00:00 2001 From: Lee Tickett <ltickett@gitlab.com> Date: Mon, 9 Jan 2023 07:50:12 +0000 Subject: [PATCH 3/6] Fix failing specs --- doc/user/crm/index.md | 2 +- ee/spec/features/groups/navbar_spec.rb | 6 +++--- .../autocomplete_sources_controller_spec.rb | 12 ++++++------ spec/features/groups/navbar_spec.rb | 6 +++--- spec/finders/crm/contacts_finder_spec.rb | 4 ++-- spec/finders/crm/organizations_finder_spec.rb | 2 +- .../customer_relations/contacts/create_spec.rb | 2 +- .../mutations/issues/set_crm_contacts_spec.rb | 4 +++- .../groups/crm/contacts_controller_spec.rb | 2 +- .../groups/crm/organizations_controller_spec.rb | 2 +- spec/services/groups/update_service_spec.rb | 14 +++++++------- .../shared_contexts/navbar_structure_context.rb | 10 ++++++++++ 12 files changed, 39 insertions(+), 27 deletions(-) diff --git a/doc/user/crm/index.md b/doc/user/crm/index.md index b664db479ffc0d5e..e27982af290a9dd0 100644 --- a/doc/user/crm/index.md +++ b/doc/user/crm/index.md @@ -34,7 +34,7 @@ For more information about what is planned for the future, see [issue 2256](http ## Enable customer relations management (CRM) -> [Enabled by default for new groups](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108378) from GitLab 15.8. +> [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108378) in GitLab 15.8. Customer relations management features are enabled at the group level. If your group also contains subgroups, and you want to use CRM features in the subgroup, diff --git a/ee/spec/features/groups/navbar_spec.rb b/ee/spec/features/groups/navbar_spec.rb index 3fd2a397472f06d1..f804e64f06ba2d3d 100644 --- a/ee/spec/features/groups/navbar_spec.rb +++ b/ee/spec/features/groups/navbar_spec.rb @@ -10,7 +10,7 @@ include_context 'group navbar structure' let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group) } + let_it_be(:group) { create(:group, :crm_disabled) } context 'for maintainers' do before do @@ -121,7 +121,7 @@ it_behaves_like 'verified navigation bar' end - context 'when customer relations feature is enabled' do + context 'when crm feature is enabled' do let(:group) { create(:group) } before do @@ -133,7 +133,7 @@ it_behaves_like 'verified navigation bar' end - context 'when customer relations feature enabled but subgroup' do + context 'when crm feature enabled but subgroup' do let(:group) { create(:group, parent: create(:group)) } before do diff --git a/spec/controllers/projects/autocomplete_sources_controller_spec.rb b/spec/controllers/projects/autocomplete_sources_controller_spec.rb index 1745dfe3af017136..2273665579547f95 100644 --- a/spec/controllers/projects/autocomplete_sources_controller_spec.rb +++ b/spec/controllers/projects/autocomplete_sources_controller_spec.rb @@ -296,11 +296,7 @@ def members_by_username(username) end context 'when feature flag is enabled' do - context 'when a group has contact relations enabled' do - before do - create(:crm_settings, group: group, enabled: true) - end - + context 'when a group has crm enabled' do context 'when a user can read contacts' do it 'lists contacts' do group.add_developer(user) @@ -321,7 +317,11 @@ def members_by_username(username) end end - context 'when a group has contact relations disabled' do + context 'when a group has crm disabled' do + before do + create(:crm_settings, group: group, enabled: false) + end + it 'renders 404' do group.add_developer(user) diff --git a/spec/features/groups/navbar_spec.rb b/spec/features/groups/navbar_spec.rb index c72fbf4fd3fd3fde..4b299d702afd6253 100644 --- a/spec/features/groups/navbar_spec.rb +++ b/spec/features/groups/navbar_spec.rb @@ -42,8 +42,8 @@ it_behaves_like 'verified navigation bar' end - context 'when customer_relations feature is enabled' do - let(:group) { create(:group) } + context 'when crm feature is disabled' do + let(:group) { create(:group, :crm_disabled) } before do if Gitlab.ee? @@ -58,7 +58,7 @@ it_behaves_like 'verified navigation bar' end - context 'when customer_relations feature is enabled but subgroup' do + context 'when crm feature is enabled but subgroup' do let(:group) { create(:group, parent: create(:group)) } before do diff --git a/spec/finders/crm/contacts_finder_spec.rb b/spec/finders/crm/contacts_finder_spec.rb index b2dd8569a9b79fbd..c54cd39534f2090b 100644 --- a/spec/finders/crm/contacts_finder_spec.rb +++ b/spec/finders/crm/contacts_finder_spec.rb @@ -44,8 +44,8 @@ end end - context 'when customer relations feature is disabled for the group' do - let_it_be(:group) { create(:group) } + context 'when crm feature is disabled for the group' do + let_it_be(:group) { create(:group, :crm_disabled) } let_it_be(:contact) { create(:contact, group: group) } before do diff --git a/spec/finders/crm/organizations_finder_spec.rb b/spec/finders/crm/organizations_finder_spec.rb index 6d955ae0d7db2ec9..fd65cd22cb509c14 100644 --- a/spec/finders/crm/organizations_finder_spec.rb +++ b/spec/finders/crm/organizations_finder_spec.rb @@ -45,7 +45,7 @@ end context 'when customer relations feature is disabled for the group' do - let_it_be(:group) { create(:group) } + let_it_be(:group) { create(:group, :crm_disabled) } let_it_be(:crm_organization) { create(:crm_organization, group: group) } before do diff --git a/spec/graphql/mutations/customer_relations/contacts/create_spec.rb b/spec/graphql/mutations/customer_relations/contacts/create_spec.rb index 0117e025a0c18bd2..2cca77b8983e72ca 100644 --- a/spec/graphql/mutations/customer_relations/contacts/create_spec.rb +++ b/spec/graphql/mutations/customer_relations/contacts/create_spec.rb @@ -41,7 +41,7 @@ end context 'when crm_enabled is false' do - let(:group) { create(:group) } + let(:group) { create(:group, :crm_disabled) } it 'raises an error' do expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) 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 a62a79180af41357..ac92d91468ff2ada 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 @@ -171,7 +171,9 @@ def expected_contacts(contacts) end context 'when crm_enabled is false' do - let(:issue) { create(:issue) } + let(:group2) { create(:group, :crm_disabled) } + let(:project2) { create(:project, group: group2) } + let(:issue) { create(:issue, project: project2) } let(:initial_contacts) { [] } it 'raises expected error' do diff --git a/spec/requests/groups/crm/contacts_controller_spec.rb b/spec/requests/groups/crm/contacts_controller_spec.rb index cb16561a921befc6..48e9bfe6ef0fd9da 100644 --- a/spec/requests/groups/crm/contacts_controller_spec.rb +++ b/spec/requests/groups/crm/contacts_controller_spec.rb @@ -37,7 +37,7 @@ end context 'when crm_enabled is false' do - let(:group) { create(:group, :private) } + let(:group) { create(:group, :private, :crm_disabled) } it_behaves_like 'response with 404 status' end diff --git a/spec/requests/groups/crm/organizations_controller_spec.rb b/spec/requests/groups/crm/organizations_controller_spec.rb index 079fc6b894aaf7ee..c180365ae55f6196 100644 --- a/spec/requests/groups/crm/organizations_controller_spec.rb +++ b/spec/requests/groups/crm/organizations_controller_spec.rb @@ -37,7 +37,7 @@ end context 'when crm_enabled is false' do - let(:group) { create(:group, :private) } + let(:group) { create(:group, :private, :crm_disabled) } it_behaves_like 'response with 404 status' end diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb index ceb0f5c45b4461b9..01e1adda18d8f1ed 100644 --- a/spec/services/groups/update_service_spec.rb +++ b/spec/services/groups/update_service_spec.rb @@ -150,29 +150,29 @@ context 'crm_enabled param' do context 'when no existing crm_settings' do - it 'when param not present, leave crm disabled' do + it 'when param not present, leave crm enabled' do params = {} described_class.new(public_group, user, params).execute updated_group = public_group.reload - expect(updated_group.crm_enabled?).to be_falsey + expect(updated_group.crm_enabled?).to be_truthy end - it 'when param set true, enables crm' do - params = { crm_enabled: true } + it 'when param set false, disables crm' do + params = { crm_enabled: false } described_class.new(public_group, user, params).execute updated_group = public_group.reload - expect(updated_group.crm_enabled?).to be_truthy + expect(updated_group.crm_enabled?).to be_falsy end end context 'with existing crm_settings' do it 'when param set true, enables crm' do params = { crm_enabled: true } - create(:crm_settings, group: public_group) + create(:crm_settings, group: public_group, enabled: false) described_class.new(public_group, user, params).execute @@ -192,7 +192,7 @@ it 'when param not present, crm remains disabled' do params = {} - create(:crm_settings, group: public_group) + create(:crm_settings, group: public_group, enabled: false) described_class.new(public_group, user, params).execute diff --git a/spec/support/shared_contexts/navbar_structure_context.rb b/spec/support/shared_contexts/navbar_structure_context.rb index 9188fa0833536480..1f958734bad87fcc 100644 --- a/spec/support/shared_contexts/navbar_structure_context.rb +++ b/spec/support/shared_contexts/navbar_structure_context.rb @@ -149,6 +149,16 @@ [_("Issues"), _("Issue board"), _("Milestones"), (_('Iterations') if Gitlab.ee?)] end + let(:customer_relations_nav_item) do + { + nav_item: _('Customer relations'), + nav_sub_items: [ + _('Contacts'), + _('Organizations') + ] + } + end + let(:structure) do [ { -- GitLab From b76a1d62a2a5df1b3af6f6a207175f1391f84fa5 Mon Sep 17 00:00:00 2001 From: Lee Tickett <ltickett@gitlab.com> Date: Tue, 6 Feb 2024 03:28:57 -0800 Subject: [PATCH 4/6] Fix specs --- app/models/group/crm_settings.rb | 4 ++++ ...05644_change_group_crm_settings_enabled_default.rb | 7 ------- ...30350_change_group_crm_settings_enabled_default.rb | 11 +++++++++++ db/schema_migrations/20230108205644 | 1 - db/schema_migrations/20240207130350 | 1 + doc/user/crm/index.md | 2 +- ee/spec/features/boards/scoped_issue_board_spec.rb | 2 +- ...p_page_with_external_authorization_service_spec.rb | 2 +- spec/features/groups/navbar_spec.rb | 10 ++++------ spec/policies/group_policy_spec.rb | 4 ++-- 10 files changed, 25 insertions(+), 19 deletions(-) delete mode 100644 db/migrate/20230108205644_change_group_crm_settings_enabled_default.rb create mode 100644 db/post_migrate/20240207130350_change_group_crm_settings_enabled_default.rb delete mode 100644 db/schema_migrations/20230108205644 create mode 100644 db/schema_migrations/20240207130350 diff --git a/app/models/group/crm_settings.rb b/app/models/group/crm_settings.rb index 30fbe6ae07f7d128..f8dfbd8a3052a838 100644 --- a/app/models/group/crm_settings.rb +++ b/app/models/group/crm_settings.rb @@ -1,10 +1,14 @@ # frozen_string_literal: true class Group::CrmSettings < ApplicationRecord + include SafelyChangeColumnDefault + self.primary_key = :group_id self.table_name = 'group_crm_settings' belongs_to :group, -> { where(type: Group.sti_name) }, foreign_key: 'group_id' validates :group, presence: true + + columns_changing_default :enabled end diff --git a/db/migrate/20230108205644_change_group_crm_settings_enabled_default.rb b/db/migrate/20230108205644_change_group_crm_settings_enabled_default.rb deleted file mode 100644 index 57bf3e6191aa32a7..0000000000000000 --- a/db/migrate/20230108205644_change_group_crm_settings_enabled_default.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -class ChangeGroupCrmSettingsEnabledDefault < Gitlab::Database::Migration[2.1] - def change - change_column_default(:group_crm_settings, :enabled, from: false, to: true) - end -end diff --git a/db/post_migrate/20240207130350_change_group_crm_settings_enabled_default.rb b/db/post_migrate/20240207130350_change_group_crm_settings_enabled_default.rb new file mode 100644 index 0000000000000000..0fa050f95c1ed3d4 --- /dev/null +++ b/db/post_migrate/20240207130350_change_group_crm_settings_enabled_default.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class ChangeGroupCrmSettingsEnabledDefault < Gitlab::Database::Migration[2.2] + milestone '16.9' + + enable_lock_retries! + + def change + change_column_default('group_crm_settings', 'enabled', from: false, to: true) + end +end diff --git a/db/schema_migrations/20230108205644 b/db/schema_migrations/20230108205644 deleted file mode 100644 index a11c47e5fb446211..0000000000000000 --- a/db/schema_migrations/20230108205644 +++ /dev/null @@ -1 +0,0 @@ -2513a093d556b741729fc6e6015f78d5f309e4deae25f7ba379e04553e7d71f2 \ No newline at end of file diff --git a/db/schema_migrations/20240207130350 b/db/schema_migrations/20240207130350 new file mode 100644 index 0000000000000000..acca6b36968690b1 --- /dev/null +++ b/db/schema_migrations/20240207130350 @@ -0,0 +1 @@ +555c4cad3d4d75edd6bc75e504ebd551a6f51172b267a3924bcc75ada25d1775 \ No newline at end of file diff --git a/doc/user/crm/index.md b/doc/user/crm/index.md index e27982af290a9dd0..34f0dd3364ac9ff3 100644 --- a/doc/user/crm/index.md +++ b/doc/user/crm/index.md @@ -34,7 +34,7 @@ For more information about what is planned for the future, see [issue 2256](http ## Enable customer relations management (CRM) -> [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108378) in GitLab 15.8. +> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108378) in GitLab 15.8. Customer relations management features are enabled at the group level. If your group also contains subgroups, and you want to use CRM features in the subgroup, diff --git a/ee/spec/features/boards/scoped_issue_board_spec.rb b/ee/spec/features/boards/scoped_issue_board_spec.rb index 2d9adec433a6827c..a4337c6fee4a3435 100644 --- a/ee/spec/features/boards/scoped_issue_board_spec.rb +++ b/ee/spec/features/boards/scoped_issue_board_spec.rb @@ -7,7 +7,7 @@ include ListboxHelpers let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group, :public) } + let_it_be(:group) { create(:group, :public, :crm_disabled) } let_it_be(:project) { create(:project, :public, namespace: group) } let_it_be(:project_2) { create(:project, :public, namespace: group) } let_it_be(:project_label) { create(:label, project: project, name: 'Planning') } diff --git a/spec/features/groups/group_page_with_external_authorization_service_spec.rb b/spec/features/groups/group_page_with_external_authorization_service_spec.rb index fe1a685899fccf36..f69fec56d3544178 100644 --- a/spec/features/groups/group_page_with_external_authorization_service_spec.rb +++ b/spec/features/groups/group_page_with_external_authorization_service_spec.rb @@ -6,7 +6,7 @@ include ExternalAuthorizationServiceHelpers let(:user) { create(:user) } - let(:group) { create(:group) } + let(:group) { create(:group, :crm_disabled) } before do sign_in user diff --git a/spec/features/groups/navbar_spec.rb b/spec/features/groups/navbar_spec.rb index 4b299d702afd6253..b1d7559a6d9923ac 100644 --- a/spec/features/groups/navbar_spec.rb +++ b/spec/features/groups/navbar_spec.rb @@ -17,6 +17,10 @@ insert_after_nav_item(_('Analyze'), new_nav_item: settings_for_maintainer_nav_item) if Gitlab.ee? insert_infrastructure_registry_nav(_('Kubernetes')) + if group.crm_enabled? && group.parent.nil? + insert_customer_relations_nav(Gitlab.ee? ? _('Iterations') : _('Milestones')) + end + stub_config(dependency_proxy: { enabled: false }) stub_config(registry: { enabled: false }) stub_group_wikis(false) @@ -46,12 +50,6 @@ let(:group) { create(:group, :crm_disabled) } before do - if Gitlab.ee? - insert_customer_relations_nav(_('Iterations')) - else - insert_customer_relations_nav(_('Milestones')) - end - visit group_path(group) end diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 150d7c75aac1b7bc..128f7f5e5d46cf2b 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -1256,7 +1256,7 @@ it_behaves_like 'Self-managed Core resource access tokens' context 'support bot' do - let_it_be_with_refind(:group) { create(:group, :private, :crm_enabled) } + let_it_be_with_refind(:group) { create(:group, :private) } let_it_be(:current_user) { Users::Internal.support_bot } before do @@ -1635,7 +1635,7 @@ let(:current_user) { owner } before do - group.crm_settings.update!(enabled: false) + Group::CrmSettings.create!(group: group, enabled: false) end it { is_expected.to be_disallowed(:read_crm_contact) } -- GitLab From 9734b07fbb13c55e326f8e93866cf44c46bef7bc Mon Sep 17 00:00:00 2001 From: Lee Tickett <ltickett@gitlab.com> Date: Thu, 8 Feb 2024 09:18:42 +0000 Subject: [PATCH 5/6] Use crm_settings factory and favour let_it_be --- spec/policies/group_policy_spec.rb | 2 +- .../api/graphql/mutations/issues/set_crm_contacts_spec.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 128f7f5e5d46cf2b..f9712b9735dd545f 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -1635,7 +1635,7 @@ let(:current_user) { owner } before do - Group::CrmSettings.create!(group: group, enabled: false) + create(:crm_settings, group: group, enabled: false) end it { is_expected.to be_disallowed(:read_crm_contact) } 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 ac92d91468ff2ada..05e0c9975574a86d 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 @@ -171,10 +171,10 @@ def expected_contacts(contacts) end context 'when crm_enabled is false' do - let(:group2) { create(:group, :crm_disabled) } - let(:project2) { create(:project, group: group2) } - let(:issue) { create(:issue, project: project2) } - let(:initial_contacts) { [] } + let_it_be(:group2) { create(:group, :crm_disabled) } + let_it_be(:project2) { create(:project, group: group2) } + let_it_be(:issue) { create(:issue, project: project2) } + let_it_be(:initial_contacts) { [] } it 'raises expected error' do issue.project.add_reporter(user) -- GitLab From 42a7cdf66440e4a5cc5b921ce46cca3d9e8b842c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Etienne=20Baqu=C3=A9?= <ebaque@gitlab.com> Date: Fri, 9 Feb 2024 13:24:50 +0000 Subject: [PATCH 6/6] Apply 2 suggestion(s) to 2 file(s) --- doc/user/crm/index.md | 2 +- ee/spec/features/groups/navbar_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/user/crm/index.md b/doc/user/crm/index.md index 34f0dd3364ac9ff3..557eebad1b047a72 100644 --- a/doc/user/crm/index.md +++ b/doc/user/crm/index.md @@ -34,7 +34,7 @@ For more information about what is planned for the future, see [issue 2256](http ## Enable customer relations management (CRM) -> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108378) in GitLab 15.8. +> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108378) in GitLab 16.9. Customer relations management features are enabled at the group level. If your group also contains subgroups, and you want to use CRM features in the subgroup, diff --git a/ee/spec/features/groups/navbar_spec.rb b/ee/spec/features/groups/navbar_spec.rb index f804e64f06ba2d3d..4aa10fc597da343b 100644 --- a/ee/spec/features/groups/navbar_spec.rb +++ b/ee/spec/features/groups/navbar_spec.rb @@ -133,7 +133,7 @@ it_behaves_like 'verified navigation bar' end - context 'when crm feature enabled but subgroup' do + context 'when crm feature is enabled on both group and parent group' do let(:group) { create(:group, parent: create(:group)) } before do -- GitLab