gitlab-housekeeper: Add helper for determining group and reviewer

This will be used by Keeps to determine which group is responsible
for a given feature category, which allows a suitable (backend)
reviewer to be selected.

gems/gitlab-housekeeper/lib/gitlab/housekeeper/change.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Housekeeper
+    class Change
+      attr_accessor :identifiers,
+        :title,
+        :description,
+        :changed_files,
+        :labels,
+        :reviewers
+
+      def mr_description
+        <<~MARKDOWN
+        #{description}
+
+        This change was generated by
+        [gitlab-housekeeper](https://gitlab.com/gitlab-org/gitlab/-/tree/master/gems/gitlab-housekeeper)
+        MARKDOWN
+      end
+
+      def commit_message
+        <<~MARKDOWN
+        #{title}
+
+        #{mr_description}
+
+        Changelog: other
+        MARKDOWN
+      end
+
+      def valid?
+        @identifiers && @title && @description && @changed_files
+      end
+    end
+  end
+end

gems/gitlab-housekeeper/lib/gitlab/housekeeper/git.rb

@@ -44,15 +44,7 @@ def create_commit(branch_name, change)
 
         Shell.execute("git", "checkout", "-b", branch_name)
         Shell.execute("git", "add", *change.changed_files)
-
-        commit_message = <<~MSG
-        #{change.title}
-
-        #{change.description}
-        MSG
-
-        Shell.execute("git", "commit", "-m", commit_message)
-
+        Shell.execute("git", "commit", "-m", change.commit_message)
       ensure
         Shell.execute("git", "checkout", current_branch)
       end

gems/gitlab-housekeeper/lib/gitlab/housekeeper/gitlab_client.rb

@@ -43,6 +43,9 @@ def non_housekeeper_changes(
           changes << :title if note['body'].start_with?("changed title from")
           changes << :description if note['body'] == "changed the description"
           changes << :code if note['body'].match?(/added \d+ commit/)
+
+          changes << :reviewers if note['body'].include?('requested review from')
+          changes << :reviewers if note['body'].include?('removed review request for')
         end
 
         resource_label_events = get_merge_request_resource_label_events(target_project_id: target_project_id, iid: iid)
@@ -61,17 +64,17 @@ def non_housekeeper_changes(
         changes.to_a
       end
 
+      # rubocop:disable Metrics/ParameterLists
       def create_or_update_merge_request(
+        change:,
         source_project_id:,
-        title:,
-        description:,
-        labels:,
         source_branch:,
         target_branch:,
         target_project_id:,
         update_title:,
         update_description:,
-        update_labels:
+        update_labels:,
+        update_reviewers:
       )
         existing_iid = get_existing_merge_request(
           source_project_id: source_project_id,
@@ -82,105 +85,48 @@ def create_or_update_merge_request(
 
         if existing_iid
           update_existing_merge_request(
+            change: change,
             existing_iid: existing_iid,
-            title: title,
-            description: description,
-            labels: labels,
             target_project_id: target_project_id,
             update_title: update_title,
             update_description: update_description,
-            update_labels: update_labels
+            update_labels: update_labels,
+            update_reviewers: update_reviewers
           )
         else
           create_merge_request(
+            change: change,
             source_project_id: source_project_id,
-            title: title,
-            description: description,
-            labels: labels,
             source_branch: source_branch,
             target_branch: target_branch,
             target_project_id: target_project_id
           )
         end
       end
+      # rubocop:enable Metrics/ParameterLists
 
       private
 
       def get_merge_request_notes(target_project_id:, iid:)
-        response = HTTParty.get(
-          "#{@base_uri}/projects/#{target_project_id}/merge_requests/#{iid}/notes",
-          query: {
-            per_page: 100
-          },
-          headers: {
-            "Private-Token" => @token
-          }
-        )
-
-        unless (200..299).cover?(response.code)
-          raise Error,
-            "Failed to get merge request notes with response code: #{response.code} and body:\n#{response.body}"
-        end
-
-        JSON.parse(response.body)
+        request(:get, "/projects/#{target_project_id}/merge_requests/#{iid}/notes", query: { per_page: 100 })
       end
 
       def get_merge_request_resource_label_events(target_project_id:, iid:)
-        response = HTTParty.get(
-          "#{@base_uri}/projects/#{target_project_id}/merge_requests/#{iid}/resource_label_events",
-          query: {
-            per_page: 100
-          },
-          headers: {
-            "Private-Token" => @token
-          }
-        )
-
-        unless (200..299).cover?(response.code)
-          raise Error,
-            "Failed to get merge request label events with response code: #{response.code} and body:\n#{response.body}"
-        end
-
-        JSON.parse(response.body)
+        request(:get, "/projects/#{target_project_id}/merge_requests/#{iid}/resource_label_events",
+          query: { per_page: 100 })
       end
 
       def current_user_id
-        @current_user_id = begin
-          response = HTTParty.get(
-            "#{@base_uri}/user",
-            headers: {
-              "Private-Token" => @token
-            }
-          )
-
-          unless (200..299).cover?(response.code)
-            raise Error,
-              "Failed with response code: #{response.code} and body:\n#{response.body}"
-          end
-
-          data = JSON.parse(response.body)
-          data['id']
-        end
+        @current_user_id ||= request(:get, "/user")['id']
       end
 
       def get_existing_merge_request(source_project_id:, source_branch:, target_branch:, target_project_id:)
-        response = HTTParty.get("#{@base_uri}/projects/#{target_project_id}/merge_requests",
-          query: {
-            state: :opened,
-            source_branch: source_branch,
-            target_branch: target_branch,
-            source_project_id: source_project_id
-          },
-          headers: {
-            'Private-Token' => @token
-          })
-
-        unless (200..299).cover?(response.code)
-          raise Error,
-            "Failed with response code: #{response.code} and body:\n#{response.body}"
-        end
-
-        data = JSON.parse(response.body)
+        data = request(:get, "/projects/#{target_project_id}/merge_requests", query: {
+          state: :opened,
+          source_branch: source_branch,
+          target_branch: target_branch,
+          source_project_id: source_project_id
+        })
 
         return nil if data.empty?
 
@@ -192,63 +138,64 @@ def get_existing_merge_request(source_project_id:, source_branch:, target_branch
       end
 
       def create_merge_request(
+        change:,
         source_project_id:,
-        title:,
-        description:,
-        labels:,
         source_branch:,
         target_branch:,
         target_project_id:
       )
-        response = HTTParty.post("#{@base_uri}/projects/#{source_project_id}/merge_requests", body: {
-          title: title,
-          description: description,
-          labels: Array(labels).join(','),
+        request(:post, "/projects/#{source_project_id}/merge_requests", body: {
+          title: change.title,
+          description: change.mr_description,
+          labels: Array(change.labels).join(','),
           source_branch: source_branch,
           target_branch: target_branch,
           target_project_id: target_project_id,
-          remove_source_branch: true
-        }.to_json,
-          headers: {
-            'Private-Token' => @token,
-            'Content-Type' => 'application/json'
-          })
-
-        return if (200..299).cover?(response.code)
-
-        raise Error,
-          "Failed with response code: #{response.code} and body:\n#{response.body}"
+          remove_source_branch: true,
+          reviewer_ids: usernames_to_ids(change.reviewers)
+        })
       end
 
       def update_existing_merge_request(
+        change:,
         existing_iid:,
-        title:,
-        description:,
-        labels:,
         target_project_id:,
         update_title:,
         update_description:,
-        update_labels:
+        update_labels:,
+        update_reviewers:
       )
         body = {}
 
-        body[:title] = title if update_title
-        body[:description] = description if update_description
-        body[:add_labels] = Array(labels).join(',') if update_labels
+        body[:title] = change.title if update_title
+        body[:description] = change.mr_description if update_description
+        body[:add_labels] = Array(change.labels).join(',') if update_labels
+        body[:reviewer_ids] = usernames_to_ids(change.reviewers) if update_reviewers
 
         return if body.empty?
 
-        response = HTTParty.put("#{@base_uri}/projects/#{target_project_id}/merge_requests/#{existing_iid}",
-          body: body.to_json,
-          headers: {
-            'Private-Token' => @token,
-            'Content-Type' => 'application/json'
-          })
+        request(:put, "/projects/#{target_project_id}/merge_requests/#{existing_iid}", body: body)
+      end
+
+      def usernames_to_ids(usernames)
+        usernames.map do |username|
+          data = request(:get, "/users", query: { username: username })
+          data[0]['id']
+        end
+      end
+
+      def request(method, path, query: {}, body: {})
+        response = HTTParty.public_send(method, "#{@base_uri}#{path}", query: query, body: body.to_json, headers: { # rubocop:disable GitlabSecurity/PublicSend
+          'Private-Token' => @token,
+          'Content-Type' => 'application/json'
+        })
 
-        return if (200..299).cover?(response.code)
+        unless (200..299).cover?(response.code)
+          raise Error,
+            "Failed with response code: #{response.code} and body:\n#{response.body}"
+        end
 
-        raise Error,
-          "Failed with response code: #{response.code} and body:\n#{response.body}"
+        JSON.parse(response.body)
       end
     end
   end

gems/gitlab-housekeeper/lib/gitlab/housekeeper/runner.rb

@@ -4,13 +4,12 @@
 require 'gitlab/housekeeper/keep'
 require 'gitlab/housekeeper/gitlab_client'
 require 'gitlab/housekeeper/git'
+require 'gitlab/housekeeper/change'
 require 'awesome_print'
 require 'digest'
 
 module Gitlab
   module Housekeeper
-    Change = Struct.new(:identifiers, :title, :description, :changed_files, :labels)
-
     class Runner
       def initialize(max_mrs: 1, dry_run: false, require: [], keeps: nil)
         @max_mrs = max_mrs
@@ -32,6 +31,11 @@ def run
           @keeps.each do |keep_class|
             keep = keep_class.new
             keep.each_change do |change|
+              unless change.valid?
+                puts "Ignoring invalid change from: #{keep_class}"
+                next
+              end
+
               branch_name = git.commit_in_branch(change)
               add_standard_change_data(change)
 
@@ -54,14 +58,6 @@ def run
       def add_standard_change_data(change)
         change.labels ||= []
         change.labels << 'automation:gitlab-housekeeper-authored'
-
-        change.description += <<~MARKDOWN
-
-        This change was generated by
-        [gitlab-housekeeper](https://gitlab.com/gitlab-org/gitlab/-/tree/master/gems/gitlab-housekeeper)
-
-        Changelog: other
-        MARKDOWN
       end
 
       def git
@@ -112,16 +108,15 @@ def create(change, branch_name)
         end
 
         gitlab_client.create_or_update_merge_request(
+          change: change,
           source_project_id: housekeeper_fork_project_id,
-          title: change.title,
-          description: change.description,
-          labels: change.labels,
           source_branch: branch_name,
           target_branch: 'master',
           target_project_id: housekeeper_target_project_id,
           update_title: !non_housekeeper_changes.include?(:title),
           update_description: !non_housekeeper_changes.include?(:description),
-          update_labels: !non_housekeeper_changes.include?(:labels)
+          update_labels: !non_housekeeper_changes.include?(:labels),
+          update_reviewers: !non_housekeeper_changes.include?(:reviewers)
         )
       end
 

gems/gitlab-housekeeper/spec/gitlab/housekeeper/change_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Gitlab::Housekeeper::Change do
+  let(:change) { described_class.new }
+
+  before do
+    change.title = 'The title'
+    change.description = 'The description'
+  end
+
+  describe '#mr_description' do
+    it 'includes standard content' do
+      expect(change.mr_description).to eq(
+        <<~MARKDOWN
+        The description
+
+        This change was generated by
+        [gitlab-housekeeper](https://gitlab.com/gitlab-org/gitlab/-/tree/master/gems/gitlab-housekeeper)
+        MARKDOWN
+      )
+    end
+  end
+
+  describe '#commit_message' do
+    it 'includes standard content' do
+      expect(change.commit_message).to eq(
+        <<~MARKDOWN
+        The title
+
+        The description
+
+        This change was generated by
+        [gitlab-housekeeper](https://gitlab.com/gitlab-org/gitlab/-/tree/master/gems/gitlab-housekeeper)
+
+
+        Changelog: other
+        MARKDOWN
+      )
+    end
+  end
+
+  describe '#valid?' do
+    it 'is not valid if missing required attributes' do
+      [:identifiers, :title, :description, :changed_files].each do |attribute|
+        change = create_change
+        expect(change).to be_valid
+        change.public_send("#{attribute}=", nil)
+        expect(change).not_to be_valid
+      end
+    end
+  end
+end

gems/gitlab-housekeeper/spec/gitlab/housekeeper/git_spec.rb

@@ -50,27 +50,21 @@ def setup_and_checkout_another_branch
     let(:test_file2) { 'files/test_file2.txt' }
 
     it 'commits the given change details to the given branch name' do
-      title = "The commit title"
-      description = <<~COMMIT
+      change = ::Gitlab::Housekeeper::Change.new
+      change.title = "The commit title"
+      change.description = <<~COMMIT
       The commit description can be
       split over multiple lines!
       COMMIT
 
-      identifiers = %w[GitlabHousekeeper TestBranch]
+      change.identifiers = %w[GitlabHousekeeper TestBranch]
 
       Dir.mkdir('files')
       File.write(test_file1, "Content in file 1!")
       File.write(test_file2, "Other content in file 2!")
       File.write(file_not_to_commit, 'Do not commit!')
 
-      changed_files = [test_file1, test_file2]
-
-      change = ::Gitlab::Housekeeper::Change.new(
-        identifiers,
-        title,
-        description,
-        changed_files
-      )
+      change.changed_files = [test_file1, test_file2]
 
       branch_name = nil
       git.with_branch_from_branch do
@@ -91,6 +85,11 @@ def setup_and_checkout_another_branch
           The commit description can be
           split over multiple lines!
 
+          This change was generated by
+          [gitlab-housekeeper](https://gitlab.com/gitlab-org/gitlab/-/tree/master/gems/gitlab-housekeeper)
+
+          Changelog: other
+
 
       diff --git a/files/test_file2.txt b/files/test_file2.txt
       new file mode 100644

gems/gitlab-housekeeper/spec/gitlab/housekeeper/gitlab_client_spec.rb

@@ -23,6 +23,24 @@
       }
     end
 
+    let(:added_reviewer_note) do
+      {
+        id: 1698248524,
+        body: "requested review from @bob",
+        author: { "id" => 1234 },
+        system: true
+      }
+    end
+
+    let(:removed_reviewer_note) do
+      {
+        id: 1698248524,
+        body: "removed review request for @bob",
+        author: { "id" => 1234 },
+        system: true
+      }
+    end
+
     let(:irrelevant_note1) do
       {
         id: 1698248523,
@@ -138,7 +156,7 @@
 
     context 'when all important things change' do
       let(:notes) do
-        [not_a_system_note, updated_title_note, updated_description_note, added_commit_note]
+        [not_a_system_note, updated_title_note, updated_description_note, added_commit_note, added_reviewer_note]
       end
 
       let(:resource_label_events) do
@@ -146,10 +164,7 @@
       end
 
       it 'returns :title, :description, :code, :labels' do
-        expect(non_housekeeper_changes).to include(:title)
-        expect(non_housekeeper_changes).to include(:description)
-        expect(non_housekeeper_changes).to include(:code)
-        expect(non_housekeeper_changes).to include(:labels)
+        expect(non_housekeeper_changes).to match_array([:title, :description, :code, :labels, :reviewers])
       end
     end
 
@@ -159,10 +174,7 @@
       end
 
       it 'returns :title' do
-        expect(non_housekeeper_changes).to include(:title)
-        expect(non_housekeeper_changes).not_to include(:description)
-        expect(non_housekeeper_changes).not_to include(:code)
-        expect(non_housekeeper_changes).not_to include(:labels)
+        expect(non_housekeeper_changes).to match_array([:title])
       end
     end
 
@@ -172,10 +184,7 @@
       end
 
       it 'returns :description' do
-        expect(non_housekeeper_changes).not_to include(:title)
-        expect(non_housekeeper_changes).to include(:description)
-        expect(non_housekeeper_changes).not_to include(:code)
-        expect(non_housekeeper_changes).not_to include(:labels)
+        expect(non_housekeeper_changes).to match_array([:description])
       end
     end
 
@@ -189,10 +198,27 @@
       end
 
       it 'returns :labels' do
-        expect(non_housekeeper_changes).not_to include(:title)
-        expect(non_housekeeper_changes).not_to include(:description)
-        expect(non_housekeeper_changes).not_to include(:code)
-        expect(non_housekeeper_changes).to include(:labels)
+        expect(non_housekeeper_changes).to match_array([:labels])
+      end
+    end
+
+    context 'when reviewers are added' do
+      let(:notes) do
+        [not_a_system_note, added_reviewer_note]
+      end
+
+      it 'returns :reviewers' do
+        expect(non_housekeeper_changes).to match_array([:reviewers])
+      end
+    end
+
+    context 'when reviewers are removed' do
+      let(:notes) do
+        [not_a_system_note, removed_reviewer_note]
+      end
+
+      it 'returns :reviewers' do
+        expect(non_housekeeper_changes).to match_array([:reviewers])
       end
     end
 
@@ -204,24 +230,39 @@
   end
 
   describe '#create_or_update_merge_request' do
+    let(:reviewer_id) { 999 }
+
+    let(:change) do
+      create_change
+    end
+
     let(:params) do
       {
+        change: change,
         source_project_id: 123,
-        title: 'A new merge request!',
-        labels: %w[label-1 label-2],
-        description: 'This merge request is pretty good.',
         source_branch: 'the-source-branch',
         target_branch: 'the-target-branch',
         target_project_id: 456,
         update_title: true,
         update_description: true,
-        update_labels: true
+        update_labels: true,
+        update_reviewers: true
       }
     end
 
     let(:existing_mrs) { [] }
 
     before do
+      # Stub the user id of the reviewers
+      stub_request(:get, "https://gitlab.com/api/v4/users")
+        .with(
+          query: { username: 'thegitlabreviewer' },
+          headers: {
+            'Private-Token' => 'the-api-token'
+          }
+        )
+        .to_return(status: 200, body: [{ id: reviewer_id }].to_json)
+
       # Stub the check to see if the merge request already exists
       stub_request(:get, "https://gitlab.com/api/v4/projects/456/merge_requests?state=opened&source_branch=the-source-branch&target_branch=the-target-branch&source_project_id=123")
         .with(
@@ -236,19 +277,20 @@
       stub = stub_request(:post, "https://gitlab.com/api/v4/projects/123/merge_requests")
         .with(
           body: {
-            title: "A new merge request!",
-            description: "This merge request is pretty good.",
-            labels: "label-1,label-2",
+            title: "The change title",
+            description: change.mr_description,
+            labels: "some-label-1,some-label-2",
             source_branch: "the-source-branch",
             target_branch: "the-target-branch",
             target_project_id: 456,
-            remove_source_branch: true
+            remove_source_branch: true,
+            reviewer_ids: [reviewer_id]
           },
           headers: {
             'Content-Type' => 'application/json',
             'Private-Token' => 'the-api-token'
           })
-        .to_return(status: 200, body: "")
+        .to_return(status: 200, body: '{}')
 
       client.create_or_update_merge_request(**params)
 
@@ -264,15 +306,16 @@
         stub = stub_request(:put, "https://gitlab.com/api/v4/projects/456/merge_requests/1234")
           .with(
             body: {
-              title: "A new merge request!",
-              description: "This merge request is pretty good.",
-              add_labels: "label-1,label-2"
+              title: "The change title",
+              description: change.mr_description,
+              add_labels: "some-label-1,some-label-2",
+              reviewer_ids: [reviewer_id]
             }.to_json,
             headers: {
               'Content-Type' => 'application/json',
               'Private-Token' => 'the-api-token'
             })
-         .to_return(status: 200, body: "")
+         .to_return(status: 200, body: '{}')
 
         client.create_or_update_merge_request(**params)
         expect(stub).to have_been_requested
@@ -293,14 +336,15 @@
           stub = stub_request(:put, "https://gitlab.com/api/v4/projects/456/merge_requests/1234")
             .with(
               body: {
-                description: "This merge request is pretty good.",
-                add_labels: "label-1,label-2"
+                description: change.mr_description,
+                add_labels: "some-label-1,some-label-2",
+                reviewer_ids: [reviewer_id]
               }.to_json,
               headers: {
                 'Content-Type' => 'application/json',
                 'Private-Token' => 'the-api-token'
               }
-            ).to_return(status: 200, body: "")
+            ).to_return(status: 200, body: '{}')
 
           client.create_or_update_merge_request(**params.merge(update_title: false))
           expect(stub).to have_been_requested
@@ -312,14 +356,15 @@
           stub = stub_request(:put, "https://gitlab.com/api/v4/projects/456/merge_requests/1234")
             .with(
               body: {
-                title: "A new merge request!",
-                add_labels: "label-1,label-2"
+                title: "The change title",
+                add_labels: "some-label-1,some-label-2",
+                reviewer_ids: [reviewer_id]
               }.to_json,
               headers: {
                 'Content-Type' => 'application/json',
                 'Private-Token' => 'the-api-token'
               }
-            ).to_return(status: 200, body: "")
+            ).to_return(status: 200, body: '{}')
 
           client.create_or_update_merge_request(**params.merge(update_description: false))
           expect(stub).to have_been_requested
@@ -331,24 +376,49 @@
           stub = stub_request(:put, "https://gitlab.com/api/v4/projects/456/merge_requests/1234")
             .with(
               body: {
-                title: "A new merge request!",
-                description: "This merge request is pretty good."
+                title: "The change title",
+                description: change.mr_description,
+                reviewer_ids: [reviewer_id]
               }.to_json,
               headers: {
                 'Content-Type' => 'application/json',
                 'Private-Token' => 'the-api-token'
               }
-            ).to_return(status: 200, body: "")
+            ).to_return(status: 200, body: '{}')
 
           client.create_or_update_merge_request(**params.merge(update_labels: false))
           expect(stub).to have_been_requested
         end
       end
 
+      context 'when update_reviewers: false' do
+        it 'does not update the reviewers' do
+          stub = stub_request(:put, "https://gitlab.com/api/v4/projects/456/merge_requests/1234")
+            .with(
+              body: {
+                title: "The change title",
+                description: change.mr_description,
+                add_labels: "some-label-1,some-label-2"
+              }.to_json,
+              headers: {
+                'Content-Type' => 'application/json',
+                'Private-Token' => 'the-api-token'
+              }
+            ).to_return(status: 200, body: '{}')
+
+          client.create_or_update_merge_request(**params.merge(update_reviewers: false))
+          expect(stub).to have_been_requested
+        end
+      end
+
       context 'when there is nothing to update' do
         it 'does not make a request' do
-          client.create_or_update_merge_request(**params.merge(update_description: false, update_title: false,
-            update_labels: false))
+          client.create_or_update_merge_request(**params.merge(
+            update_description: false,
+            update_title: false,
+            update_labels: false,
+            update_reviewers: false
+          ))
         end
       end
     end

gems/gitlab-housekeeper/spec/gitlab/housekeeper/runner_spec.rb

@@ -8,32 +8,23 @@
   let(:fake_keep) { instance_double(Class) }
 
   let(:change1) do
-    ::Gitlab::Housekeeper::Change.new(
-      %w[the identifier for the first change],
-      "The title of MR1",
-      "The description of the MR",
-      ['change1.txt', 'change2.txt'],
-      ['example-label']
+    create_change(
+      identifiers: %w[the identifier for the first change],
+      title: "The title of MR1"
     )
   end
 
   let(:change2) do
-    ::Gitlab::Housekeeper::Change.new(
-      %w[the identifier for the second change],
-      "The title of MR2",
-      "The description of the MR",
-      ['change1.txt', 'change2.txt'],
-      ['example-label']
+    create_change(
+      identifiers: %w[the identifier for the second change],
+      title: "The title of MR2"
     )
   end
 
   let(:change3) do
-    ::Gitlab::Housekeeper::Change.new(
-      %w[the identifier for the third change],
-      "The title of MR3",
-      "The description of the MR",
-      ['change1.txt', 'change2.txt'],
-      ['example-label']
+    create_change(
+      identifiers: %w[the identifier for the third change],
+      title: "The title of MR3"
     )
   end
 
@@ -98,29 +89,27 @@
       # Merge requests get created
       expect(gitlab_client).to receive(:create_or_update_merge_request)
         .with(
+          change: change1,
           source_project_id: '123',
-          title: 'The title of MR1',
-          description: /The description of the MR/,
-          labels: %w[example-label automation:gitlab-housekeeper-authored],
           source_branch: 'the-identifier-for-the-first-change',
           target_branch: 'master',
           target_project_id: '456',
           update_title: true,
           update_description: true,
-          update_labels: true
+          update_labels: true,
+          update_reviewers: true
         )
       expect(gitlab_client).to receive(:create_or_update_merge_request)
         .with(
+          change: change2,
           source_project_id: '123',
-          title: 'The title of MR2',
-          description: /The description of the MR/,
-          labels: %w[example-label automation:gitlab-housekeeper-authored],
           source_branch: 'the-identifier-for-the-second-change',
           target_branch: 'master',
           target_project_id: '456',
           update_title: true,
           update_description: true,
-          update_labels: true
+          update_labels: true,
+          update_reviewers: true
         )
 
       described_class.new(max_mrs: 2, keeps: [fake_keep]).run
@@ -135,7 +124,7 @@
             source_branch: 'the-identifier-for-the-first-change',
             target_branch: 'master',
             target_project_id: '456'
-          ).and_return([:code, :description])
+          ).and_return([:code, :description, :reviewers])
 
         # Second change has updated title and description so it should push the code
         expect(gitlab_client).to receive(:non_housekeeper_changes)
@@ -155,29 +144,27 @@
 
         expect(gitlab_client).to receive(:create_or_update_merge_request)
           .with(
+            change: change1,
             source_project_id: '123',
-            title: 'The title of MR1',
-            description: /The description of the MR/,
-            labels: %w[example-label automation:gitlab-housekeeper-authored],
             source_branch: 'the-identifier-for-the-first-change',
             target_branch: 'master',
             target_project_id: '456',
             update_title: true,
             update_description: false,
-            update_labels: true
+            update_labels: true,
+            update_reviewers: false
           )
         expect(gitlab_client).to receive(:create_or_update_merge_request)
           .with(
+            change: change2,
             source_project_id: '123',
-            title: 'The title of MR2',
-            description: /The description of the MR/,
-            labels: %w[example-label automation:gitlab-housekeeper-authored],
             source_branch: 'the-identifier-for-the-second-change',
             target_branch: 'master',
             target_project_id: '456',
             update_title: false,
             update_description: false,
-            update_labels: true
+            update_labels: true,
+            update_reviewers: true
           )
 
         described_class.new(max_mrs: 2, keeps: [fake_keep]).run

gems/gitlab-housekeeper/spec/spec_helper.rb

@@ -6,6 +6,28 @@
 require 'webmock/rspec'
 require 'gitlab/rspec/all'
 
+module HousekeeperFactory
+  def create_change(
+    identifiers: %w[the identifier],
+    title: 'The change title',
+    description: 'The change description',
+    changed_files: ['change1.txt', 'change2.txt'],
+    labels: %w[some-label-1 some-label-2],
+    reviewers: ['thegitlabreviewer']
+  )
+
+    change = ::Gitlab::Housekeeper::Change.new
+    change.identifiers = identifiers
+    change.title = title
+    change.description = description
+    change.changed_files = changed_files
+    change.labels = labels
+    change.reviewers = reviewers
+
+    change
+  end
+end
+
 RSpec.configure do |config|
   config.include StubENV
   # Enable flags like --only-failures and --next-failure
@@ -17,4 +39,6 @@
   config.expect_with :rspec do |c|
     c.syntax = :expect
   end
+
+  config.include(HousekeeperFactory)
 end

keeps/helpers/groups.rb

+# frozen_string_literal: true
+
+module Keeps
+  module Helpers
+    class Groups
+      Error = Class.new(StandardError)
+
+      def group_for_feature_category(category)
+        @groups ||= {}
+        @groups[category] ||= fetch_groups.find do |_, group|
+          group['categories'].present? && group['categories'].include?(category)
+        end&.last
+      end
+
+      def pick_reviewer(group, identifiers)
+        random_engineer = Digest::SHA256.hexdigest(identifiers.join).to_i(16) % group['backend_engineers'].size
+
+        group['backend_engineers'][random_engineer]
+      end
+
+      private
+
+      def fetch_groups
+        @groups_json ||= begin
+          response = Gitlab::HTTP.get('https://about.gitlab.com/groups.json')
+
+          unless (200..299).cover?(response.code)
+            raise Error,
+              "Failed to get group information with response code: #{response.code} and body:\n#{response.body}"
+          end
+
+          Gitlab::Json.parse(response.body)
+        end
+      end
+    end
+  end
+end

keeps/overdue_finalize_background_migration.rb

@@ -40,15 +40,16 @@ def each_change
         next unless migration_record
 
         # Finalize the migration
-        title = "Finalize migration #{job_name}"
+        change = ::Gitlab::Housekeeper::Change.new
+        change.title = "Finalize migration #{job_name}"
 
-        identifiers = [self.class.name.demodulize, job_name]
+        change.identifiers = [self.class.name.demodulize, job_name]
 
         last_migration_file = last_migration_for_job(job_name)
         next unless last_migration_file
 
         # rubocop:disable Gitlab/DocUrl -- Not running inside rails application
-        description = <<~MARKDOWN
+        change.description = <<~MARKDOWN
         This migration was finished at `#{migration_record.finished_at || migration_record.updated_at}`, you can confirm
         the status using our
         [batched background migration chatops commands](https://docs.gitlab.com/ee/development/database/batched_background_migrations.html#monitor-the-progress-and-status-of-a-batched-background-migration).
@@ -78,7 +79,7 @@ def each_change
           .source_root('generator_templates/post_deployment_migration/post_deployment_migration/')
         generator = ::PostDeploymentMigration::PostDeploymentMigrationGenerator.new([migration_name], force: true)
         migration_file = generator.invoke_all.first
-        changed_files = [migration_file]
+        change.changed_files = [migration_file]
 
         add_ensure_call_to_migration(migration_file, queue_method_node, job_name)
         ::Gitlab::Housekeeper::Shell.execute('rubocop', '-a', migration_file)
@@ -89,11 +90,10 @@ def each_change
 
         add_finalized_by_to_yaml(migration_yaml_file, generator.migration_number)
 
-        changed_files << digest_file
-        changed_files << migration_yaml_file
+        change.changed_files << digest_file
+        change.changed_files << migration_yaml_file
 
-        to_create = ::Gitlab::Housekeeper::Change.new(identifiers, title, description, changed_files)
-        yield(to_create)
+        yield(change)
       end
     end
 

spec/keeps/helpers/groups_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require './keeps/helpers/groups'
+
+RSpec.describe Keeps::Helpers::Groups, feature_category: :tooling do
+  let(:groups) do
+    {
+      'tenant_scale' => {
+        'name' => 'Tenant Scale',
+        'section' => 'core_platform',
+        'stage' => 'data_stores',
+        'categories' => %w[cell groups_and_projects user_profile organization],
+        'label' => 'group::tenant scale',
+        'extra_labels' => [],
+        'slack_channel' => 'g_tenant_scale',
+        'backend_engineers' => %w[be1 be2 be3 be4 be5],
+        'triage_ops_config' => nil
+      }
+    }
+  end
+
+  before do
+    stub_request(:get, "https://about.gitlab.com/groups.json").to_return(status: 200, body: groups.to_json)
+  end
+
+  describe '#group_for_feature_category' do
+    let(:category) { 'organization' }
+
+    subject(:group) { described_class.new.group_for_feature_category(category) }
+
+    it { is_expected.to eq(groups['tenant_scale']) }
+
+    context 'when the category does not exist' do
+      let(:category) { 'missing-category' }
+
+      it { is_expected.to eq(nil) }
+    end
+
+    context 'when the request to fetch groups fails' do
+      before do
+        stub_request(:get, "https://about.gitlab.com/groups.json").to_return(status: 404, body: '')
+      end
+
+      it 'raises an error' do
+        expect { group }.to raise_error(described_class::Error)
+      end
+    end
+  end
+
+  describe '#pick_reviewer' do
+    let(:group) { groups['tenant_scale'] }
+    let(:identifiers) { %w[example identifier] }
+    let(:expected_index) { Digest::SHA256.hexdigest(identifiers.join).to_i(16) % group['backend_engineers'].size }
+
+    subject { described_class.new.pick_reviewer(group, identifiers) }
+
+    it { is_expected.to eq(group['backend_engineers'][expected_index]) }
+  end
+end