PEO-834 | Fill rails code quality checking section

inquisition.gemspec

@@ -28,7 +28,7 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'simplecov', '~> 0.17.0'
   spec.add_development_dependency 'timecop', '~> 0.9.1'
 
-  spec.add_dependency 'active_record_doctor', '~> 1.6.0'
+  spec.add_dependency 'active_record_doctor', '~> 1.7', '>= 1.7.1'
   spec.add_dependency 'brakeman', '~> 4.6'
   spec.add_dependency 'bundler-audit', '~> 0.6'
   spec.add_dependency 'bundler-leak', '~> 0.1'

lib/inquisition.rb

@@ -11,6 +11,14 @@
 
 require 'inquisition/active_record_doctor/runner'
 require 'inquisition/active_record_doctor/issue'
+require_relative 'inquisition/active_record_doctor/unindexed_foreign_keys_runner'
+require_relative 'inquisition/active_record_doctor/extraneous_indexes_runner'
+require_relative 'inquisition/active_record_doctor/missing_foreign_keys_runner'
+require_relative 'inquisition/active_record_doctor/missing_non_null_constraint_runner'
+require_relative 'inquisition/active_record_doctor/missing_presence_validation_runner'
+require_relative 'inquisition/active_record_doctor/missing_unique_indexes_runner'
+require_relative 'inquisition/active_record_doctor/undefined_table_references_runner'
+require_relative 'inquisition/active_record_doctor/unindexed_deleted_at_runner'
 require 'inquisition/brakeman/vulnerability'
 require 'inquisition/brakeman/runner'
 require 'inquisition/rubocop'

lib/inquisition/active_record_doctor/extraneous_indexes_runner.rb

@@ -0,0 +1,17 @@
+require_relative 'runner'
+require 'active_record_doctor/tasks/extraneous_indexes'
+
+module Inquisition
+  module ActiveRecordDoctor
+    class ExtraneousIndexesRunner < ActiveRecordDoctor::Runner
+      attr_reader :issues
+      TASK = ::ActiveRecordDoctor::Tasks::ExtraneousIndexes
+
+      def run
+        TASK.run.first.each do |table, column|
+          @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK)
+        end
+      end
+    end
+  end
+end

lib/inquisition/active_record_doctor/issue.rb

@@ -11,7 +11,7 @@ def to_h
         {
           severity: Severity::LOW,
           message: create_message,
-          context: task.to_s.split('::').last
+          context: task.demodulize
         }
       end
 
@@ -20,7 +20,7 @@ def to_h
       attr_reader :task, :table, :column
 
       def create_message
-        issue_text = task.to_s.split('::').last.split(/(?=[A-Z])/).map(&:downcase).join(' ')
+        issue_text = task.demodulize.gsub(/([a-z]+)([A-Z])/, '\1 \2').downcase
         "#{table} has #{issue_text}, details: #{column ? column.join(', ') : 'n/a'}"
       end
     end

lib/inquisition/active_record_doctor/missing_foreign_keys_runner.rb

@@ -0,0 +1,17 @@
+require_relative 'runner'
+require 'active_record_doctor/tasks/missing_foreign_keys'
+
+module Inquisition
+  module ActiveRecordDoctor
+    class MissingForeignKeysRunner < ActiveRecordDoctor::Runner
+      attr_reader :issues
+      TASK = ::ActiveRecordDoctor::Tasks::MissingForeignKeys
+
+      def run
+        TASK.run.first.each do |table, column|
+          @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK)
+        end
+      end
+    end
+  end
+end

lib/inquisition/active_record_doctor/missing_non_null_constraint_runner.rb

@@ -0,0 +1,17 @@
+require_relative 'runner'
+require 'active_record_doctor/tasks/missing_non_null_constraint'
+
+module Inquisition
+  module ActiveRecordDoctor
+    class MissingNonNullConstraintRunner < ActiveRecordDoctor::Runner
+      attr_reader :issues
+      TASK = ::ActiveRecordDoctor::Tasks::MissingNonNullConstraint
+
+      def run
+        TASK.run.first.each do |table, column|
+          @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK)
+        end
+      end
+    end
+  end
+end

lib/inquisition/active_record_doctor/missing_presence_validation_runner.rb

@@ -0,0 +1,17 @@
+require_relative 'runner'
+require 'active_record_doctor/tasks/missing_presence_validation'
+
+module Inquisition
+  module ActiveRecordDoctor
+    class MissingPresenceValidationRunner < ActiveRecordDoctor::Runner
+      attr_reader :issues
+      TASK = ::ActiveRecordDoctor::Tasks::MissingPresenceValidation
+
+      def run
+        TASK.run.first.each do |table, column|
+          @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK)
+        end
+      end
+    end
+  end
+end

lib/inquisition/active_record_doctor/missing_unique_indexes_runner.rb

@@ -0,0 +1,17 @@
+require_relative 'runner'
+require 'active_record_doctor/tasks/missing_unique_indexes'
+
+module Inquisition
+  module ActiveRecordDoctor
+    class MissingUniqueIndexesRunner < ActiveRecordDoctor::Runner
+      attr_reader :issues
+      TASK = ::ActiveRecordDoctor::Tasks::MissingUniqueIndexes
+
+      def run
+        TASK.run.first.each do |table, column|
+          @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK)
+        end
+      end
+    end
+  end
+end

lib/inquisition/active_record_doctor/runner.rb

@@ -1,31 +1,22 @@
-require 'active_record_doctor/tasks/extraneous_indexes'
-require 'active_record_doctor/tasks/missing_foreign_keys'
-require 'active_record_doctor/tasks/unindexed_deleted_at'
-require 'active_record_doctor/tasks/unindexed_foreign_keys'
-require 'active_record_doctor/tasks/undefined_table_references'
-require 'active_record_doctor/tasks/missing_unique_indexes'
-require 'active_record_doctor/tasks/missing_presence_validation'
-require 'active_record_doctor/tasks/missing_non_null_constraint'
-
 module Inquisition
   module ActiveRecordDoctor
     class Runner < ::Inquisition::Runner
-      TASKS = [
-        ::ActiveRecordDoctor::Tasks::ExtraneousIndexes,
-        ::ActiveRecordDoctor::Tasks::MissingForeignKeys,
-        ::ActiveRecordDoctor::Tasks::MissingNonNullConstraint,
-        ::ActiveRecordDoctor::Tasks::MissingPresenceValidation,
-        ::ActiveRecordDoctor::Tasks::MissingUniqueIndexes,
-        ::ActiveRecordDoctor::Tasks::UndefinedTableReferences,
-        ::ActiveRecordDoctor::Tasks::UnindexedDeletedAt,
-        ::ActiveRecordDoctor::Tasks::UnindexedForeignKeys
-      ].freeze
+      RUNNERS = %w[ExtraneousIndexesRunner
+                   MissingForeignKeysRunner
+                   MissingNonNullConstraintRunner
+                   MissingPresenceValidationRunner
+                   MissingUniqueIndexesRunner
+                   UndefinedTableReferencesRunner
+                   UnindexedDeletedAtRunner
+                   UnindexedForeignKeysRunner].freeze
 
       def call
-        TASKS.each do |ard_task|
-          ard_task.run.first.each do |table, column|
-            @issues << Inquisition::Issue.new(Issue.new(ard_task, table, column).to_h.merge(runner: self))
-          end
+        RUNNERS.each do |runner|
+          runner = "Inquisition::ActiveRecordDoctor::#{runner}".constantize.new
+          runner.run
+          issues = runner.issues
+
+          issues.each { |issue| @issues << Inquisition::Issue.new(issue) }
         end
         @issues
       end

lib/inquisition/active_record_doctor/undefined_table_references_runner.rb

@@ -0,0 +1,17 @@
+require_relative 'runner'
+require 'active_record_doctor/tasks/undefined_table_references'
+
+module Inquisition
+  module ActiveRecordDoctor
+    class UndefinedTableReferencesRunner < ActiveRecordDoctor::Runner
+      attr_reader :issues
+      TASK = ::ActiveRecordDoctor::Tasks::UndefinedTableReferences
+
+      def run
+        TASK.run.first.each do |table, column|
+          @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK)
+        end
+      end
+    end
+  end
+end

lib/inquisition/active_record_doctor/unindexed_deleted_at_runner.rb

@@ -0,0 +1,17 @@
+require_relative 'runner'
+require 'active_record_doctor/tasks/unindexed_deleted_at'
+
+module Inquisition
+  module ActiveRecordDoctor
+    class UnindexedDeletedAtRunner < ActiveRecordDoctor::Runner
+      attr_reader :issues
+      TASK = ::ActiveRecordDoctor::Tasks::UnindexedDeletedAt
+
+      def run
+        TASK.run.first.each do |table, column|
+          @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK)
+        end
+      end
+    end
+  end
+end

lib/inquisition/active_record_doctor/unindexed_foreign_keys_runner.rb

@@ -0,0 +1,17 @@
+require_relative 'runner'
+require 'active_record_doctor/tasks/unindexed_foreign_keys'
+
+module Inquisition
+  module ActiveRecordDoctor
+    class UnindexedForeignKeysRunner < ActiveRecordDoctor::Runner
+      attr_reader :issues
+      TASK = ::ActiveRecordDoctor::Tasks::UnindexedForeignKeys
+
+      def run
+        TASK.run.first.each do |table, column|
+          @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK)
+        end
+      end
+    end
+  end
+end

lib/inquisition/outputter/doc/tpl/quality.rb

@@ -18,6 +18,12 @@ def rubocop
               Template.new('quality/rubocop').render(Rubocop.call(@issues))
             end
           end
+
+          def rails_best_practices
+            @rails_best_practices ||= begin
+              Template.new('quality/rails_best_practices').render(RailsBestPractices.call(@issues))
+            end
+          end
         end
       end
     end
@@ -26,3 +32,4 @@ def rubocop
 
 require_relative 'quality/rubycritic'
 require_relative 'quality/rubocop'
+require_relative 'quality/rails_best_practices'

lib/inquisition/outputter/doc/tpl/quality/rails_best_practices.rb

@@ -0,0 +1,45 @@
+module Inquisition
+  module Outputter
+    class Doc
+      module TPL
+        class Quality
+          class RailsBestPractices
+            class Wrapper < SimpleDelegator
+              def group
+                group_by(&:severity).each do |severity, collection|
+                  yield(severity.name.to_s.capitalize, collection)
+                end
+              end
+            end
+
+            def self.call(issues)
+              new(
+                Wrapper.new(Security::Collector.new(issues, ::Inquisition::RailsBestPractices::Runner).call)
+              )
+            end
+
+            def produce
+              binding
+            end
+
+            def types(issues)
+              issues.group_by(&:message).each do |warning, collection|
+                yield(warning, collection.count)
+              end
+            end
+
+            attr_reader :collection
+
+            def initialize(collection)
+              @collection = collection
+            end
+
+            def link
+              @link ||= Stack::Collector.new(['rails_best_practices']).call.first.homepage
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/fixtures/files/active_record_doctor.yml

@@ -3,7 +3,9 @@
 - severity: :low
   message: 'projects has missing foreign keys, details: user_id'
 - severity: :low
-  message: 'ActiveStorage::Attachment has missing presence validation, details: name, record_type, record_id, blob_id'
+  message: 'projects has missing non null constraint, details: name, user_id'
+- severity: :low
+  message: 'ActiveStorage::Attachment has missing presence validation, details: name, record_type'
 - severity: :low
   message: 'ActiveStorage::Blob has missing presence validation, details: key, filename, byte_size, checksum'
 - severity: :low

spec/inquisition/active_record_doctor/issue_spec.rb

@@ -2,7 +2,7 @@
   describe '#to_h' do
     subject(:vulnerability) { described_class.new(task, 'table', ['column#1', 'column#2']) }
 
-    let(:task) { ActiveRecordDoctor::Tasks::UnindexedForeignKeys }
+    let(:task) { ActiveRecordDoctor::Tasks::UnindexedForeignKeys.name }
     let(:message) { 'table has unindexed foreign keys, details: column#1, column#2' }
 
     let(:options) do

spec/inquisition/active_record_doctor/runner_spec.rb

@@ -3,6 +3,7 @@
 
   describe '#call' do
     let(:runner) { described_class.new }
+    let(:ard_runner) { 'UnindexedForeignKeysRunner' }
     let(:ard_task) { ActiveRecordDoctor::Tasks::UnindexedForeignKeys }
     let(:warning) { { 'unindexed_table' => %w[unindexed_column_1 unindexed_column_1] } }
     let(:message) do
@@ -12,7 +13,7 @@
     before { allow(ard_task).to receive(:run).and_return([warning, true]) }
 
     it 'returns issues array with specific message' do
-      stub_const('Inquisition::ActiveRecordDoctor::Runner::TASKS', [ard_task])
+      stub_const('Inquisition::ActiveRecordDoctor::Runner::RUNNERS', [ard_runner])
       expect(runner.call.first.message).to eq(message)
     end
   end

views/doc/quality.rhtml

@@ -12,41 +12,10 @@
 
 <%= rubocop %>
 
-<div class="">
-  <span class="subtitle"><a id="rails_code_quality_checking">Rails code quality checking</a></span>
-  <ul class="mt--5">
-    <li>
-      there are 612 warnings according to the rails_best_practices (the full report can be found here). We grouped them by severity:
-      <ul>
-        <li>
-          <span>High:</span>
-          <ul type="circle">
-            <li>24 of them related to the missing indexes in the database;</li>
-            <li>137 related to the unused code;</li>
-            <li>2 of them related to use model association;</li>
-          </ul>
-        </li>
-        <li>
-          <span>Medium:</span>
-          <ul type="circle">
-            <li>24 of them related to the missing indexes in the database;</li>
-            <li>137 related to the unused code;</li>
-            <li>2 of them related to use model association;</li>
-          </ul>
-        </li>
-        <li>
-          <span>Low:</span>
-          <ul type="circle">
-            <li>24 of them related to the missing indexes in the database;</li>
-            <li>137 related to the unused code;</li>
-            <li>2 of them related to use model association;</li>
-          </ul>
-        </li>
-      </ul>
-    </li>
-
+<%= rails_best_practices %>
 
-    <li>there are 41 unused routes and 10 unreachable action methods. The full report can be found here</li>
+<div class="">
+  <li>there are 41 unused routes and 10 unreachable action methods. The full report can be found here</li>
     <li>
       the business logic is stored in the:
       <ul>
@@ -79,7 +48,10 @@
         <li>workers</li>
       </ul>
     </li>
-    <li>the application does not use internationalization;</li>
-    <li>technical documentation of the project is absent.</li>
-  </ul>
+    <li>
+      <ul>
+        <li>the application does not use internationalization;</li>
+        <li>technical documentation of the project is absent.</li>
+      </ul>
+    </li>
 </div>