Commit 0f35a570 authored by Max Woolf's avatar Max Woolf

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

Add invite source tracking to member creation

See merge request gitlab-org/gitlab!62406
parents 0d951d1c 3fd25d6b
...@@ -72,6 +72,23 @@ module Members ...@@ -72,6 +72,23 @@ module Members
errors << "#{prefix}#{member.errors.full_messages.to_sentence}" errors << "#{prefix}#{member.errors.full_messages.to_sentence}"
end end
def after_execute(member:)
super
Gitlab::Tracking.event(self.class.name, 'create_member', label: invite_source, property: tracking_property(member))
end
def invite_source
params[:invite_source] || 'unknown'
end
def tracking_property(member)
# ideally invites go down the invite service class instead, but there is nothing that limits an invite
# from being used in this class and if you send emails as a comma separated list to the api/members
# endpoint, it will support invites
member.invite? ? 'net_new_user' : 'existing_user'
end
def user_limit def user_limit
limit = params.fetch(:limit, DEFAULT_INVITE_LIMIT) limit = params.fetch(:limit, DEFAULT_INVITE_LIMIT)
......
...@@ -41,6 +41,7 @@ POST /projects/:id/invitations ...@@ -41,6 +41,7 @@ POST /projects/:id/invitations
| `email` | string | yes | The email of the new member or multiple emails separated by commas | | `email` | string | yes | The email of the new member or multiple emails separated by commas |
| `access_level` | integer | yes | A valid access level | | `access_level` | integer | yes | A valid access level |
| `expires_at` | string | no | A date string in the format YEAR-MONTH-DAY | | `expires_at` | string | no | A date string in the format YEAR-MONTH-DAY |
| `invite_source` | string | no | The source of the invitation that starts the member creation process. See [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/327120). |
```shell ```shell
curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" --data "email=test@example.com&access_level=30" "https://gitlab.example.com/api/v4/groups/:id/invitations" curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" --data "email=test@example.com&access_level=30" "https://gitlab.example.com/api/v4/groups/:id/invitations"
......
...@@ -415,6 +415,7 @@ POST /projects/:id/members ...@@ -415,6 +415,7 @@ POST /projects/:id/members
| `user_id` | integer/string | yes | The user ID of the new member or multiple IDs separated by commas | | `user_id` | integer/string | yes | The user ID of the new member or multiple IDs separated by commas |
| `access_level` | integer | yes | A valid access level | | `access_level` | integer | yes | A valid access level |
| `expires_at` | string | no | A date string in the format `YEAR-MONTH-DAY` | | `expires_at` | string | no | A date string in the format `YEAR-MONTH-DAY` |
| `invite_source` | string | no | The source of the invitation that starts the member creation process. See [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/327120). |
```shell ```shell
curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" --data "user_id=1&access_level=30" "https://gitlab.example.com/api/v4/groups/:id/members" curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" --data "user_id=1&access_level=30" "https://gitlab.example.com/api/v4/groups/:id/members"
......
...@@ -23,6 +23,7 @@ module API ...@@ -23,6 +23,7 @@ module API
requires :email, types: [String, Array[String]], email_or_email_list: true, desc: 'The email address to invite, or multiple emails separated by comma' requires :email, types: [String, Array[String]], email_or_email_list: true, desc: 'The email address to invite, or multiple emails separated by comma'
requires :access_level, type: Integer, values: Gitlab::Access.all_values, desc: 'A valid access level (defaults: `30`, developer access level)' requires :access_level, type: Integer, values: Gitlab::Access.all_values, desc: 'A valid access level (defaults: `30`, developer access level)'
optional :expires_at, type: DateTime, desc: 'Date string in the format YEAR-MONTH-DAY' optional :expires_at, type: DateTime, desc: 'Date string in the format YEAR-MONTH-DAY'
optional :invite_source, type: String, desc: 'Source that triggered the member creation process', default: 'api'
end end
post ":id/invitations" do post ":id/invitations" do
params[:source] = find_source(source_type, params[:id]) params[:source] = find_source(source_type, params[:id])
......
...@@ -93,6 +93,7 @@ module API ...@@ -93,6 +93,7 @@ module API
requires :access_level, type: Integer, desc: 'A valid access level (defaults: `30`, developer access level)' requires :access_level, type: Integer, desc: 'A valid access level (defaults: `30`, developer access level)'
requires :user_id, types: [Integer, String], desc: 'The user ID of the new member or multiple IDs separated by commas.' requires :user_id, types: [Integer, String], desc: 'The user ID of the new member or multiple IDs separated by commas.'
optional :expires_at, type: DateTime, desc: 'Date string in the format YEAR-MONTH-DAY' optional :expires_at, type: DateTime, desc: 'Date string in the format YEAR-MONTH-DAY'
optional :invite_source, type: String, desc: 'Source that triggered the member creation process', default: 'api'
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
post ":id/members" do post ":id/members" do
...@@ -116,6 +117,7 @@ module API ...@@ -116,6 +117,7 @@ module API
not_allowed! # This currently can only be reached in EE not_allowed! # This currently can only be reached in EE
elsif member.valid? && member.persisted? elsif member.valid? && member.persisted?
present_members(member) present_members(member)
Gitlab::Tracking.event(::Members::CreateService.name, 'create_member', label: params[:invite_source], property: 'existing_user')
else else
render_validation_error!(member) render_validation_error!(member)
end end
......
...@@ -61,7 +61,7 @@ RSpec.describe API::Invitations do ...@@ -61,7 +61,7 @@ RSpec.describe API::Invitations do
context 'and new member is already a requester' do context 'and new member is already a requester' do
it 'does not transform the requester into a proper member' do it 'does not transform the requester into a proper member' do
expect do expect do
post api("/#{source_type.pluralize}/#{source.id}/invitations", maintainer), post invitations_url(source, maintainer),
params: { email: access_requester.email, access_level: Member::MAINTAINER } params: { email: access_requester.email, access_level: Member::MAINTAINER }
expect(response).to have_gitlab_http_status(:created) expect(response).to have_gitlab_http_status(:created)
...@@ -71,7 +71,7 @@ RSpec.describe API::Invitations do ...@@ -71,7 +71,7 @@ RSpec.describe API::Invitations do
it 'invites a new member' do it 'invites a new member' do
expect do expect do
post api("/#{source_type.pluralize}/#{source.id}/invitations", maintainer), post invitations_url(source, maintainer),
params: { email: email, access_level: Member::DEVELOPER } params: { email: email, access_level: Member::DEVELOPER }
expect(response).to have_gitlab_http_status(:created) expect(response).to have_gitlab_http_status(:created)
...@@ -82,7 +82,7 @@ RSpec.describe API::Invitations do ...@@ -82,7 +82,7 @@ RSpec.describe API::Invitations do
expect do expect do
email_list = [email, email2].join(',') email_list = [email, email2].join(',')
post api("/#{source_type.pluralize}/#{source.id}/invitations", maintainer), post invitations_url(source, maintainer),
params: { email: email_list, access_level: Member::DEVELOPER } params: { email: email_list, access_level: Member::DEVELOPER }
expect(response).to have_gitlab_http_status(:created) expect(response).to have_gitlab_http_status(:created)
...@@ -98,7 +98,7 @@ RSpec.describe API::Invitations do ...@@ -98,7 +98,7 @@ RSpec.describe API::Invitations do
project.update!(group: group) project.update!(group: group)
parent.add_developer(stranger) parent.add_developer(stranger)
post api("/#{source_type.pluralize}/#{source.id}/invitations", maintainer), post invitations_url(source, maintainer),
params: { email: stranger.email, access_level: Member::REPORTER } params: { email: stranger.email, access_level: Member::REPORTER }
expect(response).to have_gitlab_http_status(:created) expect(response).to have_gitlab_http_status(:created)
...@@ -113,7 +113,7 @@ RSpec.describe API::Invitations do ...@@ -113,7 +113,7 @@ RSpec.describe API::Invitations do
project.update!(group: group) project.update!(group: group)
parent.add_developer(stranger) parent.add_developer(stranger)
post api("/#{source_type.pluralize}/#{source.id}/invitations", maintainer), post invitations_url(source, maintainer),
params: { email: stranger.email, access_level: Member::MAINTAINER } params: { email: stranger.email, access_level: Member::MAINTAINER }
expect(response).to have_gitlab_http_status(:created) expect(response).to have_gitlab_http_status(:created)
...@@ -122,7 +122,7 @@ RSpec.describe API::Invitations do ...@@ -122,7 +122,7 @@ RSpec.describe API::Invitations do
context 'access expiry date' do context 'access expiry date' do
subject do subject do
post api("/#{source_type.pluralize}/#{source.id}/invitations", maintainer), post invitations_url(source, maintainer),
params: { email: email, access_level: Member::DEVELOPER, expires_at: expires_at } params: { email: email, access_level: Member::DEVELOPER, expires_at: expires_at }
end end
...@@ -152,8 +152,34 @@ RSpec.describe API::Invitations do ...@@ -152,8 +152,34 @@ RSpec.describe API::Invitations do
end end
end end
context 'with invite_source considerations', :snowplow do
let(:params) { { email: email, access_level: Member::DEVELOPER } }
it 'tracks the invite source as api' do
post invitations_url(source, maintainer), params: params
expect_snowplow_event(
category: 'Members::InviteService',
action: 'create_member',
label: 'api',
property: 'net_new_user'
)
end
it 'tracks the invite source from params' do
post invitations_url(source, maintainer), params: params.merge(invite_source: '_invite_source_')
expect_snowplow_event(
category: 'Members::InviteService',
action: 'create_member',
label: '_invite_source_',
property: 'net_new_user'
)
end
end
it "returns a message if member already exists" do it "returns a message if member already exists" do
post api("/#{source_type.pluralize}/#{source.id}/invitations", maintainer), post invitations_url(source, maintainer),
params: { email: developer.email, access_level: Member::MAINTAINER } params: { email: developer.email, access_level: Member::MAINTAINER }
expect(response).to have_gitlab_http_status(:created) expect(response).to have_gitlab_http_status(:created)
...@@ -161,7 +187,7 @@ RSpec.describe API::Invitations do ...@@ -161,7 +187,7 @@ RSpec.describe API::Invitations do
end end
it 'returns 404 when the email is not valid' do it 'returns 404 when the email is not valid' do
post api("/#{source_type.pluralize}/#{source.id}/invitations", maintainer), post invitations_url(source, maintainer),
params: { email: '', access_level: Member::MAINTAINER } params: { email: '', access_level: Member::MAINTAINER }
expect(response).to have_gitlab_http_status(:created) expect(response).to have_gitlab_http_status(:created)
...@@ -169,7 +195,7 @@ RSpec.describe API::Invitations do ...@@ -169,7 +195,7 @@ RSpec.describe API::Invitations do
end end
it 'returns 404 when the email list is not a valid format' do it 'returns 404 when the email list is not a valid format' do
post api("/#{source_type.pluralize}/#{source.id}/invitations", maintainer), post invitations_url(source, maintainer),
params: { email: 'email1@example.com,not-an-email', access_level: Member::MAINTAINER } params: { email: 'email1@example.com,not-an-email', access_level: Member::MAINTAINER }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
...@@ -177,14 +203,14 @@ RSpec.describe API::Invitations do ...@@ -177,14 +203,14 @@ RSpec.describe API::Invitations do
end end
it 'returns 400 when email is not given' do it 'returns 400 when email is not given' do
post api("/#{source_type.pluralize}/#{source.id}/invitations", maintainer), post invitations_url(source, maintainer),
params: { access_level: Member::MAINTAINER } params: { access_level: Member::MAINTAINER }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
it 'returns 400 when access_level is not given' do it 'returns 400 when access_level is not given' do
post api("/#{source_type.pluralize}/#{source.id}/invitations", maintainer), post invitations_url(source, maintainer),
params: { email: email } params: { email: email }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
......
...@@ -255,11 +255,39 @@ RSpec.describe API::Members do ...@@ -255,11 +255,39 @@ RSpec.describe API::Members do
expect(json_response['access_level']).to eq(Member::DEVELOPER) expect(json_response['access_level']).to eq(Member::DEVELOPER)
end end
describe 'executes the Members::CreateService for multiple user_ids' do context 'with invite_source considerations', :snowplow do
let(:params) { { user_id: stranger.id, access_level: Member::DEVELOPER } }
it 'tracks the invite source as api' do
post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
params: params
expect_snowplow_event(
category: 'Members::CreateService',
action: 'create_member',
label: 'api',
property: 'existing_user'
)
end
it 'tracks the invite source from params' do
post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
params: params.merge(invite_source: '_invite_source_')
expect_snowplow_event(
category: 'Members::CreateService',
action: 'create_member',
label: '_invite_source_',
property: 'existing_user'
)
end
end
context 'when executing the Members::CreateService for multiple user_ids' do
let(:user_ids) { [stranger.id, access_requester.id].join(',') }
it 'returns success when it successfully create all members' do it 'returns success when it successfully create all members' do
expect do expect do
user_ids = [stranger.id, access_requester.id].join(',')
post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
params: { user_id: user_ids, access_level: Member::DEVELOPER } params: { user_id: user_ids, access_level: Member::DEVELOPER }
...@@ -270,8 +298,6 @@ RSpec.describe API::Members do ...@@ -270,8 +298,6 @@ RSpec.describe API::Members do
it 'returns the error message if there was an error adding members to group' do it 'returns the error message if there was an error adding members to group' do
error_message = 'Unable to find User ID' error_message = 'Unable to find User ID'
user_ids = [stranger.id, access_requester.id].join(',')
allow_next_instance_of(::Members::CreateService) do |service| allow_next_instance_of(::Members::CreateService) do |service|
expect(service).to receive(:execute).and_return({ status: :error, message: error_message }) expect(service).to receive(:execute).and_return({ status: :error, message: error_message })
end end
...@@ -283,6 +309,34 @@ RSpec.describe API::Members do ...@@ -283,6 +309,34 @@ RSpec.describe API::Members do
expect(json_response['status']).to eq('error') expect(json_response['status']).to eq('error')
expect(json_response['message']).to eq(error_message) expect(json_response['message']).to eq(error_message)
end end
context 'with invite_source considerations', :snowplow do
let(:params) { { user_id: user_ids, access_level: Member::DEVELOPER } }
it 'tracks the invite source as api' do
post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
params: params
expect_snowplow_event(
category: 'Members::CreateService',
action: 'create_member',
label: 'api',
property: 'existing_user'
)
end
it 'tracks the invite source from params' do
post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
params: params.merge(invite_source: '_invite_source_')
expect_snowplow_event(
category: 'Members::CreateService',
action: 'create_member',
label: '_invite_source_',
property: 'existing_user'
)
end
end
end end
end end
......
...@@ -8,7 +8,8 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_ ...@@ -8,7 +8,8 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
let_it_be(:member) { create(:user) } let_it_be(:member) { create(:user) }
let_it_be(:user_ids) { member.id.to_s } let_it_be(:user_ids) { member.id.to_s }
let_it_be(:access_level) { Gitlab::Access::GUEST } let_it_be(:access_level) { Gitlab::Access::GUEST }
let(:params) { { user_ids: user_ids, access_level: access_level } } let(:additional_params) { {} }
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 } subject(:execute_service) { described_class.new(user, params.merge({ source: source })).execute }
...@@ -82,4 +83,49 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_ ...@@ -82,4 +83,49 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
expect(OnboardingProgress.completed?(source.namespace, :user_added)).to be(false) expect(OnboardingProgress.completed?(source.namespace, :user_added)).to be(false)
end end
end end
context 'when tracking the invite source', :snowplow do
context 'when invite_source is not passed' do
it 'tracks the invite source as unknown' do
execute_service
expect_snowplow_event(
category: described_class.name,
action: 'create_member',
label: 'unknown',
property: 'existing_user'
)
end
end
context 'when invite_source is not passed' do
let(:additional_params) { { invite_source: '_invite_source_' } }
it 'tracks the invite source from params' do
execute_service
expect_snowplow_event(
category: described_class.name,
action: 'create_member',
label: '_invite_source_',
property: 'existing_user'
)
end
end
context 'when it is a net_new_user' do
let(:additional_params) { { user_ids: 'email@example.org' } }
it 'tracks the invite source from params' do
execute_service
expect_snowplow_event(
category: described_class.name,
action: 'create_member',
label: 'unknown',
property: 'net_new_user'
)
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