Merge branch 'ag/341564-use-account-id-for-iframe' into 'master'

Retrieve the account id and pass it to the FE

See merge request gitlab-org/gitlab!71266

ee/app/controllers/subscriptions_controller.rb

@@ -34,7 +34,12 @@ class SubscriptionsController < ApplicationController
   def buy_minutes
     return render_404 unless ci_minutes_plan_data.present?
 
-    @group = find_group(plan_id: ci_minutes_plan_data["id"])
+    # At the moment of this comment the account id is directly available to the view.
+    # This might change in the future  given the intention to associate the account id to the namespace.
+    # See: https://gitlab.com/gitlab-org/gitlab/-/issues/338546#note_684762160
+    result = find_group(plan_id: ci_minutes_plan_data["id"])
+    @group = result[:namespace]
+    @account_id = result[:account_id]
 
     return render_404 if @group.nil?
 
@@ -44,7 +49,12 @@ class SubscriptionsController < ApplicationController
   def buy_storage
     return render_404 unless storage_plan_data.present?
 
-    @group = find_group(plan_id: storage_plan_data["id"])
+    # At the moment of this comment the account id is directly available to the view.
+    # This might change in the future  given the intention to associate the account id to the namespace.
+    # See: https://gitlab.com/gitlab-org/gitlab/-/issues/338546#note_684762160
+    result = find_group(plan_id: storage_plan_data["id"])
+    @group = result[:namespace]
+    @account_id = result[:account_id]
 
     return render_404 if @group.nil?
 
@@ -63,7 +73,7 @@ class SubscriptionsController < ApplicationController
 
   def create
     current_user.update(setup_for_company: true) if params[:setup_for_company]
-    group = params[:selected_group] ? find_group(plan_id: subscription_params[:plan_id]) : create_group
+    group = params[:selected_group] ? current_group : create_group
 
     return not_found if group.nil?
     return render json: group.errors.to_json unless group.persisted?
@@ -105,11 +115,17 @@ class SubscriptionsController < ApplicationController
   def find_group(plan_id:)
     selected_group = current_user.manageable_groups.top_most.find(params[:selected_group])
 
-    result = GitlabSubscriptions::FilterPurchaseEligibleNamespacesService
+    result = GitlabSubscriptions::FetchPurchaseEligibleNamespacesService
       .new(user: current_user, plan_id: plan_id, namespaces: Array(selected_group))
       .execute
 
-    result.success? ? result.payload.first : nil
+    return {} unless result.success?
+
+    result.payload.first || {}
+  end
+
+  def current_group
+    find_group(plan_id: subscription_params[:plan_id]).dig(:namespace)
   end
 
   def create_group
@@ -149,12 +165,17 @@ class SubscriptionsController < ApplicationController
   def load_eligible_groups
     return @eligible_groups = [] unless current_user
 
-    candidate_groups = current_user.manageable_groups.top_most.with_counts(archived: false)
+    @eligible_groups = fetch_eligible_groups
+  end
 
-    result = GitlabSubscriptions::FilterPurchaseEligibleNamespacesService
+  def fetch_eligible_groups
+    candidate_groups = current_user.manageable_groups.top_most.with_counts(archived: false)
+    result = GitlabSubscriptions::FetchPurchaseEligibleNamespacesService
                .new(user: current_user, namespaces: candidate_groups, any_self_service_plan: true)
                .execute
 
-    @eligible_groups = result.success? ? result.payload : []
+    return [] unless result.success?
+
+    (result.payload || []).map { |h| h.dig(:namespace) }
   end
 end

ee/app/helpers/subscriptions_helper.rb

@@ -16,9 +16,9 @@ module SubscriptionsHelper
     }
   end
 
-  def buy_addon_data(group, anchor, purchased_product)
+  def buy_addon_data(group, account_id, anchor, purchased_product)
     {
-      group_data: [present_group(group)].to_json,
+      group_data: [present_group(group, account_id)].to_json,
       namespace_id: params[:selected_group],
       redirect_after_success: group_usage_quotas_path(group, anchor: anchor, purchased_product: purchased_product),
       source: params[:source]
@@ -57,10 +57,10 @@ module SubscriptionsHelper
     groups.map { |namespace| present_group(namespace) }
   end
 
-  def present_group(namespace)
+  def present_group(namespace, account_id = nil)
     {
       id: namespace.id,
-      account_id: nil,
+      account_id: account_id,
       name: namespace.name,
       users: namespace.member_count,
       guests: namespace.guest_count

ee/app/services/gitlab_subscriptions/filter_purchase_eligible_namespaces_service.rb → ee/app/services/gitlab_subscriptions/fetch_purchase_eligible_namespaces_service.rb

 # frozen_string_literal: true
 
-# Filter a list of namespaces by their eligibility to purchase a new plan.
+# Fetch a list of namespaces and filter them by their eligibility to purchase a new subscription
 #
 # - 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.
+# - When present, the account id associated with the namespace will be added.
+#   This is needed in the context of add on purchase, in order to correctly initialise the payment form.
 module GitlabSubscriptions
-  class FilterPurchaseEligibleNamespacesService
+  class FetchPurchaseEligibleNamespacesService
     include ::Gitlab::Utils::StrongMemoize
 
     def initialize(user:, namespaces:, plan_id: nil, any_self_service_plan: nil)
@@ -23,9 +25,12 @@ module GitlabSubscriptions
       return missing_plan_error if plan_id.nil? && any_self_service_plan.nil?
 
       if response[:success] && response[:data]
-        eligible_ids = response[:data].map { |data| data['id'] }.to_set
-
-        data = namespaces.filter { |namespace| eligible_ids.include?(namespace.id) }
+        eligible_namespaces = response[:data].to_h { |data| [data["id"], data["accountId"]] }
+        data = namespaces.each_with_object([]) do |namespace, acc|
+          if eligible_namespaces.include?(namespace.id)
+            acc << { namespace: namespace, account_id: eligible_namespaces[namespace.id] }
+          end
+        end
 
         success(data)
       else

ee/app/views/subscriptions/buy_minutes.html.haml

 - page_title _('Buy CI Minutes')
 
-#js-buy-minutes{ data: buy_addon_data(@group, 'pipelines-quota-tab', s_('Checkout|CI minutes')) }
+#js-buy-minutes{ data: buy_addon_data(@group, @account_id, 'pipelines-quota-tab', s_('Checkout|CI minutes')) }

ee/app/views/subscriptions/buy_storage.html.haml

 - page_title _('Buy Storage')
 
-#js-buy-storage{ data: buy_addon_data(@group, 'storage-quota-tab', s_('Checkout|a storage subscription')) }
+#js-buy-storage{ data: buy_addon_data(@group, @account_id, 'storage-quota-tab', s_('Checkout|a storage subscription')) }

ee/app/views/subscriptions/new.html.haml

 - page_title _('Checkout')
 
 #js-new-subscription{ data: subscription_data(@eligible_groups) }
-

ee/lib/gitlab/subscription_portal/clients/graphql.rb

@@ -123,6 +123,7 @@ module Gitlab
             query FilterEligibleNamespaces($customerUid: Int!, $namespaces: [GitlabNamespaceInput!]!, $planId: ID, $eligibleForPurchase: Boolean) {
               namespaceEligibility(customerUid: $customerUid, namespaces: $namespaces, planId: $planId, eligibleForPurchase: $eligibleForPurchase) {
                 id
+                accountId: zuoraAccountId
               }
             }
             GQL

ee/spec/controllers/subscriptions_controller_spec.rb

@@ -40,12 +40,14 @@ RSpec.describe SubscriptionsController do
           group.add_owner(user)
 
           allow_next_instance_of(
-            GitlabSubscriptions::FilterPurchaseEligibleNamespacesService,
+            GitlabSubscriptions::FetchPurchaseEligibleNamespacesService,
             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: [{ namespace: group, account_id: nil }])
+            )
           end
         end
 
@@ -59,7 +61,7 @@ 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,
+            GitlabSubscriptions::FetchPurchaseEligibleNamespacesService,
             user: user,
             namespaces: [],
             any_self_service_plan: true
@@ -77,6 +79,7 @@ RSpec.describe SubscriptionsController do
 
   describe 'GET #buy_minutes' do
     let_it_be(:group) { create(:group) }
+    let_it_be(:plan_id) { 'ci_minutes' }
 
     subject(:buy_minutes) { get :buy_minutes, params: { selected_group: group.id } }
 
@@ -112,12 +115,14 @@ RSpec.describe SubscriptionsController do
             .and_return({ success: true, data: [{ 'id' => 'ci_minutes' }] })
 
           allow_next_instance_of(
-            GitlabSubscriptions::FilterPurchaseEligibleNamespacesService,
+            GitlabSubscriptions::FetchPurchaseEligibleNamespacesService,
             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: [{ namespace: group, account_id: nil }])
+            )
           end
         end
 
@@ -128,28 +133,24 @@ RSpec.describe SubscriptionsController do
           buy_minutes
 
           expect(assigns(:group)).to eq group
-        end
-      end
+          expect(assigns(:account_id)).to eq nil
         end
 
         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
 
           it { is_expected.to have_gitlab_http_status(:not_found) }
         end
       end
+    end
+  end
 
   describe 'GET #buy_storage' do
     let_it_be(:group) { create(:group) }
 
-    subject { get :buy_storage, params: { selected_group: group.id } }
+    subject(:buy_storage) { get :buy_storage, params: { selected_group: group.id } }
 
     context 'with authenticated user' do
       before do
@@ -172,7 +173,7 @@ RSpec.describe SubscriptionsController do
         it { is_expected.to have_gitlab_http_status(:not_found) }
       end
 
-      context 'with :new_route_storage_purchase enabled' do
+      context 'when there are groups eligible for the addon' do
         let_it_be(:group) { create(:group) }
 
         before do
@@ -183,33 +184,37 @@ RSpec.describe SubscriptionsController do
             .and_return({ success: true, data: [{ 'id' => 'storage' }] })
 
           allow_next_instance_of(
-            GitlabSubscriptions::FilterPurchaseEligibleNamespacesService,
+            GitlabSubscriptions::FetchPurchaseEligibleNamespacesService,
             user: user,
             plan_id: 'storage',
             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: [{ namespace: group, account_id: nil }])
+            )
           end
         end
 
         it { is_expected.to render_template 'layouts/checkout' }
         it { is_expected.to render_template :buy_storage }
-      end
+
+        it 'assigns the group for the addon' do
+          buy_storage
+
+          expect(assigns(:group)).to eq group
+          expect(assigns(:account_id)).to eq nil
         end
 
         context 'with :new_route_storage_purchase disabled' do
           before do
-        allow(Gitlab::SubscriptionPortal::Client)
-          .to receive(:get_plans).with(tags: ['STORAGE_PLAN'])
-          .and_return({ success: true, data: [{ 'id' => 'ci_minutes' }] })
-
-        stub_feature_flags(new_route_ci_minutes_purchase: false)
-        sign_in(user)
+            stub_feature_flags(new_route_storage_purchase: false)
           end
 
           it { is_expected.to have_gitlab_http_status(:not_found) }
         end
       end
+    end
+  end
 
   describe 'GET #payment_form' do
     subject { get :payment_form, params: { id: 'cc' } }
@@ -389,14 +394,16 @@ RSpec.describe SubscriptionsController do
             selected_group.add_owner(user)
 
             allow_next_instance_of(
-              GitlabSubscriptions::FilterPurchaseEligibleNamespacesService,
+              GitlabSubscriptions::FetchPurchaseEligibleNamespacesService,
               user: user,
               plan_id: params[:subscription][:plan_id],
               namespaces: [selected_group]
             ) do |instance|
               allow(instance)
                 .to receive(:execute)
-                .and_return(instance_double(ServiceResponse, success?: true, payload: [selected_group]))
+                .and_return(
+                  instance_double(ServiceResponse, success?: true, payload: [{ namespace: selected_group, account_id: nil }])
+                )
             end
           end
 
@@ -439,7 +446,7 @@ RSpec.describe SubscriptionsController do
             selected_group.add_owner(user)
 
             allow_next_instance_of(
-              GitlabSubscriptions::FilterPurchaseEligibleNamespacesService,
+              GitlabSubscriptions::FetchPurchaseEligibleNamespacesService,
               user: user,
               plan_id: params[:subscription][:plan_id],
               namespaces: [selected_group]

ee/spec/helpers/subscriptions_helper_spec.rb

@@ -134,10 +134,11 @@ RSpec.describe SubscriptionsHelper do
   end
 
   describe '#buy_addon_data' do
-    subject(:buy_addon_data) { helper.buy_addon_data(group, anchor, purchased_product) }
+    subject(:buy_addon_data) { helper.buy_addon_data(group, account_id, anchor, purchased_product) }
 
     let_it_be(:group) { create(:group, name: 'My Namespace') }
     let_it_be(:user) { create(:user, name: 'First Last') }
+    let_it_be(:account_id) { '111111111111' }
 
     let(:anchor) { 'pipelines-quota-tab' }
     let(:purchased_product) { 'CI Minutes' }
@@ -150,7 +151,7 @@ RSpec.describe SubscriptionsHelper do
 
     it { is_expected.to include(namespace_id: group.id.to_s) }
     it { is_expected.to include(source: 'some_source') }
-    it { is_expected.to include(group_data: %Q{[{"id":#{group.id},"account_id":null,"name":"My Namespace","users":1,"guests":0}]}) }
+    it { is_expected.to include(group_data: %Q{[{"id":#{group.id},"account_id":"#{account_id}","name":"My Namespace","users":1,"guests":0}]}) }
     it { is_expected.to include(redirect_after_success: group_usage_quotas_path(group, anchor: anchor, purchased_product: purchased_product)) }
   end
 end

ee/spec/lib/gitlab/subscription_portal/clients/graphql_spec.rb

@@ -322,6 +322,7 @@ RSpec.describe Gitlab::SubscriptionPortal::Clients::Graphql do
           query FilterEligibleNamespaces($customerUid: Int!, $namespaces: [GitlabNamespaceInput!]!, $planId: ID, $eligibleForPurchase: Boolean) {
             namespaceEligibility(customerUid: $customerUid, namespaces: $namespaces, planId: $planId, eligibleForPurchase: $eligibleForPurchase) {
               id
+              accountId: zuoraAccountId
             }
           }
         GQL
@@ -333,7 +334,9 @@ RSpec.describe Gitlab::SubscriptionPortal::Clients::Graphql do
         response = {
           data: {
             'data' => {
-              'namespaceEligibility' => [{ 'id' => 1 }, { 'id' => 3 }]
+              'namespaceEligibility' => [
+                { 'id' => 1 }, { 'id' => 3 }
+              ]
             }
           }
         }

ee/spec/services/gitlab_subscriptions/filter_purchase_eligible_namespaces_service_spec.rb → ee/spec/services/gitlab_subscriptions/fetch_purchase_eligible_namespaces_service_spec.rb

@@ -2,14 +2,14 @@
 
 require 'spec_helper'
 
-RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do
+RSpec.describe GitlabSubscriptions::FetchPurchaseEligibleNamespacesService 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
+      it 'returns an array with an empty hash', :aggregate_failures do
         result = described_class.new(user: user, plan_id: 'test', namespaces: []).execute
 
         expect(result).to be_success
@@ -71,7 +71,10 @@ RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do
         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 }])
+          .and_return(success: true, data: [
+            { 'id' => namespace_1.id, 'accountId' => nil },
+            { 'id' => namespace_2.id, 'accountId' => nil }
+          ])
       end
 
       it 'does not filter any namespaces', :aggregate_failures do
@@ -79,16 +82,21 @@ RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do
         result = described_class.new(user: user, plan_id: 'test', namespaces: namespaces).execute
 
         expect(result).to be_success
-        expect(result.payload).to eq namespaces
+        expect(result.payload).to match_array [
+           namespace_result(namespace_1, nil),
+           namespace_result(namespace_2, nil)
+        ]
       end
     end
 
     context 'when the user has a namespace ineligible' do
+      let(:account_id) { '111111111' }
+
       before do
         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 }])
+          .and_return(success: true, data: [{ 'id' => namespace_1.id, 'accountId' => account_id }])
       end
 
       it 'is filtered from the results', :aggregate_failures do
@@ -96,7 +104,9 @@ RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do
         result = described_class.new(user: user, plan_id: 'test', namespaces: namespaces).execute
 
         expect(result).to be_success
-        expect(result.payload).to eq [namespace_1]
+        expect(result.payload).to match_array [
+           namespace_result(namespace_1, account_id)
+         ]
       end
     end
 
@@ -113,8 +123,16 @@ RSpec.describe GitlabSubscriptions::FilterPurchaseEligibleNamespacesService do
         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]
+        expect(result.payload).to match_array [
+          namespace_result(namespace_1, nil)
+        ]
+      end
     end
   end
+
+  private
+
+  def namespace_result(namespace, account_id)
+    { namespace: namespace, account_id: account_id }
   end
 end