Commit 82938ec8 authored by Alex Kalderimis's avatar Alex Kalderimis

Merge branch...

Merge branch '333449-experiment-ask-admin-inviter-what-areas-they-d-like-the-invitee-to-focus-on' into 'master'

Add area_of_focus to members/invites

See merge request gitlab-org/gitlab!64956
parents 1a480295 864c59d6
...@@ -80,6 +80,11 @@ module Members ...@@ -80,6 +80,11 @@ module Members
def after_execute(member:) def after_execute(member:)
super super
track_invite_source(member)
track_areas_of_focus(member)
end
def track_invite_source(member)
Gitlab::Tracking.event(self.class.name, 'create_member', label: invite_source, property: tracking_property(member), user: current_user) Gitlab::Tracking.event(self.class.name, 'create_member', label: invite_source, property: tracking_property(member), user: current_user)
end end
...@@ -94,6 +99,16 @@ module Members ...@@ -94,6 +99,16 @@ module Members
member.invite? ? 'net_new_user' : 'existing_user' member.invite? ? 'net_new_user' : 'existing_user'
end end
def track_areas_of_focus(member)
areas_of_focus.each do |area_of_focus|
Gitlab::Tracking.event(self.class.name, 'area_of_focus', label: area_of_focus, property: member.id.to_s)
end
end
def areas_of_focus
params[:areas_of_focus] || []
end
def user_limit def user_limit
limit = params.fetch(:limit, DEFAULT_INVITE_LIMIT) limit = params.fetch(:limit, DEFAULT_INVITE_LIMIT)
......
...@@ -42,6 +42,7 @@ POST /projects/:id/invitations ...@@ -42,6 +42,7 @@ POST /projects/:id/invitations
| `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). | | `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). |
| `areas_of_focus` | string | no | Areas the inviter wants the member to focus upon. |
```shell ```shell
curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" \ curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" \
......
...@@ -418,6 +418,7 @@ POST /projects/:id/members ...@@ -418,6 +418,7 @@ POST /projects/:id/members
| `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). | | `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). |
| `areas_of_focus` | string | no | Areas the inviter wants the member to focus upon. |
```shell ```shell
curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" \ curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" \
......
...@@ -53,7 +53,7 @@ module EE ...@@ -53,7 +53,7 @@ module EE
override :create_member override :create_member
def create_member(current_user, user, source, params) def create_member(current_user, user, source, params)
member = source.add_user(user, params[:access_level], current_user: current_user, expires_at: params[:expires_at]) member = super
return false unless member return false unless member
......
...@@ -54,6 +54,14 @@ module API ...@@ -54,6 +54,14 @@ module API
source.add_user(user, params[:access_level], current_user: current_user, expires_at: params[:expires_at]) source.add_user(user, params[:access_level], current_user: current_user, expires_at: params[:expires_at])
end end
def track_areas_of_focus(member, areas_of_focus)
return unless areas_of_focus
areas_of_focus.each do |area_of_focus|
Gitlab::Tracking.event(::Members::CreateService.name, 'area_of_focus', label: area_of_focus, property: member.id.to_s)
end
end
def present_members(members) def present_members(members)
present members, with: Entities::Member, current_user: current_user, show_seat_info: params[:show_seat_info] present members, with: Entities::Member, current_user: current_user, show_seat_info: params[:show_seat_info]
end end
......
...@@ -24,6 +24,7 @@ module API ...@@ -24,6 +24,7 @@ module API
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: 'invitations-api' optional :invite_source, type: String, desc: 'Source that triggered the member creation process', default: 'invitations-api'
optional :areas_of_focus, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Areas the inviter wants the member to focus upon'
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])
...@@ -54,9 +55,9 @@ module API ...@@ -54,9 +55,9 @@ module API
success Entities::Member success Entities::Member
end end
params do params do
requires :email, type: String, desc: 'The email address of the invitation.' requires :email, type: String, desc: 'The email address of the invitation'
optional :access_level, type: Integer, values: Gitlab::Access.all_values, desc: 'A valid access level (defaults: `30`, developer access level).' optional :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 ISO 8601 format (`YYYY-MM-DDTHH:MM:SSZ`).' optional :expires_at, type: DateTime, desc: 'Date string in ISO 8601 format (`YYYY-MM-DDTHH:MM:SSZ`)'
end end
put ":id/invitations/:email", requirements: { email: /[^\/]+/ } do put ":id/invitations/:email", requirements: { email: /[^\/]+/ } do
source = find_source(source_type, params.delete(:id)) source = find_source(source_type, params.delete(:id))
......
...@@ -94,6 +94,7 @@ module API ...@@ -94,6 +94,7 @@ module API
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: 'members-api' optional :invite_source, type: String, desc: 'Source that triggered the member creation process', default: 'members-api'
optional :areas_of_focus, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Areas the inviter wants the member to focus upon'
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
post ":id/members" do post ":id/members" do
...@@ -119,7 +120,12 @@ module API ...@@ -119,7 +120,12 @@ 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', user: current_user) Gitlab::Tracking.event(::Members::CreateService.name,
'create_member',
label: params[:invite_source],
property: 'existing_user',
user: current_user)
track_areas_of_focus(member, params[:areas_of_focus])
else else
render_validation_error!(member) render_validation_error!(member)
end end
......
...@@ -152,6 +152,20 @@ RSpec.describe API::Invitations do ...@@ -152,6 +152,20 @@ RSpec.describe API::Invitations do
end end
end end
context 'with areas_of_focus', :snowplow do
it 'tracks the areas_of_focus from params' do
post invitations_url(source, maintainer),
params: { email: email, access_level: Member::DEVELOPER, areas_of_focus: 'Other' }
expect_snowplow_event(
category: 'Members::InviteService',
action: 'area_of_focus',
label: 'Other',
property: source.members.last.id.to_s
)
end
end
context 'with invite_source considerations', :snowplow do context 'with invite_source considerations', :snowplow do
let(:params) { { email: email, access_level: Member::DEVELOPER } } let(:params) { { email: email, access_level: Member::DEVELOPER } }
......
...@@ -409,6 +409,53 @@ RSpec.describe API::Members do ...@@ -409,6 +409,53 @@ RSpec.describe API::Members do
end end
end end
context 'with areas_of_focus considerations', :snowplow do
context 'when there is 1 user to add' do
let(:user_id) { stranger.id }
context 'when areas_of_focus is present in params' do
it 'tracks the areas_of_focus' do
post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
params: { user_id: user_id, access_level: Member::DEVELOPER, areas_of_focus: 'Other' }
expect_snowplow_event(
category: 'Members::CreateService',
action: 'area_of_focus',
label: 'Other',
property: source.members.last.id.to_s
)
end
end
context 'when areas_of_focus is not present in params' do
it 'does not track the areas_of_focus' do
post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
params: { user_id: user_id, access_level: Member::DEVELOPER }
expect_no_snowplow_event(category: 'Members::CreateService', action: 'area_of_focus')
end
end
end
context 'when there are multiple users to add' do
let(:user_id) { [developer.id, stranger.id].join(',') }
context 'when areas_of_focus is present in params' do
it 'tracks the areas_of_focus' do
post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
params: { user_id: user_id, access_level: Member::DEVELOPER, areas_of_focus: 'Other' }
expect_snowplow_event(
category: 'Members::CreateService',
action: 'area_of_focus',
label: 'Other',
property: source.members.last.id.to_s
)
end
end
end
end
it "returns 409 if member already exists" do it "returns 409 if member already exists" do
post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
params: { user_id: maintainer.id, access_level: Member::MAINTAINER } params: { user_id: maintainer.id, access_level: Member::MAINTAINER }
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_cache, :clean_gitlab_redis_shared_state, :sidekiq_inline do RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_cache, :clean_gitlab_redis_shared_state, :sidekiq_inline do
let_it_be(:source) { create(:project) } let_it_be(:source, reload: true) { create(:project) }
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
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 }
...@@ -89,7 +89,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_ ...@@ -89,7 +89,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
context 'when invite_source is not passed' do context 'when invite_source is not passed' do
let(:additional_params) { {} } let(:additional_params) { {} }
it 'tracks the invite source as unknown' do it 'raises an error' do
expect { execute_service }.to raise_error(ArgumentError, 'No invite source provided.') expect { execute_service }.to raise_error(ArgumentError, 'No invite source provided.')
expect_no_snowplow_event expect_no_snowplow_event
...@@ -126,4 +126,74 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_ ...@@ -126,4 +126,74 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
end end
end end
end end
context 'when tracking the areas of focus', :snowplow do
context 'when areas_of_focus is not passed' do
it 'does not track' do
execute_service
expect_no_snowplow_event(category: described_class.name, action: 'area_of_focus')
end
end
context 'when 1 areas_of_focus is passed' do
let(:additional_params) { { invite_source: '_invite_source_', areas_of_focus: ['no_selection'] } }
it 'tracks the areas_of_focus from params' do
execute_service
expect_snowplow_event(
category: described_class.name,
action: 'area_of_focus',
label: 'no_selection',
property: source.members.last.id.to_s
)
end
context 'when passing many user ids' do
let(:another_user) { create(:user) }
let(:user_ids) { [member.id, another_user.id].join(',') }
it 'tracks the areas_of_focus from params' do
execute_service
members = source.members.last(2)
expect_snowplow_event(
category: described_class.name,
action: 'area_of_focus',
label: 'no_selection',
property: members.first.id.to_s
)
expect_snowplow_event(
category: described_class.name,
action: 'area_of_focus',
label: 'no_selection',
property: members.last.id.to_s
)
end
end
end
context 'when multiple areas_of_focus are passed' do
let(:additional_params) { { invite_source: '_invite_source_', areas_of_focus: %w[no_selection Other] } }
it 'tracks the areas_of_focus from params' do
execute_service
expect_snowplow_event(
category: described_class.name,
action: 'area_of_focus',
label: 'no_selection',
property: source.members.last.id.to_s
)
expect_snowplow_event(
category: described_class.name,
action: 'area_of_focus',
label: 'Other',
property: source.members.last.id.to_s
)
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