Switching diff view styles doesn't load the correct diffs
Summary
Switching diff view styles results in a UI with all of the files, but no diffs.
Steps to reproduce
- Load an MR diffs page
- Use the UI to switch to the other view type (e.g.
inline
=>side-by-side
orside-by-side
=>inline
) - Note the diff file(s) after the page has finished loading.
Example Project
(Purely example, it can be any MR).
What is the current bug behavior?
The diff files are empty after the switch finishes loading.
What is the expected correct behavior?
The loaded files should show diffs in the selected style.
Relevant logs and/or screenshots
nick 19 days ago
I can't view blob_spec.rb in side-by--side diff mode? https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29204/diffs?view=parallel
----
nick 19 days ago
oh, there we go. page reload fixed it
----
andr3 18 days ago
Interesting, had you just switched from parallel to inline?
----
andr3 18 days ago
Moving from parallel to inline I got this.
[SCREENSHOT #1]
----
andr3 18 days ago
something failed when setting the inline lines /cc @Thomas Randolph
[SCREENSHOT #2]
----
nick 18 days ago
no, i'd just switched from inline to parallel
----
nick 18 days ago
so maybe it's broken in both ways
----
andr3 18 days ago
yeah we worked on that code recently. but it has been working for a couple of weeks already. Maybe something else was touched in the last deploy…? We’ll look into it. Seems like a recent regression.
----
Thomas Randolph 18 days ago
Hrm. This is my only recent change to diffs: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/28962/diffs#note_318816977
----
Thomas Randolph 18 days ago
It seems benign, but I've learned that nothing is benign in the diffs code...
----
andr3 18 days ago
> Deployed to gprd 18 hours ago
This is consistent with the feeling this didn’t happen before. :confused: Might be worth spending a bit investigating this. sounds like a regression from 12.10
----
Thomas Randolph 18 days ago
Errr, this is because the request for the new view style is broken:
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29204/diffs_batch.json?view=parallel&per_page=20&w=0&view=inline&page=1 two "view", switching from parallel to inline.
I recall this from when we were getting Split Diffs finalized.... should have been fixed...
SCREENSHOT # 1 | SCREENSHOT # 2 |
---|---|
![]() |
![]() |
Output of checks
This bug happens on GitLab.com
Possible fixes
It's most probable that this get
should be updated to use mergeUrlParams
like this line a bit earlier. Without mergeUrlParams
, parameters already present in the active URL (already part of the endpoint in use), are not overwritten with parameters of the same name. Instead same name parameters are tacked onto the end, resulting in duplicates and undefined behavior.
Edited by Thomas Randolph