Skip to content

Commit f894bf5

Browse files
authored
Feature/review coverage (#603) (#604)
* Add review coverage model * Fix review coverage tests * Add review coverage to the dashboard * Adjust review coverage creation and error handling
1 parent 8d3345c commit f894bf5

File tree

42 files changed

+811
-136
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+811
-136
lines changed

app/controllers/development_metrics_controller.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ def build_overall_calculations(entity_name)
5252
@merge_time_success_rate = merge_time[:success_rate]
5353
@merge_time_avg = merge_time[:avg]
5454
@pull_request_size_avg = @pull_request_size[key].first[:avg]
55+
@review_coverage_avg = @review_coverage[key].first[:avg] if @review_coverage.present?
5556
end
5657

5758
def build_metrics(entity_id, entity_name)
@@ -60,6 +61,7 @@ def build_metrics(entity_id, entity_name)
6061
.call(entity_id, @from, @to)
6162
@merge_time = metrics[:merge_time]
6263
@pull_request_size = metrics[:pull_request_size]
64+
@review_coverage = metrics[:review_coverage]
6365
end
6466

6567
def build_product_metrics(entity_id, entity_name)
@@ -84,6 +86,7 @@ def build_metrics_definitions
8486
@review_turnaround_definition = MetricDefinition.find_by(code: :review_turnaround)
8587
@merge_time_definition = MetricDefinition.find_by(code: :merge_time)
8688
@pull_request_size_definition = MetricDefinition.find_by(code: :pull_request_size)
89+
@review_coverage_definition = MetricDefinition.find_by(code: :review_coverage)
8790
end
8891

8992
def set_metrics_to_show
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
module PullRequests
2+
class ReviewCoveragePrsRepositoryController < PullRequestsController
3+
def repository
4+
Builders::Distribution::PullRequests::ReviewCoverageRepository
5+
end
6+
end
7+
end

app/helpers/modal_see_detail_helper.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ def build_bar_chart_modal(repository_name, from, to)
1414
'pull-request-size': {
1515
url: repository_pull_request_size_prs_repository_index_path(repository_name, metrics),
1616
metric: 'lines'
17+
},
18+
'review-coverage': {
19+
url: repository_review_coverage_prs_repository_index_path(repository_name, metrics),
20+
metric: '%'
1721
}
1822
}
1923

app/models/events/pull_request.rb

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ class PullRequest < ApplicationRecord
5252
dependent: :destroy, inverse_of: :pull_request
5353
has_many :events, as: :handleable, dependent: :destroy
5454
has_one :merge_time, dependent: :destroy
55+
has_one :review_coverage, dependent: :destroy
5556

5657
validates :state, inclusion: { in: states.keys }
5758
validates :github_id,
@@ -65,13 +66,5 @@ class PullRequest < ApplicationRecord
6566
:locked,
6667
inclusion: { in: [true, false] }
6768
validates :github_id, uniqueness: true, strict: PullRequests::GithubUniquenessError
68-
69-
after_validation :build_merge_time, on: :update, if: :merged_at_changed?
70-
71-
private
72-
73-
def build_merge_time
74-
Builders::MergeTime.call(self)
75-
end
7669
end
7770
end

app/models/metric_definition.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ class MetricDefinition < ApplicationRecord
1919
defect_escape_rate: 'defect_escape_rate',
2020
pull_request_size: 'pull_request_size',
2121
development_cycle: 'development_cycle',
22-
planned_to_done: 'planned_to_done' }
22+
planned_to_done: 'planned_to_done',
23+
review_coverage: 'review_coverage' }
2324

2425
validates :code, uniqueness: true
2526
validates :name, presence: true, uniqueness: true

app/models/review_coverage.rb

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# == Schema Information
2+
#
3+
# Table name: review_coverages
4+
#
5+
# id :bigint not null, primary key
6+
# coverage_percentage :decimal(, ) not null
7+
# deleted_at :datetime
8+
# files_with_comments_count :integer not null
9+
# total_files_changed :integer not null
10+
# created_at :datetime not null
11+
# updated_at :datetime not null
12+
# pull_request_id :bigint not null
13+
#
14+
# Indexes
15+
#
16+
# index_review_coverages_on_pull_request_id (pull_request_id)
17+
#
18+
# Foreign Keys
19+
#
20+
# fk_rails_... (pull_request_id => events_pull_requests.id)
21+
#
22+
23+
class ReviewCoverage < ApplicationRecord
24+
acts_as_paranoid
25+
26+
belongs_to :pull_request, class_name: 'Events::PullRequest'
27+
28+
validates :total_files_changed, :files_with_comments_count, presence: true
29+
validates :pull_request_id, uniqueness: true
30+
validates :total_files_changed, :files_with_comments_count,
31+
numericality: { greater_than_or_equal_to: 0 }
32+
end

app/services/action_handlers/pull_request.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@ def open
1616
end
1717

1818
def merged
19-
@entity.update!(merged_at: Time.current)
19+
ActiveRecord::Base.transaction do
20+
@entity.update!(merged_at: Time.current)
21+
Builders::MergeTime.call(@entity)
22+
Builders::ReviewCoverage.call(@entity)
23+
end
2024
end
2125

2226
def closed

app/services/builders/chartkick/development_metrics.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,8 @@ def entities_by_metric
7171
metrics = {
7272
review_turnaround: %w[repository users_repository repository_distribution],
7373
merge_time: %w[repository users_repository repository_distribution],
74-
pull_request_size: %w[repository_distribution]
74+
pull_request_size: %w[repository_distribution],
75+
review_coverage: %w[repository repository_distribution]
7576
}
7677
metrics
7778
end
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
module Builders
2+
module Chartkick
3+
module RepositoryDistributionDataMetrics
4+
class ReviewCoverage
5+
def retrieve_records(entity_id:, time_range:)
6+
::ReviewCoverage
7+
.joins(pull_request: :repository)
8+
.where(repositories: { id: entity_id })
9+
.where(events_pull_requests: { merged_at: time_range })
10+
.where.not(events_pull_requests: { owner: User.ignored_users })
11+
end
12+
13+
def resolve_interval(entity)
14+
Metrics::IntervalResolver::Percentage.call(entity.coverage_percentage * 100)
15+
end
16+
17+
def value_for_average(entity)
18+
entity.coverage_percentage * 100
19+
end
20+
end
21+
end
22+
end
23+
end

app/services/builders/distribution/pull_requests/pull_request_size_repository.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ def pr_sizes
2626
.where.not(
2727
events_pull_requests: { html_url: nil }
2828
)
29+
.where.not(owner: User.ignored_users)
2930
.where.not(size: nil)
3031
.order(:size)
3132
end

0 commit comments

Comments
 (0)