diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index e39bfc0f7924950ad03429d7ea4a1c32123664a6..33f3c3a921e9ae96bba53623e60459b8b70d5f88 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -13,9 +13,6 @@ var NoteList = { NoteList.setupMainTargetNoteForm(); - // get initial set of notes - NoteList.getContent(); - // Unbind events to prevent firing twice $(document).off("click", ".js-add-diff-note-button"); $(document).off("click", ".js-discussion-reply-button"); diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 242aa41182d6e99505251f9cf40381f5edc3ff46..34cd1341f7872aca489578e13ce3b2c872202223 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -24,7 +24,8 @@ class Projects::CommitController < Projects::ApplicationController @line_notes = result[:line_notes] @branches = result[:branches] @notes_count = result[:notes_count] - @target_type = :commit + @target_type = "Commit" + @notes = project.notes.for_commit_id(@commit.id).not_inline.fresh @target_id = @commit.id @comments_allowed = @reply_allowed = true diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index e7b4c837ae3612c41c007c63a9bcfac2cc4a9e6d..81f21fa0c599f05c4f4878e95465e3ce4eab42e1 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -49,7 +49,8 @@ class Projects::IssuesController < Projects::ApplicationController def show @note = @project.notes.new(noteable: @issue) - @target_type = :issue + @notes = @issue.notes.inc_author.fresh + @target_type = 'Issue' @target_id = @issue.id respond_with(@issue) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index d644026b2b21ebd9dbfea4723bbb284d0949793d..c542f31f006928f4709c3ca0c840b39566accbaa 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -198,6 +198,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController def define_show_vars # Build a note object for comment form @note = @project.notes.new(noteable: @merge_request) + @notes = @merge_request.mr_and_commit_notes.inc_author.fresh + @discussions = Note.discussions_from_notes(@notes) + @target_type = 'MergeRequest' + @target_id = @merge_request.id # Get commits from repository # or from cache if already merged @@ -205,9 +209,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController @allowed_to_merge = allowed_to_merge? @show_merge_controls = @merge_request.opened? && @commits.any? && @allowed_to_merge - - @target_type = :merge_request - @target_id = @merge_request.id end def allowed_to_merge? diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 5ff5c5b7d96d9e671d8ef5c4062562b5d1cfa36d..1044e5cb3f976420040dafbdaff676f6d47de866 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -11,7 +11,7 @@ class Projects::NotesController < Projects::ApplicationController @target_id = params[:target_id] if params[:target_type] == "merge_request" - @discussions = discussions_from_notes + @discussions = Note.discussions_from_notes(@notes) end respond_to do |format| @@ -76,36 +76,4 @@ class Projects::NotesController < Projects::ApplicationController def preview render text: view_context.markdown(params[:note]) end - - protected - - def discussion_notes_for(note) - @notes.select do |other_note| - note.discussion_id == other_note.discussion_id - end - end - - def discussions_from_notes - discussion_ids = [] - discussions = [] - - @notes.each do |note| - next if discussion_ids.include?(note.discussion_id) - - # don't group notes for the main target - if note_for_main_target?(note) - discussions << [note] - else - discussions << discussion_notes_for(note) - discussion_ids << note.discussion_id - end - end - - discussions - end - - # Helps to distinguish e.g. commit notes in mr notes list - def note_for_main_target?(note) - (@target_type.camelize == note.noteable_type && !note.for_diff_line?) - end end diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb index dd0c1a57089e51ae8f28e1b7e968ffccac14454d..4db0bc34b536c444e63b51b0fc1213d7b2e2c2a9 100644 --- a/app/controllers/projects/snippets_controller.rb +++ b/app/controllers/projects/snippets_controller.rb @@ -48,7 +48,8 @@ class Projects::SnippetsController < Projects::ApplicationController def show @note = @project.notes.new(noteable: @snippet) - @target_type = :snippet + @notes = @snippet.notes.fresh + @target_type = 'Snippet' @target_id = @snippet.id end diff --git a/app/models/note.rb b/app/models/note.rb index b23f7df7742de5aeac39f73b9baee704977e705b..48b36bcafdf3b2eb10b3a5fc80a414742aaa9177 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -56,29 +56,52 @@ class Note < ActiveRecord::Base serialize :st_diff before_create :set_diff, if: ->(n) { n.line_code.present? } - def self.create_status_change_note(noteable, project, author, status, source) - body = "_Status changed to #{status}#{' by ' + source.gfm_reference if source}_" - - create({ - noteable: noteable, - project: project, - author: author, - note: body, - system: true - }, without_protection: true) - end + class << self + def create_status_change_note(noteable, project, author, status, source) + body = "_Status changed to #{status}#{' by ' + source.gfm_reference if source}_" + + create({ + noteable: noteable, + project: project, + author: author, + note: body, + system: true + }, without_protection: true) + end + + # +noteable+ was referenced from +mentioner+, by including GFM in either +mentioner+'s description or an associated Note. + # Create a system Note associated with +noteable+ with a GFM back-reference to +mentioner+. + def create_cross_reference_note(noteable, mentioner, author, project) + create({ + noteable: noteable, + commit_id: (noteable.sha if noteable.respond_to? :sha), + project: project, + author: author, + note: "_mentioned in #{mentioner.gfm_reference}_", + system: true + }, without_protection: true) + end - # +noteable+ was referenced from +mentioner+, by including GFM in either +mentioner+'s description or an associated Note. - # Create a system Note associated with +noteable+ with a GFM back-reference to +mentioner+. - def self.create_cross_reference_note(noteable, mentioner, author, project) - create({ - noteable: noteable, - commit_id: (noteable.sha if noteable.respond_to? :sha), - project: project, - author: author, - note: "_mentioned in #{mentioner.gfm_reference}_", - system: true - }, without_protection: true) + def discussions_from_notes(notes) + discussion_ids = [] + discussions = [] + + notes.each do |note| + next if discussion_ids.include?(note.discussion_id) + + # don't group notes for the main target + if !note.for_diff_line? && note.noteable_type == "MergeRequest" + discussions << [note] + else + discussions << notes.select do |other_note| + note.discussion_id == other_note.discussion_id + end + discussion_ids << note.discussion_id + end + end + + discussions + end end # Determine whether or not a cross-reference note already exists. @@ -89,7 +112,7 @@ class Note < ActiveRecord::Base def commit_author @commit_author ||= project.users.find_by_email(noteable.author_email) || - project.users.find_by_name(noteable.author_name) + project.users.find_by_name(noteable.author_name) rescue nil end diff --git a/app/views/projects/notes/_notes.html.haml b/app/views/projects/notes/_notes.html.haml index ac8901fe70458bcdbf3bd1cb5203792a05873de1..ca60dd239b234033e6106d5466ce86689ba83d52 100644 --- a/app/views/projects/notes/_notes.html.haml +++ b/app/views/projects/notes/_notes.html.haml @@ -4,7 +4,7 @@ - if note_for_main_target?(note) = render discussion_notes - else - = render 'discussion', discussion_notes: discussion_notes + = render 'projects/notes/discussion', discussion_notes: discussion_notes - else - @notes.each do |note| - next unless note.author diff --git a/app/views/projects/notes/_notes_with_form.html.haml b/app/views/projects/notes/_notes_with_form.html.haml index ac28c7432ef98eda09ad0b7971a268c448bd1e59..133f2e0541bba50c5905c712ea969c620702a189 100644 --- a/app/views/projects/notes/_notes_with_form.html.haml +++ b/app/views/projects/notes/_notes_with_form.html.haml @@ -1,4 +1,5 @@ %ul#notes-list.notes + = render "projects/notes/notes" .js-notes-busy .js-main-target-form