From cfd9fd30d60c5a880785acda27e9f3d55b17e4ef Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 13:38:35 +0300 Subject: [PATCH] Move code for issue creation to service. The goal of suych refactoring is to get rid of observers. Its much easier to test and code when object creation and all other related actions done in one class instead of splited across observers, callbacks etc. Signed-off-by: Dmitriy Zaporozhets --- app/controllers/projects/issues_controller.rb | 4 +--- app/observers/issue_observer.rb | 7 ------ app/services/base_service.rb | 12 ++++++++++ app/services/issues/create_service.rb | 23 +++++++++++++++++++ lib/api/issues.rb | 20 ++++++++-------- spec/services/issues/create_service_spec.rb | 22 ++++++++++++++++++ 6 files changed, 67 insertions(+), 21 deletions(-) create mode 100644 app/services/issues/create_service.rb create mode 100644 spec/services/issues/create_service_spec.rb diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index eef849d8209..ca85ba6b257 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -59,9 +59,7 @@ class Projects::IssuesController < Projects::ApplicationController end def create - @issue = @project.issues.new(params[:issue]) - @issue.author = current_user - @issue.save + @issue = Issues::CreateService.new(project, current_user, params[:issue]).execute respond_to do |format| format.html do diff --git a/app/observers/issue_observer.rb b/app/observers/issue_observer.rb index 30da1f83da7..c2132ddca55 100644 --- a/app/observers/issue_observer.rb +++ b/app/observers/issue_observer.rb @@ -1,11 +1,4 @@ class IssueObserver < BaseObserver - def after_create(issue) - notification.new_issue(issue, current_user) - event_service.open_issue(issue, current_user) - issue.create_cross_references!(issue.project, current_user) - execute_hooks(issue) - end - def after_close(issue, transition) notification.close_issue(issue, current_user) event_service.close_issue(issue, current_user) diff --git a/app/services/base_service.rb b/app/services/base_service.rb index 610f0474872..9ad80923152 100644 --- a/app/services/base_service.rb +++ b/app/services/base_service.rb @@ -16,4 +16,16 @@ class BaseService def can?(object, action, subject) abilities.allowed?(object, action, subject) end + + def notification_service + NotificationService.new + end + + def event_service + EventCreateService.new + end + + def log_info message + Gitlab::AppLogger.info message + end end diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb new file mode 100644 index 00000000000..37f440fc40e --- /dev/null +++ b/app/services/issues/create_service.rb @@ -0,0 +1,23 @@ +module Issues + class CreateService < BaseService + def execute + issue = project.issues.new(params) + issue.author = current_user + + if issue.save + notification_service.new_issue(issue, current_user) + event_service.open_issue(issue, current_user) + issue.create_cross_references!(issue.project, current_user) + execute_hooks(issue) + end + + issue + end + + private + + def execute_hooks(issue) + issue.project.execute_hooks(issue.to_hook_data, :issue_hooks) + end + end +end diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 3d15c35b8cc..169c58b0075 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -48,17 +48,15 @@ module API # Example Request: # POST /projects/:id/issues post ":id/issues" do - set_current_user_for_thread do - required_attributes! [:title] - attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id] - attrs[:label_list] = params[:labels] if params[:labels].present? - @issue = user_project.issues.new attrs - @issue.author = current_user - if @issue.save - present @issue, with: Entities::Issue - else - not_found! - end + required_attributes! [:title] + attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id] + attrs[:label_list] = params[:labels] if params[:labels].present? + issue = ::Issues::CreateService.new(user_project, current_user, attrs).execute + + if issue.valid? + present issue, with: Entities::Issue + else + not_found! end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb new file mode 100644 index 00000000000..7e2d5ad2e81 --- /dev/null +++ b/spec/services/issues/create_service_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Issues::CreateService do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + + describe :execute do + context "valid params" do + before do + project.team << [user, :master] + opts = { + title: 'Awesome issue', + description: 'please fix' + } + + @issue = Issues::CreateService.new(project, user, opts).execute + end + + it { @issue.should be_valid } + end + end +end