Commit cfd9fd30d60c5a880785acda27e9f3d55b17e4ef
1 parent
3b0510a7
Exists in
spb-stable
and in
3 other branches
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 <dmitriy.zaporozhets@gmail.com>
Showing
6 changed files
with
67 additions
and
21 deletions
Show diff stats
app/controllers/projects/issues_controller.rb
| @@ -59,9 +59,7 @@ class Projects::IssuesController < Projects::ApplicationController | @@ -59,9 +59,7 @@ class Projects::IssuesController < Projects::ApplicationController | ||
| 59 | end | 59 | end |
| 60 | 60 | ||
| 61 | def create | 61 | def create |
| 62 | - @issue = @project.issues.new(params[:issue]) | ||
| 63 | - @issue.author = current_user | ||
| 64 | - @issue.save | 62 | + @issue = Issues::CreateService.new(project, current_user, params[:issue]).execute |
| 65 | 63 | ||
| 66 | respond_to do |format| | 64 | respond_to do |format| |
| 67 | format.html do | 65 | format.html do |
app/observers/issue_observer.rb
| 1 | class IssueObserver < BaseObserver | 1 | class IssueObserver < BaseObserver |
| 2 | - def after_create(issue) | ||
| 3 | - notification.new_issue(issue, current_user) | ||
| 4 | - event_service.open_issue(issue, current_user) | ||
| 5 | - issue.create_cross_references!(issue.project, current_user) | ||
| 6 | - execute_hooks(issue) | ||
| 7 | - end | ||
| 8 | - | ||
| 9 | def after_close(issue, transition) | 2 | def after_close(issue, transition) |
| 10 | notification.close_issue(issue, current_user) | 3 | notification.close_issue(issue, current_user) |
| 11 | event_service.close_issue(issue, current_user) | 4 | event_service.close_issue(issue, current_user) |
app/services/base_service.rb
| @@ -16,4 +16,16 @@ class BaseService | @@ -16,4 +16,16 @@ class BaseService | ||
| 16 | def can?(object, action, subject) | 16 | def can?(object, action, subject) |
| 17 | abilities.allowed?(object, action, subject) | 17 | abilities.allowed?(object, action, subject) |
| 18 | end | 18 | end |
| 19 | + | ||
| 20 | + def notification_service | ||
| 21 | + NotificationService.new | ||
| 22 | + end | ||
| 23 | + | ||
| 24 | + def event_service | ||
| 25 | + EventCreateService.new | ||
| 26 | + end | ||
| 27 | + | ||
| 28 | + def log_info message | ||
| 29 | + Gitlab::AppLogger.info message | ||
| 30 | + end | ||
| 19 | end | 31 | end |
| @@ -0,0 +1,23 @@ | @@ -0,0 +1,23 @@ | ||
| 1 | +module Issues | ||
| 2 | + class CreateService < BaseService | ||
| 3 | + def execute | ||
| 4 | + issue = project.issues.new(params) | ||
| 5 | + issue.author = current_user | ||
| 6 | + | ||
| 7 | + if issue.save | ||
| 8 | + notification_service.new_issue(issue, current_user) | ||
| 9 | + event_service.open_issue(issue, current_user) | ||
| 10 | + issue.create_cross_references!(issue.project, current_user) | ||
| 11 | + execute_hooks(issue) | ||
| 12 | + end | ||
| 13 | + | ||
| 14 | + issue | ||
| 15 | + end | ||
| 16 | + | ||
| 17 | + private | ||
| 18 | + | ||
| 19 | + def execute_hooks(issue) | ||
| 20 | + issue.project.execute_hooks(issue.to_hook_data, :issue_hooks) | ||
| 21 | + end | ||
| 22 | + end | ||
| 23 | +end |
lib/api/issues.rb
| @@ -48,17 +48,15 @@ module API | @@ -48,17 +48,15 @@ module API | ||
| 48 | # Example Request: | 48 | # Example Request: |
| 49 | # POST /projects/:id/issues | 49 | # POST /projects/:id/issues |
| 50 | post ":id/issues" do | 50 | post ":id/issues" do |
| 51 | - set_current_user_for_thread do | ||
| 52 | - required_attributes! [:title] | ||
| 53 | - attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id] | ||
| 54 | - attrs[:label_list] = params[:labels] if params[:labels].present? | ||
| 55 | - @issue = user_project.issues.new attrs | ||
| 56 | - @issue.author = current_user | ||
| 57 | - if @issue.save | ||
| 58 | - present @issue, with: Entities::Issue | ||
| 59 | - else | ||
| 60 | - not_found! | ||
| 61 | - end | 51 | + required_attributes! [:title] |
| 52 | + attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id] | ||
| 53 | + attrs[:label_list] = params[:labels] if params[:labels].present? | ||
| 54 | + issue = ::Issues::CreateService.new(user_project, current_user, attrs).execute | ||
| 55 | + | ||
| 56 | + if issue.valid? | ||
| 57 | + present issue, with: Entities::Issue | ||
| 58 | + else | ||
| 59 | + not_found! | ||
| 62 | end | 60 | end |
| 63 | end | 61 | end |
| 64 | 62 |
| @@ -0,0 +1,22 @@ | @@ -0,0 +1,22 @@ | ||
| 1 | +require 'spec_helper' | ||
| 2 | + | ||
| 3 | +describe Issues::CreateService do | ||
| 4 | + let(:project) { create(:empty_project) } | ||
| 5 | + let(:user) { create(:user) } | ||
| 6 | + | ||
| 7 | + describe :execute do | ||
| 8 | + context "valid params" do | ||
| 9 | + before do | ||
| 10 | + project.team << [user, :master] | ||
| 11 | + opts = { | ||
| 12 | + title: 'Awesome issue', | ||
| 13 | + description: 'please fix' | ||
| 14 | + } | ||
| 15 | + | ||
| 16 | + @issue = Issues::CreateService.new(project, user, opts).execute | ||
| 17 | + end | ||
| 18 | + | ||
| 19 | + it { @issue.should be_valid } | ||
| 20 | + end | ||
| 21 | + end | ||
| 22 | +end |