Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
8 changes: 8 additions & 0 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,6 +222,11 @@
subject
expect(response.body).to include(pull_request_size_metric_definition.explanation)
end

it 'render review coverage metric tooltip' do
subject
expect(response.body).to include(review_coverage_metric_definition.explanation)
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