From f7e7dc7ebbdac2a45d528d1f9c19055febd0bede Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Wed, 25 Dec 2013 13:32:43 +0200
Subject: [PATCH] Make note anchors actually work
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/assets/javascripts/notes.js | 3 -
app/controllers/projects/commit_controller.rb | 3 +-
app/controllers/projects/issues_controller.rb | 3 +-
.../projects/merge_requests_controller.rb | 7 +-
app/controllers/projects/notes_controller.rb | 34 +--------
.../projects/snippets_controller.rb | 3 +-
app/models/note.rb | 69 ++++++++++++-------
app/views/projects/notes/_notes.html.haml | 2 +-
.../projects/notes/_notes_with_form.html.haml | 1 +
9 files changed, 59 insertions(+), 66 deletions(-)
diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index e39bfc0f792..33f3c3a921e 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 242aa41182d..34cd1341f78 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 e7b4c837ae3..81f21fa0c59 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 d644026b2b2..c542f31f006 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 5ff5c5b7d96..1044e5cb3f9 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 dd0c1a57089..4db0bc34b53 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 b23f7df7742..48b36bcafdf 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 ac8901fe704..ca60dd239b2 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 ac28c7432ef..133f2e0541b 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
--
GitLab