From 72b7b10d52de1b150b124c017427b2802b4193e6 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 24 Oct 2017 09:38:06 +0300 Subject: [PATCH 1/9] add applications controller logic --- .../admin/applications_controller.rb | 15 +++++-------- .../oauth/applications_controller.rb | 22 +++++++++---------- app/services/applications/create_searvice.rb | 15 +++++++++++++ .../applications/create_service_spec.rb | 12 ++++++++++ 4 files changed, 42 insertions(+), 22 deletions(-) create mode 100644 app/services/applications/create_searvice.rb create mode 100644 spec/services/applications/create_service_spec.rb diff --git a/app/controllers/admin/applications_controller.rb b/app/controllers/admin/applications_controller.rb index fb6d8c0bb81..69ebe7db08b 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.merge(ip_address: request.remote_ip)).execute - 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..daba161a177 100644 --- a/app/controllers/oauth/applications_controller.rb +++ b/app/controllers/oauth/applications_controller.rb @@ -16,25 +16,16 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController end def create - @application = Doorkeeper::Application.new(application_params) + @application = Applications::CreateService.new(current_user, create_application_params).execute - @application.owner = current_user - - if @application.save - redirect_to_oauth_application_page + if @application.persisted? + flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create]) 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 +52,11 @@ 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 + params[:ip_address] = request.remote_ip + end + end end diff --git a/app/services/applications/create_searvice.rb b/app/services/applications/create_searvice.rb new file mode 100644 index 00000000000..66092261a11 --- /dev/null +++ b/app/services/applications/create_searvice.rb @@ -0,0 +1,15 @@ +module Applications + class CreateService + prepend ::EE::Applications::CreateService + + def initialize(current_user, params) + @current_user = current_user + @params = params + @ip_address = @params.delete(:ip_address) + end + + def execute + Doorkeeper::Application.create(@params) + end + end +end diff --git a/spec/services/applications/create_service_spec.rb b/spec/services/applications/create_service_spec.rb new file mode 100644 index 00000000000..0bd96a605c0 --- /dev/null +++ b/spec/services/applications/create_service_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe ::Applications::CreateService do + let(:user) { create(:user) } + let(:params) { attributes_for(:application) } + + subject { described_class.new(user, params) } + + it 'creates an application' do + expect { subject.execute }.to change { Doorkeeper::Application.count }.by(1) + end +end From d0546e6d81fa3eb9424db9beb5e34d351fabb0fc Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 24 Oct 2017 09:52:47 +0300 Subject: [PATCH 2/9] uypdated keys controller logic --- app/controllers/profiles/keys_controller.rb | 10 ++-------- app/services/keys/base_service.rb | 1 + 2 files changed, 3 insertions(+), 8 deletions(-) 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/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 From 11e5218ceb7b4179ac11f941e138ff8081edfe45 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 24 Oct 2017 10:34:04 +0300 Subject: [PATCH 3/9] fix spec --- app/services/applications/create_searvice.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/services/applications/create_searvice.rb b/app/services/applications/create_searvice.rb index 66092261a11..ccc0d0848f9 100644 --- a/app/services/applications/create_searvice.rb +++ b/app/services/applications/create_searvice.rb @@ -1,7 +1,5 @@ module Applications class CreateService - prepend ::EE::Applications::CreateService - def initialize(current_user, params) @current_user = current_user @params = params From eeacfbd5bf2c422c3cff7c141cec6f2e4d2b0dd6 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 24 Oct 2017 12:03:44 +0300 Subject: [PATCH 4/9] fix specs --- .../applications/{create_searvice.rb => create_service.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename app/services/applications/{create_searvice.rb => create_service.rb} (100%) diff --git a/app/services/applications/create_searvice.rb b/app/services/applications/create_service.rb similarity index 100% rename from app/services/applications/create_searvice.rb rename to app/services/applications/create_service.rb From 82c758c0a85433ca05442ed440f823e17e82573b Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 24 Oct 2017 14:42:20 +0300 Subject: [PATCH 5/9] fix spinach failure --- app/controllers/oauth/applications_controller.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb index daba161a177..4dbd61bfeae 100644 --- a/app/controllers/oauth/applications_controller.rb +++ b/app/controllers/oauth/applications_controller.rb @@ -20,6 +20,8 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController if @application.persisted? flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create]) + + redirect_to oauth_application_url(@application) else set_index_vars render :index From 232bdd47d9af004b09cfde68d866d3e694572774 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 31 Oct 2017 09:02:57 +0100 Subject: [PATCH 6/9] refactor code --- app/controllers/admin/applications_controller.rb | 2 +- app/controllers/oauth/applications_controller.rb | 3 +-- app/services/applications/create_service.rb | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/applications_controller.rb b/app/controllers/admin/applications_controller.rb index 69ebe7db08b..bf4e65183ad 100644 --- a/app/controllers/admin/applications_controller.rb +++ b/app/controllers/admin/applications_controller.rb @@ -19,7 +19,7 @@ class Admin::ApplicationsController < Admin::ApplicationController end def create - @application = Applications::CreateService.new(current_user, application_params.merge(ip_address: request.remote_ip)).execute + @application = Applications::CreateService.new(current_user, application_params.merge.execute(request) if @application.persisted? flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create]) diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb index 4dbd61bfeae..2443f529c7b 100644 --- a/app/controllers/oauth/applications_controller.rb +++ b/app/controllers/oauth/applications_controller.rb @@ -16,7 +16,7 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController end def create - @application = Applications::CreateService.new(current_user, create_application_params).execute + @application = Applications::CreateService.new(current_user, create_application_params).execute(request) if @application.persisted? flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create]) @@ -58,7 +58,6 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController def create_application_params application_params.tap do |params| params[:owner] = current_user - params[:ip_address] = request.remote_ip end end end diff --git a/app/services/applications/create_service.rb b/app/services/applications/create_service.rb index ccc0d0848f9..35d45f25a71 100644 --- a/app/services/applications/create_service.rb +++ b/app/services/applications/create_service.rb @@ -6,7 +6,7 @@ module Applications @ip_address = @params.delete(:ip_address) end - def execute + def execute(request = nil) Doorkeeper::Application.create(@params) end end From 37eb00ce5b684ff2bbbc83746a298a71e275c84d Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 2 Nov 2017 08:49:36 +0100 Subject: [PATCH 7/9] fix specs --- app/controllers/admin/applications_controller.rb | 2 +- spec/services/applications/create_service_spec.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/applications_controller.rb b/app/controllers/admin/applications_controller.rb index bf4e65183ad..b47a9ee2444 100644 --- a/app/controllers/admin/applications_controller.rb +++ b/app/controllers/admin/applications_controller.rb @@ -19,7 +19,7 @@ class Admin::ApplicationsController < Admin::ApplicationController end def create - @application = Applications::CreateService.new(current_user, application_params.merge.execute(request) + @application = Applications::CreateService.new(current_user, application_params.merge.execute(request)) if @application.persisted? flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create]) diff --git a/spec/services/applications/create_service_spec.rb b/spec/services/applications/create_service_spec.rb index 0bd96a605c0..47a2a9d6403 100644 --- a/spec/services/applications/create_service_spec.rb +++ b/spec/services/applications/create_service_spec.rb @@ -3,10 +3,11 @@ 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 }.to change { Doorkeeper::Application.count }.by(1) + expect { subject.execute(request) }.to change { Doorkeeper::Application.count }.by(1) end end From 32ef7ed7d6d2540a3716f1b8e40f130008b2e5c2 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 2 Nov 2017 13:14:56 +0100 Subject: [PATCH 8/9] fix specs --- app/controllers/admin/applications_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/admin/applications_controller.rb b/app/controllers/admin/applications_controller.rb index b47a9ee2444..cdac3b90a7f 100644 --- a/app/controllers/admin/applications_controller.rb +++ b/app/controllers/admin/applications_controller.rb @@ -19,7 +19,7 @@ class Admin::ApplicationsController < Admin::ApplicationController end def create - @application = Applications::CreateService.new(current_user, application_params.merge.execute(request)) + @application = Applications::CreateService.new(current_user, application_params).merge.execute(request) if @application.persisted? flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create]) From 9cb9a86f37e89ad181a6a4d024240752c50606c9 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 2 Nov 2017 14:33:56 +0100 Subject: [PATCH 9/9] fix typo --- app/controllers/admin/applications_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/admin/applications_controller.rb b/app/controllers/admin/applications_controller.rb index cdac3b90a7f..5be23c76a95 100644 --- a/app/controllers/admin/applications_controller.rb +++ b/app/controllers/admin/applications_controller.rb @@ -19,7 +19,7 @@ class Admin::ApplicationsController < Admin::ApplicationController end def create - @application = Applications::CreateService.new(current_user, application_params).merge.execute(request) + @application = Applications::CreateService.new(current_user, application_params).execute(request) if @application.persisted? flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create])