change logic on line_code

app/assets/javascripts/notes.js

@@ -71,6 +71,10 @@ var NoteList = {
     $(document).on("click",
       ".js-choose-note-attachment-button",
       NoteList.chooseNoteAttachment);
+
+    $(document).on("click",
+      ".js-show-outdated-discussion",
+      function(e) { $(this).next('.outdated-discussion').show(); e.preventDefault() });
   },
 
 

app/assets/stylesheets/sections/notes.scss

@@ -79,7 +79,7 @@ ul.notes {
       margin-top: 3px;
     }
     .attachment {
-      font-size: 16px;
+      font-size: 14px;
       margin-top: -20px;
 
       .icon-attachment {
@@ -92,7 +92,6 @@ ul.notes {
     }
     .note-body {
       margin-left: 45px;
-      padding-top: 5px;
     }
     .note-header {
       padding-bottom: 5px;
@@ -284,7 +283,7 @@ ul.notes {
       font-size: 24px;
       padding: 4px;
       position: absolute;
-      right: 0;
+      right: 10px;
     }
     .note_preview {
       background: #f5f5f5;
@@ -307,3 +306,9 @@ ul.notes {
 .notes-busy {
   margin: 18px;
 }
+
+.note-image-attach {
+  @extend .span4;
+  @extend .thumbnail;
+  margin-left: 45px;
+}

app/helpers/commits_helper.rb

@@ -9,11 +9,13 @@ module CommitsHelper
     end
   end
 
-  def build_line_anchor(index, line_new, line_old)
-    "#{index}_#{line_old}_#{line_new}"
+  def build_line_anchor(diff, line_new, line_old)
+    "#{hexdigest(diff.new_path)}_#{line_old}_#{line_new}"
   end
 
-  def each_diff_line(diff_arr, index)
+  def each_diff_line(diff, index)
+    diff_arr = diff.diff.lines.to_a
+
     line_old = 1
     line_new = 1
     type = nil
@@ -39,7 +41,7 @@ module CommitsHelper
         next
       else
         type = identification_type(line)
-        line_code = build_line_anchor(index, line_new, line_old)
+        line_code = build_line_anchor(diff, line_new, line_old)
         yield(full_line, type, line_code, line_new, line_old)
       end
 

app/helpers/notes_helper.rb

@@ -17,7 +17,7 @@ module NotesHelper
   end
 
   def link_to_merge_request_diff_line_note(note)
-    if note.for_merge_request_diff_line?
+    if note.for_merge_request_diff_line? and note.diff
       link_to "#{note.diff_file_name}:L#{note.diff_new_line}", diffs_project_merge_request_path(note.project, note.noteable_id, anchor: note.line_code)
     end
   end

app/models/note.rb

@@ -33,7 +33,7 @@ class Note < ActiveRecord::Base
   delegate :name, :email, to: :author, prefix: true
 
   validates :note, :project, presence: true
-  validates :line_code, format: { with: /\A\d+_\d+_\d+\Z/ }, allow_blank: true
+  validates :line_code, format: { with: /\A[a-z0-9]+_\d+_\d+\Z/ }, allow_blank: true
   validates :attachment, file_size: { maximum: 10.megabytes.to_i }
 
   validates :noteable_id, presence: true, if: ->(n) { n.noteable_type.present? && n.noteable_type != 'Commit' }
@@ -69,11 +69,17 @@ class Note < ActiveRecord::Base
   end
 
   def diff
-    noteable.diffs[diff_file_index]
+    if noteable.diffs.present?
+      noteable.diffs.select do |d|
+        if d.b_path
+          Digest::SHA1.hexdigest(d.b_path) == diff_file_index
+        end
+      end.first
+    end
   end
 
   def diff_file_index
-    line_code.split('_')[0].to_i
+    line_code.split('_')[0]
   end
 
   def diff_file_name
@@ -85,7 +91,7 @@ class Note < ActiveRecord::Base
   end
 
   def discussion_id
-    @discussion_id ||= [:discussion, noteable_type.underscore, noteable_id, line_code].join("-").to_sym
+    @discussion_id ||= [:discussion, noteable_type.try(:underscore), noteable_id, line_code].join("-").to_sym
   end
 
   # Returns true if this is a downvote note,

app/views/commits/_text_diff.html.haml

@@ -3,7 +3,7 @@
   %a.supp_diff_link Diff suppressed. Click to show
 
 %table{class: "#{'hide' if too_big}"}
-  - each_diff_line(diff.diff.lines.to_a, index) do |line, type, line_code, line_new, line_old|
+  - each_diff_line(diff, index) do |line, type, line_code, line_new, line_old|
     %tr.line_holder{ id: line_code }
       - if type == "match"
         %td.old_line= "..."

app/views/notes/_discussion.html.haml

@@ -12,8 +12,15 @@
     %div
       = link_to note.author_name, project_team_member_path(@project, @project.team_member_by_id(note.author)), class: "note-author"
       - if note.for_merge_request?
-        started a discussion on this merge request diff
-        = link_to_merge_request_diff_line_note(note)
+        - if note.diff
+          started a discussion on this merge request diff
+          = link_to_merge_request_diff_line_note(note)
+        - else
+          started
+          %strong
+            %i.icon-remove
+            outdated
+          discussion on this merge request diff
       - elsif note.for_commit?
         started a discussion on commit
         #{link_to note.noteable.short_id, project_commit_path(@project, note.noteable)}
@@ -29,8 +36,17 @@
         ago
   .discussion-body
     - if note.for_diff_line?
-      .content
-        .diff_file= render "notes/discussion_diff", discussion_notes: discussion_notes, note: note
+      - if note.diff
+        .content
+          .diff_file= render "notes/discussion_diff", discussion_notes: discussion_notes, note: note
+      - else
+        = link_to 'show outdated discussion', '#', class: 'js-show-outdated-discussion'
+        %div.hide.outdated-discussion
+          .content
+            .notes{ rel: discussion_notes.first.discussion_id }
+              = render discussion_notes
+
+
     - else
       .content
         .notes{ rel: discussion_notes.first.discussion_id }

app/views/notes/_discussion_diff.html.haml

@@ -9,7 +9,7 @@
   %br/
 .diff_file_content
   %table
-    - each_diff_line(diff.diff.lines.to_a, note.diff_file_index) do |line, type, line_code, line_new, line_old|
+    - each_diff_line(diff, note.diff_file_index) do |line, type, line_code, line_new, line_old|
       %tr.line_holder{ id: line_code }
         - if type == "match"
           %td.old_line= "..."

app/views/notes/_note.html.haml

@@ -6,8 +6,8 @@
         Link here
        
       - if(note.author_id == current_user.id) || can?(current_user, :admin_note, @project)
-        = link_to project_note_path(@project, note), title: "Remove comment", method: :delete, confirm: 'Are you sure?', remote: true, class: "danger js-note-delete" do
-          %i.icon-remove-circle
+        = link_to project_note_path(@project, note), title: "Remove comment", method: :delete, confirm: 'Are you sure you want to remove comment?', remote: true, class: "danger js-note-delete" do
+          %i.icon-trash.cred
     = image_tag gravatar_icon(note.author.email), class: "avatar s32"
     = link_to note.author_name, project_team_member_path(@project, @project.team_member_by_id(note.author)), class: "note-author"
     %span.note-last-update
@@ -29,7 +29,7 @@
       = markdown(note.note)
   - if note.attachment.url
     - if note.attachment.image?
-      = image_tag note.attachment.url, class: 'thumbnail span4'
+      = image_tag note.attachment.url, class: 'note-image-attach'
     .attachment.right
       = link_to note.attachment.url, target: "_blank" do
         %i.icon-attachment