Skip to content

Ensure mergeability check runs on specific cases

Patrick Bajao requested to merge 351190-fix-stuck-mergeability-check into master

What does this MR do and why?

We call MergeRequest#check_mergeability on MR page load to trigger mergeability check. Other than that, we rely on calls for MergeRequest#mergeable? to eventually call it as well. This can cause stale MergeRequest#merge_status when:

  1. Race condition occurred between the MR page load and the time NewMergeRequestWorker runs.
  2. The MergeRequest#merge_status gets updated after the MR page has already loaded (e.g. new changes were pushed to the MR's target branch).

As a fix, we are calling MergeRequest#check_mergeability in more places to ensure we check for it:

  1. We now call it on MergeRequests::AfterCreateService to prevent race condition issue. This service is called by the NewMergeRequestWorker. This way, we ensure that we check for MR's mergeability after it was created.
  2. MergeRequestPollCachedWidgetEntity#merge_status has been updated so whenever the MergeRequest#merge_status gets updated and the MR widget polls for updated status, we will recheck for mergeability as well. On next poll, it should show the updated merge_status.
  3. The check_mergeability_async_in_widget FF has also been removed since it doesn't seem to be used at all. We also need to keep the call for MergeRequest#check_mergeability on the show action so we won't need to wait for poll in case the merge status gets updated and the user decides to refresh the page.

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

  1. Create a MR.
  2. Check that MR widget is updated and able to merge.
  3. Update the merge_status of the MR manually via Rails console and set it to unchecked by calling mark_as_unchecked!.
  4. Wait for the MR widget to poll and eventually update to a mergeable state.

MR acceptance checklist

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

Related to #351190 (closed)

Merge request reports

Loading