Add zero comments percetage

app/controllers/development_metrics_controller.rb

@@ -49,11 +49,12 @@ def products_metrics
   def build_overall_calculations(entity_name)
     key = "per_#{entity_name.downcase}_distribution".to_sym
 
-    merge_time = @merge_time[key].first
-    @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?
+    @merge_time_interval_metrics = @merge_time[key].first[:interval_metrics]
+    @pull_request_size_interval_metrics = @pull_request_size[key].first[:interval_metrics]
+
+    return if @review_coverage.blank?
+
+    @review_coverage_interval_metrics = @review_coverage[key].first[:interval_metrics]
   end
 
   def build_metrics(entity_id, entity_name)

app/models/review_coverage.rb

@@ -29,4 +29,6 @@ class ReviewCoverage < ApplicationRecord
   validates :pull_request_id, uniqueness: true
   validates :total_files_changed, :files_with_comments_count,
             numericality: { greater_than_or_equal_to: 0 }
+
+  scope :with_zero_coverage, -> { where(coverage_percentage: 0) }
 end

app/services/alerts_service.rb

@@ -40,7 +40,7 @@ def build_success_rates(metric_name, entity_id, entity_name)
 
     data = distribution_data.first
 
-    success_rate = data[:success_rate] if data.present?
+    success_rate = data[:interval_metrics][:success_rate] if data.present?
 
     success_rate[:rate] if success_rate
   end

app/services/builders/chartkick/repository_distribution_data.rb

@@ -2,13 +2,10 @@ module Builders
   module Chartkick
     class RepositoryDistributionData < Builders::Chartkick::Base
       def call
-        intervals = build_distribution_data(retrieve_records)
-
         [{
           name: repository_name,
           data: intervals,
-          success_rate: build_success_rate(repository_name, metric_name, intervals),
-          avg: build_average
+          interval_metrics: build_interval_metrics
         }]
       end
 
@@ -34,20 +31,41 @@ def metric
         @metric ||= RepositoryDistributionDataMetrics.const_get(metric_name.to_s.camelize).new
       end
 
+      def intervals
+        @intervals ||= build_distribution_data(retrieve_records)
+      end
+
       def resolve_interval(entity)
         metric.resolve_interval(entity)
       end
 
-      def build_average
-        return if retrieve_records.empty?
+      def total_value
+        @total_value ||= retrieve_records.sum { |record| metric.value_for_average(record) }
+      end
+
+      def total_records
+        @total_records ||= retrieve_records.size
+      end
+
+      def success_rate
+        build_success_rate(repository_name, metric_name, intervals)
+      end
 
-        total_value = retrieve_records.sum { |record| metric.value_for_average(record) }
-        total_records = retrieve_records.count
+      def build_interval_metrics
+        return if retrieve_records.empty?
 
-        {
+        interval_data = {
+          success_rate: success_rate,
           avg_number: (total_value.to_f / total_records).round(1),
           total: total_records
         }
+
+        if metric_name == :review_coverage
+          interval_data[:zero_coverage_percentage] =
+            metric.zero_coverage_percentage(retrieve_records)
+        end
+
+        interval_data
       end
     end
   end

app/services/builders/chartkick/repository_distribution_data_metrics/review_coverage.rb

@@ -22,6 +22,15 @@ def resolve_interval(entity)
         def value_for_average(entity)
           entity.coverage_percentage * 100
         end
+
+        def zero_coverage_percentage(records)
+          total_count = records.count
+          return 0 if total_count.zero?
+
+          zero_coverage_count = records.with_zero_coverage.count
+
+          ((zero_coverage_count.to_f / total_count) * 100).round
+        end
       end
     end
   end

app/views/development_metrics/repositories.erb

@@ -47,19 +47,19 @@
         <div class='row'>
           <div class="col-md-12">
             <div class="box-title" >
-              <% if @merge_time_avg.present? %>
+              <% if @merge_time_interval_metrics.present? %>
                 <span class="badge badge-info success_rate_btn">
-                  Average: <%= @merge_time_avg[:avg_number] %> hours
+                  Average: <%= @merge_time_interval_metrics[:avg_number] %> hours
                 </span>
               <% end %>
-              <% if @merge_time_success_rate.present? %>
+              <% if @merge_time_interval_metrics.present? && @merge_time_interval_metrics[:success_rate].present? %>
                 <span class="badge badge-info success_rate_btn">
-                  Success rate: <%= @merge_time_success_rate[:rate] %>%
+                  Success rate: <%= @merge_time_interval_metrics[:success_rate][:rate] %>%
                 </span>
               <% end %>
-              <% if @merge_time_avg.present? %>
+              <% if @merge_time_interval_metrics.present? %>
                 <span class="badge badge-info success_rate_btn">
-                  Total: <%= @merge_time_avg[:total] %> PRs (bots excluded)
+                  Total: <%= @merge_time_interval_metrics[:total] %> PRs (bots excluded)
                 </span>
               <% end %>
             </div>
@@ -88,12 +88,12 @@
               <span data-placement='right' class='metric_tooltip' aria-hidden='true' data-toggle='tooltip' title='<%= pull_request_size_presenter.metric_explanation %>'>¡</span>
             </div>
             <div class="box-title" >
-              <% if @pull_request_size_avg.present? %>
+              <% if @pull_request_size_interval_metrics.present? %>
                 <span class="badge badge-info success_rate_btn">
-                  Average: <%= @pull_request_size_avg[:avg_number].round %> lines
+                  Average: <%= @pull_request_size_interval_metrics[:avg_number].round %> lines
                 </span>
                 <span class="badge badge-info success_rate_btn">
-                  Total: <%= @pull_request_size_avg[:total] %> PRs (bots excluded)
+                  Total: <%= @pull_request_size_interval_metrics[:total] %> PRs (bots excluded)
                 </span>
               <% end %>
             </div>
@@ -125,12 +125,17 @@
               <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? %>
+              <% if @review_coverage_interval_metrics.present? %>
                 <span class="badge badge-info success_rate_btn">
-                  Average: <%= @review_coverage_avg[:avg_number] %>%
+                  Average: <%= @review_coverage_interval_metrics[:avg_number] %>%
                 </span>
+                <% if @review_coverage_interval_metrics[:zero_coverage_percentage].present? %>
+                  <span class="badge badge-warning success_rate_btn">
+                    PRs with 0 Comments: <%= @review_coverage_interval_metrics[:zero_coverage_percentage] %>%
+                  </span>
+                <% end %>
                 <span class="badge badge-info success_rate_btn">
-                  Total: <%= @review_coverage_avg[:total] %> PRs (bots excluded)
+                  Total: <%= @review_coverage_interval_metrics[:total] %> PRs (bots excluded)
                 </span>
               <% end %>
             </div>

spec/presenters/metric_presenter_spec.rb

@@ -7,7 +7,11 @@
       per_users_repository: [{ name: 'horacio', data: { "2022-10-24": 3.0 } },
                              { name: 'hvilloria', data: { "2022-10-24": 5.0 } },
                              { name: 'sandro', data: { "2022-10-24": 14.0 } }],
-      per_repository_distribution: [{ name: 'forecast', data: [], success_rate: nil }]
+      per_repository_distribution: [{
+        name: 'forecast',
+        data: [],
+        interval_metrics: { success_rate: nil }
+      }]
     }
   end
   let(:metric_def) { build(:metric_definition) }

spec/services/builders/chartkick/department_distribution_data_spec.rb

@@ -24,14 +24,12 @@
         let(:metric_name) { :merge_time }
 
         it_behaves_like 'merge time data distribution'
-        it_behaves_like 'success rate'
       end
 
       context 'when name is review turnaround' do
         let(:metric_name) { :review_turnaround }
 
         it_behaves_like 'review turnaround data distribution'
-        it_behaves_like 'success rate'
       end
 
       context 'when name is pull request size' do

spec/services/builders/chartkick/repository_distribution_data_spec.rb

@@ -45,6 +45,38 @@
         it_behaves_like 'pull request size data distribution'
         it_behaves_like 'average'
       end
+
+      context 'when name is review coverage' do
+        let(:metric_name) { :review_coverage }
+
+        before do
+          pr_with_zero = create(
+            :pull_request,
+            :merged,
+            repository: repository,
+            merged_at: range.begin + 1.hour
+          )
+          pr_with_zero.review_coverage.update!(coverage_percentage: 0.0)
+
+          pr_with_coverage = create(
+            :pull_request,
+            :merged,
+            repository: repository,
+            merged_at: range.begin + 2.hours
+          )
+          pr_with_coverage.review_coverage.update!(coverage_percentage: 0.5)
+        end
+
+        it 'includes zero_coverage_percentage in interval_metrics data' do
+          result = subject.first
+          expect(result[:interval_metrics]).to include(:zero_coverage_percentage)
+        end
+
+        it 'calculates zero coverage percentage correctly' do
+          result = subject.first
+          expect(result[:interval_metrics][:zero_coverage_percentage]).to eq(50)
+        end
+      end
     end
   end
 end

spec/support/shared_examples/average.rb

@@ -15,16 +15,16 @@
     query.merge!(name: metric_name)
   end
 
-  it 'returns an array with avg key' do
-    expect(subject.first).to have_key(:avg)
+  it 'returns an array with interval_metrics key' do
+    expect(subject.first).to have_key(:interval_metrics)
   end
 
   it 'returns an array with filled value' do
-    expect(subject.first[:avg][:avg_number]).to eq(average_value)
+    expect(subject.first[:interval_metrics][:avg_number]).to eq(average_value)
   end
 
   it 'returns total number of records' do
-    expect(subject.first[:avg][:total]).to eq(values_for_average.count)
+    expect(subject.first[:interval_metrics][:total]).to eq(values_for_average.count)
   end
 
   context 'when there is an ignored user' do
@@ -42,15 +42,15 @@
     end
 
     it 'does not include ignored user in the total' do
-      expect(subject.first[:avg][:total]).to eq(values_for_average.count)
+      expect(subject.first[:interval_metrics][:total]).to eq(values_for_average.count)
     end
   end
 
   context 'when there are no records' do
     let(:values_for_average) { [] }
 
-    it 'returns nil for avg' do
-      expect(subject.first[:avg]).to be_nil
+    it 'returns nil for interval_metrics' do
+      expect(subject.first[:interval_metrics]).to be_nil
     end
   end
 end

spec/support/shared_examples/success_rate.rb

@@ -9,27 +9,27 @@
     query.merge!(name: :merge_time)
   end
 
-  it 'returns an array with success_rate key' do
-    expect(subject.first).to have_key(:success_rate)
+  it 'returns an array with interval_metrics key' do
+    expect(subject.first).to have_key(:interval_metrics)
   end
 
-  it 'returns an array with filled value' do
-    expect(subject.first[:success_rate]).to be_present
+  it 'returns success_rate nested inside interval_metrics' do
+    expect(subject.first[:interval_metrics][:success_rate]).to be_present
   end
 
   it 'returns rate value' do
-    expect(subject.first[:success_rate][:rate]).to be_present
+    expect(subject.first[:interval_metrics][:success_rate][:rate]).to be_present
   end
 
   it 'returns number of items on the first 24hs' do
-    expect(subject.first[:success_rate][:successful]).to be_present
+    expect(subject.first[:interval_metrics][:success_rate][:successful]).to be_present
   end
 
   it 'returns total of items' do
-    expect(subject.first[:success_rate][:total]).to be_present
+    expect(subject.first[:interval_metrics][:success_rate][:total]).to be_present
   end
 
   it 'returns metric settings' do
-    expect(subject.first[:success_rate][:metric_detail]).to be_present
+    expect(subject.first[:interval_metrics][:success_rate][:metric_detail]).to be_present
   end
 end