account params wip

Author: mayorova

app/controllers/admin/api/signups_controller.rb

@@ -7,16 +7,30 @@ class Admin::Api::SignupsController < Admin::Api::BaseController
   def create
     authorize!(:create, Account) if current_user
 
-    @signup_result = Signup::DeveloperAccountManager.new(current_account).create(signup_params)
+    @signup_result = account_manager.create(signup_params)
 
     check_creation_errors
     respond_with(@signup_result.account, user_options: { with_apps: true })
   end
 
   private
 
+  def account_manager
+    @account_manager ||= Signup::DeveloperAccountManager.new(current_account)
+  end
+
   def user_params
-    flat_params.merge({signup_type: :minimal})
+    @user_params ||= begin
+      allowed_attrs = account_manager.user.defined_fields_names | %i(password signup_type)
+      flat_params.permit(*allowed_attrs).merge({signup_type: :minimal})
+    end
+  end
+
+  def account_params
+    @account_params ||= begin
+      allowed_attrs = account_manager.account.defined_fields_names - %i(billing_address country) + %i(country_id)
+      flat_params.permit(*allowed_attrs)
+    end
   end
 
   def check_creation_errors
@@ -31,7 +45,7 @@ def check_creation_errors
   end
 
   def signup_params
-    Signup::SignupParams.new(plans: plans, user_attributes: user_params, account_attributes: flat_params, defaults: defaults)
+    Signup::SignupParams.new(plans: plans, user_attributes: user_params, account_attributes: account_params, defaults: defaults)
   end
 
   def defaults

app/controllers/buyers/accounts_controller.rb

@@ -9,6 +9,7 @@ class Buyers::AccountsController < Buyers::BaseController
 
   before_action :set_plans, :only => %i[new create]
   before_action :find_account, except: %i[index new create]
+  before_action :init_signup_account_manager, only: %i[create]
 
   activate_menu :buyers, :accounts, :listing
 
@@ -41,7 +42,7 @@ def update
   end
 
   def create
-    signup_result = Signup::DeveloperAccountManager.new(current_account).create(signup_params)
+    signup_result = @account_manager.create(signup_params)
     @buyer = signup_result.account
 
     if signup_result.persisted?
@@ -84,7 +85,7 @@ def show
 
   protected
 
-  attr_reader :account
+  attr_reader :account, :user
 
   def find_account
     with_deleted = %w[show resume].include?(action_name)
@@ -109,13 +110,19 @@ def signup_params
     Signup::SignupParams.new(plans: [], user_attributes: user_params.merge(signup_type: :created_by_provider), account_attributes: account_params, validate_fields: false)
   end
 
-  # TODO: using `permit` later
   def account_params
-    @account_params ||= params.require(:account).except(:user)
+    @account_params ||= begin
+      allowed_attrs = account.defined_builtin_fields_names - %i(billing_address country) + %i(country_id)
+      params.require(:account).permit(*allowed_attrs, extra_fields: account.defined_extra_fields_names)
+    end
   end
 
   def user_params
-    params.require(:account).fetch(:user, {})
+    @user_params ||= begin
+      allowed_attrs = user.defined_builtin_fields_names | %i(password signup_type)
+      params.require(:account).fetch(:user, ActionController::Parameters.new)
+            .permit(*allowed_attrs, extra_fields: user.defined_extra_fields_names)
+    end
   end
 
   def set_plans
@@ -131,4 +138,10 @@ def presenter
                                                       user: current_user,
                                                       params: params)
   end
+
+  def init_signup_account_manager
+    @account_manager = Signup::DeveloperAccountManager.new(current_account)
+    @account = @account_manager.account
+    @user = @account_manager.user
+  end
 end

app/lib/signup/account_manager.rb

@@ -4,13 +4,17 @@ module Signup
   class AccountManager
     def initialize(manager_account)
       @manager_account = manager_account
+      @account = manager_account.buyers.new
+      @user = @account.users.new
     end
 
-    attr_reader :manager_account
+    attr_reader :manager_account, :account, :user
 
     def create(signup_params, signup_result_class = ::Signup::Result)
       transaction do
-        signup_result = build_signup_result(signup_params, signup_result_class)
+        assign_attributes_for_account(signup_params)
+        assign_attributes_for_user(signup_params)
+        signup_result = build_signup_result(signup_result_class)
         yield(signup_result) if block_given?
         save_result_with_plans(signup_result, signup_params) if signup_result.valid?
         signup_result
@@ -44,21 +48,22 @@ def persist!(*)
       raise NotImplementedError, 'persist! should be implemented in subclasses'
     end
 
-    def build_signup_result(signup_params, signup_result_class)
-      account = build_account(signup_params)
-      signup_result_class.new(user: build_user(signup_params, account), account: account)
+    def build_signup_result(signup_result_class)
+      signup_result_class.new(user: user, account: account)
     end
 
-    def build_user(signup_params, account)
-      user             = signup_params.build_user_with_attributes_for_account(account)
-      user.role        = :admin
-      user
+    def assign_attributes_for_account(signup_params)
+      account.validate_fields! if signup_params.validate_fields
+      account_attributes = signup_params.attributes[:account]
+      account_attributes.delete('name') if account_attributes['org_name'].present?
+      account.assign_attributes(account_attributes)
+      account_builder.call(account)
     end
 
-    def build_account(signup_params)
-      account = signup_params.build_account_with_attributes_for_provider_account(manager_account)
-      account_builder.call(account)
-      account
+    def assign_attributes_for_user(signup_params)
+      user.validate_fields! if signup_params.validate_fields
+      user.assign_attributes(signup_params.attributes[:user])
+      user.role = :admin
     end
 
     def create_contract_plans_for_account!(account, plans, defaults)

app/lib/signup/signup_params.rb

@@ -8,7 +8,7 @@ def initialize(user_attributes: {}, account_attributes: {}, plans: [], defaults:
       @defaults = defaults
       @validate_fields = validate_fields
     end
-    attr_reader :plans, :defaults
+    attr_reader :plans, :defaults, :attributes, :validate_fields
 
     def build_user_with_attributes_for_account(account)
       user = account.users.new
@@ -29,8 +29,6 @@ def build_account_with_attributes_for_provider_account(provider_account)
 
     private
 
-    attr_reader :attributes, :validate_fields
-
     def account_attributes
       attributes[:account]
     end

app/services/signup_service.rb

@@ -2,19 +2,18 @@
 
 class SignupService
 
-  attr_reader :provider, :plans, :session, :account_params, :user_params, :authentication_provider
+  attr_reader :provider, :plans, :session, :authentication_provider, :account_manager
 
-  def initialize(provider:, plans:, session:, account_params:, user_params:, authentication_provider: nil)
+  def initialize(provider:, plans:, session:, authentication_provider: nil)
     @provider       = provider
     @plans          = plans
     @session        = session
-    @account_params = account_params
-    @user_params    = user_params.merge(signup_type: :new_signup)
     @authentication_provider = authentication_provider
+    @account_manager = Signup::DeveloperAccountManager.new(provider)
   end
 
-  def create
-    signup_result = Signup::DeveloperAccountManager.new(provider).create(signup_params) do |signup|
+  def create(account_params:, user_params:)
+    signup_result = account_manager.create(signup_params(account_params:, user_params:)) do |signup|
       strategy.on_new_user(signup.user, session)
       yield(signup) if block_given?
     end
@@ -40,8 +39,8 @@ def account_should_be_approved?(signup_result)
     authentication_provider.automatically_approve_accounts? && !signup_result.account_approved?
   end
 
-  def signup_params
-    Signup::SignupParams.new(plans: plans, user_attributes: user_params, account_attributes: account_params)
+  def signup_params(account_params:, user_params:)
+    Signup::SignupParams.new(plans: plans, user_attributes: user_params.merge(signup_type: :new_signup), account_attributes: account_params)
   end
 
   def push_webhooks(user)

lib/developer_portal/app/controllers/developer_portal/signup_controller.rb

@@ -11,6 +11,7 @@ class SignupController < DeveloperPortal::BaseController
     before_action :deny_if_signup_disabled
     before_action :find_plans, :except => :success
     before_action :set_strategy, :only => %i[show create]
+    before_action :init_signup_service, only: %i[create]
     skip_before_action :finish_signup_for_paid_plan
 
     self.builtin_template_scope = 'signup'
@@ -30,10 +31,7 @@ def show
     end
 
     def create
-      account_params = filter_readonly_params(params[:account], Account)
-      user_params    = filter_readonly_params(account_params.try(:delete, :user), User)
-
-      if signup_user!(account_params, user_params)
+      if signup_user!
         if @user.can_login?
           self.current_user = @user
           create_user_session!
@@ -67,9 +65,9 @@ def authentication_provider
         .find_by(system_name: session[:authentication_provider])
     end
 
-    def signup_user!(account_params, user_params)
+    def signup_user!
       Account.transaction do
-        SignupService.create(**signup_service_params(account_params, user_params)) do |signup_result|
+        @signup_service.create(account_params:, user_params:) do |signup_result|
           @signup_result = signup_result
           @user  = signup_result.user
           @buyer = signup_result.account
@@ -80,16 +78,6 @@ def signup_user!(account_params, user_params)
       @signup_result.persisted?
     end
 
-    def signup_service_params(account_params, user_params)
-      { provider:       @provider,
-        plans:          @plans,
-        session:        session,
-        account_params: account_params,
-        user_params:    user_params,
-        authentication_provider: authentication_provider
-      }
-    end
-
     def redirect_if_logged_in
       redirect_to admin_dashboard_path if logged_in?
     end
@@ -134,5 +122,31 @@ def convert_legacy_params
         params[:plans] << params[type] if params[type].present?
       end
     end
+
+    def init_signup_service
+      signup_service_params = {
+        provider: @provider,
+        plans: @plans,
+        session: session,
+        authentication_provider: authentication_provider
+      }
+      @signup_service = SignupService.new(**signup_service_params)
+    end
+
+    def account_params
+      @account_params ||= begin
+        account = @signup_service.account_manager.account
+        allowed_attrs = account.defined_fields_names - %i(billing_address country) + %i(country_id)
+        filter_readonly_params(params.fetch(:account, {}), Account).permit(*allowed_attrs)
+      end
+    end
+
+    def user_params
+      @user_params ||= begin
+        user = @signup_service.account_manager.user
+        allowed_attrs = user.defined_fields_names | %i(password signup_type)
+        filter_readonly_params(params.fetch(:account, {})[:user], User).permit(*allowed_attrs).merge({ signup_type: :minimal })
+      end
+    end
   end
 end

test/integration/buyers/accounts_controller_test.rb

@@ -32,9 +32,9 @@ def setup
       assert_equal 'hi', user.extra_fields['created_by']
     end
 
-    test 'billing address extra field and webhooks' do
+    test 'legal address extra field and webhooks' do
       FactoryBot.create(:fields_definition, account: @provider,
-                         target: 'Account', name: 'billing_address', read_only: true)
+                         target: 'Account', name: 'org_legaladdress')
 
       @provider.settings.allow_web_hooks!
       WebHook.delete_all