From 3f72af9994554c66a51cdfb4302e48da0edd043a Mon Sep 17 00:00:00 2001 From: Riyad Preukschas Date: Wed, 10 Oct 2012 12:06:30 +0200 Subject: [PATCH 1/3] Make notes for merge requests include commit notes and add helpers --- app/contexts/notes/load_context.rb | 2 +- app/controllers/notes_controller.rb | 5 +++++ app/helpers/notes_helper.rb | 5 +++++ app/models/note.rb | 6 +++++- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/app/contexts/notes/load_context.rb b/app/contexts/notes/load_context.rb index f92a7801..f3949149 100644 --- a/app/contexts/notes/load_context.rb +++ b/app/contexts/notes/load_context.rb @@ -13,7 +13,7 @@ module Notes when "issue" project.issues.find(target_id).notes.inc_author.fresh.limit(20) when "merge_request" - project.merge_requests.find(target_id).notes.inc_author.fresh.limit(20) + project.merge_requests.find(target_id).mr_and_commit_notes.inc_author.fresh.limit(20) when "snippet" project.snippets.find(target_id).notes.fresh when "wall" diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index 7f5f5cd2..d794f368 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -7,6 +7,11 @@ class NotesController < ProjectResourceController def index notes + if params[:target_type] == "merge_request" + @mixed_targets = true + @main_target_type = params[:target_type].camelize + end + respond_with(@notes) end diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 65389e38..3e875023 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -7,6 +7,11 @@ module NotesHelper params[:loading_new].present? end + # Helps to distinguish e.g. commit notes in mr notes list + def note_for_main_target?(note) + !@mixed_targets || @main_target_type == note.noteable_type + end + def note_vote_class(note) if note.upvote? "vote upvote" diff --git a/app/models/note.rb b/app/models/note.rb index 65b20fe0..ae51e486 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -49,7 +49,7 @@ class Note < ActiveRecord::Base end def target - if noteable_type == "Commit" + if commit? project.commit(noteable_id) else noteable @@ -82,6 +82,10 @@ class Note < ActiveRecord::Base noteable_type == "Commit" end + def line_note? + line_code.present? + end + def commit_author @commit_author ||= project.users.find_by_email(target.author_email) || From fb0279f3113f58b1cbdbe04acabe874ac4d231f9 Mon Sep 17 00:00:00 2001 From: Riyad Preukschas Date: Wed, 10 Oct 2012 12:09:45 +0200 Subject: [PATCH 2/3] Fix vote counting to only count main target notes (not mixed in ones) --- app/assets/javascripts/notes.js | 2 +- app/helpers/notes_helper.rb | 6 ------ app/views/notes/_note.html.haml | 23 ++++++++++++++--------- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index e1ad1d2f..558643d5 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -230,7 +230,7 @@ var NoteList = { updateVotes: function() { var votes = $("#votes .votes"); - var notes = $("#notes-list, #new-notes-list").find(".note.vote"); + var notes = $("#notes-list, #new-notes-list").find(".note .vote"); // only update if there is a vote display if (votes.size()) { diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 3e875023..ba02ca6b 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -12,11 +12,5 @@ module NotesHelper !@mixed_targets || @main_target_type == note.noteable_type end - def note_vote_class(note) - if note.upvote? - "vote upvote" - elsif note.downvote? - "vote downvote" - end end end diff --git a/app/views/notes/_note.html.haml b/app/views/notes/_note.html.haml index 5234e55d..57946163 100644 --- a/app/views/notes/_note.html.haml +++ b/app/views/notes/_note.html.haml @@ -1,4 +1,4 @@ -%li{id: dom_id(note), class: "note #{note_vote_class(note)}"} +%li{id: dom_id(note), class: "note"} = image_tag gravatar_icon(note.author.email), class: "avatar s32" %div.note-author %strong= note.author_name @@ -6,14 +6,19 @@ %cite.cgray = time_ago_in_words(note.updated_at) ago - - if note.upvote? - %span.label.label-success - %i.icon-thumbs-up - \+1 - - if note.downvote? - %span.label.label-error - %i.icon-thumbs-down - \-1 + + -# only show vote if it's a note for the main target + - if note_for_main_target?(note) + - if note.upvote? + %span.vote.upvote.label.label-success + %i.icon-thumbs-up + \+1 + - if note.downvote? + %span.vote.downvote.label.label-error + %i.icon-thumbs-down + \-1 + + -# remove button - if(note.author_id == current_user.id) || can?(current_user, :admin_note, @project) = link_to [@project, note], confirm: 'Are you sure?', method: :delete, remote: true, class: "cred delete-note btn very_small" do %i.icon-trash From 81ee69381d9e01997a204b89cdeabf9854138967 Mon Sep 17 00:00:00 2001 From: Riyad Preukschas Date: Wed, 10 Oct 2012 12:14:48 +0200 Subject: [PATCH 3/3] Add links to the note source if the note is mixed in --- app/helpers/notes_helper.rb | 10 ++++++++++ app/views/commits/_text_file.html.haml | 2 +- app/views/notes/_note.html.haml | 6 ++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index ba02ca6b..99db8b6f 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -12,5 +12,15 @@ module NotesHelper !@mixed_targets || @main_target_type == note.noteable_type end + def link_to_commit_diff_line_note(note) + return unless note.line_note? + + commit = note.target + diff_index, diff_old_line, diff_new_line = note.line_code.split('_') + + link_file = commit.diffs[diff_index.to_i].new_path + link_line = diff_new_line + + link_to "#{link_file}:L#{link_line}", project_commit_path(@project, commit, anchor: note.line_code) end end diff --git a/app/views/commits/_text_file.html.haml b/app/views/commits/_text_file.html.haml index 9f5b5345..02117386 100644 --- a/app/views/commits/_text_file.html.haml +++ b/app/views/commits/_text_file.html.haml @@ -4,7 +4,7 @@ %table{class: "#{'hide' if too_big}"} - each_diff_line(diff.diff.lines.to_a, index) do |line, type, line_code, line_new, line_old| - %tr.line_holder + %tr.line_holder{ id: line_code } - if type == "match" %td.old_line= "..." %td.new_line= "..." diff --git a/app/views/notes/_note.html.haml b/app/views/notes/_note.html.haml index 57946163..0901e4b8 100644 --- a/app/views/notes/_note.html.haml +++ b/app/views/notes/_note.html.haml @@ -7,6 +7,12 @@ = time_ago_in_words(note.updated_at) ago + - unless note_for_main_target?(note) + - if note.commit? + %span.cgray + on #{link_to note.target.short_id, project_commit_path(@project, note.target)} + = link_to_commit_diff_line_note(note) if note.line_note? + -# only show vote if it's a note for the main target - if note_for_main_target?(note) - if note.upvote?