Commit c4e81ed9de6a5bbfe089e9b61ca0400167e489f3
1 parent
cfd9fd30
Exists in
spb-stable
and in
3 other branches
Move update issue code to separate service
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Showing
8 changed files
with
74 additions
and
28 deletions
Show diff stats
app/controllers/projects/issues_controller.rb
| @@ -74,8 +74,7 @@ class Projects::IssuesController < Projects::ApplicationController | @@ -74,8 +74,7 @@ class Projects::IssuesController < Projects::ApplicationController | ||
| 74 | end | 74 | end |
| 75 | 75 | ||
| 76 | def update | 76 | def update |
| 77 | - @issue.update_attributes(params[:issue]) | ||
| 78 | - @issue.reset_events_cache | 77 | + @issue = Issues::UpdateService.new(project, current_user, params[:issue]).execute(issue) |
| 79 | 78 | ||
| 80 | respond_to do |format| | 79 | respond_to do |format| |
| 81 | format.js | 80 | format.js |
app/observers/issue_observer.rb
| @@ -12,16 +12,6 @@ class IssueObserver < BaseObserver | @@ -12,16 +12,6 @@ class IssueObserver < BaseObserver | ||
| 12 | execute_hooks(issue) | 12 | execute_hooks(issue) |
| 13 | end | 13 | end |
| 14 | 14 | ||
| 15 | - def after_update(issue) | ||
| 16 | - if issue.is_being_reassigned? | ||
| 17 | - notification.reassigned_issue(issue, current_user) | ||
| 18 | - create_assignee_note(issue) | ||
| 19 | - end | ||
| 20 | - | ||
| 21 | - issue.notice_added_references(issue.project, current_user) | ||
| 22 | - execute_hooks(issue) | ||
| 23 | - end | ||
| 24 | - | ||
| 25 | protected | 15 | protected |
| 26 | 16 | ||
| 27 | # Create issue note with service comment like 'Status changed to closed' | 17 | # Create issue note with service comment like 'Status changed to closed' |
| @@ -0,0 +1,19 @@ | @@ -0,0 +1,19 @@ | ||
| 1 | +module Issues | ||
| 2 | + class BaseService < ::BaseService | ||
| 3 | + | ||
| 4 | + private | ||
| 5 | + | ||
| 6 | + # Create issue note with service comment like 'Status changed to closed' | ||
| 7 | + def create_note(issue) | ||
| 8 | + Note.create_status_change_note(issue, issue.project, current_user, issue.state, current_commit) | ||
| 9 | + end | ||
| 10 | + | ||
| 11 | + def create_assignee_note(issue) | ||
| 12 | + Note.create_assignee_change_note(issue, issue.project, current_user, issue.assignee) | ||
| 13 | + end | ||
| 14 | + | ||
| 15 | + def execute_hooks(issue) | ||
| 16 | + issue.project.execute_hooks(issue.to_hook_data, :issue_hooks) | ||
| 17 | + end | ||
| 18 | + end | ||
| 19 | +end |
app/services/issues/create_service.rb
| @@ -0,0 +1,19 @@ | @@ -0,0 +1,19 @@ | ||
| 1 | +module Issues | ||
| 2 | + class UpdateService < BaseService | ||
| 3 | + def execute(issue) | ||
| 4 | + if issue.update_attributes(params) | ||
| 5 | + issue.reset_events_cache | ||
| 6 | + | ||
| 7 | + if issue.is_being_reassigned? | ||
| 8 | + notification.reassigned_issue(issue, current_user) | ||
| 9 | + create_assignee_note(issue) | ||
| 10 | + end | ||
| 11 | + | ||
| 12 | + issue.notice_added_references(issue.project, current_user) | ||
| 13 | + execute_hooks(issue) | ||
| 14 | + end | ||
| 15 | + | ||
| 16 | + issue | ||
| 17 | + end | ||
| 18 | + end | ||
| 19 | +end |
lib/api/issues.rb
| @@ -74,18 +74,18 @@ module API | @@ -74,18 +74,18 @@ module API | ||
| 74 | # Example Request: | 74 | # Example Request: |
| 75 | # PUT /projects/:id/issues/:issue_id | 75 | # PUT /projects/:id/issues/:issue_id |
| 76 | put ":id/issues/:issue_id" do | 76 | put ":id/issues/:issue_id" do |
| 77 | - set_current_user_for_thread do | ||
| 78 | - @issue = user_project.issues.find(params[:issue_id]) | ||
| 79 | - authorize! :modify_issue, @issue | 77 | + issue = user_project.issues.find(params[:issue_id]) |
| 78 | + authorize! :modify_issue, issue | ||
| 80 | 79 | ||
| 81 | - attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id, :state_event] | ||
| 82 | - attrs[:label_list] = params[:labels] if params[:labels].present? | 80 | + attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id, :state_event] |
| 81 | + attrs[:label_list] = params[:labels] if params[:labels].present? | ||
| 82 | + | ||
| 83 | + issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue) | ||
| 83 | 84 | ||
| 84 | - if @issue.update_attributes attrs | ||
| 85 | - present @issue, with: Entities::Issue | ||
| 86 | - else | ||
| 87 | - not_found! | ||
| 88 | - end | 85 | + if issue.valid? |
| 86 | + present issue, with: Entities::Issue | ||
| 87 | + else | ||
| 88 | + not_found! | ||
| 89 | end | 89 | end |
| 90 | end | 90 | end |
| 91 | 91 |
spec/services/issues/create_service_spec.rb
| @@ -0,0 +1,24 @@ | @@ -0,0 +1,24 @@ | ||
| 1 | +require 'spec_helper' | ||
| 2 | + | ||
| 3 | +describe Issues::UpdateService do | ||
| 4 | + let(:project) { create(:empty_project) } | ||
| 5 | + let(:user) { create(:user) } | ||
| 6 | + let(:issue) { create(:issue) } | ||
| 7 | + | ||
| 8 | + describe :execute do | ||
| 9 | + context "valid params" do | ||
| 10 | + before do | ||
| 11 | + project.team << [user, :master] | ||
| 12 | + opts = { | ||
| 13 | + title: 'New title', | ||
| 14 | + description: 'Also please fix' | ||
| 15 | + } | ||
| 16 | + | ||
| 17 | + @issue = Issues::UpdateService.new(project, user, opts).execute(issue) | ||
| 18 | + end | ||
| 19 | + | ||
| 20 | + it { @issue.should be_valid } | ||
| 21 | + it { @issue.title.should == 'New title' } | ||
| 22 | + end | ||
| 23 | + end | ||
| 24 | +end |