Raw diff and rendered diff produce different results

Zendesk: https://gitlab.zendesk.com/agent/tickets/18305

The raw diff endpoint (merge_requests/####.diff) produces a diff with lots more changes than the regular merge request diffs tab. Maybe this is simply a lack of understanding about the differences between the two, but it seems like a bug.

Possibly related, and interesting, is that merge_requests/####.patch yields yet a third result.

I used !3388 (merged) as a test case.

.diff URL

Regular diff view

.patch URL

nothing is generated

cc/ @zj @brodock Can you explain or see what's going on here, please?

Is this a duplicate of #3017 (closed)?

@stanhu Related, definitely. Probably caused by the same thing, but the other doesn't mention the diff or patch URLs.

mentioned in issue #3017 (closed)

@dblessing @stanhu

I think the problem is that, when you access a .diff, or a .patch, it compares with target_branch, instead of comparing with diff_base_commit that the normal html render does.

format.diff { render text: @merge_request.to_diff(current_user) }
format.patch { render text: @merge_request.to_patch(current_user) }

  def to_diff(current_user)
    target_project.repository.diff_text(target_branch, source_sha)
  end

  def to_patch(current_user)
    target_project.repository.format_patch(target_branch, source_sha)
  end

This is how the html rendered diff is calculated:

  def diffs
    apply_diff_view_cookie!

    @commit = @merge_request.last_commit
    @base_commit = @merge_request.diff_base_commit

    # MRs created before 8.4 don't have a diff_base_commit,
    # but we need it for the "View file @ ..." link by deleted files
    @base_commit ||= @merge_request.first_commit.parent || @merge_request.first_commit

    @comments_allowed = @reply_allowed = true
    @comments_target = {
      noteable_type: 'MergeRequest',
      noteable_id: @merge_request.id
    }
    @line_notes = @merge_request.notes.where("line_code is not null")

    respond_to do |format|
      format.html
      format.json { render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") } }
    end
  end

@brodock Do you think this should be altered so the diff and patch use the base commit?

yes, definitely.

I made a search and it's the only place we are using this two methods, so this should be easy to fix, but hard to make a spec :)

Reassigned to @brodock

Milestone changed to 8.7

mentioned in merge request !3450 (merged)

Status changed to closed by merge request !3450 (merged)

mentioned in commit 3d484861

mentioned in commit 0a3f5adf

Mentioned in commit maxraab/gitlab-ce@0a3f5adf