Merge request loads CI widget of a different MR

Description

Under unknown circumstances it seems that the merge request view loads the CI widget of a different merge request. I observed this in !3566 (merged). This MR should only close #14931 (closed). Instead as shown in the screenshot below it indicates that it closes #14845 (closed), #14944 (closed), and #6060 (closed).

I looked into the source of the page and the Merge link really points to /gitlab-org/gitlab-ce/merge_requests/3557/merge - Here we have a different MR, namely !3557 (merged)

Boom, when merging this MR actually !3557 (merged) gets merged instead of !3566 (merged)

@jvanbaarsen mentioned such a weird behavior in the slack channel.

/cc @dblessing @haynes @jvanbaarsen

@razer6 did you do any actions in !3557 (merged) right before this happend?

@haynes I posted a view notes on that MR.

mentioned in issue #14939 (closed)

/cc @rspeicher @rymai

Just for our own awareness, https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3567 is the only MR I could find that was merged because of this so far.

Just noticed a similar issue on https://gitlab.com/gitlab-org/University/merge_requests/9

This one is baffling. I did some investigation:

1. The only page view cache during the merge request page is the description (because of Markdown).
2. When the merge request controller loads, it looks up the merge request by iid here and sets @merge_request.
3. The controller then sets @closes_issueshere.
4. The controller displays the #show method and renders the CI status widget.
5. Since the MR is open, the show view renders this partial and the "Accepting this merge request will close XXX" message".

Since JavaScript widgets aren't involved with step 5, this seems to suggest the backend set the wrong @closes_issues. In addition, it may have fed the wrong @merge_request data to the JS MergeRequestWidget too.

I'm wondering a few things:

1. Is there a SQL query cache that somehow messed up the lookup in step 2?
2. Is there some mixup with the iid and id? I'd be curious to know what each value were for !3566 (merged) and !3567 (merged).

I think I've got it.

This is “memoizing” window.merge_request_widget on a global level: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/views/projects/merge_requests/widget/_show.html.haml#L23-25

If we browse from one MR to a second MR, that second MR page will have ​_the same_​ window.merge_request_widget.

If that second MR has the merge status set to unchecked at time of load, we will find this line: http://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/views/projects/merge_requests/widget/open/_check.html.haml#L7

getMergeStatus will use merge_check_url from that window.merge_request_widget.

merge_check_url is set to the merge_check URL of the ​_original MR_​.

When called, that merge_check action will render widget/show for the ​_original MR_​ and insert it into the page for the ​_new MR_​

Boom.

Reassigned to @jschatz1

mentioned in merge request !3582 (merged)

Status changed to closed by commit 8f094531

mentioned in commit 8f094531

@DouweM Indeed, great finding!!! Once you know that, it's very easy to reproduce: you just have to navigate from a MR to another MR that is unchecked (at the beginning the widget is good but then it's being replaced by the original MR's I visited!):