Skip to content

Gracefully handle notes on deleted commits in merge requests

Stan Hu requested to merge stanhu/gitlab-ce:fix-mr-comment-commit-pruned into master

Here's how to reproduce:

  1. Create a new MR with commit A
  2. Go directly to the commit A (not within the MR) and write a comment
  3. Force push to the branch of this MR
  4. In the repo directory, run git gc --prune=all
  5. Visit the MR

An Error 500 results:

Completed 500 Internal Server Error in 850ms (ActiveRecord: 15.9ms)

ActionView::Template::Error (undefined method `short_id' for nil:NilClass):
     8:     %div
     9:       = link_to_member(@project, note.author, avatar: false)
    10:       started a discussion on commit
    11:       = link_to(note.noteable.short_id, namespace_project_commit_path(note.project.namespace, note.project, note.noteable), class: 'monospace')
    12:     .last-update.hide.js-toggle-content
    13:       - last_note = discussion_notes.last
    14:       last updated by
  app/views/projects/notes/discussions/_commit.html.haml:11:in `_app_views_projects_notes_discussions__commit_html_haml__1803003857195861105_49071460'
  app/views/projects/notes/_discussion.html.haml:13:in `_app_views_projects_notes__discussion_html_haml___1504502595006611869_46489940'
  app/views/projects/notes/_notes.html.haml:9:in `block in _app_views_projects_notes__notes_html_haml___2999529581037780893_46368000'
  app/views/projects/notes/_notes.html.haml:2:in `each'
  app/views/projects/notes/_notes.html.haml:2:in `_app_views_projects_notes__notes_html_haml___2999529581037780893_46368000'
  app/views/projects/notes/_notes_with_form.html.haml:2:in `_app_views_projects_notes__notes_with_form_html_haml___148775219672431171_46764600'
  app/views/projects/merge_requests/_discussion.html.haml:8:in `_app_views_projects_merge_requests__discussion_html_haml___3124216701411407902_46913820'
  app/views/projects/merge_requests/_show.html.haml:77:in `_app_views_projects_merge_requests__show_html_haml__2349524690606977109_52608360'
  app/views/projects/merge_requests/show.html.haml:1:in `_app_views_projects_merge_requests_show_html_haml___859080177316653739_44352240'
  app/controllers/projects/merge_requests_controller.rb:57:in `show'
  lib/gitlab/middleware/go.rb:16:in `call'

Now if a comment is listed for a deleted comment, the note will look like:

image

Closes #3250 (closed)

Merge request reports