Include MR version information when loading file diffs
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.
-
I have evaluated the MR acceptance checklist for this MR.