Skip to content

Fix target_branch_path in merge request sticky header

Andrea Bolognani requested to merge abologna/gitlab:merge-request-target-branch into master

What does this MR do and why?

The target branch for a merge request can be displayed in one of two places:

  • in the regular header, which is shown when the page is initially loaded;
  • in the more compact sticky header, shown after scrolling down the page a bit.

In the sticky header version, however, the target branch is assumed to be part of the source repository instead of the target repository. As a result, the link to the target branch is initially correct, but once you scroll down the page and the sticky header takes over it starts pointing to the wrong branch.

Screenshots or screen recordings

Correct behavior in the initial view (link points to the actual target branch, https://gitlab.com/libvirt/libvirt-rust/-/tree/master): Screenshot_from_2023-10-06_20-35-10

Incorrect behavior once the sticky header takes over (link points to the branch of the same name as the target branch but in the source repository, https://gitlab.com/abologna/libvirt-rust/-/tree/master): Screenshot_from_2023-10-06_20-35-25

How to set up and validate locally

  1. Pick any of the projects that are pre-loaded into the GDK environment;
  2. fork it under the root user's account;
  3. clone it locally;
  4. make some random changes;
  5. push the branch and use it as the basis for a merge request;
  6. check the merge request's page.

This behavior cannot be encountered in a freshly-provisioned GDK environment because, even though it is populated with a number of dummy merge requests, they are all set up so that the source repository and the target repository match. It might be a good idea to augment the dummy data so that merge request coming from an external repository, a very common usage scenario, are represented as well.

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 haven't actually extended the test suite so that it covers the scenario that I'm addressing. I'm willing to take a stab at it if it doesn't involve creating a lot of new scaffolding or becoming intimately familiar with the code base.

Merge request reports