Commit fd755652 authored by Josianne Hyson's avatar Josianne Hyson

Update CI Minutes eligibility check to use new API

We want to use the new API for checking if a namespace is eligible for
CI minutes as the previous one assumed that the purchase check was for
new license subscriptions. This would filter out namespaces that have a
Premium/Ultimate subscription, when those namespaces are actually
eligible for a CI minutes purchase.

Update the subscriptions controller to fetch and supply the plan ID to
this API.

Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/326663
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66981
parent c45281a9
......@@ -32,7 +32,9 @@ class SubscriptionsController < ApplicationController
end
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?
......@@ -51,7 +53,7 @@ class SubscriptionsController < ApplicationController
def create
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 render json: group.errors.to_json unless group.persisted?
......@@ -90,11 +92,11 @@ class SubscriptionsController < ApplicationController
params.require(:subscription).permit(:plan_id, :payment_method_id, :quantity, :source)
end
def find_group
def find_group(plan_id:)
selected_group = current_user.manageable_groups.top_most.find(params[:selected_group])
result = GitlabSubscriptions::FilterPurchaseEligibleNamespacesService
.new(user: current_user, namespaces: Array(selected_group))
.new(user: current_user, plan_id: plan_id, namespaces: Array(selected_group))
.execute
result.success? ? result.payload.first : nil
......@@ -118,13 +120,21 @@ class SubscriptionsController < ApplicationController
redirect_to new_user_registration_path(redirect_from: from)
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
return unless current_user
return @eligible_groups = [] unless current_user
candidate_groups = current_user.manageable_groups.top_most.with_counts(archived: false)
result = GitlabSubscriptions::FilterPurchaseEligibleNamespacesService
.new(user: current_user, namespaces: candidate_groups)
.new(user: current_user, namespaces: candidate_groups, any_self_service_plan: true)
.execute
@eligible_groups = result.success? ? result.payload : []
......
# 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
class FilterPurchaseEligibleNamespacesService
include ::Gitlab::Utils::StrongMemoize
def initialize(user:, namespaces:)
def initialize(user:, namespaces:, plan_id: nil, any_self_service_plan: nil)
@user = user
@namespaces = namespaces
@plan_id = plan_id
@any_self_service_plan = any_self_service_plan
end
def execute
return success([]) if namespaces.empty?
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
data = namespaces.filter { |namespace| eligible_ids.include?(namespace.id) }
......@@ -26,7 +35,7 @@ module GitlabSubscriptions
private
attr_reader :user, :namespaces
attr_reader :user, :namespaces, :plan_id, :any_self_service_plan
def success(payload)
ServiceResponse.success(payload: payload)
......@@ -43,9 +52,21 @@ module GitlabSubscriptions
error(message)
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
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
......
......@@ -98,10 +98,30 @@ module Gitlab
end
end
def filter_purchase_eligible_namespaces(user, namespaces)
def get_plans(tags:)
query = <<~GQL
query FilterEligibleNamespaces($customerUid: Int!, $namespaces: [GitlabNamespaceInput!]!) {
namespaceEligibility(customerUid: $customerUid, namespaces: $namespaces, eligibleForPurchase: true) {
query getPlans($tags: [PlanTag!]) {
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
}
}
......@@ -112,18 +132,25 @@ module Gitlab
id: namespace.id,
parentId: namespace.parent_id,
plan: namespace.actual_plan_name,
trial: !!namespace.trial?
trial: !!namespace.trial?,
kind: namespace.kind,
membersCountWithDescendants: namespace.group? ? namespace.users_with_descendants.count : nil
}
end
response = http_post(
"graphql",
'graphql',
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]
if response['errors'].blank?
{ success: true, data: response.dig('data', 'namespaceEligibility') }
if response['errors'].blank? && (data = response.dig('data', 'namespaceEligibility'))
{ success: true, data: data }
else
track_error(query, response)
......
......@@ -38,7 +38,12 @@ RSpec.describe SubscriptionsController do
before do
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]))
end
end
......@@ -52,7 +57,12 @@ RSpec.describe SubscriptionsController do
context 'when there are no eligible groups for the subscription' 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: []))
end
......@@ -77,13 +87,36 @@ RSpec.describe SubscriptionsController do
sign_in(user)
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
let_it_be(:group) { create(:group) }
before do
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]))
end
end
......@@ -101,6 +134,10 @@ RSpec.describe SubscriptionsController do
context 'with :new_route_ci_minutes_purchase disabled' 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)
sign_in(user)
end
......@@ -289,6 +326,7 @@ RSpec.describe SubscriptionsController do
allow_next_instance_of(
GitlabSubscriptions::FilterPurchaseEligibleNamespacesService,
user: user,
plan_id: params[:subscription][:plan_id],
namespaces: [selected_group]
) do |instance|
allow(instance)
......@@ -338,6 +376,7 @@ RSpec.describe SubscriptionsController do
allow_next_instance_of(
GitlabSubscriptions::FilterPurchaseEligibleNamespacesService,
user: user,
plan_id: params[:subscription][:plan_id],
namespaces: [selected_group]
) do |instance|
allow(instance)
......
......@@ -204,12 +204,91 @@ RSpec.describe Gitlab::SubscriptionPortal::Clients::Graphql do
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
subject do
client.filter_purchase_eligible_namespaces(user, [user_namespace, group_namespace, subgroup])
subject(:filter_purchase_eligible_namespaces) do
client.filter_purchase_eligible_namespaces(
user,
[user_namespace, group_namespace, subgroup],
plan_id: plan_id,
any_self_service_plan: any_self_service_plan
)
end
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(:group_namespace) { create(:group) }
let_it_be(:subgroup) { create(:group, parent: group_namespace) }
......@@ -226,10 +305,12 @@ RSpec.describe Gitlab::SubscriptionPortal::Clients::Graphql do
let(:variables) do
{
customerUid: user.id,
planId: plan_id,
eligibleForPurchase: true,
namespaces: [
{ id: user_namespace.id, parentId: nil, plan: "default", trial: false },
{ id: group_namespace.id, parentId: nil, plan: "default", trial: false },
{ id: subgroup.id, parentId: group_namespace.id, 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, kind: 'group', membersCountWithDescendants: 0 },
{ id: subgroup.id, parentId: group_namespace.id, plan: "default", trial: false, kind: 'group', membersCountWithDescendants: 0 }
]
}
end
......@@ -238,8 +319,8 @@ RSpec.describe Gitlab::SubscriptionPortal::Clients::Graphql do
{
variables: variables,
query: <<~GQL
query FilterEligibleNamespaces($customerUid: Int!, $namespaces: [GitlabNamespaceInput!]!) {
namespaceEligibility(customerUid: $customerUid, namespaces: $namespaces, eligibleForPurchase: true) {
query FilterEligibleNamespaces($customerUid: Int!, $namespaces: [GitlabNamespaceInput!]!, $planId: ID, $eligibleForPurchase: Boolean) {
namespaceEligibility(customerUid: $customerUid, namespaces: $namespaces, planId: $planId, eligibleForPurchase: $eligibleForPurchase) {
id
}
}
......
......@@ -5,18 +5,34 @@ require 'spec_helper'
RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do
describe '#execute' do
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
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.payload).to eq []
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
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
expect(Gitlab::ErrorTracking)
......@@ -32,12 +48,13 @@ RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do
end
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
allow(Gitlab::SubscriptionPortal::Client).to receive(:filter_purchase_eligible_namespaces).and_return(
success: false, data: { errors: 'error' }
)
allow(Gitlab::SubscriptionPortal::Client)
.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
it 'returns an error message', :aggregate_failures do
......@@ -50,18 +67,16 @@ RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do
end
context 'when all the namespaces are eligible' do
let(:namespace_1) { create(:namespace) }
let(:namespace_2) { create(:namespace) }
before do
allow(Gitlab::SubscriptionPortal::Client).to receive(:filter_purchase_eligible_namespaces).and_return(
success: true, data: [{ 'id' => namespace_1.id }, { 'id' => namespace_2.id }]
)
allow(Gitlab::SubscriptionPortal::Client)
.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
it 'does not filter any namespaces', :aggregate_failures do
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 namespaces
......@@ -69,18 +84,33 @@ RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do
end
context 'when the user has a namespace ineligible' do
let(:namespace_1) { create(:namespace) }
let(:namespace_2) { create(:namespace) }
before do
allow(Gitlab::SubscriptionPortal::Client).to receive(:filter_purchase_eligible_namespaces).and_return(
success: true, data: [{ 'id' => namespace_1.id }]
)
allow(Gitlab::SubscriptionPortal::Client)
.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
it 'is filtered from the results', :aggregate_failures do
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.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