diff options
author | Douwe Maan <douwe@selenight.nl> | 2017-03-10 04:29:11 +0300 |
---|---|---|
committer | Luke "Jared" Bennett <lbennett@gitlab.com> | 2017-04-05 19:44:14 +0300 |
commit | 08bbb9fce66cb46d3262e6cd4c4379b59f065be0 (patch) | |
tree | 159eeb7ca43419f29926d6e77637db18bddd20a9 /app/controllers | |
parent | 8bdfee8ba5fb0a8f48501e63274c8f9ce5708007 (diff) |
Add option to start a new discussion on an MR
Diffstat (limited to 'app/controllers')
-rw-r--r-- | app/controllers/projects/commit_controller.rb | 20 | ||||
-rw-r--r-- | app/controllers/projects/discussions_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/projects/issues_controller.rb | 10 | ||||
-rwxr-xr-x | app/controllers/projects/merge_requests_controller.rb | 30 | ||||
-rw-r--r-- | app/controllers/projects/notes_controller.rb | 86 | ||||
-rw-r--r-- | app/controllers/projects/snippets_controller.rb | 5 |
6 files changed, 70 insertions, 83 deletions
diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index cc67f688d51..98cd4ce344a 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -2,6 +2,7 @@ # # Not to be confused with CommitsController, plural. class Projects::CommitController < Projects::ApplicationController + include NotesHelper include CreatesCommit include DiffForPath include DiffHelper @@ -111,22 +112,19 @@ class Projects::CommitController < Projects::ApplicationController end def define_note_vars - @grouped_diff_discussions = commit.notes.grouped_diff_discussions - @notes = commit.notes.non_diff_notes.fresh - - Banzai::NoteRenderer.render( - @grouped_diff_discussions.values.flat_map(&:notes) + @notes, - @project, - current_user, - ) - + @noteable = @commit @note = @project.build_commit_note(commit) - @noteable = @commit - @comments_target = { + @new_diff_note_attrs = { noteable_type: 'Commit', commit_id: @commit.id } + + @grouped_diff_discussions = commit.grouped_diff_discussions + @discussions = commit.discussions + + @notes = (@grouped_diff_discussions.values + @discussions).flat_map(&:notes) + @notes = prepare_notes_for_rendering(@notes) end def assign_change_commit_vars diff --git a/app/controllers/projects/discussions_controller.rb b/app/controllers/projects/discussions_controller.rb index 1349b015a63..f4a18a5e8f7 100644 --- a/app/controllers/projects/discussions_controller.rb +++ b/app/controllers/projects/discussions_controller.rb @@ -28,7 +28,7 @@ class Projects::DiscussionsController < Projects::ApplicationController end def discussion - @discussion ||= @merge_request.find_diff_discussion(params[:id]) || render_404 + @discussion ||= @merge_request.find_discussion(params[:id]) || render_404 end def authorize_resolve_discussion! diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index d984e6d3918..0c6267f065b 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -84,15 +84,11 @@ class Projects::IssuesController < Projects::ApplicationController end def show - raw_notes = @issue.notes.inc_relations_for_view.fresh - - @notes = Banzai::NoteRenderer. - render(raw_notes, @project, current_user, @path, @project_wiki, @ref) - - @note = @project.notes.new(noteable: @issue) @noteable = @issue + @note = @project.notes.new(noteable: @issue) - preload_max_access_for_authors(@notes, @project) + @discussions = @issue.discussions + @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes)) respond_to do |format| format.html diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 37e3ac05916..7863fe18f32 100755 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -570,20 +570,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @note = @project.notes.new(noteable: @merge_request) @discussions = @merge_request.discussions - - preload_noteable_for_regular_notes(@discussions.flat_map(&:notes)) - - # This is not executed lazily - @notes = Banzai::NoteRenderer.render( - @discussions.flat_map(&:notes), - @project, - current_user, - @path, - @project_wiki, - @ref - ) - - preload_max_access_for_authors(@notes, @project) + @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes)) end def define_widget_vars @@ -596,22 +583,15 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def define_diff_comment_vars - @comments_target = { + @new_diff_note_attrs = { noteable_type: 'MergeRequest', noteable_id: @merge_request.id } @use_legacy_diff_notes = !@merge_request.has_complete_diff_refs? - @grouped_diff_discussions = @merge_request.notes.inc_relations_for_view.grouped_diff_discussions - - Banzai::NoteRenderer.render( - @grouped_diff_discussions.values.flat_map(&:notes), - @project, - current_user, - @path, - @project_wiki, - @ref - ) + + @grouped_diff_discussions = @merge_request.grouped_diff_discussions + @notes = prepare_notes_for_rendering(@grouped_diff_discussions.values.flat_map(&:notes)) end def define_pipelines_vars diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index d00177e7612..f80313bbee0 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -6,13 +6,14 @@ class Projects::NotesController < Projects::ApplicationController before_action :authorize_create_note!, only: [:create] before_action :authorize_admin_note!, only: [:update, :destroy] before_action :authorize_resolve_note!, only: [:resolve, :unresolve] - before_action :find_current_user_notes, only: [:index] def index current_fetched_at = Time.now.to_i notes_json = { notes: [], last_fetched_at: current_fetched_at } + @notes = notes_finder.execute.inc_author + @notes.each do |note| next if note.cross_reference_not_visible_for?(current_user) @@ -23,7 +24,11 @@ class Projects::NotesController < Projects::ApplicationController end def create - create_params = note_params.merge(merge_request_diff_head_sha: params[:merge_request_diff_head_sha]) + create_params = note_params.merge( + merge_request_diff_head_sha: params[:merge_request_diff_head_sha], + in_reply_to_discussion_id: params[:in_reply_to_discussion_id], + new_discussion: params[:new_discussion], + ) @note = Notes::CreateService.new(project, current_user, create_params).execute if @note.is_a?(Note) @@ -111,6 +116,17 @@ class Projects::NotesController < Projects::ApplicationController ) end + def discussion_html(discussion) + return if discussion.render_as_individual_notes? + + render_to_string( + "discussions/_discussion", + layout: false, + formats: [:html], + locals: { discussion: discussion } + ) + end + def diff_discussion_html(discussion) return unless discussion.diff_discussion? @@ -135,17 +151,6 @@ class Projects::NotesController < Projects::ApplicationController ) end - def discussion_html(discussion) - return unless discussion.diff_discussion? - - render_to_string( - "discussions/_discussion", - layout: false, - formats: [:html], - locals: { discussion: discussion } - ) - end - def note_json(note) attrs = { id: note.id @@ -156,33 +161,22 @@ class Projects::NotesController < Projects::ApplicationController attrs.merge!( valid: true, - discussion_id: note.discussion_id, + discussion_id: note.discussion_id(noteable), html: note_html(note), note: note.note ) - if note.diff_note? - discussion = note.to_discussion - + discussion = note.to_discussion(noteable) + unless discussion.render_as_individual_notes? attrs.merge!( diff_discussion_html: diff_discussion_html(discussion), - discussion_html: discussion_html(discussion) - ) + discussion_html: discussion_html(discussion), - # The discussion_id is used to add the comment to the correct discussion - # element on the merge request page. Among other things, the discussion_id - # contains the sha of head commit of the merge request. - # When new commits are pushed into the merge request after the initial - # load of the merge request page, the discussion elements will still have - # the old discussion_ids, with the old head commit sha. The new comment, - # however, will have the new discussion_id with the new commit sha. - # To ensure that these new comments will still end up in the correct - # discussion element, we also send the original discussion_id, with the - # old commit sha, along, and fall back on this value when no discussion - # element with the new discussion_id could be found. - if note.new_diff_note? && note.position != note.original_position - attrs[:original_discussion_id] = note.original_discussion_id - end + # Since the `discussion_id` can change, for example when new commits are pushed into an MR, + # the never-changing `original_discussion_id` is used as a fallback to the find the relevant + # discussion container to add this note to. + original_discussion_id: note.original_discussion_id + ) end else attrs.merge!( @@ -205,14 +199,30 @@ class Projects::NotesController < Projects::ApplicationController def note_params params.require(:note).permit( - :note, :noteable, :noteable_id, :noteable_type, :project_id, - :attachment, :line_code, :commit_id, :type, :position + :project_id, + :noteable_type, + :noteable_id, + :commit_id, + :noteable, + :type, + + :note, + :attachment, + + # LegacyDiffNote + :line_code, + + # DiffNote + :position ) end - def find_current_user_notes - @notes = NotesFinder.new(project, current_user, params.merge(last_fetched_at: last_fetched_at)) - .execute.inc_author + def notes_finder + @notes_finder ||= NotesFinder.new(project, current_user, params.merge(last_fetched_at: last_fetched_at)) + end + + def noteable + @noteable ||= notes_finder.target end def last_fetched_at diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb index ea1a97b7cf0..2600a9d7256 100644 --- a/app/controllers/projects/snippets_controller.rb +++ b/app/controllers/projects/snippets_controller.rb @@ -1,4 +1,5 @@ class Projects::SnippetsController < Projects::ApplicationController + include NotesHelper include ToggleAwardEmoji include SpammableActions include SnippetsActions @@ -55,8 +56,10 @@ class Projects::SnippetsController < Projects::ApplicationController def show @note = @project.notes.new(noteable: @snippet) - @notes = Banzai::NoteRenderer.render(@snippet.notes.fresh, @project, current_user) @noteable = @snippet + + @discussions = @snippet.discussions + @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes)) end def destroy |