Skip to content

Fix suggested reviewers job not run for merge requests created on branch

Tan Le requested to merge 387161-missing-suggested-reviewers-on-mr-branch into master

What does this MR do and why?

This fixes a regression issue introduced by !102907 (merged) which causes the suggested reviewers job to stop running on merge requests created from a branch.

The modified_paths check for FetchSuggestedReviewersWorker depends on merge_request_diff being persisted as part of NewMergeRequestWorker. However, in a recent refactor, NewMergeRequestWorker is scheduled to run after the check is invoked and hence causes this bug.

The user-facing feature is gated behind a feature flag suggested_reviewers_control that is disabled by default to no change log is required.

🔬 Implementations

A sequence diagram of the new flow:

  • this MR ensures these actions highlighted in #f97f71 are performed in the correct order

How to set up and validate locally

  1. Ensure a SaaS (Gitlab.com) environment
    1. One way of doing this is to add a env.runit file to the root GDK folder with the following snippet 
      export GITLAB_SIMULATE_SAAS=1
  2. Set ultimate license on a group http://gdk.test:3000/admin/groups
  3. Create a project in the ultimate group or use an existing one, e.g. http://gdk.test:3000/gitlab-org/gitlab-test
  4. Set the feature flag on rails console bundle exec rails c 
    project = Project.find(2)
Feature.enable(:suggested_reviewers_control, project)
  5. Enable suggested_reviewers_enabled project settings 
    project.project_setting.update!(suggested_reviewers_enabled: true)
  6. Edit a file (like README.md) and create a merge request on the project
  7. Observe some failed workers in the Sidekiq logs (due to lack of authentication) and some failed jobs in the admin panel (http://gdk.test:3000/admin/sidekiq) 
    gdk tail rails-background-jobs | grep --line-buffered FetchSuggestedReviewersWorker

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 #387161 (closed)

Edited by Tan Le

Merge request reports