Merge branch 'refactor-services-for-audit-events-ce' into 'master'

[CE] Refactor controller calls into services

Closes gitlab-ee#3544

See merge request gitlab-org/gitlab-ce!15023

6 files changed, 44 insertions, 29 deletions

diff --git a/app/controllers/admin/applications_controller.rb b/app/controllers/admin/applications_controller.rb
index fb6d8c0bb81..5be23c76a95 100644
--- a/app/controllers/admin/applications_controller.rb
+++ b/app/controllers/admin/applications_controller.rb
@@ -19,10 +19,12 @@ class Admin::ApplicationsController < Admin::ApplicationController
   end
 
   def create
-    @application = Doorkeeper::Application.new(application_params)
+    @application = Applications::CreateService.new(current_user, application_params).execute(request)
 
-    if @application.save
-      redirect_to_admin_page
+    if @application.persisted?
+      flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create])
+
+      redirect_to admin_application_url(@application)
     else
       render :new
     end
@@ -41,13 +43,6 @@ class Admin::ApplicationsController < Admin::ApplicationController
     redirect_to admin_applications_url, status: 302, notice: 'Application was successfully destroyed.'
   end
 
-  protected
-
-  def redirect_to_admin_page
-    flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create])
-    redirect_to admin_application_url(@application)
-  end
-
   private
 
   def set_application
diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb
index b02e64a132b..2443f529c7b 100644
--- a/app/controllers/oauth/applications_controller.rb
+++ b/app/controllers/oauth/applications_controller.rb
@@ -16,25 +16,18 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
   end
 
   def create
-    @application = Doorkeeper::Application.new(application_params)
+    @application = Applications::CreateService.new(current_user, create_application_params).execute(request)
 
-    @application.owner = current_user
+    if @application.persisted?
+      flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create])
 
-    if @application.save
-      redirect_to_oauth_application_page
+      redirect_to oauth_application_url(@application)
     else
       set_index_vars
       render :index
     end
   end
 
-  protected
-
-  def redirect_to_oauth_application_page
-    flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create])
-    redirect_to oauth_application_url(@application)
-  end
-
   private
 
   def verify_user_oauth_applications_enabled
@@ -61,4 +54,10 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
   rescue_from ActiveRecord::RecordNotFound do |exception|
     render "errors/not_found", layout: "errors", status: 404
   end
+
+  def create_application_params
+    application_params.tap do |params|
+      params[:owner] = current_user
+    end
+  end
 end
diff --git a/app/controllers/profiles/keys_controller.rb b/app/controllers/profiles/keys_controller.rb
index 069e6a810f2..f0e5d2aa94e 100644
--- a/app/controllers/profiles/keys_controller.rb
+++ b/app/controllers/profiles/keys_controller.rb
@@ -11,10 +11,10 @@ class Profiles::KeysController < Profiles::ApplicationController
   end
 
   def create
-    @key = Keys::CreateService.new(current_user, key_params).execute
+    @key = Keys::CreateService.new(current_user, key_params.merge(ip_address: request.remote_ip)).execute
 
     if @key.persisted?
-      redirect_to_profile_key_path
+      redirect_to profile_key_path(@key)
     else
       @keys = current_user.keys.select(&:persisted?)
       render :index
@@ -50,12 +50,6 @@ class Profiles::KeysController < Profiles::ApplicationController
     end
   end
 
-  protected
-
-  def redirect_to_profile_key_path
-    redirect_to profile_key_path(@key)
-  end
-
   private
 
   def key_params
diff --git a/app/services/applications/create_service.rb b/app/services/applications/create_service.rb
new file mode 100644
index 00000000000..35d45f25a71
--- /dev/null
+++ b/app/services/applications/create_service.rb
@@ -0,0 +1,13 @@
+module Applications
+  class CreateService
+    def initialize(current_user, params)
+      @current_user = current_user
+      @params = params
+      @ip_address = @params.delete(:ip_address)
+    end
+
+    def execute(request = nil)
+      Doorkeeper::Application.create(@params)
+    end
+  end
+end
diff --git a/app/services/keys/base_service.rb b/app/services/keys/base_service.rb
index 545832d0bd4..f78791932a7 100644
--- a/app/services/keys/base_service.rb
+++ b/app/services/keys/base_service.rb
@@ -4,6 +4,7 @@ module Keys
 
     def initialize(user, params)
       @user, @params = user, params
+      @ip_address = @params.delete(:ip_address)
     end
 
     def notification_service
diff --git a/spec/services/applications/create_service_spec.rb b/spec/services/applications/create_service_spec.rb
new file mode 100644
index 00000000000..47a2a9d6403
--- /dev/null
+++ b/spec/services/applications/create_service_spec.rb
@@ -0,0 +1,13 @@
+require 'spec_helper'
+
+describe ::Applications::CreateService do
+  let(:user) { create(:user) }
+  let(:params) { attributes_for(:application) }
+  let(:request) { ActionController::TestRequest.new(remote_ip: '127.0.0.1') }
+
+  subject { described_class.new(user, params) }
+
+  it 'creates an application' do
+    expect { subject.execute(request) }.to change { Doorkeeper::Application.count }.by(1)
+  end
+end