Merge branch 'andysoiron/refactor-jira-connect-dev-info' into 'master'

Refactor JiraConnect dev info

See merge request gitlab-org/gitlab!82201

lib/atlassian/jira_connect/client.rb

@@ -14,14 +14,14 @@ module Atlassian
 
       def send_info(project:, update_sequence_id: nil, **args)
         common = { project: project, update_sequence_id: update_sequence_id }
-        dev_info = args.slice(:commits, :branches, :merge_requests)
+        dev_info = DevInfo.new(**common.merge(args.slice(:commits, :branches, :merge_requests)))
         build_info = args.slice(:pipelines)
         deploy_info = args.slice(:deployments)
         ff_info = args.slice(:feature_flags)
 
         responses = []
 
-        responses << store_dev_info(**common, **dev_info) if dev_info.present?
+        responses << store_dev_info(dev_info) if dev_info.present?
         responses << store_build_info(**common, **build_info) if build_info.present?
         responses << store_deploy_info(**common, **deploy_info) if deploy_info.present?
         responses << store_ff_info(**common, **ff_info) if ff_info.present?
@@ -93,17 +93,8 @@ module Atlassian
         handle_response(r, 'builds') { |data| errors(data, 'rejectedBuilds') }
       end
 
-      def store_dev_info(project:, commits: nil, branches: nil, merge_requests: nil, update_sequence_id: nil)
-        repo = ::Atlassian::JiraConnect::Serializers::RepositoryEntity.represent(
-          project,
-          commits: commits,
-          branches: branches,
-          merge_requests: merge_requests,
-          user_notes_count: user_notes_count(merge_requests),
-          update_sequence_id: update_sequence_id
-        )
-
-        post('/rest/devinfo/0.10/bulk', { repositories: [repo] })
+      def store_dev_info(dev_info)
+        post(dev_info.url, dev_info.body)
       end
 
       def post(path, payload)
@@ -157,14 +148,6 @@ module Atlassian
         { 'errorMessages' => messages }
       end
 
-      def user_notes_count(merge_requests)
-        return unless merge_requests
-
-        Note.count_for_collection(merge_requests.map(&:id), 'MergeRequest').to_h do |count_group|
-          [count_group.noteable_id, count_group.count]
-        end
-      end
-
       def jwt_token(http_method, uri)
         claims = Atlassian::Jwt.build_claims(
           Atlassian::JiraConnect.app_key,

lib/atlassian/jira_connect/dev_info.rb

+# frozen_string_literal: true
+
+module Atlassian
+  module JiraConnect
+    class DevInfo
+      URL = '/rest/devinfo/0.10/bulk'
+
+      def initialize(project:, commits: nil, branches: nil, merge_requests: nil, update_sequence_id: nil)
+        @project = project
+        @commits = commits
+        @branches = branches
+        @merge_requests = merge_requests
+        @update_sequence_id = update_sequence_id
+      end
+
+      def url
+        URL
+      end
+
+      def body
+        repo = ::Atlassian::JiraConnect::Serializers::RepositoryEntity.represent(
+          @project,
+          commits: @commits,
+          branches: @branches,
+          merge_requests: @merge_requests,
+          user_notes_count: user_notes_count,
+          update_sequence_id: @update_sequence_id
+        )
+
+        { repositories: [repo] }
+      end
+
+      def present?
+        [@commits, @branches, @merge_requests].any?(&:present?)
+      end
+
+      private
+
+      def user_notes_count
+        return unless @merge_requests
+
+        Note.count_for_collection(@merge_requests.map(&:id), 'MergeRequest').to_h do |count_group|
+          [count_group.noteable_id, count_group.count]
+        end
+      end
+    end
+  end
+end

spec/fixtures/api/schemas/jira_connect/dev_info.json

+{
+  "repositories": {
+    "type": "array",
+    "items": {
+      "$ref": "./repository.json"
+    }
+  }
+}

spec/lib/atlassian/jira_connect/client_spec.rb

@@ -58,12 +58,16 @@ RSpec.describe Atlassian::JiraConnect::Client do
         deployments: :q
       ).and_return(:deploys_stored)
 
-      expect(subject).to receive(:store_dev_info).with(
+      expect(Atlassian::JiraConnect::DevInfo).to receive(:new).with(
         project: project,
         update_sequence_id: :x,
         commits: :a,
         branches: :b,
         merge_requests: :c
+      ).and_call_original
+
+      expect(subject).to receive(:store_dev_info).with(
+        instance_of(Atlassian::JiraConnect::DevInfo)
       ).and_return(:dev_stored)
 
       args = {
@@ -83,9 +87,7 @@ RSpec.describe Atlassian::JiraConnect::Client do
 
     it 'only calls methods that we need to call' do
       expect(subject).to receive(:store_dev_info).with(
-        project: project,
-        update_sequence_id: :x,
-        commits: :a
+        instance_of(Atlassian::JiraConnect::DevInfo)
       ).and_return(:dev_stored)
 
       args = {
@@ -402,15 +404,7 @@ RSpec.describe Atlassian::JiraConnect::Client do
     end
 
     it "calls the API with auth headers" do
-      subject.send(:store_dev_info, project: project)
-    end
-
-    it 'avoids N+1 database queries' do
-      control_count = ActiveRecord::QueryRecorder.new { subject.send(:store_dev_info, project: project, merge_requests: merge_requests) }.count
-
-      merge_requests << create(:merge_request, :unique_branches)
-
-      expect { subject.send(:store_dev_info, project: project, merge_requests: merge_requests) }.not_to exceed_query_limit(control_count)
+      subject.send(:store_dev_info, Atlassian::JiraConnect::DevInfo.new(project: project))
     end
   end
 end

spec/lib/atlassian/jira_connect/dev_info_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Atlassian::JiraConnect::DevInfo do
+  let_it_be(:project) { create_default(:project, :repository).freeze }
+
+  let(:update_sequence_id) { '123' }
+
+  describe '#url' do
+    subject { described_class.new(project: project).url }
+
+    it { is_expected.to eq('/rest/devinfo/0.10/bulk') }
+  end
+
+  describe '#body' do
+    let_it_be(:merge_request) { create(:merge_request, :unique_branches, title: 'TEST-123') }
+    let_it_be(:note) { create(:note, noteable: merge_request, project: merge_request.project) }
+    let_it_be(:branches) do
+      project.repository.create_branch('TEST-123', project.default_branch_or_main)
+      [project.repository.find_branch('TEST-123')]
+    end
+
+    let(:merge_requests) { [merge_request] }
+
+    subject(:body) { described_class.new(project: project, branches: branches, merge_requests: merge_requests, update_sequence_id: update_sequence_id).body.to_json }
+
+    it 'matches the schema' do
+      expect(body).to match_schema('jira_connect/dev_info')
+    end
+
+    it 'avoids N+1 database queries' do
+      control_count = ActiveRecord::QueryRecorder.new { subject }.count
+
+      merge_requests << create(:merge_request, :unique_branches)
+
+      expect { subject }.not_to exceed_query_limit(control_count)
+    end
+  end
+
+  describe '#present?' do
+    let(:arguments) { { commits: nil, branches: nil, merge_requests: nil } }
+
+    subject { described_class.new(**{ project: project, update_sequence_id: update_sequence_id }.merge(arguments)).present? }
+
+    it { is_expected.to eq(false) }
+
+    context 'with commits, branches or merge requests' do
+      let(:arguments) { { commits: anything, branches: anything, merge_requests: anything } }
+
+      it { is_expected.to eq(true) }
+    end
+  end
+end