Error 500 when rendering diffs from forks

This is likely a regression caused by https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10602. It looks like the helper is not being passed a path when the diff controller is in use.

We are reverting this from 9.1 RC6. I'm not quite sure why that MR was labeled with Pick into Stable.

https://sentry.gitlap.com/gitlab/gitlabcom/issues/28035/

TypeError: no implicit conversion of nil into String
  app/helpers/tree_helper.rb:34:in `join'
    File.join(*args)
  app/helpers/tree_helper.rb:34:in `tree_join'
    File.join(*args)
  app/helpers/blob_helper.rb:13:in `edit_path'
    tree_join(ref, path),
  app/helpers/blob_helper.rb:32:in `edit_blob_link'
    to: edit_path,
  app/views/projects/diffs/_file.html.haml:15:in `_app_views_projects_diffs__file_html_haml___1121649036413133914_70307416411280'
    = edit_blob_link(@merge_request.source_project, @merge_request.source_branch, diff_file.new_path,
...
(159 additional frame(s) were not displayed)

ActionView::Template::Error: no implicit conversion of nil into String

ActionView::Template::Error: no implicit conversion of nil into String

To reproduce this problem:

1. Fork project gitlab-org/gitlab-ce (or any project)
2. Submit a MR from your fork to the main project
3. Sign in as a different user who does NOT have access to that fork
4. View the MR

added regression label

added frontend label

Example failing URLs:

• https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10384/diffs
• https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/422/diffs

I updated the description to describe how to reproduce the problem.

!10602 (merged) has been reverted from 9.1.0.

/cc: @MadLittleMods, @filipa

assigned to @MadLittleMods

mentioned in commit 8b3ab912

It's probably labeled with Pick into Stable because the original issue was labeled with regression and Next Patch Release with milestone set as %9.1 #30637 (closed)

I am not sure the status of that.

@DouweM ^ Since you opened the original issue: #30637 (closed), do you know after we reverted !10602 (merged) in 9.1 stable branch, would the issue come back? If so we might need another MR targets stable branch directly, or just ignore it?

@MadLittleMods Do we need to pick the patch you're working on into 9.1 stable branch?

@godfat I think it is safe to ignore. It is a consistency issue vs a functional issue.

In case we do want to merge, fix is ready timeline update: the fix itself is ready and I am working on adding some tests for the missing case that caused us to revert, https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10868/

mentioned in commit 3a12ce70

mentioned in merge request !10868 (merged)

mentioned in commit 2c0afc97

mentioned in commit a27b59f6

closed via merge request !10868 (merged)

mentioned in commit 9d2ca3f5

mentioned in commit nick.thomas/gitlab-ee@0748960b

mentioned in merge request !14785