Review changes and specs clean up

ee/app/assets/javascripts/registrations/company/new/index.js

@@ -5,16 +5,13 @@ import RegistrationForm from 'ee/registrations/components/company_form.vue';
 export default () => {
   const el = document.querySelector('#js-company-registration-form');
 
-  const { submitPath, trial, role, jtbd, comment } = el.dataset;
+  const { submitPath, trial } = el.dataset;
 
   return new Vue({
     el,
     apolloProvider,
     provide: {
       submitPath,
-      role,
-      jtbd,
-      comment,
     },
     render(createElement) {
       return createElement(RegistrationForm, {

ee/app/assets/javascripts/registrations/components/company_form.vue

@@ -28,7 +28,7 @@ export default {
     CountryOrRegionSelector,
     RegistrationTrialToggle,
   },
-  inject: ['submitPath', 'role', 'jtbd', 'comment'],
+  inject: ['submitPath'],
   props: {
     trial: {
       type: Boolean,
@@ -86,9 +86,6 @@ export default {
 <template>
   <gl-form :action="submitPath" method="post">
     <input :value="$options.csrf.token" type="hidden" name="authenticity_token" />
-    <input :value="role" type="hidden" name="role" data-testid="role" />
-    <input :value="jtbd" type="hidden" name="jtbd" data-testid="jtbd" />
-    <input :value="comment" type="hidden" name="comment" data-testid="comment" />
     <gl-form-text class="gl-font-base gl-text-gray-400 gl-pb-3">{{ descriptionText }}</gl-form-text>
     <div class="gl-display-flex gl-flex-direction-column gl-sm-flex-direction-row gl-mt-5">
       <gl-form-group

ee/app/controllers/registrations/company_controller.rb

@@ -12,15 +12,13 @@ module Registrations
     end
 
     def create
-      result = GitlabSubscriptions::CreateTrialOrLeadService.new.execute(
-        user: current_user,
-        params: permitted_params
-      )
+      result = GitlabSubscriptions::CreateTrialOrLeadService.new(user: current_user, params: permitted_params).execute
 
-      if result[:success]
+      if result.success?
         redirect_to new_users_sign_up_groups_project_path(redirect_param)
       else
-        render :new
+        flash.now[:alert] = result[:message]
+        render :new, status: result.http_status
       end
     end
 

ee/app/helpers/ee/trial_helper.rb

@@ -26,12 +26,9 @@ module EE
     end
 
     def create_company_form_data
+      submit_params = glm_params.merge(params.slice(:trial, :role, :jtbd, :comment).to_unsafe_h.symbolize_keys)
       {
-        submit_path: users_sign_up_company_path(glm_params),
-        trial: params[:trial],
-        role: params[:role],
-        jtbd: params[:jtbd],
-        comment: params[:comment]
+        submit_path: users_sign_up_company_path(submit_params)
       }
     end
 

ee/app/services/gitlab_subscriptions/create_trial_or_lead_service.rb

 # frozen_string_literal: true
-
 module GitlabSubscriptions
   class CreateTrialOrLeadService
-    def execute(user:, params:)
-      params = params.merge(hardcoded_values).merge(user_values(user))
-
-      response = if Gitlab::Utils.to_boolean(params[:trial])
-                   client.generate_trial(trial_user: params)
-                 else
-                   client.generate_hand_raise_lead(params)
+    def initialize(user:, params:)
+      @params = params.merge(hardcoded_values).merge(user_values(user))
     end
 
-      if response[:success]
-        ServiceResponse.success
-      else
-        ServiceResponse.error(message: response.dig(:data, :errors))
-      end
+    def execute
+      generate_response
+      result
     end
 
     private
 
+    attr_reader :response, :params
+
     def hardcoded_values
       {
         provider: 'gitlab',
@@ -39,6 +33,26 @@ module GitlabSubscriptions
       }
     end
 
+    def result
+      if response[:success]
+        ServiceResponse.success
+      else
+        ServiceResponse.error(message: response.dig(:data, :errors), http_status: :unprocessable_entity)
+      end
+    end
+
+    def generate_response
+      @response = if trial?
+                    client.generate_trial(trial_user: params)
+                  else
+                    client.generate_hand_raise_lead(params)
+                  end
+    end
+
+    def trial?
+      Gitlab::Utils.to_boolean(params[:trial])
+    end
+
     def client
       Gitlab::SubscriptionPortal::Client
     end

ee/spec/controllers/registrations/company_controller_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Registrations::CompanyController do
-  let_it_be(:user) { create(:user, email_opted_in: true, last_name: 'Doe') }
+  let_it_be(:user) { create(:user) }
 
   let(:logged_in) { true }
 
@@ -70,15 +70,17 @@ RSpec.describe Registrations::CompanyController do
       end
 
       with_them do
-        it 'creates trial or lead and redirects to the corect path' do
-          expect_next_instance_of(GitlabSubscriptions::CreateTrialOrLeadService) do |service|
-            expect(service).to receive(:execute).with({
+        it 'creates trial or lead and redirects to the correct path' do
+          expect_next_instance_of(
+            GitlabSubscriptions::CreateTrialOrLeadService,
             user: user,
             params: ActionController::Parameters.new(params.merge({ trial: trial })).permit!
-            }).and_return({ success: true })
+          ) do |service|
+            expect(service).to receive(:execute).and_return(ServiceResponse.success)
           end
 
           post :create, params: params.merge({ trial: trial })
+
           expect(response).to have_gitlab_http_status(:redirect)
           expect(response).to redirect_to(new_users_sign_up_groups_project_path(redirect_query))
         end
@@ -95,6 +97,7 @@ RSpec.describe Registrations::CompanyController do
           end
 
           post :create, params: params.merge({ trial: trial })
+
           expect(response).to have_gitlab_http_status(:ok)
           expect(response).to render_template(:new)
         end

ee/spec/features/trial_registrations/company_information_spec.rb

@@ -19,6 +19,7 @@ RSpec.describe 'Company Information', :js do
 
   context 'send company information to create lead' do
     using RSpec::Parameterized::TableSyntax
+
     let(:params) do
       {
         company_name: 'GitLab',
@@ -27,16 +28,17 @@ RSpec.describe 'Company Information', :js do
         country: 'US',
         state: 'CA',
         website_url: 'gitlab.com',
-        role: '',
-        jtbd: '',
-        comment: ''
+        trial: 'false'
       }
     end
 
-    where(trial: %w[true false])
+    where(:service_response, :current_path) do
+      ServiceResponse.success                   | new_users_sign_up_groups_project_path
+      ServiceResponse.error(message: 'failed')  | users_sign_up_company_path
+    end
 
     with_them do
-      it 'proceeds to next step' do
+      it 'redirects to correct path' do
         fill_in 'company_name', with: 'GitLab'
         select '1 - 99', from: 'company_size'
         select 'United States of America', from: 'country'
@@ -44,18 +46,17 @@ RSpec.describe 'Company Information', :js do
         fill_in 'website_url', with: 'gitlab.com'
         fill_in 'phone_number', with: '+1 23 456-78-90'
 
-        # defaults to trial off, click to turn on
-        click_button class: 'gl-toggle' if Gitlab::Utils.to_boolean(trial)
-
-        expect_next_instance_of(GitlabSubscriptions::CreateTrialOrLeadService) do |service|
-          expect(service).to receive(:execute).with({
+        expect_next_instance_of(
+          GitlabSubscriptions::CreateTrialOrLeadService,
           user: user,
-            params: ActionController::Parameters.new(params.merge({ trial: trial })).permit!
-          }).and_return({ success: true })
+          params: ActionController::Parameters.new(params).permit!
+        ) do |service|
+          expect(service).to receive(:execute).and_return(service_response)
         end
 
         click_button 'Continue'
-        expect(page).to have_current_path(new_users_sign_up_groups_project_path, ignore_query: true)
+
+        expect(page).to have_current_path(current_path, ignore_query: true)
       end
     end
   end

ee/spec/frontend/registrations/company/new/components/company_form_spec.js

@@ -18,9 +18,6 @@ describe('RegistrationForm', () => {
       localVue,
       provide: {
         submitPath: SUBMIT_PATH,
-        role: 'Software Engineer',
-        jtbd: 'Jobs to be done',
-        comment: 'A comment',
       },
       propsData: { trial: true },
     });
@@ -71,15 +68,6 @@ describe('RegistrationForm', () => {
     `('has the correct form input in the form content', ({ testid }) => {
       expect(findFormInput(testid).exists()).toBe(true);
     });
-
-    it.each`
-      testid       | value
-      ${'role'}    | ${'Software Engineer'}
-      ${'jtbd'}    | ${'Jobs to be done'}
-      ${'comment'} | ${'A comment'}
-    `('has the hidden injected value for $testid', ({ testid, value }) => {
-      expect(findFormInput(testid).attributes('value')).toBe(value);
-    });
   });
 
   describe('submitting', () => {

ee/spec/helpers/ee/trial_helper_spec.rb

@@ -57,19 +57,35 @@ RSpec.describe EE::TrialHelper do
   end
 
   describe '#create_company_form_data' do
-    let(:user) do
-      double('User', first_name: '_first_name_', last_name: '_last_name_')
+    let(:extra_params) do
+      {
+        role: '_params_role_',
+        jtbd: '_params_jtbd_',
+        comment: '_params_comment_'
+      }
+    end
+
+    let(:params) do
+      ActionController::Parameters.new(extra_params)
     end
 
     before do
-      allow(helper).to receive(:current_user).and_return(user)
+      allow(helper).to receive(:params).and_return(params)
     end
 
     it 'provides expected form data' do
-      keys = [:submit_path, :trial, :role, :jtbd, :comment]
+      keys = [:submit_path]
 
       expect(helper.create_company_form_data.keys.map(&:to_sym)).to match_array(keys)
     end
+
+    it 'allows overriding data with params' do
+      submit_path = {
+        submit_path: '/users/sign_up/company?comment=_params_comment_&jtbd=_params_jtbd_&role=_params_role_'
+      }
+
+      expect(helper.create_company_form_data).to match(submit_path)
+    end
   end
 
   describe '#should_ask_company_question?' do

ee/spec/services/gitlab_subscriptions/create_trial_or_lead_service_spec.rb

@@ -7,6 +7,7 @@ RSpec.describe GitlabSubscriptions::CreateTrialOrLeadService do
 
   describe '#execute' do
     using RSpec::Parameterized::TableSyntax
+
     where(:trial, :service) do
       'true'  | :generate_trial
       'false' | :generate_hand_raise_lead
@@ -16,7 +17,8 @@ RSpec.describe GitlabSubscriptions::CreateTrialOrLeadService do
       it 'successfully creates a trial or lead' do
         allow(Gitlab::SubscriptionPortal::Client).to receive(service).and_return({ success: true })
 
-        result = described_class.new.execute(**{ user: user, params: { trial: trial } })
+        result = described_class.new(**{ user: user, params: { trial: trial } }).execute
+
         expect(result.is_a?(ServiceResponse)).to be true
         expect(result.success?).to be true
       end
@@ -24,7 +26,8 @@ RSpec.describe GitlabSubscriptions::CreateTrialOrLeadService do
       it 'error while creating trial or lead' do
         allow(Gitlab::SubscriptionPortal::Client).to receive(service).and_return({ success: false })
 
-        result = described_class.new.execute(**{ user: user, params: { trial: trial } })
+        result = described_class.new(**{ user: user, params: { trial: trial } }).execute
+
         expect(result.is_a?(ServiceResponse)).to be true
         expect(result.success?).to be false
       end