Merge branch '327120-add-invite-source-tracking-5' into 'master'

Add backend sources for inviting members to be recorded

See merge request gitlab-org/gitlab!62732

app/controllers/admin/groups_controller.rb

@@ -62,7 +62,7 @@ class Admin::GroupsController < Admin::ApplicationController
 
   def members_update
     member_params = params.permit(:user_ids, :access_level, :expires_at)
-    result = Members::CreateService.new(current_user, member_params.merge(limit: -1, source: @group)).execute
+    result = Members::CreateService.new(current_user, member_params.merge(limit: -1, source: @group, invite_source: 'admin-group-page')).execute
 
     if result[:status] == :success
       redirect_to [:admin, @group], notice: _('Users were successfully added.')

app/controllers/concerns/membership_actions.rb

@@ -6,7 +6,7 @@ module MembershipActions
 
   def create
     create_params = params.permit(:user_ids, :access_level, :expires_at)
-    result = Members::CreateService.new(current_user, create_params.merge({ source: membershipable })).execute
+    result = Members::CreateService.new(current_user, create_params.merge({ source: membershipable, invite_source: "#{source_type}-members-page" })).execute
 
     if result[:status] == :success
       redirect_to members_page_url, notice: _('Users were successfully added.')

app/services/members/create_service.rb

@@ -16,6 +16,7 @@ module Members
     end
 
     def execute
+      validate_invite_source!
       validate_invites!
 
       add_members
@@ -33,6 +34,10 @@ module Members
       params[:user_ids]
     end
 
+    def validate_invite_source!
+      raise ArgumentError, s_('AddMember|No invite source provided.') unless invite_source.present?
+    end
+
     def validate_invites!
       raise BlankInvitesError, blank_invites_message if invites.blank?
 
@@ -79,7 +84,7 @@ module Members
     end
 
     def invite_source
-      params[:invite_source] || 'unknown'
+      params[:invite_source]
     end
 
     def tracking_property(member)

ee/app/controllers/concerns/group_invite_members.rb

@@ -3,11 +3,12 @@
 module GroupInviteMembers
   private
 
-  def invite_members(group)
+  def invite_members(group, invite_source:)
     invite_params = {
       source: group,
       user_ids: emails_param[:emails]&.reject(&:blank?)&.join(','),
-      access_level: Gitlab::Access::DEVELOPER
+      access_level: Gitlab::Access::DEVELOPER,
+      invite_source: invite_source
     }
 
     result = Members::CreateService.new(current_user, invite_params).execute

ee/app/controllers/ee/groups_controller.rb

@@ -115,7 +115,7 @@ module EE
     def successful_creation_hooks
       super
 
-      invite_members(group)
+      invite_members(group, invite_source: 'group-creation-page')
     end
   end
 end

ee/app/controllers/registrations/group_invites_controller.rb

@@ -39,7 +39,8 @@ module Registrations
       {
         source: group,
         user_ids: emails_param[:emails]&.reject(&:blank?)&.join(','),
-        access_level: Gitlab::Access::DEVELOPER
+        access_level: Gitlab::Access::DEVELOPER,
+        invite_source: 'registrations-group-invite'
       }
     end
 

ee/spec/requests/registrations/group_invites_spec.rb

@@ -71,12 +71,19 @@ RSpec.describe 'view group invites' do
           let(:valid_emails) { %w[a@a.a b@b.b] }
           let(:group_params) { { group_id: group.id, emails: valid_emails + ['', '', 'x', 'y'] } }
 
-          it 'adds users with developer access and ignores blank and invalid emails', :aggregate_failures do
+          it 'adds users with developer access and ignores blank and invalid emails', :aggregate_failures, :snowplow do
             post_request
 
             invited_members = group.members.invite
             expect(invited_members.pluck(:invite_email)).to match_array(valid_emails)
             expect(invited_members.pluck(:access_level).uniq).to match([Gitlab::Access::DEVELOPER])
+            expect_snowplow_event(
+              category: 'Members::CreateService',
+              action: 'create_member',
+              label: 'registrations-group-invite',
+              property: 'net_new_user',
+              user: user
+            )
           end
 
           it 'tracks experiment as expected', :experiment do

ee/spec/services/ee/members/create_service_spec.rb

@@ -10,7 +10,13 @@ RSpec.describe Members::CreateService do
   let_it_be(:subgroup_project) { create(:project, group: subgroup) }
   let_it_be(:project_users) { create_list(:user, 2) }
 
-  let(:params) { { user_ids: project_users.map(&:id).join(','), access_level: Gitlab::Access::GUEST } }
+  let(:params) do
+    {
+      user_ids: project_users.map(&:id).join(','),
+      access_level: Gitlab::Access::GUEST,
+      invite_source: '_invite_source_'
+    }
+  end
 
   subject { described_class.new(user, params.merge({ source: project })).execute }
 

ee/spec/services/ee/members/invite_service_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe Members::InviteService, :aggregate_failures do
   let_it_be(:subgroup) { create(:group, parent: root_ancestor) }
   let_it_be(:subgroup_project) { create(:project, group: subgroup) }
 
-  let(:base_params) { { access_level: Gitlab::Access::GUEST, source: project } }
+  let(:base_params) { { access_level: Gitlab::Access::GUEST, source: project, invite_source: '_invite_source_' } }
   let(:params) { { email: %w[email@example.org email2@example.org] } }
 
   subject(:result) { described_class.new(user, base_params.merge(params)).execute }

ee/spec/support/shared_examples/controllers/concerns/group_invite_members_shared_examples.rb

@@ -33,6 +33,13 @@ RSpec.shared_examples GroupInviteMembers do
         subject
 
         expect_snowplow_event(category: anything, action: 'invite_members', label: 'new_group_form', user: user)
+        expect_snowplow_event(
+          category: 'Members::CreateService',
+          action: 'create_member',
+          label: 'group-creation-page',
+          property: 'net_new_user',
+          user: user
+        )
       end
     end
   end

locale/gitlab.pot

@@ -2095,6 +2095,9 @@ msgstr ""
 msgid "AddMember|Invite limit of %{daily_invites} per day exceeded"
 msgstr ""
 
+msgid "AddMember|No invite source provided."
+msgstr ""
+
 msgid "AddMember|No users specified."
 msgstr ""
 

spec/controllers/admin/groups_controller_spec.rb

@@ -3,9 +3,9 @@
 require 'spec_helper'
 
 RSpec.describe Admin::GroupsController do
-  let(:group) { create(:group) }
-  let(:project) { create(:project, namespace: group) }
-  let(:admin) { create(:admin) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, namespace: group) }
+  let_it_be(:admin) { create(:admin) }
 
   before do
     sign_in(admin)
@@ -46,9 +46,9 @@ RSpec.describe Admin::GroupsController do
   end
 
   describe 'PUT #members_update' do
-    let(:group_user) { create(:user) }
+    let_it_be(:group_user) { create(:user) }
 
-    it 'adds user to members' do
+    it 'adds user to members', :aggregate_failures, :snowplow do
       put :members_update, params: {
                              id: group,
                              user_ids: group_user.id,
@@ -58,9 +58,16 @@ RSpec.describe Admin::GroupsController do
       expect(controller).to set_flash.to 'Users were successfully added.'
       expect(response).to redirect_to(admin_group_path(group))
       expect(group.users).to include group_user
+      expect_snowplow_event(
+        category: 'Members::CreateService',
+        action: 'create_member',
+        label: 'admin-group-page',
+        property: 'existing_user',
+        user: admin
+      )
     end
 
-    it 'can add unlimited members' do
+    it 'can add unlimited members', :aggregate_failures do
       put :members_update, params: {
                              id: group,
                              user_ids: 1.upto(1000).to_a.join(','),
@@ -71,7 +78,7 @@ RSpec.describe Admin::GroupsController do
       expect(response).to redirect_to(admin_group_path(group))
     end
 
-    it 'adds no user to members' do
+    it 'adds no user to members', :aggregate_failures do
       put :members_update, params: {
                              id: group,
                              user_ids: '',

spec/controllers/groups/group_members_controller_spec.rb

@@ -17,7 +17,7 @@ RSpec.describe Groups::GroupMembersController do
   end
 
   describe 'GET index' do
-    it 'renders index with 200 status code' do
+    it 'renders index with 200 status code', :aggregate_failures do
       get :index, params: { group_id: group }
 
       expect(response).to have_gitlab_http_status(:ok)
@@ -118,7 +118,7 @@ RSpec.describe Groups::GroupMembersController do
         group.add_developer(user)
       end
 
-      it 'returns 403' do
+      it 'returns 403', :aggregate_failures do
         post :create, params: {
                         group_id: group,
                         user_ids: group_user.id,
@@ -135,7 +135,7 @@ RSpec.describe Groups::GroupMembersController do
         group.add_owner(user)
       end
 
-      it 'adds user to members' do
+      it 'adds user to members', :aggregate_failures, :snowplow do
         post :create, params: {
                         group_id: group,
                         user_ids: group_user.id,
@@ -145,9 +145,16 @@ RSpec.describe Groups::GroupMembersController do
         expect(controller).to set_flash.to 'Users were successfully added.'
         expect(response).to redirect_to(group_group_members_path(group))
         expect(group.users).to include group_user
+        expect_snowplow_event(
+          category: 'Members::CreateService',
+          action: 'create_member',
+          label: 'group-members-page',
+          property: 'existing_user',
+          user: user
+        )
       end
 
-      it 'adds no user to members' do
+      it 'adds no user to members', :aggregate_failures do
         post :create, params: {
                         group_id: group,
                         user_ids: '',
@@ -177,7 +184,7 @@ RSpec.describe Groups::GroupMembersController do
       context 'when set to a date in the past' do
         let(:expires_at) { 2.days.ago }
 
-        it 'does not add user to members' do
+        it 'does not add user to members', :aggregate_failures do
           subject
 
           expect(flash[:alert]).to include('Expires at cannot be a date in the past')
@@ -189,7 +196,7 @@ RSpec.describe Groups::GroupMembersController do
       context 'when set to a date in the future' do
         let(:expires_at) { 5.days.from_now }
 
-        it 'adds user to members' do
+        it 'adds user to members', :aggregate_failures do
           subject
 
           expect(controller).to set_flash.to 'Users were successfully added.'
@@ -326,7 +333,7 @@ RSpec.describe Groups::GroupMembersController do
           group.add_developer(user)
         end
 
-        it 'returns 403' do
+        it 'returns 403', :aggregate_failures do
           delete :destroy, params: { group_id: group, id: member }
 
           expect(response).to have_gitlab_http_status(:forbidden)
@@ -339,7 +346,7 @@ RSpec.describe Groups::GroupMembersController do
           group.add_owner(user)
         end
 
-        it '[HTML] removes user from members' do
+        it '[HTML] removes user from members', :aggregate_failures do
           delete :destroy, params: { group_id: group, id: member }
 
           expect(controller).to set_flash.to 'User was successfully removed from group.'
@@ -348,7 +355,7 @@ RSpec.describe Groups::GroupMembersController do
           expect(sub_group.members).to include sub_member
         end
 
-        it '[HTML] removes user from members including subgroups and projects' do
+        it '[HTML] removes user from members including subgroups and projects', :aggregate_failures do
           delete :destroy, params: { group_id: group, id: member, remove_sub_memberships: true }
 
           expect(controller).to set_flash.to 'User was successfully removed from group and any subgroups and projects.'
@@ -357,7 +364,7 @@ RSpec.describe Groups::GroupMembersController do
           expect(sub_group.members).not_to include sub_member
         end
 
-        it '[JS] removes user from members' do
+        it '[JS] removes user from members', :aggregate_failures do
           delete :destroy, params: { group_id: group, id: member }, xhr: true
 
           expect(response).to be_successful
@@ -386,7 +393,7 @@ RSpec.describe Groups::GroupMembersController do
           group.add_developer(user)
         end
 
-        it 'removes user from members' do
+        it 'removes user from members', :aggregate_failures do
           delete :leave, params: { group_id: group }
 
           expect(controller).to set_flash.to "You left the \"#{group.name}\" group."
@@ -394,7 +401,7 @@ RSpec.describe Groups::GroupMembersController do
           expect(group.users).not_to include user
         end
 
-        it 'supports json request' do
+        it 'supports json request', :aggregate_failures do
           delete :leave, params: { group_id: group }, format: :json
 
           expect(response).to have_gitlab_http_status(:ok)
@@ -421,7 +428,7 @@ RSpec.describe Groups::GroupMembersController do
           group.request_access(user)
         end
 
-        it 'removes user from members' do
+        it 'removes user from members', :aggregate_failures do
           delete :leave, params: { group_id: group }
 
           expect(controller).to set_flash.to 'Your access request to the group has been withdrawn.'
@@ -438,7 +445,7 @@ RSpec.describe Groups::GroupMembersController do
       sign_in(user)
     end
 
-    it 'creates a new GroupMember that is not a team member' do
+    it 'creates a new GroupMember that is not a team member', :aggregate_failures do
       post :request_access, params: { group_id: group }
 
       expect(controller).to set_flash.to 'Your request for access has been queued for review.'
@@ -469,7 +476,7 @@ RSpec.describe Groups::GroupMembersController do
           group.add_developer(user)
         end
 
-        it 'returns 403' do
+        it 'returns 403', :aggregate_failures do
           post :approve_access_request, params: { group_id: group, id: member }
 
           expect(response).to have_gitlab_http_status(:forbidden)
@@ -482,7 +489,7 @@ RSpec.describe Groups::GroupMembersController do
           group.add_owner(user)
         end
 
-        it 'adds user to members' do
+        it 'adds user to members', :aggregate_failures do
           post :approve_access_request, params: { group_id: group, id: member }
 
           expect(response).to redirect_to(group_group_members_path(group))

spec/services/members/create_service_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
   let_it_be(:member) { create(:user) }
   let_it_be(:user_ids) { member.id.to_s }
   let_it_be(:access_level) { Gitlab::Access::GUEST }
-  let(:additional_params) { {} }
+  let(:additional_params) { { invite_source: '_invite_source_' } }
   let(:params) { { user_ids: user_ids, access_level: access_level }.merge(additional_params) }
 
   subject(:execute_service) { described_class.new(user, params.merge({ source: source })).execute }
@@ -86,22 +86,16 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
 
   context 'when tracking the invite source', :snowplow do
     context 'when invite_source is not passed' do
+      let(:additional_params) { {} }
+
       it 'tracks the invite source as unknown' do
-        execute_service
+        expect { execute_service }.to raise_error(ArgumentError, 'No invite source provided.')
 
-        expect_snowplow_event(
-          category: described_class.name,
-          action: 'create_member',
-          label: 'unknown',
-          property: 'existing_user',
-          user: user
-        )
+        expect_no_snowplow_event
       end
     end
 
-    context 'when invite_source is not passed' do
-      let(:additional_params) { { invite_source: '_invite_source_' } }
-
+    context 'when invite_source is passed' do
       it 'tracks the invite source from params' do
         execute_service
 
@@ -116,7 +110,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
     end
 
     context 'when it is a net_new_user' do
-      let(:additional_params) { { user_ids: 'email@example.org' } }
+      let(:additional_params) { { invite_source: '_invite_source_', user_ids: 'email@example.org' } }
 
       it 'tracks the invite source from params' do
         execute_service
@@ -124,7 +118,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
         expect_snowplow_event(
           category: described_class.name,
           action: 'create_member',
-          label: 'unknown',
+          label: '_invite_source_',
           property: 'net_new_user',
           user: user
         )

spec/services/members/invite_service_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
   let_it_be(:project_user) { create(:user) }
   let_it_be(:namespace) { project.namespace }
   let(:params) { {} }
-  let(:base_params) { { access_level: Gitlab::Access::GUEST, source: project } }
+  let(:base_params) { { access_level: Gitlab::Access::GUEST, source: project, invite_source: '_invite_source_' } }
 
   subject(:result) { described_class.new(user, base_params.merge(params) ).execute }