Commit 1e214a79 authored by Max Woolf's avatar Max Woolf

Merge branch 'jh-eligible_add_on_packs' into 'master'

Use plan ID to filter namespace eligibility for purchase

See merge request gitlab-org/gitlab!66981
parents 6b1e2864 fd755652
...@@ -32,7 +32,9 @@ class SubscriptionsController < ApplicationController ...@@ -32,7 +32,9 @@ class SubscriptionsController < ApplicationController
end end
def buy_minutes def buy_minutes
@group = find_group return render_404 unless ci_minutes_plan_data.present?
@group = find_group(plan_id: ci_minutes_plan_data["id"])
return render_404 if @group.nil? return render_404 if @group.nil?
...@@ -51,7 +53,7 @@ class SubscriptionsController < ApplicationController ...@@ -51,7 +53,7 @@ class SubscriptionsController < ApplicationController
def create def create
current_user.update(setup_for_company: true) if params[:setup_for_company] current_user.update(setup_for_company: true) if params[:setup_for_company]
group = params[:selected_group] ? find_group : create_group group = params[:selected_group] ? find_group(plan_id: subscription_params[:plan_id]) : create_group
return not_found if group.nil? return not_found if group.nil?
return render json: group.errors.to_json unless group.persisted? return render json: group.errors.to_json unless group.persisted?
...@@ -90,11 +92,11 @@ class SubscriptionsController < ApplicationController ...@@ -90,11 +92,11 @@ class SubscriptionsController < ApplicationController
params.require(:subscription).permit(:plan_id, :payment_method_id, :quantity, :source) params.require(:subscription).permit(:plan_id, :payment_method_id, :quantity, :source)
end end
def find_group def find_group(plan_id:)
selected_group = current_user.manageable_groups.top_most.find(params[:selected_group]) selected_group = current_user.manageable_groups.top_most.find(params[:selected_group])
result = GitlabSubscriptions::FilterPurchaseEligibleNamespacesService result = GitlabSubscriptions::FilterPurchaseEligibleNamespacesService
.new(user: current_user, namespaces: Array(selected_group)) .new(user: current_user, plan_id: plan_id, namespaces: Array(selected_group))
.execute .execute
result.success? ? result.payload.first : nil result.success? ? result.payload.first : nil
...@@ -118,13 +120,21 @@ class SubscriptionsController < ApplicationController ...@@ -118,13 +120,21 @@ class SubscriptionsController < ApplicationController
redirect_to new_user_registration_path(redirect_from: from) redirect_to new_user_registration_path(redirect_from: from)
end end
def ci_minutes_plan_data
strong_memoize(:ci_minutes_plan_data) do
plan_response = client.get_plans(tags: ['CI_1000_MINUTES_PLAN'])
plan_response[:success] ? plan_response[:data].first : nil
end
end
def load_eligible_groups def load_eligible_groups
return unless current_user return @eligible_groups = [] unless current_user
candidate_groups = current_user.manageable_groups.top_most.with_counts(archived: false) candidate_groups = current_user.manageable_groups.top_most.with_counts(archived: false)
result = GitlabSubscriptions::FilterPurchaseEligibleNamespacesService result = GitlabSubscriptions::FilterPurchaseEligibleNamespacesService
.new(user: current_user, namespaces: candidate_groups) .new(user: current_user, namespaces: candidate_groups, any_self_service_plan: true)
.execute .execute
@eligible_groups = result.success? ? result.payload : [] @eligible_groups = result.success? ? result.payload : []
......
# frozen_string_literal: true # frozen_string_literal: true
# Filter a list of namespaces by their eligibility to purchase a new plan.
#
# - When `plan_id: ID` is supplied the eligibility will be checked for that specific plan ID.
# This param should be supplied when checking add on pack eligibility.
# - When `any_self_service_plan: Boolean` is supplied, the eligibility to have a new self-service plan
# (ie Premium/Ultimate) in general is checked.
module GitlabSubscriptions module GitlabSubscriptions
class FilterPurchaseEligibleNamespacesService class FilterPurchaseEligibleNamespacesService
include ::Gitlab::Utils::StrongMemoize include ::Gitlab::Utils::StrongMemoize
def initialize(user:, namespaces:) def initialize(user:, namespaces:, plan_id: nil, any_self_service_plan: nil)
@user = user @user = user
@namespaces = namespaces @namespaces = namespaces
@plan_id = plan_id
@any_self_service_plan = any_self_service_plan
end end
def execute def execute
return success([]) if namespaces.empty? return success([]) if namespaces.empty?
return missing_user_error if user.nil? return missing_user_error if user.nil?
return missing_plan_error if plan_id.nil? && any_self_service_plan.nil?
if response[:success] if response[:success] && response[:data]
eligible_ids = response[:data].map { |data| data['id'] }.to_set eligible_ids = response[:data].map { |data| data['id'] }.to_set
data = namespaces.filter { |namespace| eligible_ids.include?(namespace.id) } data = namespaces.filter { |namespace| eligible_ids.include?(namespace.id) }
...@@ -26,7 +35,7 @@ module GitlabSubscriptions ...@@ -26,7 +35,7 @@ module GitlabSubscriptions
private private
attr_reader :user, :namespaces attr_reader :user, :namespaces, :plan_id, :any_self_service_plan
def success(payload) def success(payload)
ServiceResponse.success(payload: payload) ServiceResponse.success(payload: payload)
...@@ -43,9 +52,21 @@ module GitlabSubscriptions ...@@ -43,9 +52,21 @@ module GitlabSubscriptions
error(message) error(message)
end end
def missing_plan_error
message = 'plan_id and any_self_service_plan cannot both be nil'
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(ArgumentError.new(message))
error(message)
end
def response def response
strong_memoize(:response) do strong_memoize(:response) do
Gitlab::SubscriptionPortal::Client.filter_purchase_eligible_namespaces(user, namespaces) Gitlab::SubscriptionPortal::Client.filter_purchase_eligible_namespaces(
user,
namespaces,
plan_id: plan_id,
any_self_service_plan: any_self_service_plan
)
end end
end end
end end
......
...@@ -98,10 +98,30 @@ module Gitlab ...@@ -98,10 +98,30 @@ module Gitlab
end end
end end
def filter_purchase_eligible_namespaces(user, namespaces) def get_plans(tags:)
query = <<~GQL query = <<~GQL
query FilterEligibleNamespaces($customerUid: Int!, $namespaces: [GitlabNamespaceInput!]!) { query getPlans($tags: [PlanTag!]) {
namespaceEligibility(customerUid: $customerUid, namespaces: $namespaces, eligibleForPurchase: true) { plans(planTags: $tags) {
id
}
}
GQL
response = http_post('graphql', admin_headers, { query: query, variables: { tags: tags } })[:data]
if response['errors'].blank? && (data = response.dig('data', 'plans'))
{ success: true, data: data }
else
track_error(query, response)
error(response['errors'])
end
end
def filter_purchase_eligible_namespaces(user, namespaces, plan_id: nil, any_self_service_plan: nil)
query = <<~GQL
query FilterEligibleNamespaces($customerUid: Int!, $namespaces: [GitlabNamespaceInput!]!, $planId: ID, $eligibleForPurchase: Boolean) {
namespaceEligibility(customerUid: $customerUid, namespaces: $namespaces, planId: $planId, eligibleForPurchase: $eligibleForPurchase) {
id id
} }
} }
...@@ -112,18 +132,25 @@ module Gitlab ...@@ -112,18 +132,25 @@ module Gitlab
id: namespace.id, id: namespace.id,
parentId: namespace.parent_id, parentId: namespace.parent_id,
plan: namespace.actual_plan_name, plan: namespace.actual_plan_name,
trial: !!namespace.trial? trial: !!namespace.trial?,
kind: namespace.kind,
membersCountWithDescendants: namespace.group? ? namespace.users_with_descendants.count : nil
} }
end end
response = http_post( response = http_post(
"graphql", 'graphql',
admin_headers, admin_headers,
{ query: query, variables: { customerUid: user.id, namespaces: namespace_data } } { query: query, variables: {
customerUid: user.id,
namespaces: namespace_data,
planId: plan_id,
eligibleForPurchase: any_self_service_plan
} }
)[:data] )[:data]
if response['errors'].blank? if response['errors'].blank? && (data = response.dig('data', 'namespaceEligibility'))
{ success: true, data: response.dig('data', 'namespaceEligibility') } { success: true, data: data }
else else
track_error(query, response) track_error(query, response)
......
...@@ -38,7 +38,12 @@ RSpec.describe SubscriptionsController do ...@@ -38,7 +38,12 @@ RSpec.describe SubscriptionsController do
before do before do
group.add_owner(user) group.add_owner(user)
allow_next_instance_of(GitlabSubscriptions::FilterPurchaseEligibleNamespacesService, user: user, namespaces: [group]) do |instance| allow_next_instance_of(
GitlabSubscriptions::FilterPurchaseEligibleNamespacesService,
user: user,
namespaces: [group],
any_self_service_plan: true
) do |instance|
allow(instance).to receive(:execute).and_return(instance_double(ServiceResponse, success?: true, payload: [group])) allow(instance).to receive(:execute).and_return(instance_double(ServiceResponse, success?: true, payload: [group]))
end end
end end
...@@ -52,7 +57,12 @@ RSpec.describe SubscriptionsController do ...@@ -52,7 +57,12 @@ RSpec.describe SubscriptionsController do
context 'when there are no eligible groups for the subscription' do context 'when there are no eligible groups for the subscription' do
it 'assigns eligible groups as an empty array' do it 'assigns eligible groups as an empty array' do
allow_next_instance_of(GitlabSubscriptions::FilterPurchaseEligibleNamespacesService, user: user, namespaces: []) do |instance| allow_next_instance_of(
GitlabSubscriptions::FilterPurchaseEligibleNamespacesService,
user: user,
namespaces: [],
any_self_service_plan: true
) do |instance|
allow(instance).to receive(:execute).and_return(instance_double(ServiceResponse, success?: true, payload: [])) allow(instance).to receive(:execute).and_return(instance_double(ServiceResponse, success?: true, payload: []))
end end
...@@ -77,13 +87,36 @@ RSpec.describe SubscriptionsController do ...@@ -77,13 +87,36 @@ RSpec.describe SubscriptionsController do
sign_in(user) sign_in(user)
end end
context 'when the add-on plan cannot be found' do
let_it_be(:group) { create(:group) }
before do
group.add_owner(user)
allow(Gitlab::SubscriptionPortal::Client)
.to receive(:get_plans).with(tags: ['CI_1000_MINUTES_PLAN'])
.and_return({ success: false, data: [] })
end
it { is_expected.to have_gitlab_http_status(:not_found) }
end
context 'when there are groups eligible for the addon' do context 'when there are groups eligible for the addon' do
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
before do before do
group.add_owner(user) group.add_owner(user)
allow_next_instance_of(GitlabSubscriptions::FilterPurchaseEligibleNamespacesService, user: user, namespaces: [group]) do |instance| allow(Gitlab::SubscriptionPortal::Client)
.to receive(:get_plans).with(tags: ['CI_1000_MINUTES_PLAN'])
.and_return({ success: true, data: [{ 'id' => 'ci_minutes' }] })
allow_next_instance_of(
GitlabSubscriptions::FilterPurchaseEligibleNamespacesService,
user: user,
plan_id: 'ci_minutes',
namespaces: [group]
) do |instance|
allow(instance).to receive(:execute).and_return(instance_double(ServiceResponse, success?: true, payload: [group])) allow(instance).to receive(:execute).and_return(instance_double(ServiceResponse, success?: true, payload: [group]))
end end
end end
...@@ -101,6 +134,10 @@ RSpec.describe SubscriptionsController do ...@@ -101,6 +134,10 @@ RSpec.describe SubscriptionsController do
context 'with :new_route_ci_minutes_purchase disabled' do context 'with :new_route_ci_minutes_purchase disabled' do
before do before do
allow(Gitlab::SubscriptionPortal::Client)
.to receive(:get_plans).with(tags: ['CI_1000_MINUTES_PLAN'])
.and_return({ success: true, data: [{ 'id' => 'ci_minutes' }] })
stub_feature_flags(new_route_ci_minutes_purchase: false) stub_feature_flags(new_route_ci_minutes_purchase: false)
sign_in(user) sign_in(user)
end end
...@@ -289,6 +326,7 @@ RSpec.describe SubscriptionsController do ...@@ -289,6 +326,7 @@ RSpec.describe SubscriptionsController do
allow_next_instance_of( allow_next_instance_of(
GitlabSubscriptions::FilterPurchaseEligibleNamespacesService, GitlabSubscriptions::FilterPurchaseEligibleNamespacesService,
user: user, user: user,
plan_id: params[:subscription][:plan_id],
namespaces: [selected_group] namespaces: [selected_group]
) do |instance| ) do |instance|
allow(instance) allow(instance)
...@@ -338,6 +376,7 @@ RSpec.describe SubscriptionsController do ...@@ -338,6 +376,7 @@ RSpec.describe SubscriptionsController do
allow_next_instance_of( allow_next_instance_of(
GitlabSubscriptions::FilterPurchaseEligibleNamespacesService, GitlabSubscriptions::FilterPurchaseEligibleNamespacesService,
user: user, user: user,
plan_id: params[:subscription][:plan_id],
namespaces: [selected_group] namespaces: [selected_group]
) do |instance| ) do |instance|
allow(instance) allow(instance)
......
...@@ -204,12 +204,91 @@ RSpec.describe Gitlab::SubscriptionPortal::Clients::Graphql do ...@@ -204,12 +204,91 @@ RSpec.describe Gitlab::SubscriptionPortal::Clients::Graphql do
end end
end end
describe '#get_plans' do
subject { client.get_plans(tags: ['test-plan-id']) }
let(:headers) do
{
"Accept" => "application/json",
"Content-Type" => "application/json",
"X-Admin-Email" => "gl_com_api@gitlab.com",
"X-Admin-Token" => "customer_admin_token"
}
end
let(:params) do
{
variables: { tags: ['test-plan-id'] },
query: <<~GQL
query getPlans($tags: [PlanTag!]) {
plans(planTags: $tags) {
id
}
}
GQL
}
end
context 'when the request is successful' do
it 'returns the data' do
response = { data: { 'data' => { 'plans' => [{ 'id' => 1 }, { 'id' => 3 }] } } }
expect(client).to receive(:http_post).with('graphql', headers, params).and_return(response)
expect(subject).to eq(success: true, data: [{ 'id' => 1 }, { 'id' => 3 }])
end
end
context 'when the request is unsuccessful' do
it 'returns a failure response and logs the error' do
response = {
data: {
"data" => { "plans" => nil },
"errors" => [
{
"message" => "You must be logged in to access this resource",
"locations" => [{ "line" => 2, "column" => 3 }],
"path" => ["getPlans"]
}
]
}
}
expect(Gitlab::ErrorTracking)
.to receive(:track_and_raise_for_dev_exception)
.with(
a_kind_of(Gitlab::SubscriptionPortal::Client::ResponseError),
query: params[:query],
response: response[:data]
)
expect(client).to receive(:http_post).with('graphql', headers, params).and_return(response)
expect(subject).to eq(
success: false,
errors: [{
"locations" => [{ "column" => 3, "line" => 2 }],
"message" => "You must be logged in to access this resource",
"path" => ["getPlans"]
}]
)
end
end
end
describe '#filter_purchase_eligible_namespaces' do describe '#filter_purchase_eligible_namespaces' do
subject do subject(:filter_purchase_eligible_namespaces) do
client.filter_purchase_eligible_namespaces(user, [user_namespace, group_namespace, subgroup]) client.filter_purchase_eligible_namespaces(
user,
[user_namespace, group_namespace, subgroup],
plan_id: plan_id,
any_self_service_plan: any_self_service_plan
)
end end
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:plan_id) { 'test-plan' }
let_it_be(:any_self_service_plan) { true }
let_it_be(:user_namespace) { user.namespace } let_it_be(:user_namespace) { user.namespace }
let_it_be(:group_namespace) { create(:group) } let_it_be(:group_namespace) { create(:group) }
let_it_be(:subgroup) { create(:group, parent: group_namespace) } let_it_be(:subgroup) { create(:group, parent: group_namespace) }
...@@ -226,10 +305,12 @@ RSpec.describe Gitlab::SubscriptionPortal::Clients::Graphql do ...@@ -226,10 +305,12 @@ RSpec.describe Gitlab::SubscriptionPortal::Clients::Graphql do
let(:variables) do let(:variables) do
{ {
customerUid: user.id, customerUid: user.id,
planId: plan_id,
eligibleForPurchase: true,
namespaces: [ namespaces: [
{ id: user_namespace.id, parentId: nil, plan: "default", trial: false }, { id: user_namespace.id, parentId: nil, plan: "default", trial: false, kind: 'user', membersCountWithDescendants: nil },
{ id: group_namespace.id, parentId: nil, plan: "default", trial: false }, { id: group_namespace.id, parentId: nil, plan: "default", trial: false, kind: 'group', membersCountWithDescendants: 0 },
{ id: subgroup.id, parentId: group_namespace.id, plan: "default", trial: false } { id: subgroup.id, parentId: group_namespace.id, plan: "default", trial: false, kind: 'group', membersCountWithDescendants: 0 }
] ]
} }
end end
...@@ -238,8 +319,8 @@ RSpec.describe Gitlab::SubscriptionPortal::Clients::Graphql do ...@@ -238,8 +319,8 @@ RSpec.describe Gitlab::SubscriptionPortal::Clients::Graphql do
{ {
variables: variables, variables: variables,
query: <<~GQL query: <<~GQL
query FilterEligibleNamespaces($customerUid: Int!, $namespaces: [GitlabNamespaceInput!]!) { query FilterEligibleNamespaces($customerUid: Int!, $namespaces: [GitlabNamespaceInput!]!, $planId: ID, $eligibleForPurchase: Boolean) {
namespaceEligibility(customerUid: $customerUid, namespaces: $namespaces, eligibleForPurchase: true) { namespaceEligibility(customerUid: $customerUid, namespaces: $namespaces, planId: $planId, eligibleForPurchase: $eligibleForPurchase) {
id id
} }
} }
......
...@@ -5,18 +5,34 @@ require 'spec_helper' ...@@ -5,18 +5,34 @@ require 'spec_helper'
RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do
describe '#execute' do describe '#execute' do
let_it_be(:user) { build(:user) } let_it_be(:user) { build(:user) }
let_it_be(:namespace_1) { create(:namespace) }
let_it_be(:namespace_2) { create(:namespace) }
context 'when no namespaces are supplied' do context 'when no namespaces are supplied' do
it 'returns an empty array', :aggregate_failures do it 'returns an empty array', :aggregate_failures do
result = described_class.new(user: user, namespaces: []).execute result = described_class.new(user: user, plan_id: 'test', namespaces: []).execute
expect(result).to be_success expect(result).to be_success
expect(result.payload).to eq [] expect(result.payload).to eq []
end end
end end
context 'when no plan_id or any_self_service_plan flag is supplied' do
it 'logs and returns an error message', :aggregate_failures do
expect(Gitlab::ErrorTracking)
.to receive(:track_and_raise_for_dev_exception)
.with an_instance_of(ArgumentError)
result = described_class.new(user: user, plan_id: nil, namespaces: [namespace_1]).execute
expect(result).to be_error
expect(result.message).to eq 'plan_id and any_self_service_plan cannot both be nil'
expect(result.payload).to be_nil
end
end
context 'when no user is supplied' do context 'when no user is supplied' do
subject(:service) { described_class.new(user: nil, namespaces: [build(:namespace)]) } subject(:service) { described_class.new(user: nil, plan_id: 'test', namespaces: [namespace_1]) }
it 'logs and returns an error message', :aggregate_failures do it 'logs and returns an error message', :aggregate_failures do
expect(Gitlab::ErrorTracking) expect(Gitlab::ErrorTracking)
...@@ -32,12 +48,13 @@ RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do ...@@ -32,12 +48,13 @@ RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do
end end
context 'when the http request fails' do context 'when the http request fails' do
subject(:service) { described_class.new(user: user, namespaces: [create(:namespace)]) } subject(:service) { described_class.new(user: user, plan_id: 'test', namespaces: [namespace_1]) }
before do before do
allow(Gitlab::SubscriptionPortal::Client).to receive(:filter_purchase_eligible_namespaces).and_return( allow(Gitlab::SubscriptionPortal::Client)
success: false, data: { errors: 'error' } .to receive(:filter_purchase_eligible_namespaces)
) .with(user, [namespace_1], plan_id: 'test', any_self_service_plan: nil)
.and_return(success: false, data: { errors: 'error' })
end end
it 'returns an error message', :aggregate_failures do it 'returns an error message', :aggregate_failures do
...@@ -50,18 +67,16 @@ RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do ...@@ -50,18 +67,16 @@ RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do
end end
context 'when all the namespaces are eligible' do context 'when all the namespaces are eligible' do
let(:namespace_1) { create(:namespace) }
let(:namespace_2) { create(:namespace) }
before do before do
allow(Gitlab::SubscriptionPortal::Client).to receive(:filter_purchase_eligible_namespaces).and_return( allow(Gitlab::SubscriptionPortal::Client)
success: true, data: [{ 'id' => namespace_1.id }, { 'id' => namespace_2.id }] .to receive(:filter_purchase_eligible_namespaces)
) .with(user, [namespace_1, namespace_2], plan_id: 'test', any_self_service_plan: nil)
.and_return(success: true, data: [{ 'id' => namespace_1.id }, { 'id' => namespace_2.id }])
end end
it 'does not filter any namespaces', :aggregate_failures do it 'does not filter any namespaces', :aggregate_failures do
namespaces = [namespace_1, namespace_2] namespaces = [namespace_1, namespace_2]
result = described_class.new(user: user, namespaces: namespaces).execute result = described_class.new(user: user, plan_id: 'test', namespaces: namespaces).execute
expect(result).to be_success expect(result).to be_success
expect(result.payload).to eq namespaces expect(result.payload).to eq namespaces
...@@ -69,18 +84,33 @@ RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do ...@@ -69,18 +84,33 @@ RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do
end end
context 'when the user has a namespace ineligible' do context 'when the user has a namespace ineligible' do
let(:namespace_1) { create(:namespace) }
let(:namespace_2) { create(:namespace) }
before do before do
allow(Gitlab::SubscriptionPortal::Client).to receive(:filter_purchase_eligible_namespaces).and_return( allow(Gitlab::SubscriptionPortal::Client)
success: true, data: [{ 'id' => namespace_1.id }] .to receive(:filter_purchase_eligible_namespaces)
) .with(user, [namespace_1, namespace_2], plan_id: 'test', any_self_service_plan: nil)
.and_return(success: true, data: [{ 'id' => namespace_1.id }])
end end
it 'is filtered from the results', :aggregate_failures do it 'is filtered from the results', :aggregate_failures do
namespaces = [namespace_1, namespace_2] namespaces = [namespace_1, namespace_2]
result = described_class.new(user: user, namespaces: namespaces).execute result = described_class.new(user: user, plan_id: 'test', namespaces: namespaces).execute
expect(result).to be_success
expect(result.payload).to eq [namespace_1]
end
end
context "when supplied the any_self_service_plan flag" do
before do
allow(Gitlab::SubscriptionPortal::Client)
.to receive(:filter_purchase_eligible_namespaces)
.with(user, [namespace_1, namespace_2], plan_id: nil, any_self_service_plan: true)
.and_return(success: true, data: [{ 'id' => namespace_1.id }])
end
it 'filters the results by eligibility for any self service plan' do
namespaces = [namespace_1, namespace_2]
result = described_class.new(user: user, namespaces: namespaces, any_self_service_plan: true).execute
expect(result).to be_success expect(result).to be_success
expect(result.payload).to eq [namespace_1] expect(result.payload).to eq [namespace_1]
......
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