feat(APIv2): RHINENG-10187 sorting reports by compliant percentage

app/models/v2/report.rb

@@ -7,10 +7,6 @@ class Report < ApplicationRecord
     self.table_name = :v2_policies
     self.primary_key = :id
 
-    def percent_compliant
-      0
-    end
-
     SYSTEM_COUNT = lambda do
       AN::NamedFunction.new('COUNT', [V2::System.arel_table[:id]]).filter(
         Pundit.policy_scope(User.current, V2::System).where_clause.ast
@@ -33,6 +29,44 @@ def percent_compliant
       )
     end
 
+    # rubocop:disable Metrics/BlockLength
+    PERCENT_COMPLIANT = lambda do
+      AN::NamedFunction.new(
+        'CAST',
+        [
+          AN::NamedFunction.new(
+            'FLOOR',
+            [
+              AN::Multiplication.new(
+                AN::NamedFunction.new(
+                  'COALESCE',
+                  [
+                    AN::Division.new(
+                      AN::NamedFunction.new(
+                        'CAST',
+                        [
+                          COMPLIANT_SYSTEM_COUNT.call.as('FLOAT')
+                        ]
+                      ),
+                      AN::NamedFunction.new(
+                        'NULLIF',
+                        [
+                          AN::NamedFunction.new('CAST', [SYSTEM_COUNT.call.as('FLOAT')]),
+                          Arel.sql('0')
+                        ]
+                      )
+                    ), Arel.sql('0')
+                  ]
+                ),
+                Arel.sql('100')
+              )
+            ]
+          ).as('INTEGER')
+        ]
+      )
+    end
+    # rubocop:enable Metrics/BlockLength
+
     # To prevent an autojoin with itself, there should not be an inverse relationship specified
     belongs_to :policy, class_name: 'V2::Policy', foreign_key: :id # rubocop:disable Rails/InverseOf
     belongs_to :account
@@ -44,12 +78,27 @@ def percent_compliant
     has_many :report_systems, class_name: 'V2::ReportSystem', dependent: nil # rubocop:disable Rails/InverseOf
     has_many :systems, class_name: 'V2::System', through: :report_systems
     has_many :reported_systems, class_name: 'V2::System', through: :test_results, source: :system, dependent: nil
+    has_many :reporting_and_non_reporting_systems, lambda {
+      # joining TestResult and System to correctly count the system with content to report
+      system_test_results = arel_table.join(V2::System.arel_table, AN::InnerJoin)
+                                      .on(V2::System.arel_table[:id].eq(V2::ReportSystem.arel_table[:system_id]))
+                                      .join(V2::TestResult.arel_table, AN::OuterJoin)
+                                      .on(V2::TestResult.arel_table[:system_id].eq(V2::System.arel_table[:id]))
+                                      .join_sources
+
+      # aggregation to prevent optimizer (possibly buggy) filtering out join
+      agg = V2::TestResult.arel_table[:id]
+                          .not_eq(nil)
+                          .or(V2::TestResult.arel_table[:id].eq(nil))
+
+      joins(system_test_results).where(agg)
+    }, class_name: 'V2::ReportSystem', dependent: nil, inverse_of: false
 
     sortable_by :title
     sortable_by :os_major_version
     sortable_by :business_objective
     sortable_by :compliance_threshold
-    sortable_by :compliance_percentage, Arel.sql('0')
+    sortable_by :percent_compliant, 'aggregate_percent_compliant'
 
     searchable_by :title, %i[like unlike eq ne in notin]
     searchable_by :os_major_version, %i[eq ne in notin] do |_key, op, val|

app/serializers/v2/report_serializer.rb

@@ -9,8 +9,8 @@ class ReportSerializer < V2::ApplicationSerializer
     derived_attribute :profile_title, profile: [:title]
     derived_attribute :ref_id, profile: [:ref_id]
     derived_attribute :all_systems_exposed, :total_system_count
-    derived_attribute :percent_compliant
 
+    aggregated_attribute :percent_compliant, :reporting_and_non_reporting_systems, V2::Report::PERCENT_COMPLIANT
     aggregated_attribute :assigned_system_count, :systems, V2::Report::SYSTEM_COUNT
     aggregated_attribute :compliant_system_count, :reported_systems, V2::Report::COMPLIANT_SYSTEM_COUNT
     aggregated_attribute :unsupported_system_count, :reported_systems, V2::Report::UNSUPPORTED_SYSTEM_COUNT

spec/controllers/v2/reports_controller_spec.rb

@@ -13,10 +13,10 @@
       business_objective: :business_objective,
       all_systems_exposed: -> { true },
       compliance_threshold: :compliance_threshold,
-      percent_compliant: :percent_compliant,
-      assigned_system_count: -> { 0 },
-      reported_system_count: -> { 0 },
-      compliant_system_count: -> { 0 },
+      percent_compliant: -> { 50 },
+      assigned_system_count: -> { 2 },
+      reported_system_count: -> { 2 },
+      compliant_system_count: -> { 1 },
       unsupported_system_count: -> { 0 }
     }
   end
@@ -44,7 +44,9 @@
       let(:items) do
         FactoryBot.create_list(
           :v2_report, item_count,
-          assigned_system_count: 0,
+          assigned_system_count: 2,
+          compliant_system_count: 1,
+          unsupported_system_count: 0,
           os_major_version: 8,
           supports_minors: [0, 1],
           account: current_user.account
@@ -68,7 +70,7 @@
             business_objective: :business_objective,
             all_systems_exposed: -> { true },
             compliance_threshold: :compliance_threshold,
-            percent_compliant: :percent_compliant,
+            percent_compliant: -> { 25 },
             assigned_system_count: -> { 4 },
             reported_system_count: -> { 4 },
             compliant_system_count: -> { 1 },
@@ -98,7 +100,7 @@
               business_objective: :business_objective,
               all_systems_exposed: -> { false },
               compliance_threshold: :compliance_threshold,
-              percent_compliant: :percent_compliant,
+              percent_compliant: -> { 25 },
               assigned_system_count: -> { 4 },
               reported_system_count: -> { 4 },
               compliant_system_count: -> { 1 },
@@ -142,13 +144,33 @@
     end
 
     describe 'GET show' do
+      let(:attributes) do
+        {
+          title: :title,
+          os_major_version: :os_major_version,
+          ref_id: :ref_id,
+          description: :description,
+          profile_title: :profile_title,
+          business_objective: :business_objective,
+          all_systems_exposed: -> { true },
+          compliance_threshold: :compliance_threshold,
+          percent_compliant: -> { 50 },
+          assigned_system_count: -> { 2 },
+          reported_system_count: -> { 2 },
+          compliant_system_count: -> { 1 },
+          unsupported_system_count: -> { 0 }
+        }
+      end
+
       let(:extra_params) { { account: current_user.account, id: item.id } }
 
       let(:item) do
         FactoryBot.create(
           :v2_report,
           os_major_version: 9,
-          assigned_system_count: 0,
+          assigned_system_count: 2,
+          compliant_system_count: 1,
+          unsupported_system_count: 0,
           supports_minors: [0, 1, 2],
           account: current_user.account
         )
@@ -167,7 +189,7 @@
             business_objective: :business_objective,
             all_systems_exposed: -> { true },
             compliance_threshold: :compliance_threshold,
-            percent_compliant: :percent_compliant,
+            percent_compliant: -> { 25 },
             assigned_system_count: -> { 4 },
             reported_system_count: -> { 4 },
             compliant_system_count: -> { 1 },
@@ -200,7 +222,7 @@
               business_objective: :business_objective,
               all_systems_exposed: -> { false },
               compliance_threshold: :compliance_threshold,
-              percent_compliant: :percent_compliant,
+              percent_compliant: -> { 25 },
               assigned_system_count: -> { 4 },
               reported_system_count: -> { 4 },
               compliant_system_count: -> { 1 },
@@ -240,6 +262,62 @@
 
           it_behaves_like 'individual'
         end
+
+        context 'with mixed inventory groups access' do
+          let(:attributes) do
+            {
+              title: :title,
+              os_major_version: :os_major_version,
+              ref_id: :ref_id,
+              description: :description,
+              profile_title: :profile_title,
+              business_objective: :business_objective,
+              all_systems_exposed: -> { false },
+              compliance_threshold: :compliance_threshold,
+              percent_compliant: -> { 33 },
+              assigned_system_count: -> { 3 },
+              reported_system_count: -> { 3 },
+              compliant_system_count: -> { 1 },
+              unsupported_system_count: -> { 1 }
+            }
+          end
+
+          let!(:item) do
+            FactoryBot.create(
+              :v2_report,
+              os_major_version: 9,
+              assigned_system_count: 5,
+              compliant_system_count: 1,
+              unsupported_system_count: 1,
+              supports_minors: [0, 1, 2],
+              account: current_user.account
+            )
+          end
+
+          before do
+            stub_rbac_permissions(
+              Rbac::INVENTORY_HOSTS_READ => [{
+                attribute_filter: {
+                  key: 'group.id',
+                  operation: 'in',
+                  value: [nil] # access to ungrouped hosts
+                }
+              }]
+            )
+
+            # change groups for some of the non-compliant systems
+            non_compliant_systems = item.test_results.select do |tr|
+              tr.score.try(:<, tr.report.compliance_threshold)
+            end.take(2)
+            non_compliant_systems.each do |tr|
+              tr.system.update!(groups: [Faker::Internet])
+            end
+          end
+
+          let(:extra_params) { { account: current_user.account, id: item.id } }
+
+          it_behaves_like 'individual'
+        end
       end
     end
 
@@ -375,7 +453,7 @@
           business_objective: :business_objective,
           compliance_threshold: :compliance_threshold,
           all_systems_exposed: -> { false }, # this is inconcistent but we don't care
-          percent_compliant: :percent_compliant,
+          percent_compliant: -> { 0 },
           reported_system_count: -> { 0 },
           compliant_system_count: -> { 0 },
           unsupported_system_count: -> { 0 }

spec/fixtures/files/sortable/reports_controller.yaml

@@ -6,6 +6,10 @@
     :os_major_version: ${rhel_9}
     :system_id: ${system_id}
     :account: ${account}
+    :assigned_system_count: 7
+    :compliant_system_count: 1
+    :unsupported_system_count: 2
+    :supports_minors: [0, 1, 2]
 
   - :factory: :v2_report
     :title: 'bac'
@@ -14,6 +18,10 @@
     :os_major_version: ${rhel_8}
     :system_id: ${system_id}
     :account: ${account}
+    :assigned_system_count: 7
+    :compliant_system_count: 2
+    :unsupported_system_count: 2
+    :supports_minors: [0, 1, 2]
 
   - :factory: :v2_report
     :title: 'abc'
@@ -22,6 +30,10 @@
     :os_major_version: ${rhel_8}
     :system_id: ${system_id}
     :account: ${account}
+    :assigned_system_count: 7
+    :compliant_system_count: 3
+    :unsupported_system_count: 2
+    :supports_minors: [0, 1, 2]
 
   - :factory: :v2_report
     :title: 'bca'
@@ -30,6 +42,10 @@
     :os_major_version: ${rhel_7}
     :system_id: ${system_id}
     :account: ${account}
+    :assigned_system_count: 7
+    :compliant_system_count: 4
+    :unsupported_system_count: 2
+    :supports_minors: [0, 1, 2]
 
   - :factory: :v2_report
     :title: 'aab'
@@ -38,6 +54,10 @@
     :os_major_version: ${rhel_7}
     :system_id: ${system_id}
     :account: ${account}
+    :assigned_system_count: 7
+    :compliant_system_count: 5
+    :unsupported_system_count: 2
+    :supports_minors: [0, 1, 2]
 
 :queries:
   - :sort_by:
@@ -57,6 +77,7 @@
   - :sort_by:
       - 'os_major_version'
       - 'compliance_threshold'
-    :result: [4, 3, [1, 2], 0]
+      - 'percent_compliant'
+    :result: [4, 3, 1, 2, 0]
     :except_parents:
-      - :systems
+      - :systems

spec/integration/v2/reports_spec.rb

@@ -164,12 +164,15 @@
   end
 
   path '/systems/{system_id}/reports' do
-    let(:system_id) { FactoryBot.create(:system, account: user.account, os_major_version: 8, os_minor_version: 0).id }
+    let(:system_id) do
+      FactoryBot.create(:system, account: user.account, os_major_version: 8, group_count: 2, os_minor_version: 0).id
+    end
 
     before do
       FactoryBot.create_list(
         :v2_report, 5,
         system_id: system_id,
+        assigned_system_count: 5,
         os_major_version: 8,
         supports_minors: [0],
         account: user.account