Skip to content

Include MR version information when loading file diffs

Thomas Randolph requested to merge tor/defect/mr-versions-wrong-file-diff into master

What does this MR do and why?

For #34643 (closed)

The bulk of the logic of why the code is the way it is is documented in the first commit.

Problem

The problem is basically that MR Changes view doesn't take the diff_id or start_sha into account when requesting a single-file diff, which is what happens when a diff is collapsed.

If a diff is collapsed and the user is looking at an MR "version" other than BASE=>HEAD, they will receive the wrong diff.

Solution

We don't really have a great way to get the diff_id or start_sha when expanding a file.
Neither value is included directly in either the representation of the file itself OR the MR metadata.

However, the MR metadata does have a collection of other URLs, which should always include the version_path (even for "default" views that are not previous versions).
The version_path includes the diff_id and start_sha, so the fix here is to extend our utility function that derives MR info from a path to also derive the version information.

This introduces many cases where the version information returned by the MR information derivation function will be blank (since many paths don't include the necessary info), but this shouldn't be harmful.

Finally, once the information is derived from the version_path, it's included in the parameters of the loadCollapsedDiff request.

Notes

diff_id and start_sha are in the URL, but the MR metadata paths are the authoritative sources of URLs, so I immediately discarded the idea of pulling these values from the browser location bar.

Screenshots or screen recordings

Before After
mr-version-diff-before mr-version-diff-after

How to set up and validate locally

  • Please refer to the linked issue to see replication steps (and a demo MR)

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Thomas Randolph

Merge request reports

Loading