Training data migration (#1075)

* Redesign assessment logic and data and integrate with new response feature

* Draft logic to migrate training records, copying PKs and create missing
associations as needed

* Rework and test

* YAML AST powered migration testing

* Revert feedback spec during active development

* Seed Gov One account and test manual training data migration task

* Resume and validate functionality

* Improve coverage and debugging output for active development

* Restore code lost in merge with main

.env.example

@@ -2,6 +2,12 @@
 #
 # ------------------------------------------------------------------------------
 
+#
+DEV_EMAIL=[email protected]
+DEV_GOV_ONE_TOKEN=urn:fdc:gov.uk:2022:23-random-alpha-numeric
+DEV_FIRST_NAME=First
+DEV_LAST_NAME=Last
+
 # Display console information
 VERBOSE=true
 # Display debugging information

.github/workflows/ci.yml

@@ -84,11 +84,11 @@ jobs:
         name: Run rubocop
         run: bundle exec rubocop
       # Answer migration feature test
-      # -
-      #   name: Run test suite
-      #   run: bundle exec rspec
-      #   env:
-      #     DISABLE_USER_ANSWER: true
+      -
+        name: Run test suite
+        run: bundle exec rspec
+        env:
+          DISABLE_USER_ANSWER: true
       # Gov One feature test
       -
         name: Run test suite

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config --no-auto-gen-timestamp`
-# using RuboCop version 1.59.0.
+# using RuboCop version 1.60.2.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -11,11 +11,12 @@ Lint/ShadowedException:
   Exclude:
     - 'app/models/concerns/to_csv.rb'
 
-# Offense count: 1
+# Offense count: 2
 # Configuration parameters: AllowComments, AllowNil.
 Lint/SuppressedException:
   Exclude:
     - 'lib/tasks/cms.rake'
+    - 'spec/lib/migrate_training_spec.rb'
 
 # Offense count: 2
 # Configuration parameters: Database, Include.
@@ -25,13 +26,14 @@ Rails/BulkChangeTable:
   Exclude:
     - 'db/migrate/20220617020303_add_summative_assessment_fields_to_user_answers.rb'
 
-# Offense count: 3
+# Offense count: 4
 # Configuration parameters: Include.
 # Include: db/**/*.rb
 Rails/CreateTableWithTimestamps:
   Exclude:
     - 'db/migrate/20220419121105_create_ahoy_visits_and_events.rb'
     - 'db/migrate/20230316130014_create_releases.rb'
+    - 'db/migrate/20240119091634_create_assessments.rb'
 
 # Offense count: 4
 # This cop supports unsafe autocorrection (--autocorrect-all).
@@ -44,16 +46,19 @@ Rails/Output:
     - 'app/services/content_integrity.rb'
     - 'app/services/dashboard.rb'
 
-# Offense count: 10
+# Offense count: 17
 # This cop supports unsafe autocorrection (--autocorrect-all).
 # Configuration parameters: AllowImplicitReturn, AllowedReceivers.
 Rails/SaveBang:
   Exclude:
     - 'app/controllers/registrations_controller.rb'
     - 'app/controllers/training/responses_controller.rb'
     - 'app/forms/registration/setting_type_form.rb'
+    - 'app/models/user.rb'
+    - 'app/services/assessment_progress.rb'
     - 'config/sitemap.rb'
     - 'lib/content_seed.rb'
+    - 'lib/migrate_training.rb'
     - 'lib/seed_images.rb'
     - 'spec/forms/registration/setting_type_form_spec.rb'
 
@@ -65,3 +70,9 @@ Rails/Validation:
   Exclude:
     - 'app/models/course.rb'
     - 'app/models/training/module.rb'
+
+# Offense count: 2
+# This cop supports unsafe autocorrection (--autocorrect-all).
+Style/IdenticalConditionalBranches:
+  Exclude:
+    - 'app/services/module_progress.rb'

README.md

@@ -330,6 +330,23 @@ settings the following classes can be added:
 - `data-hj-suppress` to redact additional user information
 - `data-hj-allow` to allow data that is automatically redacted
 
+## Azure
+
+Production console access
+
+- https://eyrecovery-dev.scm.azurewebsites.net/webssh/host
+- https://eyrecovery-stage.scm.azurewebsites.net/webssh/host
+- https://eyrecovery-prod.scm.azurewebsites.net/webssh/host
+
+
+## Programmatic testing
+
+```ruby
+bravo = Training::Module.by_name('bravo')
+data = ContentTestSchema.new(mod: bravo).call(pass: false).compact
+file = Rails.root.join('spec/support/ast/bravo-fail.yml')
+File.open(file, 'w') { |file| file.write(data.to_yaml) }
+```
 
 ---
 

app/controllers/training/assessments_controller.rb

@@ -13,7 +13,7 @@ class AssessmentsController < ApplicationController
     layout 'hero'
 
     def new
-      assessment.archive!
+      assessment.retake! # TODO: permit testers to try again
       redirect_to training_module_page_path(mod.name, mod.assessment_intro_page.name)
     end
 

app/controllers/training/questions_controller.rb

@@ -1,3 +1,14 @@
+# Question flavours:
+#
+#   Tests:
+#     - formative question    (immediate feedback)
+#     - summative question    (grouped by assessment)
+#
+#   Opinions:
+#     - confidence question   (static options)
+#     - feedback question     (dynamic options)
+#
+#
 module Training
   class QuestionsController < ApplicationController
     include Learning

app/controllers/training/responses_controller.rb

@@ -1,3 +1,8 @@
+#
+# TODO: Logic around potential FK question_name changes that could cause an assessment to contain more than 10 responses
+# TODO: Checks that scores are numeric, otherwise zero is recorded
+#
+#
 module Training
   class ResponsesController < ApplicationController
     include Learning
@@ -25,7 +30,7 @@ def update
     # @note migrate from user_answer to response
     # @see User#response_for
     def response_params
-      if ENV['DISABLE_USER_ANSWER'].present?
+      if Rails.application.migrated_answers?
         params.require(:response).permit!
       else
         params.require(:user_answer).permit!
@@ -38,8 +43,8 @@ def response_params
     def save_response!
       correct_answers = content.confidence_question? ? true : content.correct_answers.eql?(user_answers)
 
-      if ENV['DISABLE_USER_ANSWER'].present?
-        current_user_response.update(answers: user_answers, correct: correct_answers, schema: content.schema)
+      if Rails.application.migrated_answers?
+        current_user_response.update(answers: user_answers, correct: correct_answers)
       else
         current_user_response.update(answer: user_answers, correct: correct_answers)
       end
@@ -51,7 +56,7 @@ def user_answers
     end
 
     def redirect
-      assessment.complete! if content.last_assessment?
+      assessment.grade! if content.last_assessment?
 
       if content.formative_question?
         redirect_to training_module_question_path(mod.name, content.name)
@@ -62,7 +67,7 @@ def redirect
 
     # @return [Event] Update action
     def track_question_answer
-      if ENV['DISABLE_USER_ANSWER'].present?
+      if Rails.application.migrated_answers?
         track('questionnaire_answer',
               uid: content.id,
               mod_uid: mod.id,

app/helpers/gov_one_helper.rb → app/helpers/authentication_helper.rb

@@ -1,4 +1,4 @@
-module GovOneHelper
+module AuthenticationHelper
   # @return [URI]
   def login_uri
     params = {

app/helpers/content_helper.rb

@@ -1,6 +1,7 @@
 module ContentHelper
   # @see [CustomMarkdown]
   # @param key [String]
+  # @param args [Hash]
   # @return [String]
   def m(key, headings_start_with: 'l', **args)
     markdown = I18n.exists?(key, scope: args[:scope]) ? t(key, **args) : key.to_s
@@ -41,15 +42,13 @@ def icon(icon, size: 2, **)
                 aria: { label: "#{icon} icon" }
   end
 
-  # @param success [Boolean]
-  # @param score [Integer]
+  # @param assessment [AssessmentProgress]
   # @return [String]
-  def results_banner(success:, score:)
-    state = success ? :pass : :fail
-    title = t(".#{state}.heading")
-    text = t(".#{state}.text", score: score)
-
-    govuk_notification_banner(title_text: title, text: m(text))
+  def assessment_banner(assessment)
+    govuk_notification_banner(
+      title_text: t("training.assessments.show.#{assessment.status}.heading"),
+      text: m("training.assessments.show.#{assessment.status}.text", score: assessment.score.to_i),
+    )
   end
 
   # @param status [String, Symbol]

app/helpers/link_helper.rb

@@ -62,12 +62,16 @@ def link_to_previous
   # @param mod [Training::Module]
   # @return [String, nil]
   def link_to_retake_or_results(mod)
-    return unless assessment_progress_service(mod).attempted?
+    if Rails.application.migrated_answers?
+      return unless assessment_progress_service(mod).graded?
+    else
+      return unless assessment_progress_service(mod).attempted?
+    end
 
     if assessment_progress_service(mod).failed?
       govuk_link_to 'Retake end of module test', new_training_module_assessment_path(mod.name), no_visited_state: true, class: 'card-link--retake'
     else
-      govuk_link_to 'View previous test result', training_module_assessment_path(mod.name, mod.assessment_results_page.name)
+      govuk_link_to 'View previous test result', training_module_assessment_path(mod.name, mod.assessment_results_page.name), no_visited_state: true, class: 'card-link--retake'
     end
   end
 end

app/models/assessment.rb

@@ -0,0 +1,22 @@
+class Assessment < ApplicationRecord
+  include ToCsv
+
+  belongs_to :user
+  has_many :responses, -> { where(question_type: 'summative') }
+
+  scope :incomplete, -> { where(completed_at: nil) }
+  scope :complete, -> { where.not(completed_at: nil) }
+
+  scope :passed, -> { where(passed: true) }
+  scope :failed, -> { where(passed: false) }
+
+  # @return [Boolean]
+  def passed?
+    passed
+  end
+
+  # @return [Boolean]
+  def graded?
+    score.present?
+  end
+end

app/models/data_analysis/average_pass_scores.rb

@@ -11,14 +11,22 @@ def column_names
         ]
       end
 
-      # TODO: Upcoming changes to UserAssessment will make this type coercion unnecessary
       # @return [Array<Hash{Symbol => Mixed}>]
       def dashboard
-        UserAssessment.summative.passes.group(:module).average('CAST(score AS float)').map do |module_name, score|
-          {
-            module_name: module_name,
-            pass_score: score,
-          }
+        if Rails.application.migrated_answers?
+          Assessment.passed.group(:training_module).average(:score).map do |module_name, score|
+            {
+              module_name: module_name,
+              pass_score: score,
+            }
+          end
+        else
+          UserAssessment.summative.passes.group(:module).average('CAST(score AS float)').map do |module_name, score|
+            {
+              module_name: module_name,
+              pass_score: score,
+            }
+          end
         end
       end
     end

app/models/data_analysis/high_fail_questions.rb

@@ -27,7 +27,7 @@ def dashboard
 
       # @return [Hash{Array<String> => Integer}]
       def question_attempts
-        if ENV['DISABLE_USER_ANSWER'].present?
+        if Rails.application.migrated_answers?
           Response.summative.group(:training_module, :question_name).count
         else
           UserAnswer.summative.group(:module, :name).count
@@ -36,8 +36,8 @@ def question_attempts
 
       # @return [Hash{Array<String> => Integer}]
       def question_failures
-        if ENV['DISABLE_USER_ANSWER'].present?
-          Response.summative.where(correct: false).group(:training_module, :question_name).count
+        if Rails.application.migrated_answers?
+          Response.summative.incorrect.group(:training_module, :question_name).count
         else
           UserAnswer.summative.where(correct: false).group(:module, :name).count
         end

app/models/data_analysis/modules_per_month.rb

@@ -30,12 +30,20 @@ def dashboard
 
       # @return [Hash]
       def assessments_by_month
-        UserAssessment.summative.group_by { |assessment| assessment.created_at.strftime('%B %Y') }
+        if Rails.application.migrated_answers?
+          Assessment.all.group_by { |assessment| assessment.completed_at.strftime('%B %Y') }
+        else
+          UserAssessment.all.group_by { |assessment| assessment.created_at.strftime('%B %Y') }
+        end
       end
 
       # @return [Hash]
       def assessments_by_module_by_month
-        assessments_by_month.transform_values { |assessments| assessments.group_by(&:module) }
+        if Rails.application.migrated_answers?
+          assessments_by_month.transform_values { |assessments| assessments.group_by(&:training_module) }
+        else
+          assessments_by_month.transform_values { |assessments| assessments.group_by(&:module) }
+        end
       end
     end
   end

app/models/data_analysis/resits_per_user.rb

@@ -34,7 +34,11 @@ def user_roles
 
       # @return [Hash{Array => Integer}]
       def assessments
-        UserAssessment.summative.group(:module, :user_id).count
+        if Rails.application.migrated_answers?
+          Assessment.order(:user_id).group(:training_module, :user_id).count
+        else
+          UserAssessment.summative.group(:module, :user_id).count
+        end
       end
 
       # @return [Hash{Array => Integer}]

app/models/data_analysis/users_not_passing.rb

@@ -25,12 +25,21 @@ def dashboard
 
       # @return [Hash{String => Integer}]
       def total_users_not_passing_per_module
-        UserAssessment.summative
-          .group(:module, :user_id)
-          .count
-          .reject { |(module_name, user_id), _| UserAssessment.summative.where(module: module_name, user_id: user_id).passes.exists? }
-          .group_by { |(module_name, _), _| module_name }
-          .transform_values(&:size)
+        if Rails.application.migrated_answers?
+          Assessment.all
+            .group(:training_module, :user_id)
+            .count
+            .reject { |(module_name, user_id), _| Assessment.all.where(training_module: module_name, user_id: user_id).passed.exists? }
+            .group_by { |(module_name, _), _| module_name }
+            .transform_values(&:size)
+        else
+          UserAssessment.all
+            .group(:module, :user_id)
+            .count
+            .reject { |(module_name, user_id), _| UserAssessment.all.where(module: module_name, user_id: user_id).passes.exists? }
+            .group_by { |(module_name, _), _| module_name }
+            .transform_values(&:size)
+        end
       end
     end
   end