Commit 88383160 authored by Andy Soiron's avatar Andy Soiron Committed by Alex Kalderimis

Refactor JiraConnect dev info

This refactor aims to improve testability for the dev info call.
It doesn't change the logic.
parent 35f1dd93
...@@ -14,14 +14,14 @@ module Atlassian ...@@ -14,14 +14,14 @@ module Atlassian
def send_info(project:, update_sequence_id: nil, **args) def send_info(project:, update_sequence_id: nil, **args)
common = { project: project, update_sequence_id: update_sequence_id } 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) build_info = args.slice(:pipelines)
deploy_info = args.slice(:deployments) deploy_info = args.slice(:deployments)
ff_info = args.slice(:feature_flags) ff_info = args.slice(:feature_flags)
responses = [] 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_build_info(**common, **build_info) if build_info.present?
responses << store_deploy_info(**common, **deploy_info) if deploy_info.present? responses << store_deploy_info(**common, **deploy_info) if deploy_info.present?
responses << store_ff_info(**common, **ff_info) if ff_info.present? responses << store_ff_info(**common, **ff_info) if ff_info.present?
...@@ -93,17 +93,8 @@ module Atlassian ...@@ -93,17 +93,8 @@ module Atlassian
handle_response(r, 'builds') { |data| errors(data, 'rejectedBuilds') } handle_response(r, 'builds') { |data| errors(data, 'rejectedBuilds') }
end end
def store_dev_info(project:, commits: nil, branches: nil, merge_requests: nil, update_sequence_id: nil) def store_dev_info(dev_info)
repo = ::Atlassian::JiraConnect::Serializers::RepositoryEntity.represent( post(dev_info.url, dev_info.body)
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] })
end end
def post(path, payload) def post(path, payload)
...@@ -157,14 +148,6 @@ module Atlassian ...@@ -157,14 +148,6 @@ module Atlassian
{ 'errorMessages' => messages } { 'errorMessages' => messages }
end 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) def jwt_token(http_method, uri)
claims = Atlassian::Jwt.build_claims( claims = Atlassian::Jwt.build_claims(
Atlassian::JiraConnect.app_key, Atlassian::JiraConnect.app_key,
......
# 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
{
"repositories": {
"type": "array",
"items": {
"$ref": "./repository.json"
}
}
}
...@@ -58,12 +58,16 @@ RSpec.describe Atlassian::JiraConnect::Client do ...@@ -58,12 +58,16 @@ RSpec.describe Atlassian::JiraConnect::Client do
deployments: :q deployments: :q
).and_return(:deploys_stored) ).and_return(:deploys_stored)
expect(subject).to receive(:store_dev_info).with( expect(Atlassian::JiraConnect::DevInfo).to receive(:new).with(
project: project, project: project,
update_sequence_id: :x, update_sequence_id: :x,
commits: :a, commits: :a,
branches: :b, branches: :b,
merge_requests: :c merge_requests: :c
).and_call_original
expect(subject).to receive(:store_dev_info).with(
instance_of(Atlassian::JiraConnect::DevInfo)
).and_return(:dev_stored) ).and_return(:dev_stored)
args = { args = {
...@@ -83,9 +87,7 @@ RSpec.describe Atlassian::JiraConnect::Client do ...@@ -83,9 +87,7 @@ RSpec.describe Atlassian::JiraConnect::Client do
it 'only calls methods that we need to call' do it 'only calls methods that we need to call' do
expect(subject).to receive(:store_dev_info).with( expect(subject).to receive(:store_dev_info).with(
project: project, instance_of(Atlassian::JiraConnect::DevInfo)
update_sequence_id: :x,
commits: :a
).and_return(:dev_stored) ).and_return(:dev_stored)
args = { args = {
...@@ -402,15 +404,7 @@ RSpec.describe Atlassian::JiraConnect::Client do ...@@ -402,15 +404,7 @@ RSpec.describe Atlassian::JiraConnect::Client do
end end
it "calls the API with auth headers" do it "calls the API with auth headers" do
subject.send(:store_dev_info, project: project) subject.send(:store_dev_info, Atlassian::JiraConnect::DevInfo.new(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)
end end
end end
end end
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment