Merge branch 'use-gitlab-roulette-in-danger-roulette' into 'master'

Use the JSON from the gitlab-roulette project for Danger roulette

Closes #121583

See merge request gitlab-org/gitlab!33757

danger/ce_ee_vue_templates/Dangerfile

@@ -10,9 +10,7 @@ def get_vue_files_with_ce_and_ee_versions(files)
                            "ee/#{file}"
                          end
 
-      escaped_path = CGI.escape(counterpart_path)
-      api_endpoint = "https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab-ee/repository/files/#{escaped_path}?ref=master"
-      response = HTTParty.get(api_endpoint) # rubocop:disable Gitlab/HTTParty
+      response = gitlab.api.get_file(gitlab.mr_json['project_id'], counterpart_path, 'master')
       response.code != 404
     else
       false

lib/gitlab/danger/roulette.rb

@@ -5,7 +5,7 @@ require_relative 'teammate'
 module Gitlab
   module Danger
     module Roulette
-      ROULETTE_DATA_URL = 'https://about.gitlab.com/roulette.json'
+      ROULETTE_DATA_URL = 'https://gitlab-org.gitlab.io/gitlab-roulette/roulette.json'
       OPTIONAL_CATEGORIES = [:qa, :test].freeze
 
       Spin = Struct.new(:category, :reviewer, :maintainer, :optional_role)
@@ -90,7 +90,7 @@ module Gitlab
       # @param [Teammate] person
       # @return [Boolean]
       def valid_person?(person)
-        !mr_author?(person) && person.available?
+        !mr_author?(person) && person.available && person.has_capacity
       end
 
       # @param [Teammate] person

lib/gitlab/danger/teammate.rb

 # frozen_string_literal: true
 
-require 'cgi'
-require 'set'
-
 module Gitlab
   module Danger
     class Teammate
-      attr_reader :name, :username, :role, :projects
-
-      AT_CAPACITY_EMOJI = Set.new(%w[red_circle]).freeze
-      OOO_EMOJI = Set.new(%w[
-        palm_tree
-        beach beach_umbrella beach_with_umbrella
-      ]).freeze
+      attr_reader :username, :name, :markdown_name, :role, :projects, :available, :has_capacity
 
+      # The options data are produced by https://gitlab.com/gitlab-org/gitlab-roulette/-/blob/master/lib/team_member.rb
       def initialize(options = {})
         @username = options['username']
-        @name = options['name'] || @username
+        @name = options['name']
+        @markdown_name = options['markdown_name']
         @role = options['role']
         @projects = options['projects']
-      end
-
-      def markdown_name
-        "[#{name}](https://gitlab.com/#{username}) (`@#{username}`)"
+        @available = options['available']
+        @has_capacity = options['has_capacity']
       end
 
       def in_project?(name)
@@ -43,42 +34,8 @@ module Gitlab
         has_capability?(project, category, :maintainer, labels)
       end
 
-      def status
-        return @status if defined?(@status)
-
-        @status ||=
-          begin
-            Gitlab::Danger::RequestHelper.http_get_json(status_api_endpoint)
-          rescue Gitlab::Danger::RequestHelper::HTTPError, JSON::ParserError
-            nil # better no status than a crashing Danger
-          end
-      end
-
-      # @return [Boolean]
-      def available?
-        !out_of_office? && has_capacity?
-      end
-
       private
 
-      def status_api_endpoint
-        "https://gitlab.com/api/v4/users/#{CGI.escape(username)}/status"
-      end
-
-      def status_emoji
-        status&.dig("emoji")
-      end
-
-      # @return [Boolean]
-      def out_of_office?
-        status&.dig("message")&.match?(/OOO/i) || OOO_EMOJI.include?(status_emoji)
-      end
-
-      # @return [Boolean]
-      def has_capacity?
-        !AT_CAPACITY_EMOJI.include?(status_emoji)
-      end
-
       def has_capability?(project, category, kind, labels)
         case category
         when :test

spec/lib/gitlab/danger/roulette_spec.rb

@@ -11,7 +11,9 @@ describe Gitlab::Danger::Roulette do
       username: 'backend-maintainer',
       name: 'Backend maintainer',
       role: 'Backend engineer',
-      projects: { 'gitlab' => 'maintainer backend' }
+      projects: { 'gitlab' => 'maintainer backend' },
+      available: true,
+      has_capacity: true
     }
   end
   let(:frontend_reviewer) do
@@ -19,7 +21,9 @@ describe Gitlab::Danger::Roulette do
       username: 'frontend-reviewer',
       name: 'Frontend reviewer',
       role: 'Frontend engineer',
-      projects: { 'gitlab' => 'reviewer frontend' }
+      projects: { 'gitlab' => 'reviewer frontend' },
+      available: true,
+      has_capacity: true
     }
   end
   let(:frontend_maintainer) do
@@ -27,7 +31,9 @@ describe Gitlab::Danger::Roulette do
       username: 'frontend-maintainer',
       name: 'Frontend maintainer',
       role: 'Frontend engineer',
-      projects: { 'gitlab' => "maintainer frontend" }
+      projects: { 'gitlab' => "maintainer frontend" },
+      available: true,
+      has_capacity: true
     }
   end
   let(:software_engineer_in_test) do
@@ -38,7 +44,9 @@ describe Gitlab::Danger::Roulette do
       projects: {
         'gitlab' => 'reviewer qa',
         'gitlab-qa' => 'maintainer'
-      }
+      },
+      available: true,
+      has_capacity: true
     }
   end
   let(:engineering_productivity_reviewer) do
@@ -46,7 +54,9 @@ describe Gitlab::Danger::Roulette do
       username: 'eng-prod-reviewer',
       name: 'EP engineer',
       role: 'Engineering Productivity',
-      projects: { 'gitlab' => 'reviewer backend' }
+      projects: { 'gitlab' => 'reviewer backend' },
+      available: true,
+      has_capacity: true
     }
   end
 
@@ -73,10 +83,17 @@ describe Gitlab::Danger::Roulette do
 
   def matching_spin(category, reviewer: { username: nil }, maintainer: { username: nil }, optional: nil)
     satisfy do |spin|
-      spin.category == category &&
-        spin.reviewer&.username == reviewer[:username] &&
-        spin.maintainer&.username == maintainer[:username] &&
-        spin.optional_role == optional
+      bool = spin.category == category
+      bool &&= spin.reviewer&.username == reviewer[:username]
+
+      bool &&=
+        if maintainer
+          spin.maintainer&.username == maintainer[:username]
+        else
+          spin.maintainer.nil?
+        end
+
+      bool && spin.optional_role == optional
     end
   end
 
@@ -85,66 +102,76 @@ describe Gitlab::Danger::Roulette do
     let!(:branch_name) { 'a-branch' }
     let!(:mr_labels) { ['backend', 'devops::create'] }
     let!(:author) { Gitlab::Danger::Teammate.new('username' => 'filipa') }
-
-    before do
-      [
-        backend_maintainer,
-        frontend_reviewer,
-        frontend_maintainer,
-        software_engineer_in_test,
-        engineering_productivity_reviewer
-      ].each do |person|
-        stub_person_status(instance_double(Gitlab::Danger::Teammate, username: person[:username]), message: 'making GitLab magic')
-      end
-
+    let(:spins) do
+      # Stub the request at the latest time so that we can modify the raw data, e.g. available and has_capacity fields.
       WebMock
         .stub_request(:get, described_class::ROULETTE_DATA_URL)
         .to_return(body: teammate_json)
+
+      subject.spin(project, categories, branch_name)
+    end
+
+    before do
       allow(subject).to receive_message_chain(:gitlab, :mr_author).and_return(author.username)
       allow(subject).to receive_message_chain(:gitlab, :mr_labels).and_return(mr_labels)
     end
 
     context 'when change contains backend category' do
-      it 'assigns backend reviewer and maintainer' do
-        categories = [:backend]
-        spins = subject.spin(project, categories, branch_name)
+      let(:categories) { [:backend] }
 
+      it 'assigns backend reviewer and maintainer' do
         expect(spins).to contain_exactly(matching_spin(:backend, reviewer: engineering_productivity_reviewer, maintainer: backend_maintainer))
       end
+
+      context 'when teammate is not available' do
+        before do
+          backend_maintainer[:available] = false
+        end
+
+        it 'assigns backend reviewer and no maintainer' do
+          expect(spins).to contain_exactly(matching_spin(:backend, reviewer: engineering_productivity_reviewer, maintainer: nil))
+        end
+      end
+
+      context 'when teammate has no capacity' do
+        before do
+          backend_maintainer[:has_capacity] = false
+        end
+
+        it 'assigns backend reviewer and no maintainer' do
+          expect(spins).to contain_exactly(matching_spin(:backend, reviewer: engineering_productivity_reviewer, maintainer: nil))
+        end
+      end
     end
 
     context 'when change contains frontend category' do
-      it 'assigns frontend reviewer and maintainer' do
-        categories = [:frontend]
-        spins = subject.spin(project, categories, branch_name)
+      let(:categories) { [:frontend] }
 
+      it 'assigns frontend reviewer and maintainer' do
         expect(spins).to contain_exactly(matching_spin(:frontend, reviewer: frontend_reviewer, maintainer: frontend_maintainer))
       end
     end
 
     context 'when change contains QA category' do
-      it 'assigns QA reviewer and sets optional QA maintainer' do
-        categories = [:qa]
-        spins = subject.spin(project, categories, branch_name)
+      let(:categories) { [:qa] }
 
+      it 'assigns QA reviewer and sets optional QA maintainer' do
         expect(spins).to contain_exactly(matching_spin(:qa, reviewer: software_engineer_in_test, optional: :maintainer))
       end
     end
 
     context 'when change contains Engineering Productivity category' do
-      it 'assigns Engineering Productivity reviewer and fallback to backend maintainer' do
-        categories = [:engineering_productivity]
-        spins = subject.spin(project, categories, branch_name)
+      let(:categories) { [:engineering_productivity] }
 
+      it 'assigns Engineering Productivity reviewer and fallback to backend maintainer' do
         expect(spins).to contain_exactly(matching_spin(:engineering_productivity, reviewer: engineering_productivity_reviewer, maintainer: backend_maintainer))
       end
     end
 
     context 'when change contains test category' do
-      it 'assigns corresponding SET and sets optional test maintainer' do
-        categories = [:test]
-        spins = subject.spin(project, categories, branch_name)
+      let(:categories) { [:test] }
 
+      it 'assigns corresponding SET and sets optional test maintainer' do
         expect(spins).to contain_exactly(matching_spin(:test, reviewer: software_engineer_in_test, optional: :maintainer))
       end
     end
@@ -217,20 +244,13 @@ describe Gitlab::Danger::Roulette do
   end
 
   describe '#spin_for_person' do
-    let(:person1) { Gitlab::Danger::Teammate.new('username' => 'rymai') }
-    let(:person2) { Gitlab::Danger::Teammate.new('username' => 'godfat') }
-    let(:author) { Gitlab::Danger::Teammate.new('username' => 'filipa') }
-    let(:ooo) { Gitlab::Danger::Teammate.new('username' => 'jacopo-beschi') }
-    let(:no_capacity) { Gitlab::Danger::Teammate.new('username' => 'uncharged') }
+    let(:person1) { Gitlab::Danger::Teammate.new('username' => 'rymai', 'available' => true, 'has_capacity' => true) }
+    let(:person2) { Gitlab::Danger::Teammate.new('username' => 'godfat', 'available' => true, 'has_capacity' => true) }
+    let(:author) { Gitlab::Danger::Teammate.new('username' => 'filipa', 'available' => true, 'has_capacity' => true) }
+    let(:ooo) { Gitlab::Danger::Teammate.new('username' => 'jacopo-beschi', 'available' => false, 'has_capacity' => true) }
+    let(:no_capacity) { Gitlab::Danger::Teammate.new('username' => 'uncharged', 'available' => true, 'has_capacity' => false) }
 
     before do
-      stub_person_status(person1, message: 'making GitLab magic')
-      stub_person_status(person2, message: 'making GitLab magic')
-      stub_person_status(ooo, message: 'OOO till 15th')
-      stub_person_status(no_capacity, message: 'At capacity for the next few days', emoji: 'red_circle')
-      # we don't stub Filipa, as she is the author and
-      # we should not fire request checking for her
-
       allow(subject).to receive_message_chain(:gitlab, :mr_author).and_return(author.username)
     end
 
@@ -254,14 +274,4 @@ describe Gitlab::Danger::Roulette do
       expect(subject.spin_for_person([no_capacity], random: Random.new)).to be_nil
     end
   end
-
-  private
-
-  def stub_person_status(person, message: 'dummy message', emoji: 'unicorn')
-    body = { message: message, emoji: emoji }.to_json
-
-    WebMock
-      .stub_request(:get, "https://gitlab.com/api/v4/users/#{person.username}/status")
-      .to_return(body: body)
-  end
 end

spec/lib/gitlab/danger/teammate_spec.rb

@@ -114,79 +114,4 @@ describe Gitlab::Danger::Teammate do
       expect(subject.maintainer?(project, :frontend, labels)).to be_falsey
     end
   end
-
-  describe '#status' do
-    let(:capabilities) { ['dish washing'] }
-
-    context 'with empty cache' do
-      context 'for successful request' do
-        it 'returns the response' do
-          mock_status = double(does_not: 'matter')
-          expect(Gitlab::Danger::RequestHelper).to receive(:http_get_json)
-                                                       .and_return(mock_status)
-
-          expect(subject.status).to be mock_status
-        end
-      end
-
-      context 'for failing request' do
-        it 'returns nil' do
-          expect(Gitlab::Danger::RequestHelper).to receive(:http_get_json)
-                                                       .and_raise(Gitlab::Danger::RequestHelper::HTTPError.new)
-
-          expect(subject.status).to be nil
-        end
-      end
-    end
-
-    context 'with filled cache' do
-      it 'returns the cached response' do
-        mock_status = double(does_not: 'matter')
-        expect(Gitlab::Danger::RequestHelper).to receive(:http_get_json)
-                                                     .and_return(mock_status)
-        subject.status
-
-        expect(Gitlab::Danger::RequestHelper).not_to receive(:http_get_json)
-        expect(subject.status).to be mock_status
-      end
-    end
-  end
-
-  describe '#available?' do
-    using RSpec::Parameterized::TableSyntax
-
-    let(:capabilities) { ['dry head'] }
-
-    where(:status, :result) do
-      {}                               | true
-      { message: 'dear reader' }       | true
-      { message: 'OOO: massage' }      | false
-      { message: 'love it SOOO much' } | false
-      { emoji: 'red_circle' }          | false
-      { emoji: 'palm_tree' }           | false
-      { emoji: 'beach' }               | false
-      { emoji: 'beach_umbrella' }      | false
-      { emoji: 'beach_with_umbrella' } | false
-      { emoji: nil }                   | true
-      { emoji: '' }                    | true
-      { emoji: 'dancer' }              | true
-    end
-
-    with_them do
-      before do
-        expect(Gitlab::Danger::RequestHelper).to receive(:http_get_json)
-                                                     .and_return(status&.stringify_keys)
-      end
-
-      it { expect(subject.available?).to be result }
-    end
-
-    it 'returns true if request fails' do
-      expect(Gitlab::Danger::RequestHelper)
-        .to receive(:http_get_json)
-        .and_raise(Gitlab::Danger::RequestHelper::HTTPError.new)
-
-      expect(subject.available?).to be true
-    end
-  end
 end