Merge pull request code-dot-org#10162 from code-dot-org/lastAttempt

Changes User.last_attempt to return a UserLevel rather than an Activity.

Author: ashercodeorg

dashboard/app/controllers/api_controller.rb

@@ -167,13 +167,13 @@ def user_progress_for_stage
     level = params[:level] ? Script.cache_find_level(params[:level].to_i) : script_level.oldest_active_level
 
     if current_user
-      last_activity = current_user.last_attempt(level)
-      level_source = last_activity.try(:level_source).try(:data)
+      user_level = current_user.last_attempt(level)
+      level_source = user_level.try(:level_source).try(:data)
 
       response[:progress] = current_user.user_progress_by_stage(stage)
-      if last_activity
+      if user_level
         response[:lastAttempt] = {
-          timestamp: last_activity.updated_at.to_datetime.to_milliseconds,
+          timestamp: user_level.updated_at.to_datetime.to_milliseconds,
           source: level_source
         }
       end

dashboard/app/controllers/script_levels_controller.rb

@@ -152,6 +152,8 @@ def load_level_source
       readonly_view_options
     elsif @user && current_user && @user != current_user
       # load other user's solution for teachers viewing their students' solution
+      # TODO(asher): Determine if the ordering of level_source and @user_level
+      # assignment can be reversed to make level_source rely on @user_level.
       level_source = @user.last_attempt(@level).try(:level_source)
       @user_level = @user.user_level_for(@script_level, @level)
       readonly_view_options

dashboard/app/models/user.rb

@@ -518,14 +518,16 @@ def unpassed_progression_level?(script_level, user_levels)
     end
   end
 
-  # Returns the most recent (via created_at) activity for the specified level.
-  # TODO(asher): Change this to use UserLevel rather than Activity.
+  # Returns the most recent (via updated_at) user_level for the specified
+  # level.
   def last_attempt(level)
-    Activity.where(user_id: self.id, level_id: level.id).order('id desc').first
+    UserLevel.where(user_id: self.id, level_id: level.id).
+      order('updated_at DESC').
+      first
   end
 
-  # Returns the most recent (via updated_at) user_level for the specified
-  # level.
+  # Returns the most recent (via updated_at) user_level for any of the specified
+  # levels.
   def last_attempt_for_any(levels)
     level_ids = levels.map(&:id)
     UserLevel.where(user_id: self.id, level_id: level_ids).

dashboard/test/controllers/api_controller_test.rb

@@ -233,6 +233,9 @@ def make_progress_in_section(script)
   end
 
   test "should get surveys for section with script with anonymous level_group assessment" do
+    # Seed the RNG deterministically so we get the same "random" shuffling of results.
+    srand 1
+
     script = create :script
 
     sub_level1 = create :text_match, name: 'level_free_response', type: 'TextMatch'
@@ -252,52 +255,47 @@ def make_progress_in_section(script)
     level1.save!
     create :script_level, script: script, levels: [level1], assessment: true
 
-    # student_1 did the survey
-    create(:activity, user: @student_1, level: level1, level_source: create(:level_source, level: level1))
-
-    create(:activity, user: @student_1, level: sub_level1,
-      level_source: create(:level_source, level: sub_level1, data: "This is a free response"))
-    create(:activity, user: @student_1, level: sub_level2,
-      level_source: create(:level_source, level: sub_level2, data: "0"))
-    create(:activity, user: @student_1, level: sub_level3,
-      level_source: create(:level_source, level: sub_level3, data: "1"))
-    create(:activity, user: @student_1, level: sub_level4,
-      level_source: create(:level_source, level: sub_level4, data: "-1"))
-
-    # student_2 also did the survey
-    create(:activity, user: @student_2, level: level1, level_source: create(:level_source, level: level1))
-
-    create(:activity, user: @student_2, level: sub_level1,
-      level_source: create(:level_source, level: sub_level1, data: "This is a different free response"))
-    create(:activity, user: @student_2, level: sub_level2,
-      level_source: create(:level_source, level: sub_level2, data: "-1"))
-    create(:activity, user: @student_2, level: sub_level3,
-      level_source: create(:level_source, level: sub_level3, data: "2"))
-    create(:activity, user: @student_2, level: sub_level4,
-      level_source: create(:level_source, level: sub_level4, data: "3"))
-
-    # student_3 through student_5 also did the survey, just submitting a free response.
-    [@student_3, @student_4, @student_5].each_with_index do |student, student_index|
-      create(:activity, user: student, level: level1, level_source: create(:level_source, level: level1))
-
-      create(:activity, user: student, level: sub_level1,
-        level_source: create(:level_source, level: sub_level1, data: "Free response from student #{student_index + 3}"))
-      create(:activity, user: student, level: sub_level2,
-        level_source: create(:level_source, level: sub_level2, data: "-1"))
-      create(:activity, user: student, level: sub_level3,
-        level_source: create(:level_source, level: sub_level3, data: "-1"))
-      create(:activity, user: student, level: sub_level4,
-        level_source: create(:level_source, level: sub_level4, data: "-1"))
-    end
-
     updated_at = Time.now
 
+    # All students did the LevelGroup.
     [@student_1, @student_2, @student_3, @student_4, @student_5].each do |student|
-      create :user_level, user: student, best_result: 100, script: script, level: level1, submitted: true, updated_at: updated_at
+      create :user_level, user: student, script: script, level: level1,
+        level_source: create(:level_source, level: level1), best_result: 100,
+        submitted: true, updated_at: updated_at
     end
 
-    # Seed the RNG with the same thing so we get the same "random" shuffling of results.
-    srand 1
+    # student_1 did the survey.
+    create :user_level, user: @student_1, script: script, level: sub_level1,
+      level_source: create(:level_source, level: sub_level1, data: "This is a free response")
+    create :user_level, user: @student_1, script: script, level: sub_level2,
+      level_source: create(:level_source, level: sub_level2, data: "0")
+    create :user_level, user: @student_1, script: script, level: sub_level3,
+      level_source: create(:level_source, level: sub_level3, data: "1")
+    create :user_level, user: @student_1, script: script, level: sub_level4,
+      level_source: create(:level_source, level: sub_level4, data: "-1")
+
+    # student_2 did the survey.
+    create :user_level, user: @student_2, script: script, level: sub_level1,
+      level_source: create(:level_source, level: sub_level1, data: "This is a different free response")
+    create :user_level, user: @student_2, script: script, level: sub_level2,
+      level_source: create(:level_source, level: sub_level2, data: "-1")
+    create :user_level, user: @student_2, script: script, level: sub_level3,
+      level_source: create(:level_source, level: sub_level3, data: "2")
+    create :user_level, user: @student_2, script: script, level: sub_level4,
+      level_source: create(:level_source, level: sub_level4, data: "3")
+
+    # student_3, student_4, and student_5 did only the free response part of the
+    # survey....
+    [@student_3, @student_4, @student_5].each_with_index do |student, student_index|
+      create :user_level, user: student, script: script, level: sub_level1,
+        level_source: create(:level_source, level: sub_level1, data: "Free response from student #{student_index + 3}")
+      create :user_level, user: student, script: script, level: sub_level2,
+        level_source: create(:level_source, level: sub_level2, data: "-1")
+      create :user_level, user: student, script: script, level: sub_level3,
+        level_source: create(:level_source, level: sub_level3, data: "-1")
+      create :user_level, user: student, script: script, level: sub_level4,
+        level_source: create(:level_source, level: sub_level4, data: "-1")
+    end
 
     get :section_surveys, section_id: @section.id, script_id: script.id
     assert_response :success
@@ -811,10 +809,11 @@ def make_progress_in_section(script)
 
     script_level = script.script_levels[0]
     level = script_level.level
-    create :user_level, user: user, best_result: 100, script: script, level: level
+    level_source = create :level_source, level: level, data: 'level source'
 
-    create(:activity, user: user, level: level,
-           level_source: create(:level_source, level: level, data: 'level source'))
+    create :user_level, user: user, best_result: 100, script: script,
+      level: level, level_source: level_source
+    create :activity, user: user, level: level, level_source: level_source
 
     get :user_progress_for_stage, script_name: script.name, stage_position: 1, level_position: 1
     assert_response :success
@@ -895,17 +894,10 @@ def make_progress_in_section(script)
     stage = create :stage, script: script
     level1a = create :maze, name: 'maze 1'
     level1b = create :maze, name: 'maze 1 new'
+    level_source = create :level_source, level: level1a, data: 'level source'
     create :script_level, script: script, stage: stage, levels: [level1a, level1b], properties: "{'maze 1': {active: false}}"
-    create(
-      :activity,
-      user: @student_1,
-      level: level1a,
-      level_source: create(
-        :level_source,
-        level: level1a,
-        data: 'level source'
-      )
-    )
+    create :user_level, user: @student_1, script: script, level: level1a, level_source: level_source
+    create :activity, user: @student_1, level: level1a, level_source: level_source
 
     get :user_progress_for_stage, script_name: script.name, stage_position: 1, level_position: 1, level: level1a.id
     body = JSON.parse(response.body)

dashboard/test/controllers/script_levels_controller_test.rb

@@ -751,7 +751,6 @@ def assert_caching_enabled(cache_control_header, max_age, proxy_max_age)
     last_attempt_data = 'test'
     level = @custom_s1_l1.level
     level_source = LevelSource.find_identical_or_create(level, last_attempt_data)
-    Activity.create!(level: level, user: @student, level_source: level_source)
     UserLevel.create!(level: level, user: @student, level_source: level_source)
 
     get :show, script_id: @custom_script, stage_position: @custom_stage_1.absolute_position, id: @custom_s1_l1.position
@@ -766,7 +765,6 @@ def assert_caching_enabled(cache_control_header, max_age, proxy_max_age)
     last_attempt_data = 'test'
     level = @custom_s1_l1.level
     level_source = LevelSource.find_identical_or_create(level, last_attempt_data)
-    Activity.create!(level: level, user: @student, level_source: level_source)
     UserLevel.create!(script: @custom_script, level: level, user: @student, level_source: level_source)
 
     get :show, script_id: @custom_script, stage_position: @custom_stage_1.absolute_position, id: @custom_s1_l1.position, user_id: @student.id, section_id: @section.id

dashboard/test/helpers/users_helper_test.rb

@@ -104,14 +104,14 @@ def test_summarize_user_progress_with_pages
     # Create a ScriptLevel joining this level to the script.
     create :script_level, script: script, levels: [level], assessment: true
 
-    # Create a UserLevel joining this level to the user.
-    ul = create :user_level, user: user, best_result: ActivityConstants::BEST_PASS_RESULT, level: level, script: script
-
     # The Activity record will point at a LevelSource with JSON data in which
     # page one has all valid answers and page two has no valid answers.
     level_source = create :level_source,
       data: "{\"#{sub_level1.id}\":{\"valid\":true},\"#{sub_level2.id}\":{\"valid\":true},\"#{sub_level3.id}\":{\"valid\":false},\"#{sub_level4.id}\":{\"valid\":false}}"
 
+    # Create a UserLevel joining this level to the user.
+    ul = create :user_level, user: user, best_result: ActivityConstants::BEST_PASS_RESULT, level: level, script: script, level_source: level_source
+
     # And now create the Activity record.
     create :activity, level_id: level.id,
       user_id: user.id,

dashboard/test/models/plc/course_unit_module_selection_test.rb

@@ -84,6 +84,7 @@ class CourseUnitModuleSelectionTest < ActionView::TestCase
 
     @evaluation = LevelGroup.create_from_level_builder({name: 'evaluation'}, {dsl_text: levelgroup_dsl})
     create(:script_level, script: @course_unit.script, levels: [@evaluation])
+    @user_level = create(:user_level, user: @user, script: @course_unit.script, level: @evaluation)
     @activity = create(:activity, user: @user, level: @evaluation)
     @user_level = create(:user_level, user: @user, level: @evaluation)
   end
@@ -128,6 +129,7 @@ def get_preferred_modules_for_answers(answers_map)
     end
 
     level_source = create(:level_source, level: @evaluation, data: answers_data.to_json)
+    @user_level.update(level_source: level_source)
     @activity.update(level_source: level_source)
     @user_level.update(level_source: level_source)
     @course_unit.determine_preferred_learning_modules @user