Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
path: root/app
diff options
context:
space:
mode:
authorBob Van Landuyt <bob@vanlanduyt.co>2016-12-22 15:31:12 +0300
committerBob Van Landuyt <bob@gitlab.com>2017-03-13 10:27:51 +0300
commit0267b83898d604181e70c5ea8ac4a84108d2e6d6 (patch)
tree3ce8cc81ba90185c976f3b19b7e548c67856370f /app
parent9ed3db915026c6e0cd266a1c276386e3e96d2151 (diff)
Delegate a single discussion to a new issue
Delegate a discussion in a merge request into a new issue. The discussion wil be marked as resolved and a system note will be added linking to the newly created issue.
Diffstat (limited to 'app')
-rw-r--r--app/assets/javascripts/diff_notes/components/new_issue_for_discussion.js.es629
-rw-r--r--app/assets/javascripts/diff_notes/diff_notes_bundle.js3
-rw-r--r--app/assets/stylesheets/pages/note_form.scss21
-rw-r--r--app/controllers/projects/issues_controller.rb24
-rw-r--r--app/finders/notes_finder.rb10
-rw-r--r--app/services/discussions/resolve_service.rb8
-rw-r--r--app/services/issues/base_service.rb25
-rw-r--r--app/services/issues/build_service.rb21
-rw-r--r--app/services/issues/create_service.rb17
-rw-r--r--app/views/discussions/_new_issue_for_discussion.html.haml8
-rw-r--r--app/views/discussions/_notes.html.haml4
-rw-r--r--app/views/shared/icons/_icon_mr_issue.svg1
-rw-r--r--app/views/shared/issuable/_form.html.haml18
13 files changed, 160 insertions, 29 deletions
diff --git a/app/assets/javascripts/diff_notes/components/new_issue_for_discussion.js.es6 b/app/assets/javascripts/diff_notes/components/new_issue_for_discussion.js.es6
new file mode 100644
index 00000000000..e86bef47172
--- /dev/null
+++ b/app/assets/javascripts/diff_notes/components/new_issue_for_discussion.js.es6
@@ -0,0 +1,29 @@
+/* global Vue */
+/* global CommentsStore */
+
+(() => {
+ const NewIssueForDiscussion = Vue.extend({
+ props: {
+ discussionId: {
+ type: String,
+ required: true,
+ },
+ },
+ data() {
+ return {
+ discussions: CommentsStore.state,
+ };
+ },
+ computed: {
+ discussion() {
+ return this.discussions[this.discussionId];
+ },
+ showButton() {
+ if (this.discussion) return !this.discussion.isResolved();
+ return false;
+ },
+ },
+ });
+
+ Vue.component('new-issue-for-discussion-btn', NewIssueForDiscussion);
+})();
diff --git a/app/assets/javascripts/diff_notes/diff_notes_bundle.js b/app/assets/javascripts/diff_notes/diff_notes_bundle.js
index 7d8316dfd63..4f6b86a917c 100644
--- a/app/assets/javascripts/diff_notes/diff_notes_bundle.js
+++ b/app/assets/javascripts/diff_notes/diff_notes_bundle.js
@@ -14,10 +14,11 @@ require('./components/resolve_btn');
require('./components/resolve_count');
require('./components/resolve_discussion_btn');
require('./components/diff_note_avatars');
+require('./components/new_issue_for_discussion');
$(() => {
const projectPath = document.querySelector('.merge-request').dataset.projectPath;
- const COMPONENT_SELECTOR = 'resolve-btn, resolve-discussion-btn, jump-to-discussion, comment-and-resolve-btn';
+ const COMPONENT_SELECTOR = 'resolve-btn, resolve-discussion-btn, jump-to-discussion, comment-and-resolve-btn, new-issue-for-discussion-btn';
window.gl = window.gl || {};
window.gl.diffNoteApps = {};
diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss
index f984b469609..c2156a5ac69 100644
--- a/app/assets/stylesheets/pages/note_form.scss
+++ b/app/assets/stylesheets/pages/note_form.scss
@@ -178,8 +178,25 @@
padding-right: 5px;
}
- &:last-child {
- padding-left: 5px;
+ }
+
+ .discussion-actions {
+ display: table;
+
+ .new-issue-for-discussion path {
+ fill: $gray-darkest;
+ }
+
+ .btn-group {
+ display: table-cell;
+
+ &:first-child {
+ padding-right: 0;
+ }
+
+ &:first-child:not(:last-child) > div {
+ border-right: 0;
+ }
}
}
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 1151555b8fa..d7eb73d7ff5 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -64,7 +64,12 @@ class Projects::IssuesController < Projects::ApplicationController
params[:issue] ||= ActionController::Parameters.new(
assignee_id: ""
)
- build_params = issue_params.merge(merge_request_for_resolving_discussions: merge_request_for_resolving_discussions)
+
+ build_params = issue_params.merge(
+ merge_request_for_resolving_discussions: merge_request_for_resolving_discussions,
+ discussion_to_resolve: discussion_to_resolve
+ )
+
@issue = @noteable = Issues::BuildService.new(project, current_user, build_params).execute
respond_with(@issue)
@@ -94,10 +99,12 @@ class Projects::IssuesController < Projects::ApplicationController
end
def create
- create_params = issue_params
- .merge(merge_request_for_resolving_discussions: merge_request_for_resolving_discussions)
- .merge(spammable_params)
-
+ create_params = issue_params.
+ merge(
+ merge_request_for_resolving_discussions: merge_request_for_resolving_discussions,
+ discussion_to_resolve: discussion_to_resolve
+ ).
+ merge(spammable_params)
@issue = Issues::CreateService.new(project, current_user, create_params).execute
respond_to do |format|
@@ -193,6 +200,13 @@ class Projects::IssuesController < Projects::ApplicationController
find_by(iid: merge_request_iid)
end
+ def discussion_to_resolve
+ return unless discussion_id = params[:discussion_to_resolve]
+
+ @discussion_to_resolve ||= NotesFinder.new(project, current_user, discussion_id: discussion_id).
+ first_discussion
+ end
+
def authorize_read_issue!
return render_404 unless can?(current_user, :read_issue, @issue)
end
diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb
index 6630c6384f2..c93e6b05a67 100644
--- a/app/finders/notes_finder.rb
+++ b/app/finders/notes_finder.rb
@@ -11,6 +11,7 @@ class NotesFinder
# target_type: string
# target_id: integer
# last_fetched_at: time
+ # discussion_id: string
# search: string
#
def initialize(project, current_user, params = {})
@@ -22,9 +23,14 @@ class NotesFinder
def execute
@notes = since_fetch_at(@params[:last_fetched_at]) if @params[:last_fetched_at]
+ @notes = for_discussion(@params[:discussion_id]) if @params[:discussion_id]
@notes
end
+ def first_discussion
+ execute.discussions.first
+ end
+
private
def init_collection
@@ -100,4 +106,8 @@ class NotesFinder
@notes.where('updated_at > ?', last_fetched_at - FETCH_OVERLAP).fresh
end
+
+ def for_discussion(discussion_id)
+ @notes.where(Note.arel_table[:discussion_id].eq(discussion_id))
+ end
end
diff --git a/app/services/discussions/resolve_service.rb b/app/services/discussions/resolve_service.rb
index 0437195f588..4a803b47bba 100644
--- a/app/services/discussions/resolve_service.rb
+++ b/app/services/discussions/resolve_service.rb
@@ -9,7 +9,13 @@ module Discussions
discussion.resolve!(current_user)
- MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(merge_request)
+ notify_discussion_resolved(discussion)
+ end
+
+ def notify_discussion_resolved(discussion)
+ noteable = merge_request || discussion.noteable
+
+ MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(noteable)
SystemNoteService.discussion_continued_in_issue(discussion, project, current_user, follow_up_issue) if follow_up_issue
end
diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb
index 35af867a098..3aa842eb9a8 100644
--- a/app/services/issues/base_service.rb
+++ b/app/services/issues/base_service.rb
@@ -1,11 +1,12 @@
module Issues
class BaseService < ::IssuableBaseService
- attr_reader :merge_request_for_resolving_discussions
+ attr_reader :merge_request_for_resolving_discussions, :discussion_to_resolve
def initialize(*args)
super
@merge_request_for_resolving_discussions ||= params.delete(:merge_request_for_resolving_discussions)
+ @discussion_to_resolve ||= params.delete(:discussion_to_resolve)
end
def hook_data(issue, action)
@@ -15,6 +16,28 @@ module Issues
issue_data
end
+ def merge_request_for_resolving_discussions
+ @merge_request_for_resolving_discussions ||= discussion_to_resolve.try(:noteable)
+ end
+
+ def for_all_discussions_in_a_merge_request?
+ discussion_to_resolve.nil? && merge_request_for_resolving_discussions
+ end
+
+ def for_single_discussion?
+ discussion_to_resolve && discussion_to_resolve.noteable == merge_request_for_resolving_discussions
+ end
+
+ def discussions_to_resolve
+ @discussions_to_resolve ||= if for_all_discussions_in_a_merge_request?
+ merge_request_for_resolving_discussions.resolvable_discussions
+ elsif for_single_discussion?
+ Array(discussion_to_resolve)
+ else
+ []
+ end
+ end
+
private
def execute_hooks(issue, action = 'open')
diff --git a/app/services/issues/build_service.rb b/app/services/issues/build_service.rb
index 7cd927d8005..1bcc5abd3f2 100644
--- a/app/services/issues/build_service.rb
+++ b/app/services/issues/build_service.rb
@@ -4,32 +4,35 @@ module Issues
@issue = project.issues.new(issue_params)
end
- def issue_params_with_info_from_merge_request
+ def issue_params_with_info_from_discussions
return {} unless merge_request_for_resolving_discussions
- { title: title_from_merge_request, description: description_from_merge_request }
+ { title: title_for_merge_request, description: description_for_discussions }
end
- def title_from_merge_request
+ def title_for_merge_request
"Follow-up from \"#{merge_request_for_resolving_discussions.title}\""
end
- def description_from_merge_request
- if merge_request_for_resolving_discussions.resolvable_discussions.empty?
+ def description_for_discussions
+ if discussions_to_resolve.empty?
return "There are no unresolved discussions. "\
"Review the conversation in #{merge_request_for_resolving_discussions.to_reference}"
end
- description = "The following discussions from #{merge_request_for_resolving_discussions.to_reference} should be addressed:"
+ description = "The following #{'discussion'.pluralize(discussions_to_resolve.size)} "\
+ "from #{merge_request_for_resolving_discussions.to_reference} "\
+ "should be addressed:"
+
[description, *items_for_discussions].join("\n\n")
end
def items_for_discussions
- merge_request_for_resolving_discussions.resolvable_discussions.map { |discussion| item_for_discussion(discussion) }
+ discussions_to_resolve.map { |discussion| item_for_discussion(discussion) }
end
def item_for_discussion(discussion)
- first_note = discussion.first_note_to_resolve
+ first_note = discussion.first_note_to_resolve || discussion.first_note
other_note_count = discussion.notes.size - 1
creation_time = first_note.created_at.to_s(:medium)
note_url = Gitlab::UrlBuilder.build(first_note)
@@ -44,7 +47,7 @@ module Issues
end
def issue_params
- @issue_params ||= issue_params_with_info_from_merge_request.merge(whitelisted_issue_params)
+ @issue_params ||= issue_params_with_info_from_discussions.merge(whitelisted_issue_params)
end
def whitelisted_issue_params
diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb
index 85b6eb3fe3d..d2cc70ed0dc 100644
--- a/app/services/issues/create_service.rb
+++ b/app/services/issues/create_service.rb
@@ -5,7 +5,11 @@ module Issues
def execute
filter_spam_check_params
- issue_attributes = params.merge(merge_request_for_resolving_discussions: merge_request_for_resolving_discussions)
+ issue_attributes = params.merge(
+ merge_request_for_resolving_discussions: merge_request_for_resolving_discussions,
+ discussion_to_resolve: discussion_to_resolve
+ )
+
@issue = BuildService.new(project, current_user, issue_attributes).execute
create(@issue)
@@ -21,17 +25,16 @@ module Issues
notification_service.new_issue(issuable, current_user)
todo_service.new_issue(issuable, current_user)
user_agent_detail_service.create
-
- if merge_request_for_resolving_discussions.try(:discussions_can_be_resolved_by?, current_user)
- resolve_discussions_in_merge_request(issuable)
- end
+ resolve_discussions_with_issue(issuable)
end
- def resolve_discussions_in_merge_request(issue)
+ 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(merge_request_for_resolving_discussions.resolvable_discussions)
+ execute(discussions_to_resolve)
end
private
diff --git a/app/views/discussions/_new_issue_for_discussion.html.haml b/app/views/discussions/_new_issue_for_discussion.html.haml
new file mode 100644
index 00000000000..de28ded9820
--- /dev/null
+++ b/app/views/discussions/_new_issue_for_discussion.html.haml
@@ -0,0 +1,8 @@
+- if discussion.can_resolve?(current_user) && can?(current_user, :create_issue, @project)
+ %new-issue-for-discussion-btn{ ":discussion-id" => "'#{discussion.id}'",
+ "inline-template" => true }
+ .btn-group{ role: "group", "v-if" => "showButton" }
+ .btn.btn-default.discussion-create-issue-btn.has-tooltip{ title: "Resolve this discussion in a new issue",
+ "aria-label" => "Resolve this discussion in a new issue",
+ "data-container" => "body" }
+ = link_to custom_icon('icon_mr_issue'), new_namespace_project_issue_path(@project.namespace, @project, discussion_to_resolve: discussion.id), title: "Resolve this discussion in a new issue", class: 'new-issue-for-discussion'
diff --git a/app/views/discussions/_notes.html.haml b/app/views/discussions/_notes.html.haml
index dfdbdf1f969..4a1e93cec3a 100644
--- a/app/views/discussions/_notes.html.haml
+++ b/app/views/discussions/_notes.html.haml
@@ -11,6 +11,8 @@
= link_to_reply_discussion(discussion, line_type)
= render "discussions/resolve_all", discussion: discussion
- if discussion.for_merge_request?
- = render "discussions/jump_to_next", discussion: discussion
+ .btn-group.discussion-actions
+ = render "discussions/new_issue_for_discussion", discussion: discussion
+ = render "discussions/jump_to_next", discussion: discussion
- else
= link_to_reply_discussion(discussion)
diff --git a/app/views/shared/icons/_icon_mr_issue.svg b/app/views/shared/icons/_icon_mr_issue.svg
new file mode 100644
index 00000000000..ae219a3ded2
--- /dev/null
+++ b/app/views/shared/icons/_icon_mr_issue.svg
@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16"><g fill-rule="evenodd"><path d="m8.411 1.012c-.136-.008-.273-.012-.411-.012-3.866 0-7 3.134-7 7 0 3.866 3.134 7 7 7 3.866 0 7-3.134 7-7 0-.138-.004-.275-.012-.411-.464.201-.964.334-1.488.386 0 .008 0 .016 0 .025 0 3.038-2.462 5.5-5.5 5.5-3.038 0-5.5-2.462-5.5-5.5 0-3.038 2.462-5.5 5.5-5.5.008 0 .016 0 .025 0 .052-.524.185-1.024.386-1.488"/><path d="m12 2h-1.01c-.54 0-.991.448-.991 1 0 .556.444 1 .991 1h1.01v1.01c0 .54.448.991 1 .991.556 0 1-.444 1-.991v-1.01h1.01c.54 0 .991-.448.991-1 0-.556-.444-1-.991-1h-1.01v-1.01c0-.54-.448-.991-1-.991-.556 0-1 .444-1 .991v1.01m-5 4.01c0-.557.444-1.01 1-1.01.552 0 1 .443 1 1.01v1.981c0 .557-.444 1.01-1 1.01-.552 0-1-.443-1-1.01v-1.981m1 5.991c.552 0 1-.448 1-1 0-.552-.448-1-1-1-.552 0-1 .448-1 1 0 .552.448 1 1 1"/></g></svg> \ No newline at end of file
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 70470c83c51..e93fe0f6cb5 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -48,18 +48,32 @@
- if @merge_request_for_resolving_discussions
.form-group
.col-sm-10.col-sm-offset-2
+ = icon('exclamation-triangle')
- if @merge_request_for_resolving_discussions.discussions_can_be_resolved_by?(current_user)
- = icon('exclamation-triangle')
Creating this issue will mark all discussions in
= link_to @merge_request_for_resolving_discussions.to_reference, merge_request_path(@merge_request_for_resolving_discussions)
as resolved.
= hidden_field_tag 'merge_request_for_resolving_discussions', @merge_request_for_resolving_discussions.iid
- else
- = icon('exclamation-triangle')
You can't automatically mark all discussions in
= link_to @merge_request_for_resolving_discussions.to_reference, merge_request_path(@merge_request_for_resolving_discussions)
as resolved. Ask someone with sufficient rights to resolve the them.
+- if @discussion_to_resolve
+ .form-group
+ .col-sm-10.col-sm-offset-2
+ = icon('exclamation-triangle')
+ - if @discussion_to_resolve.can_resolve?(current_user)
+ Creating this issue will mark the discussion at
+ = link_to @discussion_to_resolve.noteable.to_reference, Gitlab::UrlBuilder.build(@discussion_to_resolve.first_note)
+ as resolved.
+ = hidden_field_tag 'discussion_to_resolve', @discussion_to_resolve.id
+ - else
+ You can't automatically mark the discussion at
+ = link_to @discussion_to_resolve.noteable.to_reference, Gitlab::UrlBuilder.build(@discussion_to_resolve.first_note)
+ as resolved. Ask someone with sufficient rights to resolve it.
+
+
- is_footer = !(issuable.is_a?(MergeRequest) && issuable.new_record?)
.row-content-block{ class: (is_footer ? "footer-block" : "middle-block") }
.pull-right