Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions app/controllers/development_metrics_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def build_overall_calculations(entity_name)
@merge_time_success_rate = merge_time[:success_rate]
@merge_time_avg = merge_time[:avg]
@pull_request_size_avg = @pull_request_size[key].first[:avg]
@review_coverage_avg = @review_coverage[key].first[:avg] if @review_coverage.present?
end

def build_metrics(entity_id, entity_name)
Expand All @@ -60,6 +61,7 @@ def build_metrics(entity_id, entity_name)
.call(entity_id, @from, @to)
@merge_time = metrics[:merge_time]
@pull_request_size = metrics[:pull_request_size]
@review_coverage = metrics[:review_coverage]
end

def build_product_metrics(entity_id, entity_name)
Expand All @@ -84,6 +86,7 @@ def build_metrics_definitions
@review_turnaround_definition = MetricDefinition.find_by(code: :review_turnaround)
@merge_time_definition = MetricDefinition.find_by(code: :merge_time)
@pull_request_size_definition = MetricDefinition.find_by(code: :pull_request_size)
@review_coverage_definition = MetricDefinition.find_by(code: :review_coverage)
end

def set_metrics_to_show
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module PullRequests
class ReviewCoveragePrsRepositoryController < PullRequestsController
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a fan of using inheritance for controllers but ok

def repository
Builders::Distribution::PullRequests::ReviewCoverageRepository
end
end
end
4 changes: 4 additions & 0 deletions app/helpers/modal_see_detail_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ def build_bar_chart_modal(repository_name, from, to)
'pull-request-size': {
url: repository_pull_request_size_prs_repository_index_path(repository_name, metrics),
metric: 'lines'
},
'review-coverage': {
url: repository_review_coverage_prs_repository_index_path(repository_name, metrics),
metric: '%'
}
}

Expand Down
3 changes: 2 additions & 1 deletion app/services/builders/chartkick/development_metrics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ def entities_by_metric
metrics = {
review_turnaround: %w[repository users_repository repository_distribution],
merge_time: %w[repository users_repository repository_distribution],
pull_request_size: %w[repository_distribution]
pull_request_size: %w[repository_distribution],
review_coverage: %w[repository repository_distribution]
}
metrics
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
module Builders
module Chartkick
module RepositoryDistributionDataMetrics
class ReviewCoverage
def retrieve_records(entity_id:, time_range:)
::ReviewCoverage
.joins(pull_request: :repository)
.where(repositories: { id: entity_id })
.where(events_pull_requests: { merged_at: time_range })
.where.not(events_pull_requests: { owner: User.ignored_users })
end

def resolve_interval(entity)
Metrics::IntervalResolver::Percentage.call(entity.coverage_percentage * 100)
end

def value_for_average(entity)
entity.coverage_percentage * 100
end
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def pr_sizes
.where.not(
events_pull_requests: { html_url: nil }
)
.where.not(owner: User.ignored_users)
.where.not(size: nil)
.order(:size)
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
module Builders
module Distribution
module PullRequests
class ReviewCoverageRepository < BaseService
def initialize(repository_name:, from:, to:)
@repository_name = repository_name
@from = from.to_datetime.beginning_of_day
@to = to.to_datetime.end_of_day
end

def call
review_coverages.each_with_object(hash_of_arrays) { |review_coverage, hash|
coverage = review_coverage.coverage_percentage * 100
interval = Metrics::IntervalResolver::Percentage.call(coverage)
pr_values = { html_url: review_coverage.pull_request.html_url, value: coverage }
hash[interval] << pr_values
}.sort.to_h
end

private

def review_coverages
@review_coverages ||= ::ReviewCoverage
.joins(pull_request: :repository)
.where(repositories: { name: @repository_name })
.where(pull_request: { merged_at: @from..@to })
.where.not(pull_request: { owner: User.ignored_users })
.order(:coverage_percentage)
end

def hash_of_arrays
Hash.new { |hash, key| hash[key] = [] }
end
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ def merge_times
.joins(pull_request: :repository)
.where(repositories: { name: @repository_name })
.where.not(events_pull_requests: { html_url: nil })
.where.not(
events_pull_requests: { owner: User.ignored_users }
)
.includes(:pull_request)
.order(:value)
end
Expand Down
9 changes: 9 additions & 0 deletions app/services/metrics/interval_resolver/percentage.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Metrics
module IntervalResolver
class Percentage < Metrics::IntervalResolver::Base
def ranges
{ '0-10': 10, '10-20': 20, '20-40': 40, '40-60': 60, '60-80': 80 }
end
end
end
end
24 changes: 24 additions & 0 deletions app/services/metrics/review_coverage/per_repository.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
module Metrics
module ReviewCoverage
class PerRepository < Metrics::Base
private

def process
week_intervals.flat_map do |week|
interval = build_interval(week)
query(interval).map do |repository_id, metric_value|
Metric.new(repository_id, interval.first, metric_value)
end
end
end

def query(interval)
Repository.joins(pull_requests: :review_coverage)
.where(events_pull_requests: { merged_at: interval })
.where(id: @entity_id)
.group(:id)
.pluck(:id, Arel.sql('AVG(review_coverages.coverage_percentage)'))
end
end
end
end
47 changes: 43 additions & 4 deletions app/views/development_metrics/repositories.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
review_turnaround_presenter = RepositoryMetricPresenter.new(@review_turnaround, @review_turnaround_definition)
merge_time_presenter = RepositoryMetricPresenter.new(@merge_time, @merge_time_definition)
pull_request_size_presenter = RepositoryMetricPresenter.new(@pull_request_size, @pull_request_size_definition)
review_coverage_presenter = RepositoryMetricPresenter.new(@review_coverage, @review_coverage_definition)
%>

<div class="metrics-container">
Expand Down Expand Up @@ -103,13 +104,51 @@
<% end %>
</div>
</div>
<div class="col-md-1">
<div class="distribution-report-button mr-5">
<%= link_to 'See details',
<div class="col-md-1">
<div class="distribution-report-button mr-5">
<%= link_to 'See details',
repository_pull_request_size_prs_repository_index_path(@repository.name, { metric: { from: params&.dig(:metric, :from), to: params&.dig(:metric, :to) }}),
class: 'btn btn-detail' if pull_request_size_presenter.per_repository_distribution_has_data_to_display?
%>
%>
</div>
</div>
</div>
</div>
<div class="metrics">
<div class='row'>
<div class="col-md-11">
<div class="box-title" >
<%= review_coverage_presenter.metric_name %>
<span data-placement='right' class='metric_tooltip' aria-hidden='true' data-toggle='tooltip' title='<%= review_coverage_presenter.metric_explanation %>'>¡</span>
</div>
<div class="box-title" >
<% if @review_coverage_avg.present? %>
<span class="badge badge-info success_rate_btn">
Average: <%= @review_coverage_avg[:avg_number] %>%
</span>
<span class="badge badge-info success_rate_btn">
Total: <%= @review_coverage_avg[:total] %> PRs (bots excluded)
</span>
<% end %>
</div>
<div class="graph">
<% if review_coverage_presenter.per_repository_distribution_has_data_to_display? %>
<%= render 'development_metrics/review_coverage/main_metric', review_coverage: review_coverage_presenter.metric %>
<% else %>
<div class="p-3 text-center">
<span class="text-danger extra-padding"><h5>No data to display for the selected period</h5></span>
</div>
<% end %>
</div>
</div>
<div class="col-md-1">
<div class="distribution-report-button mr-5">
<%= link_to 'See details',
repository_review_coverage_prs_repository_index_path(@repository.name, { metric: { from: params&.dig(:metric, :from), to: params&.dig(:metric, :to) }}),
class: 'btn btn-detail' if review_coverage_presenter.per_repository_distribution_has_data_to_display?
%>
</div>
</div>
</div>
</div>
<% else %>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<%= render partial: 'shared/column-chart',
locals: {
chart_id: 'review-coverage',
metric_name: 'Distribution',
suffix: " pull requests",
metric: review_coverage[:per_repository_distribution]
} if review_coverage && review_coverage[:per_repository_distribution] %>
17 changes: 17 additions & 0 deletions app/views/pull_requests/review_coverage_prs_repository/index.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<%= render 'development_metrics/development_metrics_sidebar', enabled_users_section: @enabled_users_section %>
<div class="metrics-container">
<div class="row title_sidebar">
<div class="col-md-6">
<b class="rute_title_grey"><%= @repository_name %>/</b>
<b class="rute_title">Review Coverage</b>
</div>
<div class="col-md-6 interval d-flex flex-row-reverse">
<%= simple_form_for :metric, url: repository_review_coverage_prs_repository_index_path, method: 'GET', html: {
id: 'nav-filter-form', style:"display: flex;"} do |f|%>
<%= hidden_field_tag 'repository_name', @repository_name %>
<%= render 'shared/interval-filter', form: f %>
<% end %>
</div>
</div>
<%= render partial: 'shared/pull_request_list' , locals: { pull_requests: @pull_requests, kpi_label: '%' } %>
</div>
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
resources :time_to_merge_prs_repository, only: :index, module: :pull_requests
resources :time_to_second_review_prs_repository, only: :index, module: :pull_requests
resources :pull_request_size_prs_repository, only: :index, module: :pull_requests
resources :review_coverage_prs_repository, only: :index, module: :pull_requests
end
resources :users, only: [] do
resources :repositories, only: :index, controller: 'users/repositories'
Expand Down
17 changes: 7 additions & 10 deletions spec/controllers/development_metrics_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
let!(:pull_request_size_metric_definition) do
create(:metric_definition, code: :pull_request_size)
end
let!(:review_coverage_metric_definition) do
create(:metric_definition, code: :review_coverage)
end

describe '#index' do
context 'when metric params are empty' do
Expand Down Expand Up @@ -219,17 +222,11 @@
subject
expect(response.body).to include(pull_request_size_metric_definition.explanation)
end
end
end

context '#departments' do
before { params[:department_name] = repository.language.department.name }

it 'calls CodeClimate summary retriever class' do
expect(CodeClimate::RepositoriesSummaryService)
.to receive(:call).and_return(code_climate_metric)

get :departments, params: params
it 'render review coverage metric tooltip' do
subject
expect(response.body).to include(review_coverage_metric_definition.explanation)
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/factories/events/pull_requests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
size { Faker::Number.within(range: 0..10_000) }
state { 'open' }
html_url { 'https://github.com/Codertocat/Hello-World/pull/2' }
opened_at { Faker::Date.between(from: 1.month.ago, to: Time.zone.now) }
opened_at { Faker::Time.between(from: 1.month.ago, to: Time.zone.now) }
locked { false }
draft { false }
repository
Expand Down
Loading
Loading