Move functionality for resolving discussions into a concern

This commit is contained in:
Bob Van Landuyt 2017-02-27 18:12:52 +01:00 committed by Bob Van Landuyt
parent 6ffa988ac7
commit ffe135ccf6
6 changed files with 54 additions and 42 deletions

View file

@ -64,16 +64,15 @@ class Projects::IssuesController < Projects::ApplicationController
params[:issue] ||= ActionController::Parameters.new(
assignee_id: ""
)
build_params = issue_params.merge(
merge_request_for_resolving_discussions: params[:merge_request_for_resolving_discussions],
discussion_to_resolve: params[:discussion_to_resolve]
)
service = Issues::BuildService.new(project, current_user, build_params)
@merge_request_for_resolving_discussions = service.merge_request_for_resolving_discussions
@discussion_to_resolve = service.discussions_to_resolve.first if params[:discussion_to_resolve]
@issue = @noteable = service.execute
@merge_request_for_resolving_discussions = service.merge_request_for_resolving_discussions
@discussion_to_resolve = service.discussions_to_resolve.first if params[:discussion_to_resolve]
respond_with(@issue)
end

View file

@ -1,13 +1,5 @@
module Issues
class BaseService < ::IssuableBaseService
attr_reader :merge_request_for_resolving_discussions_iid, :discussion_to_resolve_id
def initialize(*args)
super
@merge_request_for_resolving_discussions_iid ||= params.delete(:merge_request_for_resolving_discussions)
@discussion_to_resolve_id ||= params.delete(:discussion_to_resolve)
end
def hook_data(issue, action)
issue_data = issue.to_hook_data(current_user)
issue_url = Gitlab::UrlBuilder.build(issue)
@ -15,25 +7,6 @@ module Issues
issue_data
end
def merge_request_for_resolving_discussions
@merge_request_for_resolving_discussions ||= MergeRequestsFinder.new(current_user, project_id: project.id).
execute.
find_by(iid: merge_request_for_resolving_discussions_iid)
end
def discussions_to_resolve
return [] unless merge_request_for_resolving_discussions
@discussions_to_resolve ||= NotesFinder.new(project, current_user, {
discussion_id: discussion_to_resolve_id,
target_type: MergeRequest.name.underscore,
target_id: merge_request_for_resolving_discussions.id
}).
execute.
discussions.
select(&:to_be_resolved?)
end
private
def execute_hooks(issue, action = 'open')

View file

@ -1,6 +1,9 @@
module Issues
class BuildService < Issues::BaseService
include ResolveDiscussions
def execute
filter_resolve_discussion_params
@issue = project.issues.new(issue_params)
end

View file

@ -1,9 +1,11 @@
module Issues
class CreateService < Issues::BaseService
include SpamCheckService
include ResolveDiscussions
def execute
filter_spam_check_params
filter_resolve_discussion_params
issue_attributes = params.merge(
merge_request_for_resolving_discussions: merge_request_for_resolving_discussions_iid,
@ -28,15 +30,6 @@ module Issues
resolve_discussions_with_issue(issuable)
end
def resolve_discussions_with_issue(issue)
return if discussions_to_resolve.empty?
Discussions::ResolveService.new(project, current_user,
merge_request: merge_request_for_resolving_discussions,
follow_up_issue: issue).
execute(discussions_to_resolve)
end
private
def user_agent_detail_service

View file

@ -0,0 +1,35 @@
module Issues
module ResolveDiscussions
attr_reader :merge_request_for_resolving_discussions_iid, :discussion_to_resolve_id
def filter_resolve_discussion_params
@merge_request_for_resolving_discussions_iid ||= params.delete(:merge_request_for_resolving_discussions)
@discussion_to_resolve_id ||= params.delete(:discussion_to_resolve)
end
def resolve_discussions_with_issue(issue)
return if discussions_to_resolve.empty?
Discussions::ResolveService.new(project, current_user,
merge_request: merge_request_for_resolving_discussions,
follow_up_issue: issue).
execute(discussions_to_resolve)
end
def merge_request_for_resolving_discussions
@merge_request_for_resolving_discussions ||= MergeRequestsFinder.new(current_user, project_id: project.id).
execute.
find_by(iid: merge_request_for_resolving_discussions_iid)
end
def discussions_to_resolve
return [] unless merge_request_for_resolving_discussions
@discussions_to_resolve ||= NotesFinder.new(project, current_user, {
discussion_id: discussion_to_resolve_id,
target_type: MergeRequest.name.underscore,
target_id: merge_request_for_resolving_discussions.id
}).execute.discussions.select(&:to_be_resolved?)
end
end
end

View file

@ -1,6 +1,15 @@
require 'spec_helper.rb'
describe Issues::BaseService, services: true do
class DummyService < Issues::BaseService
include ::Issues::ResolveDiscussions
def initialize(*args)
super
filter_resolve_discussion_params
end
end
describe DummyService, services: true do
let(:project) { create(:project) }
let(:user) { create(:user) }
@ -46,7 +55,7 @@ describe Issues::BaseService, services: true do
end
it "contains only unresolved discussions" do
second_discussion = Discussion.new([create(:diff_note_on_merge_request, :resolved,
_second_discussion = Discussion.new([create(:diff_note_on_merge_request, :resolved,
noteable: merge_request,
project: merge_request.target_project,
line_number: 15,