Automatic merge of gitlab-org/gitlab master

changelogs/unreleased/290008-fix-background-migration-arguments.yml

+---
+title: Fix argument type for background migration
+merge_request: 51475
+author:
+type: fixed

db/post_migrate/20201207165956_remove_duplicate_services.rb

 # frozen_string_literal: true
 
 class RemoveDuplicateServices < ActiveRecord::Migration[6.0]
-  include Gitlab::Database::MigrationHelpers
-
   DOWNTIME = false
-  INTERVAL = 2.minutes
-  BATCH_SIZE = 5_000
-  MIGRATION = 'RemoveDuplicateServices'
 
   disable_ddl_transaction!
 
   def up
-    project_ids_with_duplicates = Gitlab::BackgroundMigration::RemoveDuplicateServices::Service.project_ids_with_duplicates
-
-    project_ids_with_duplicates.each_batch(of: BATCH_SIZE, column: :project_id) do |batch, index|
-      migrate_in(
-        INTERVAL * index,
-        MIGRATION,
-        batch.pluck(:project_id)
-      )
-    end
+    # noop, replaced by 20210112143418_remove_duplicate_services.rb
   end
 
   def down

db/post_migrate/20210112143418_remove_duplicate_services2.rb

+# frozen_string_literal: true
+
+# This replaces the previous post-deployment migration 20201207165956_remove_duplicate_services_spec.rb,
+# we have to run this again due to a bug in how we were receiving the arguments in the background migration.
+class RemoveDuplicateServices2 < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INTERVAL = 2.minutes
+  BATCH_SIZE = 5_000
+  MIGRATION = 'RemoveDuplicateServices'
+
+  disable_ddl_transaction!
+
+  def up
+    project_ids_with_duplicates = Gitlab::BackgroundMigration::RemoveDuplicateServices::Service.project_ids_with_duplicates
+
+    project_ids_with_duplicates.each_batch(of: BATCH_SIZE, column: :project_id) do |batch, index|
+      migrate_in(
+        INTERVAL * index,
+        MIGRATION,
+        batch.pluck(:project_id)
+      )
+    end
+  end
+
+  def down
+  end
+end

db/schema_migrations/20210112143418

+05d45e25ab9ef1565c04ca6515e0b01f2f98c5e98b1eeb09fa9dd43ebbe3c4d0
\ No newline at end of file

ee/app/services/gitlab_subscriptions/apply_trial_service.rb

@@ -6,6 +6,9 @@ module GitlabSubscriptions
       response = client.generate_trial(apply_trial_params)
 
       if response[:success]
+        namespace_id = apply_trial_params.dig(:trial_user, :namespace_id)
+        record_onboarding_progress(namespace_id) if namespace_id
+
         { success: true }
       else
         { success: false, errors: response.dig(:data, :errors) }
@@ -17,5 +20,12 @@ module GitlabSubscriptions
     def client
       Gitlab::SubscriptionPortal::Client
     end
+
+    def record_onboarding_progress(namespace_id)
+      namespace = Namespace.find_by(id: namespace_id) # rubocop: disable CodeReuse/ActiveRecord
+      return unless namespace
+
+      OnboardingProgressService.new(namespace).execute(action: :trial_started)
+    end
   end
 end

ee/spec/services/gitlab_subscriptions/apply_trial_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe GitlabSubscriptions::ApplyTrialService do
+  subject(:execute) { described_class.new.execute(apply_trial_params) }
+
+  let_it_be(:namespace) { create(:namespace) }
+  let(:apply_trial_params) do
+    {
+      trial_user: {
+        namespace_id: namespace.id
+      }
+    }
+  end
+
+  describe '#execute' do
+    before do
+      allow(Gitlab::SubscriptionPortal::Client).to receive(:generate_trial).and_return(response)
+    end
+
+    context 'trial applied successfully' do
+      let(:response) { { success: true }}
+
+      it 'returns success: true' do
+        expect(execute).to eq({ success: true })
+      end
+
+      it 'records a namespace onboarding progress action' do
+        expect_next_instance_of(OnboardingProgressService) do |service|
+          expect(service).to receive(:execute).with(action: :trial_started)
+        end
+
+        execute
+      end
+    end
+
+    context 'error while applying the trial' do
+      let(:response) { { success: false, data: { errors: ['some error'] } }}
+
+      it 'returns success: false with errors' do
+        expected_response = {
+          success: false,
+          errors: ['some error']
+        }
+
+        expect(execute).to eq(expected_response)
+      end
+
+      it 'does not record a namespace onboarding progress action' do
+        expect(OnboardingProgressService).not_to receive(:new)
+
+        execute
+      end
+    end
+  end
+end

lib/gitlab/background_migration/remove_duplicate_services.rb

@@ -29,7 +29,7 @@ module Gitlab
         end
       end
 
-      def perform(project_ids)
+      def perform(*project_ids)
         types_with_duplicates = Service.types_with_duplicates(project_ids).pluck(:project_id, :type)
 
         types_with_duplicates.each do |project_id, type|

spec/lib/gitlab/background_migration/remove_duplicate_services_spec.rb

@@ -82,7 +82,7 @@ RSpec.describe Gitlab::BackgroundMigration::RemoveDuplicateServices, :migration,
     end
 
     expect do
-      subject.perform([project2.id, project3.id])
+      subject.perform(project2.id, project3.id)
     end.to change { services.count }.from(21).to(12)
 
     services1 = services.where(project_id: project1.id)
@@ -109,13 +109,13 @@ RSpec.describe Gitlab::BackgroundMigration::RemoveDuplicateServices, :migration,
 
   it 'does not delete services without duplicates' do
     expect do
-      subject.perform([project1.id, project4.id])
+      subject.perform(project1.id, project4.id)
     end.not_to change { services.count }
   end
 
   it 'only deletes duplicate services for the current batch' do
     expect do
-      subject.perform([project2.id])
+      subject.perform(project2.id)
     end.to change { services.count }.by(-3)
   end
 end

spec/migrations/20201207165956_remove_duplicate_services_spec.rb → spec/migrations/20210112143418_remove_duplicate_services2_spec.rb

 # frozen_string_literal: true
 
 require 'spec_helper'
-require Rails.root.join('db', 'post_migrate', '20201207165956_remove_duplicate_services.rb')
+require Rails.root.join('db', 'post_migrate', '20210112143418_remove_duplicate_services2.rb')
 
-RSpec.describe RemoveDuplicateServices do
+RSpec.describe RemoveDuplicateServices2 do
   let_it_be(:namespaces) { table(:namespaces) }
   let_it_be(:projects) { table(:projects) }
   let_it_be(:services) { table(:services) }