500 on MR because a nil DiffNote's diff_line isn't accounted for

Zendesk (internal link): https://gitlab.zendesk.com/agent/tickets/99353

This customer is running 10.8.4-ee and is getting the following 500 error when trying to view a merge request:

Started GET "/<namespace>/<project>/merge_requests/6" for <IP> at 2018-06-27 08:42:49 -0400 
Processing by Projects::MergeRequestsController#show as HTML 
Parameters: {"namespace_id"=>"<namespace>", "project_id"=>"<project>", "id"=>"6"} 
Completed 500 Internal Server Error in 396ms (ActiveRecord: 44.8ms | Elasticsearch: 0.0ms)

ActionView::Template::Error (undefined method `index' for nil:NilClass): 
17: %table 
18: - if expanded 
19: - discussions = { discussion.original_line_code => [discussion] } 
20: = render partial: "projects/diffs/line", 
21: collection: discussion.truncated_diff_lines, 
22: as: :line, 
23: locals: { diff_file: diff_file, 
app/models/concerns/discussion_on_diff.rb:44:in `truncated_diff_lines' 
app/views/discussions/_diff_with_notes.html.haml:20:in `_app_views_discussions__diff_with_notes_html_haml___2271884178805127179_70054995643080' 
app/views/discussions/_discussion.html.haml:52:in `_app_views_discussions__discussion_html_haml__1728541228339610768_70054930286920' 
app/views/shared/notes/_notes.html.haml:6:in `block in _app_views_shared_notes__notes_html_haml___1948692214257223412_70054893354300' 
app/views/shared/notes/_notes.html.haml:2:in `each' 
app/views/shared/notes/_notes.html.haml:2:in `_app_views_shared_notes__notes_html_haml___1948692214257223412_70054893354300' 
app/views/shared/notes/_notes_with_form.html.haml:6:in `_app_views_shared_notes__notes_with_form_html_haml___3230466369440081720_70054891283080' 
app/views/projects/merge_requests/_discussion.html.haml:11:in `_app_views_projects_merge_requests__discussion_html_haml__4544443769467627885_70054887719340' 
app/views/projects/merge_requests/show.html.haml:92:in `_app_views_projects_merge_requests_show_html_haml___1445104828519208308_70054914939540' 
app/controllers/projects/merge_requests_controller.rb:56:in `block (2 levels) in show' 
app/controllers/projects/merge_requests_controller.rb:41:in `show' 
lib/gitlab/i18n.rb:50:in `with_locale' 
lib/gitlab/i18n.rb:56:in `with_user_locale' 
app/controllers/application_controller.rb:363:in `set_locale' 
lib/gitlab/middleware/multipart.rb:95:in `call' 
lib/gitlab/request_profiler/middleware.rb:14:in `call' 
ee/lib/gitlab/jira/middleware.rb:15:in `call' 
lib/gitlab/middleware/go.rb:17:in `call' 
lib/gitlab/etag_caching/middleware.rb:11:in `call' 
lib/gitlab/middleware/read_only/controller.rb:28:in `call' 
lib/gitlab/middleware/read_only.rb:16:in `call' 
lib/gitlab/request_context.rb:18:in `call' 
lib/gitlab/metrics/requests_rack_middleware.rb:27:in `call' 
lib/gitlab/middleware/release_env.rb:10:in `call'

It looks like it's failing on this line, where diff_line is nil so index can't be called on it.

In the rails console, we confirmed the MR has 10 discussions, none of which are a LegacyDiffDiscussion. However, there are a few DiffNote objects:

irb(main):004:0* mr.discussions.each{|d| d.notes.map{|n| puts "#{n.class}: #{n.id}" }}; nil 
Note: 633205 
Note: 633207 
Note: 633213 
Note: 633339 
DiffNote: 633397 
DiffNote: 633400 
DiffNote: 633401 
DiffNote: 633402 
Note: 633403 
Note: 633816 
=> nil 

...and they're all missing diff_line entries:

irb(main):019:0* diff_note = DiffNote.find(633397); diff_note.diff_line 
=> nil 
irb(main):020:0> diff_note = DiffNote.find(633400); diff_note.diff_line 
=> nil 
irb(main):021:0> diff_note = DiffNote.find(633401); diff_note.diff_line 
=> nil 
irb(main):022:0> diff_note = DiffNote.find(633402); diff_note.diff_line 
=> nil

Earlier in the method, here, we catch it if it's nil AND comes from a LegacyDiffNote object, but don't account for when it's nil and comes from a DiffNote object. This looks like a bug.

Please see the ticket (an internal link) for more: https://gitlab.zendesk.com/agent/tickets/99353

Edited by Harish Ramachandran