Commit dbbecb88 authored by Vijay Hawoldar's avatar Vijay Hawoldar Committed by Douglas Barbosa Alexandre

Establish billable member membership type with a Group

parent b93a4223
...@@ -288,7 +288,8 @@ Example response: ...@@ -288,7 +288,8 @@ Example response:
"state": "active", "state": "active",
"avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon", "avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon",
"web_url": "http://192.168.1.8:3000/root", "web_url": "http://192.168.1.8:3000/root",
"last_activity_on": "2021-01-27" "last_activity_on": "2021-01-27",
"membership_type": "group_member"
}, },
{ {
"id": 2, "id": 2,
...@@ -298,7 +299,8 @@ Example response: ...@@ -298,7 +299,8 @@ Example response:
"avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon", "avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon",
"web_url": "http://192.168.1.8:3000/root", "web_url": "http://192.168.1.8:3000/root",
"email": "john@example.com", "email": "john@example.com",
"last_activity_on": "2021-01-25" "last_activity_on": "2021-01-25",
"membership_type": "group_member"
}, },
{ {
"id": 3, "id": 3,
...@@ -307,7 +309,8 @@ Example response: ...@@ -307,7 +309,8 @@ Example response:
"state": "active", "state": "active",
"avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon", "avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon",
"web_url": "http://192.168.1.8:3000/root", "web_url": "http://192.168.1.8:3000/root",
"last_activity_on": "2021-01-20" "last_activity_on": "2021-01-20",
"membership_type": "group_invite"
} }
] ]
``` ```
......
...@@ -8,12 +8,18 @@ class BilledUsersFinder ...@@ -8,12 +8,18 @@ class BilledUsersFinder
end end
def execute def execute
return User.none unless group_billed_user_ids.any? return {} unless user_ids.any?
users = ::User.id_in(group_billed_user_ids) users = ::User.id_in(user_ids)
users = users.search(search_term) if search_term users = users.search(search_term) if search_term
users.sort_by_attribute(order_by) {
users: users.sort_by_attribute(order_by),
group_member_user_ids: group_billed_user_ids[:group_member_user_ids],
project_member_user_ids: group_billed_user_ids[:project_member_user_ids],
shared_group_user_ids: group_billed_user_ids[:shared_group_user_ids],
shared_project_user_ids: group_billed_user_ids[:shared_project_user_ids]
}
end end
private private
...@@ -23,4 +29,8 @@ class BilledUsersFinder ...@@ -23,4 +29,8 @@ class BilledUsersFinder
def group_billed_user_ids def group_billed_user_ids
@group_billed_user_ids ||= group.billed_user_ids @group_billed_user_ids ||= group.billed_user_ids
end end
def user_ids
group_billed_user_ids[:user_ids]
end
end end
...@@ -295,31 +295,21 @@ module EE ...@@ -295,31 +295,21 @@ module EE
override :billable_members_count override :billable_members_count
def billable_members_count(requested_hosted_plan = nil) def billable_members_count(requested_hosted_plan = nil)
billed_user_ids(requested_hosted_plan).count billable_ids = billed_user_ids(requested_hosted_plan)
billable_ids[:user_ids].count
end end
# For now, we are not billing for members with a Guest role for subscriptions # For now, we are not billing for members with a Guest role for subscriptions
# with a Gold/Ultimate plan. The other plans will treat Guest members as a regular member # with a Gold/Ultimate plan. The other plans will treat Guest members as a regular member
# for billing purposes. # for billing purposes.
# #
# We are plucking the user_ids from the "Members" table in an array and # For the user_ids key, we are plucking the user_ids from the "Members" table in an array and
# converting the array of user_ids to a Set which will have unique user_ids. # converting the array of user_ids to a Set which will have unique user_ids.
def billed_user_ids(requested_hosted_plan = nil) def billed_user_ids(requested_hosted_plan = nil)
if ([actual_plan_name, requested_hosted_plan] & [::Plan::GOLD, ::Plan::ULTIMATE]).any? exclude_guests = ([actual_plan_name, requested_hosted_plan] & [::Plan::GOLD, ::Plan::ULTIMATE]).any?
strong_memoize(:billed_user_ids) do
(billed_group_members.non_guests.distinct.pluck(:user_id) + exclude_guests ? billed_user_ids_excluding_guests : billed_user_ids_including_guests
billed_project_members.non_guests.distinct.pluck(:user_id) +
billed_shared_non_guests_group_members.non_guests.distinct.pluck(:user_id) +
billed_invited_non_guests_group_to_project_members.non_guests.distinct.pluck(:user_id)).to_set
end
else
strong_memoize(:non_billed_user_ids) do
(billed_group_members.distinct.pluck(:user_id) +
billed_project_members.distinct.pluck(:user_id) +
billed_shared_group_members.distinct.pluck(:user_id) +
billed_invited_group_to_project_members.distinct.pluck(:user_id)).to_set
end
end
end end
override :supports_events? override :supports_events?
...@@ -516,6 +506,40 @@ module EE ...@@ -516,6 +506,40 @@ module EE
errors.add(:custom_project_templates_group_id, 'has to be a subgroup of the group') errors.add(:custom_project_templates_group_id, 'has to be a subgroup of the group')
end end
def billed_user_ids_excluding_guests
strong_memoize(:billed_user_ids_excluding_guests) do
group_member_ids = billed_group_members.non_guests.distinct.pluck(:user_id)
project_member_ids = billed_project_members.non_guests.distinct.pluck(:user_id)
shared_group_ids = billed_shared_non_guests_group_members.non_guests.distinct.pluck(:user_id)
shared_project_ids = billed_invited_non_guests_group_to_project_members.non_guests.distinct.pluck(:user_id)
{
user_ids: (group_member_ids + project_member_ids + shared_group_ids + shared_project_ids).to_set,
group_member_user_ids: group_member_ids.to_set,
project_member_user_ids: project_member_ids.to_set,
shared_group_user_ids: shared_group_ids.to_set,
shared_project_user_ids: shared_project_ids.to_set
}
end
end
def billed_user_ids_including_guests
strong_memoize(:billed_user_ids_including_guests) do
group_member_ids = billed_group_members.distinct.pluck(:user_id)
project_member_ids = billed_project_members.distinct.pluck(:user_id)
shared_group_ids = billed_shared_group_members.distinct.pluck(:user_id)
shared_project_ids = billed_invited_group_to_project_members.distinct.pluck(:user_id)
{
user_ids: (group_member_ids + project_member_ids + shared_group_ids + shared_project_ids).to_set,
group_member_user_ids: group_member_ids.to_set,
project_member_user_ids: project_member_ids.to_set,
shared_group_user_ids: shared_group_ids.to_set,
shared_project_user_ids: shared_project_ids.to_set
}
end
end
# Members belonging directly to Group or its subgroups # Members belonging directly to Group or its subgroups
def billed_group_members def billed_group_members
::GroupMember.active_without_invites_and_requests.where( ::GroupMember.active_without_invites_and_requests.where(
......
...@@ -277,7 +277,13 @@ module EE ...@@ -277,7 +277,13 @@ module EE
# This method is overwritten in Group where we made the calculation # This method is overwritten in Group where we made the calculation
# for Group namespaces. # for Group namespaces.
def billed_user_ids(_requested_hosted_plan = nil) def billed_user_ids(_requested_hosted_plan = nil)
[owner_id] {
user_ids: [owner_id],
group_member_user_ids: [],
project_member_user_ids: [],
shared_group_user_ids: [],
shared_project_user_ids: []
}
end end
def eligible_for_trial? def eligible_for_trial?
......
...@@ -281,7 +281,7 @@ module EE ...@@ -281,7 +281,7 @@ module EE
namespace.present? && namespace.present? &&
active? && active? &&
!namespace.root_ancestor.free_plan? && !namespace.root_ancestor.free_plan? &&
namespace.root_ancestor.billed_user_ids.include?(self.id) namespace.root_ancestor.billed_user_ids[:user_ids].include?(self.id)
end end
def group_sso?(group) def group_sso?(group)
......
---
title: Return billable member relationship to a group in REST API
merge_request: 60600
author:
type: changed
...@@ -6,6 +6,20 @@ module EE ...@@ -6,6 +6,20 @@ module EE
class BillableMember < ::API::Entities::UserBasic class BillableMember < ::API::Entities::UserBasic
expose :public_email, as: :email expose :public_email, as: :email
expose :last_activity_on expose :last_activity_on
expose :membership_type
private
def membership_type
return 'group_member' if user_in_array?(:group_member_user_ids)
return 'project_member' if user_in_array?(:project_member_user_ids)
return 'group_invite' if user_in_array?(:shared_group_user_ids)
return 'project_invite' if user_in_array?(:shared_project_user_ids)
end
def user_in_array?(name)
options.fetch(name, []).include?(object.id)
end
end end
end end
end end
......
...@@ -69,13 +69,18 @@ module EE ...@@ -69,13 +69,18 @@ module EE
bad_request!(nil) unless ::Ability.allowed?(current_user, :admin_group_member, group) bad_request!(nil) unless ::Ability.allowed?(current_user, :admin_group_member, group)
sorting = params[:sort] || 'id_asc' sorting = params[:sort] || 'id_asc'
users = paginate(
BilledUsersFinder.new(group,
search_term: params[:search],
order_by: sorting).execute
)
present users, with: ::EE::API::Entities::BillableMember, current_user: current_user result = BilledUsersFinder.new(group,
search_term: params[:search],
order_by: sorting).execute
present paginate(result[:users]),
with: ::EE::API::Entities::BillableMember,
current_user: current_user,
group_member_user_ids: result[:group_member_user_ids],
project_member_user_ids: result[:project_member_user_ids],
shared_group_user_ids: result[:shared_group_user_ids],
shared_project_user_ids: result[:shared_project_user_ids]
end end
desc 'Get the memberships of a billable user of a root group.' do desc 'Get the memberships of a billable user of a root group.' do
...@@ -93,7 +98,7 @@ module EE ...@@ -93,7 +98,7 @@ module EE
user = ::User.find(params[:user_id]) user = ::User.find(params[:user_id])
not_found!('User') unless group.billed_user_ids.include?(user.id) not_found!('User') unless group.billed_user_ids[:user_ids].include?(user.id)
memberships = user.members.in_hierarchy(group).including_source memberships = user.members.in_hierarchy(group).including_source
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe BilledUsersFinder do RSpec.describe BilledUsersFinder do
let_it_be(:group) { create(:group) } let_it_be_with_refind(:group) { create(:group) }
let(:search_term) { nil } let(:search_term) { nil }
let(:order_by) { nil } let(:order_by) { nil }
...@@ -17,10 +17,10 @@ RSpec.describe BilledUsersFinder do ...@@ -17,10 +17,10 @@ RSpec.describe BilledUsersFinder do
subject { described_class.new(group, search_term: search_term, order_by: order_by) } subject { described_class.new(group, search_term: search_term, order_by: order_by) }
context 'when a group does not have any billed users' do context 'when a group does not have any billed users' do
it 'returns an empty array' do it 'returns an empty object' do
allow(group).to receive(:billed_user_ids).and_return([]) allow(group).to receive(:billed_user_ids).and_return({ user_ids: [] })
expect(subject.execute).to be_empty expect(subject.execute).to eq({})
end end
end end
...@@ -31,7 +31,7 @@ RSpec.describe BilledUsersFinder do ...@@ -31,7 +31,7 @@ RSpec.describe BilledUsersFinder do
let(:order_by) { 'name_desc' } let(:order_by) { 'name_desc' }
it 'sorts results accordingly' do it 'sorts results accordingly' do
expect(subject.execute).to eq([john_smith, john_doe].map(&:user)) expect(subject.execute[:users]).to eq([john_smith, john_doe].map(&:user))
end end
end end
...@@ -39,7 +39,7 @@ RSpec.describe BilledUsersFinder do ...@@ -39,7 +39,7 @@ RSpec.describe BilledUsersFinder do
subject { described_class.new(group, search_term: search_term) } subject { described_class.new(group, search_term: search_term) }
it 'sorts expected results in name_asc order' do it 'sorts expected results in name_asc order' do
expect(subject.execute).to eq([john_doe, john_smith].map(&:user)) expect(subject.execute[:users]).to eq([john_doe, john_smith].map(&:user))
end end
end end
end end
...@@ -50,7 +50,7 @@ RSpec.describe BilledUsersFinder do ...@@ -50,7 +50,7 @@ RSpec.describe BilledUsersFinder do
it 'returns expected users in name asc order when a sorting is not provided either' do it 'returns expected users in name asc order when a sorting is not provided either' do
allow(group).to receive(:billed_user_members).and_return([john_doe, john_smith, sophie, maria]) allow(group).to receive(:billed_user_members).and_return([john_doe, john_smith, sophie, maria])
expect(subject.execute).to eq([john_doe, john_smith, maria, sophie].map(&:user)) expect(subject.execute[:users]).to eq([john_doe, john_smith, maria, sophie].map(&:user))
end end
context 'and when a sorting parameter is provided (eg name descending)' do context 'and when a sorting parameter is provided (eg name descending)' do
...@@ -59,7 +59,38 @@ RSpec.describe BilledUsersFinder do ...@@ -59,7 +59,38 @@ RSpec.describe BilledUsersFinder do
subject { described_class.new(group, search_term: search_term, order_by: order_by) } subject { described_class.new(group, search_term: search_term, order_by: order_by) }
it 'sorts results accordingly' do it 'sorts results accordingly' do
expect(subject.execute).to eq([sophie, maria, john_smith, john_doe].map(&:user)) expect(subject.execute[:users]).to eq([sophie, maria, john_smith, john_doe].map(&:user))
end
end
end
context 'with billable group members including shared members' do
let_it_be(:shared_with_group_member) { create(:group_member, user: create(:user, name: 'Shared Group User')) }
let_it_be(:shared_with_project_member) { create(:group_member, user: create(:user, name: 'Shared Project User')) }
let_it_be(:project) { create(:project, group: group) }
before do
create(:group_group_link, shared_group: group, shared_with_group: shared_with_group_member.group)
create(:project_group_link, group: shared_with_project_member.group, project: project)
end
it 'returns a hash of users and user ids' do
expect(subject.execute.keys).to eq([
:users,
:group_member_user_ids,
:project_member_user_ids,
:shared_group_user_ids,
:shared_project_user_ids
])
end
it 'returns the correct user ids' do
result = subject.execute
aggregate_failures do
expect(result[:group_member_user_ids]).to contain_exactly(*[maria, john_smith, john_doe, sophie].map(&:user_id))
expect(result[:shared_group_user_ids]).to contain_exactly(shared_with_group_member.user_id)
expect(result[:shared_project_user_ids]).to contain_exactly(shared_with_project_member.user_id)
end end
end end
end end
......
...@@ -5,9 +5,17 @@ require 'spec_helper' ...@@ -5,9 +5,17 @@ require 'spec_helper'
RSpec.describe ::EE::API::Entities::BillableMember do RSpec.describe ::EE::API::Entities::BillableMember do
let(:last_activity_on) { Date.today } let(:last_activity_on) { Date.today }
let(:public_email) { nil } let(:public_email) { nil }
let(:member) { build(:user, public_email: public_email, email: 'private@email.com', last_activity_on: last_activity_on) } let(:member) { build(:user, id: non_existing_record_id, public_email: public_email, email: 'private@email.com', last_activity_on: last_activity_on) }
let(:options) do
{
group_member_user_ids: [],
project_member_user_ids: [],
shared_group_user_ids: [],
shared_project_user_ids: []
}
end
subject(:entity_representation) { described_class.new(member).as_json } subject(:entity_representation) { described_class.new(member, options).as_json }
it 'returns the last_activity_on attribute' do it 'returns the last_activity_on attribute' do
expect(entity_representation[:last_activity_on]).to eq last_activity_on expect(entity_representation[:last_activity_on]).to eq last_activity_on
...@@ -35,4 +43,34 @@ RSpec.describe ::EE::API::Entities::BillableMember do ...@@ -35,4 +43,34 @@ RSpec.describe ::EE::API::Entities::BillableMember do
end end
end end
end end
context 'with different group membership types' do
using RSpec::Parameterized::TableSyntax
where(:user_ids, :expected_value) do
:group_member_user_ids | 'group_member'
:project_member_user_ids | 'project_member'
:shared_group_user_ids | 'group_invite'
:shared_project_user_ids | 'project_invite'
end
with_them do
let(:options) { super().merge(user_ids => [member.id]) }
it 'returns the expected value' do
expect(entity_representation[:membership_type]).to eq expected_value
end
end
context 'with a missing membership type' do
before do
options.delete(:group_member_user_ids)
end
it 'does not raise an error' do
expect(options[:group_member_user_ids]).to be_nil
expect { entity_representation }.not_to raise_error
end
end
end
end end
This diff is collapsed.
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