Merge branch 'winh-teammate-out-of-office' into 'master'

Move out_of_office? from reviewer roulette to Teammate class

See merge request gitlab-org/gitlab!17504

Dangerfile

 # frozen_string_literal: true
 
 require_relative 'lib/gitlab_danger'
+require_relative 'lib/gitlab/danger/request_helper'
 
 danger.import_plugin('danger/plugins/helper.rb')
 danger.import_plugin('danger/plugins/roulette.rb')

lib/gitlab/danger/request_helper.rb

+# frozen_string_literal: true
+
+require 'net/http'
+require 'json'
+
+module Gitlab
+  module Danger
+    module RequestHelper
+      HTTPError = Class.new(RuntimeError)
+
+      # @param [String] url
+      def self.http_get_json(url)
+        rsp = Net::HTTP.get_response(URI.parse(url))
+
+        unless rsp.is_a?(Net::HTTPOK)
+          raise HTTPError, "Failed to read #{url}: #{rsp.code} #{rsp.message}"
+        end
+
+        JSON.parse(rsp.body)
+      end
+    end
+  end
+end

lib/gitlab/danger/roulette.rb

 # frozen_string_literal: true
 
-require 'net/http'
-require 'json'
-require 'cgi'
-
 require_relative 'teammate'
 
 module Gitlab
   module Danger
     module Roulette
       ROULETTE_DATA_URL = 'https://about.gitlab.com/roulette.json'
-      HTTPError = Class.new(RuntimeError)
 
       # Looks up the current list of GitLab team members and parses it into a
       # useful form
@@ -19,7 +14,7 @@ module Gitlab
       def team
         @team ||=
           begin
-            data = http_get_json(ROULETTE_DATA_URL)
+            data = Gitlab::Danger::RequestHelper.http_get_json(ROULETTE_DATA_URL)
             data.map { |hash| ::Gitlab::Danger::Teammate.new(hash) }
           rescue JSON::ParserError
             raise "Failed to parse JSON response from #{ROULETTE_DATA_URL}"
@@ -44,6 +39,7 @@ module Gitlab
 
       # Known issue: If someone is rejected due to OOO, and then becomes not OOO, the
       # selection will change on next spin
+      # @param [Array<Teammate>] people
       def spin_for_person(people, random:)
         people.shuffle(random: random)
           .find(&method(:valid_person?))
@@ -51,32 +47,17 @@ module Gitlab
 
       private
 
+      # @param [Teammate] person
+      # @return [Boolean]
       def valid_person?(person)
-        !mr_author?(person) && !out_of_office?(person)
+        !mr_author?(person) && !person.out_of_office?
       end
 
+      # @param [Teammate] person
+      # @return [Boolean]
       def mr_author?(person)
         person.username == gitlab.mr_author
       end
-
-      def out_of_office?(person)
-        username = CGI.escape(person.username)
-        api_endpoint = "https://gitlab.com/api/v4/users/#{username}/status"
-        response = http_get_json(api_endpoint)
-        response["message"]&.match?(/OOO/i)
-      rescue HTTPError, JSON::ParserError
-        false # this is no worse than not checking for OOO
-      end
-
-      def http_get_json(url)
-        rsp = Net::HTTP.get_response(URI.parse(url))
-
-        unless rsp.is_a?(Net::HTTPSuccess)
-          raise HTTPError, "Failed to read #{url}: #{rsp.code} #{rsp.message}"
-        end
-
-        JSON.parse(rsp.body)
-      end
     end
   end
 end

lib/gitlab/danger/teammate.rb

 # frozen_string_literal: true
 
+require 'cgi'
+
 module Gitlab
   module Danger
     class Teammate
@@ -34,6 +36,18 @@ module Gitlab
         has_capability?(project, category, :maintainer, labels)
       end
 
+      def status
+        api_endpoint = "https://gitlab.com/api/v4/users/#{CGI.escape(username)}/status"
+        @status ||= Gitlab::Danger::RequestHelper.http_get_json(api_endpoint)
+      rescue Gitlab::Danger::RequestHelper::HTTPError, JSON::ParserError
+        nil # better no status than a crashing Danger
+      end
+
+      # @return [Boolean]
+      def out_of_office?
+        status&.dig("message")&.match?(/OOO/i) || false
+      end
+
       private
 
       def has_capability?(project, category, kind, labels)

spec/lib/gitlab/danger/teammate_spec.rb

@@ -2,11 +2,13 @@
 
 require 'fast_spec_helper'
 
+require 'rspec-parameterized'
+
 require 'gitlab/danger/teammate'
 
 describe Gitlab::Danger::Teammate do
-  subject { described_class.new(options) }
-  let(:options) { { 'projects' => projects, 'role' => role } }
+  subject { described_class.new(options.stringify_keys) }
+  let(:options) { { username: 'luigi', projects: projects, role: role } }
   let(:projects) { { project => capabilities } }
   let(:role) { 'Engineer, Manage' }
   let(:labels) { [] }
@@ -95,4 +97,64 @@ 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 '#out_of_office?' do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:capabilities) { ['dry head'] }
+
+    where(:status, :result) do
+      nil                              | false
+      {}                               | false
+      { message: 'dear reader' }       | false
+      { message: 'OOO: massage' }      | true
+      { message: 'love it SOOO much' } | 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.out_of_office?).to be result }
+    end
+  end
 end