Data Dashboard Refinement (#751)

* Tweak CSV exporting errors

* Tweak CSV exporting errors

align

* - refactor resits
- refactor non-linear activity comparison
- scope linear query since feature introduced
- fix bug when checking notes for closed accounts
- add scope for user answers to only confidence check

* Update pending message for CMS only specs

* - refactor module completion and csv specs
- refactor csv concern to support custom headers
- add task to get sight of data volumes

* - update pending message for CMS only specs
- to_csv debug message

* - Refactor
- Scope
- Reformat

* Mitigate flakey order in pipeline spec

* Explicitly order LA data correctly

.rubocop_todo.yml

@@ -11,6 +11,11 @@ Lint/MixedRegexpCaptureTypes:
   Exclude:
     - 'lib/seed_course_entries.rb'
 
+# Offense count: 1
+Lint/ShadowedException:
+  Exclude:
+    - 'app/models/concerns/to_csv.rb'
+
 # Offense count: 1
 # Configuration parameters: AllowComments, AllowNil.
 Lint/SuppressedException:
@@ -33,7 +38,7 @@ Rails/CreateTableWithTimestamps:
     - 'db/migrate/20220419121105_create_ahoy_visits_and_events.rb'
     - 'db/migrate/20230316130014_create_releases.rb'
 
-# Offense count: 5
+# Offense count: 6
 # This cop supports unsafe autocorrection (--autocorrect-all).
 # Configuration parameters: Include.
 # Include: app/**/*.rb, config/**/*.rb, db/**/*.rb, lib/**/*.rb
@@ -42,6 +47,7 @@ Rails/Output:
     - 'app/jobs/content_check_job.rb'
     - 'app/jobs/dashboard_job.rb'
     - 'app/jobs/fill_page_views_job.rb'
+    - 'app/models/concerns/to_csv.rb'
     - 'app/services/contentful_data_integrity.rb'
     - 'app/services/dashboard.rb'
 

app/decorators/coercion_decorator.rb

@@ -1,11 +1,8 @@
 class CoercionDecorator
-  extend Dry::Initializer
-
-  param :input, type: Types::Strict::Array.of(Types::Strict::Hash)
-
-  # @return [Array<Hash>]
-  def call
-    input.each { |row| row.map { |key, value| row[key] = format_value(key, value) } }
+  # @param input [Hash]
+  # @return [Hash]
+  def call(input)
+    input.each { |key, value| input[key] = format_value(key, value) }
   end
 
 private
@@ -15,23 +12,23 @@ def call
   # @return [Mixed]
   def format_value(key, value)
     if value.is_a?(Time) || value.is_a?(DateTime)
-      format_datetime(value)
-    elsif key.to_s.include?('percentage')
-      format_percentage(value)
+      as_strftime(value)
+    elsif key.to_s.ends_with?('percentage')
+      as_percentage(value)
     else
       value
     end
   end
 
   # @param value [Numeric]
   # @return [String]
-  def format_percentage(value)
+  def as_percentage(value)
     "#{(value * 100).round(2)}%"
   end
 
   # @param value [Time, DateTime]
   # @return [String]
-  def format_datetime(value)
+  def as_strftime(value)
     value.strftime('%Y-%m-%d %H:%M:%S')
   end
 end

app/models/ahoy/event.rb

@@ -29,6 +29,9 @@ class Ahoy::Event < ApplicationRecord
                                      user_registration.where("properties -> 'controller' ?| array[:values]", values: controllers)
                                    }
 
+  scope :since_non_linear, -> { where(time: Rails.application.non_linear_launch_date..Time.zone.now) }
+  # scope :since_cms, -> { where(time: Rails.application.cms_launch_date..Time.zone.now) }
+
   # @see ContentPagesController#track_events
   # ----------------------------------------------------------------------------
   scope :page_view, -> { where(name: 'module_content_page') }

app/models/ahoy/visit.rb

@@ -5,4 +5,6 @@ class Ahoy::Visit < ApplicationRecord
 
   has_many :events, class_name: 'Ahoy::Event'
   belongs_to :user, optional: true
+
+  scope :dashboard, -> { where.not(referrer: nil) }
 end

app/models/concerns/to_csv.rb

@@ -3,39 +3,49 @@
 module ToCsv
   extend ActiveSupport::Concern
 
+  class ExportError < StandardError
+  end
+
   class_methods do
-    # Returns an array of strings representing the column names for the data export
     # @return [Array<String>]
-    def column_names
-      super
-    rescue NoMethodError
-      raise NoMethodError, 'ToCsv.to_csv a column names method must be defined for bespoke data export models to serve as csv column headers'
+    def dashboard_headers
+      column_names
+    rescue NoMethodError, NameError
+      raise ExportError, "#{name}.dashboard_headers is required for bespoke models"
     end
 
-    # Returns an array of hashes representing the rows of data to be exported or an ActiveRecord::Relation
     # @return [ActiveRecord::Relation, Array<Hash{Symbol => Mixed}>]
     def dashboard
       all
     rescue NoMethodError
-      raise NoMethodError, 'ToCsv.to_csv a dashboard method must be defined for bespoke data export models to serve as csv data'
+      raise ExportError, "#{name}.dashboard is required for bespoke models"
     end
 
-    # @return [String]
     # @param batch_size [Integer]
+    # @return [String]
     def to_csv(batch_size: 1_000)
+      puts "Starting #{name}.to_csv"
+      decorator = CoercionDecorator.new
+
       CSV.generate(headers: true) do |csv|
-        csv << column_names
+        csv << dashboard_headers
 
-        unformatted = dashboard.is_a?(Array) ? dashboard : dashboard.find_each(batch_size: batch_size).map(&:dashboard_attributes)
-        formatted = CoercionDecorator.new(unformatted).call
-        formatted.each { |row| csv << row.values }
+        if dashboard.is_a?(Array)
+          dashboard.to_enum.each do |record|
+            csv << decorator.call(record).values
+          end
+        else
+          dashboard.find_each(batch_size: batch_size) do |record|
+            csv << decorator.call(record.dashboard_row).values
+          end
+        end
       end
     end
   end
 
   included do
-    # @return [Hash] default to database fields
-    def dashboard_attributes
+    # @return [Hash]
+    def dashboard_row
       attributes
     end
   end

app/models/data/average_pass_scores.rb

@@ -5,7 +5,10 @@ class AveragePassScores
     class << self
       # @return [Array<String>]
       def column_names
-        ['Module', 'Average Pass Score']
+        [
+          'Module',
+          'Average Pass Score',
+        ]
       end
 
       # TODO: Upcoming changes to UserAssessment will make this type coercion unnecessary

app/models/data/high_fail_questions.rb

@@ -5,7 +5,11 @@ class HighFailQuestions
     class << self
       # @return [Array<String>]
       def column_names
-        ['Module', 'Question', 'Failure Rate Percentage']
+        [
+          'Module',
+          'Question',
+          'Failure Rate Percentage',
+        ]
       end
 
       # @return [Array<Hash{Symbol => Mixed}>]

app/models/data/local_authority_user.rb

@@ -5,30 +5,33 @@ class LocalAuthorityUser
     class << self
       # @return [String]
       def column_names
-        ['Local Authority', 'Users']
+        [
+          'Local Authority',
+          'Users',
+        ]
       end
 
-      # @return [Array<Hash{Symbol => Mixed}>]
+      # @return [Array<Hash>]
       def dashboard
-        count_by_local_authority.map do |authority, count|
+        authorities.map do |authority, count|
           {
             local_authority: authority,
             users: count,
           }
         end
       end
 
-  private
+    private
 
-      # @return [Hash{Symbol=>Integer}]
-      def count_by_local_authority
-        public_beta_users.group(:local_authority).count
+      # @return [Hash{Symbol => Integer}]
+      def authorities
+        public_beta_users.group(:local_authority).order(:local_authority).count
       end
 
-      # @return [ActiveRecord::Relation<User>]
+      # @return [User::ActiveRecord_Relation]
       def public_beta_users
         User.since_public_beta.with_local_authority
       end
-  end
+    end
   end
 end

app/models/data/modules_per_month.rb

@@ -5,45 +5,38 @@ class ModulesPerMonth
     class << self
       # @return [Array<String>]
       def column_names
-        ['Month', 'Module', 'Pass Percentage', 'Pass Count', 'Fail Percentage', 'Fail Count']
+        [
+          'Month',
+          'Module',
+          'Pass Percentage',
+          'Pass Count',
+          'Fail Percentage',
+          'Fail Count',
+        ]
       end
 
-      # @return [Array<Hash{Symbol => Mixed}>]
+      # @return [Array<Hash>]
       def dashboard
-        grouped_assessments.flat_map do |month, module_data|
-          module_data.flat_map do |module_name, assessments|
-            row_builder(assessments).merge({ module_name: module_name, month: month })
+        assessments_by_module_by_month.flat_map do |month, by_module|
+          by_module.flat_map do |mod_name, assessments|
+            pass_fail = SummativeQuiz.pass_fail(total: assessments.count, pass: assessments.count(&:passed?))
+
+            { month: month, module_name: mod_name, **pass_fail }
           end
         end
       end
 
-  private
-
-      # @return [Hash{Symbol => Mixed}]
-      def row_builder(assessments)
-        pass_count = assessments.count(&:passed?)
-        total_count = assessments.size
-        fail_count = total_count - pass_count
-        pass_percentage = pass_count / total_count.to_f
-        fail_percentage = 1 - pass_percentage
+    private
 
-        {
-          pass_percentage: pass_percentage,
-          pass_count: pass_count,
-          fail_percentage: fail_percentage,
-          fail_count: fail_count,
-        }
-      end
-
-      # @return [Hash{String => Array<UserAssessment>}}]
-      def modules_by_month
+      # @return [Hash]
+      def assessments_by_month
         UserAssessment.summative.group_by { |assessment| assessment.created_at.strftime('%B %Y') }
       end
 
-      # @return [Hash{String => Hash{String => Array<UserAssessment>}}]
-      def grouped_assessments
-        modules_by_month.transform_values { |assessments| assessments.group_by(&:module) }
+      # @return [Hash]
+      def assessments_by_module_by_month
+        assessments_by_month.transform_values { |assessments| assessments.group_by(&:module) }
       end
-  end
+    end
   end
 end

app/models/data/resits_per_user.rb

@@ -5,42 +5,42 @@ class ResitsPerUser
     class << self
       # @return [Array<String>]
       def column_names
-        ['Module', 'User ID', 'Role', 'Resit Attempts']
+        [
+          'Module',
+          'User ID',
+          'Role',
+          'Resit Attempts',
+        ]
       end
 
-      # @return [Array<Hash{Symbol => Mixed}>]
+      # @return [Array<Hash{Symbol => String,Integer}>]
       def dashboard
-        resit_attempts_per_user.flat_map do |module_name, user_attempts|
-          user_attempts.map do |user_id, attempts|
-            {
-              module_name: module_name,
-              user_id: user_id,
-              role_type: user_roles[user_id],
-              resit_attempts: attempts,
-            }
-          end
+        resits.map do |(module_name, user_id), attempts|
+          {
+            module_name: module_name,
+            user_id: user_id,
+            role_type: user_roles[user_id],
+            resit_attempts: attempts - 1,
+          }
         end
       end
 
-  private
+    private
 
       # @return [Hash{Integer => String}]
       def user_roles
         User.pluck(:id, :role_type).to_h
       end
 
-      # @return [Hash{Symbol => Hash{Integer => Integer}}]
-      def resit_attempts_per_user
-        UserAssessment.summative
-          .group(:module, :user_id)
-          .count
-          .each_with_object({}) do |((module_name, user_id), count), resit_attempts_per_module|
-            if count > 1
-              resit_attempts_per_module[module_name] ||= {}
-              resit_attempts_per_module[module_name][user_id] = count - 1
-            end
-          end
+      # @return [Hash{Array => Integer}]
+      def assessments
+        UserAssessment.summative.group(:module, :user_id).count
       end
-  end
+
+      # @return [Hash{Array => Integer}]
+      def resits
+        assessments.select { |_k, v| v > 1 }
+      end
+    end
   end
 end