Merge pull request #38670 from code-dot-org/revert-38532-hbergam_LP-1…

…693-progress-tab

Revert "Reconciling status differences between assessment and progress tab by…"

apps/src/code-studio/components/progress/TeacherPanelProgressBubble.jsx

@@ -75,7 +75,7 @@ export class TeacherPanelProgressBubble extends React.Component {
     const style = {
       ...styles.main,
       ...(userLevel.isConceptLevel && styles.diamond),
-      ...levelProgressStyle(userLevel.status, userLevel.kind, false)
+      ...levelProgressStyle(userLevel, false)
     };
 
     // Outer div here is used to make sure our bubbles all take up equivalent

apps/src/templates/progress/ProgressBubble.jsx

@@ -158,7 +158,7 @@ class ProgressBubble extends React.Component {
       ...(smallBubble && styles.small),
       ...(level.isConceptLevel &&
         (smallBubble ? styles.smallDiamond : styles.largeDiamond)),
-      ...levelProgressStyle(level.status, level.kind, disabled),
+      ...levelProgressStyle(level, disabled),
       ...(disabled && level.bonus && styles.disabledStageExtras)
     };
 

apps/src/templates/progress/ProgressPill.jsx

@@ -87,8 +87,7 @@ class ProgressPill extends React.Component {
     let style = {
       ...styles.levelPill,
       ...(url && hoverStyle),
-      ...(!multiLevelStep &&
-        levelProgressStyle(levels[0].status, levels[0].kind, false))
+      ...(!multiLevelStep && levelProgressStyle(levels[0], false))
     };
 
     // If we're passed a tooltip, we also need to reference it from our div

apps/src/templates/progress/progressStyles.js

@@ -1,11 +1,10 @@
 import color from '@cdo/apps/util/color';
-import {LevelStatus, LevelKind} from '@cdo/apps/util/sharedConstants';
+import {LevelStatus} from '@cdo/apps/util/sharedConstants';
 
 export const DOT_SIZE = 30;
 export const DIAMOND_DOT_SIZE = 22;
 export const SMALL_DOT_SIZE = 9;
 export const SMALL_DIAMOND_SIZE = 6;
-export const BUBBLE_BORDER_WIDTH = 2;
 
 // Hard-coded container width to ensure all big bubbles have the same width
 // regardless of shape (circle vs. diamond).
@@ -57,48 +56,16 @@ export const hoverStyle = {
     'background-color .2s ease-out, border-color .2s ease-out, color .2s ease-out'
 };
 
-const assessmentStatusStyle = {
-  [LevelStatus.attempted]: {
-    borderColor: color.level_submitted
-  },
-  [LevelStatus.submitted]: {
-    borderColor: color.level_submitted,
-    backgroundColor: color.level_submitted,
-    color: color.white
-  },
-  [LevelStatus.completed_assessment]: {
-    borderColor: color.level_submitted,
-    backgroundColor: color.level_submitted,
-    color: color.white
-  },
-  [LevelStatus.perfect]: {
-    borderColor: color.level_submitted,
-    backgroundColor: color.level_submitted,
-    color: color.white
-  },
-  [LevelStatus.readonly]: {
-    borderColor: color.level_submitted,
-    backgroundColor: color.level_submitted,
-    color: color.white
-  }
-};
-
-const levelStatusStyle = {
-  [LevelStatus.attempted]: {
-    borderColor: color.level_perfect
-  },
+const statusStyle = {
   [LevelStatus.perfect]: {
-    borderColor: color.level_perfect,
     backgroundColor: color.level_perfect,
     color: color.white
   },
   [LevelStatus.free_play_complete]: {
-    borderColor: color.level_perfect,
     backgroundColor: color.level_perfect,
     color: color.white
   },
   [LevelStatus.passed]: {
-    borderColor: color.level_perfect,
     backgroundColor: color.level_passed
   },
   // Note: There are submittable levels that are not assessments.
@@ -118,50 +85,48 @@ const levelStatusStyle = {
     backgroundColor: color.level_submitted,
     color: color.white
   },
-  // Below are used by peer reviews
+  // Below three are used by peer reviews
   [LevelStatus.review_rejected]: {
     color: color.white,
     borderColor: color.level_review_rejected,
     backgroundColor: color.level_review_rejected
   },
   [LevelStatus.review_accepted]: {
     color: color.white,
-    borderColor: color.level_perfect,
     backgroundColor: color.level_perfect
+  },
+  [LevelStatus.locked]: {
+    // Don't want our green border even though status isn't not_tried
+    borderColor: color.lighter_gray
   }
 };
 
 /**
  * Given a level object, figure out styling related to its color, border color,
  * and background color
  */
-export const levelProgressStyle = (levelStatus, levelKind, disabled) => {
+export const levelProgressStyle = (level, disabled) => {
   let style = {
-    borderWidth: BUBBLE_BORDER_WIDTH,
-    borderColor: color.lighter_gray,
-    borderStyle: 'solid',
+    borderWidth: 2,
     color: color.charcoal,
     backgroundColor: color.level_not_tried
   };
 
-  // We don't return early for disabled assessments that have been submitted
-  // so that they still show their submitted status.
-  if (
-    (disabled && levelStatus !== LevelStatus.submitted) ||
-    !levelStatus ||
-    levelStatus === levelStatus.not_tried ||
-    levelStatus === LevelStatus.locked
-  ) {
-    return style;
-  }
+  if (disabled) {
+    style = {
+      ...style,
+      ...(!disabled && hoverStyle)
+    };
+  } else {
+    if (level.status !== LevelStatus.not_tried) {
+      style.borderColor = color.level_perfect;
+    }
 
-  const statusStyle =
-    levelKind === LevelKind.assessment
-      ? assessmentStatusStyle[levelStatus]
-      : levelStatusStyle[levelStatus];
+    style = {
+      ...style,
+      ...statusStyle[level.status]
+    };
+  }
 
-  return {
-    ...style,
-    ...statusStyle
-  };
+  return style;
 };

dashboard/app/models/user_level.rb

@@ -36,6 +36,8 @@ class UserLevel < ApplicationRecord
   after_save :after_submit, if: :submitted_or_resubmitted?
   before_save :before_unsubmit, if: ->(ul) {ul.submitted_changed? from: true, to: false}
 
+  validate :readonly_requires_submitted
+
   # TODO(asher): Consider making these scopes and the methods below more consistent, in tense and in
   # word choice.
   scope :attempted, -> {where.not(best_result: nil)}
@@ -47,6 +49,12 @@ def self.by_stage(stage)
     where(script: stage.script, level: levels)
   end
 
+  def readonly_requires_submitted
+    if readonly_answers? && !submitted?
+      errors.add(:readonly_answers, 'readonly_answers only valid on submitted UserLevel')
+    end
+  end
+
   def attempted?
     !best_result.nil?
   end
@@ -159,6 +167,7 @@ def self.update_lockable_state(user_id, level_id, script_id, locked, readonly_an
     return if !user_level.persisted? && locked
 
     user_level.assign_attributes(
+      submitted: locked || readonly_answers,
       readonly_answers: !locked && readonly_answers,
       unlocked_at: locked ? nil : Time.now,
       # level_group, which is the only levels that we lock, always sets best_result to 100 when complete

dashboard/test/controllers/api_controller_test.rb

@@ -383,7 +383,7 @@ class ApiControllerTest < ActionController::TestCase
 
     post :update_lockable_state, params: {updates: updates}
     user_level = UserLevel.find_by(user_level_data)
-    assert_equal false, user_level.submitted?
+    assert_equal true, user_level.submitted?
     assert_equal true, user_level.readonly_answers?
     assert_not_nil user_level.unlocked_at
 
@@ -436,7 +436,7 @@ class ApiControllerTest < ActionController::TestCase
 
       post :update_lockable_state, params: {updates: updates}
       user_level = UserLevel.find_by(user_level_data)
-      assert_equal false, user_level.submitted?
+      assert_equal true, user_level.submitted?
       assert_equal false, user_level.readonly_answers?
       assert_nil user_level.unlocked_at
       # update_lockable_state does not modify updated_at
@@ -456,7 +456,7 @@ class ApiControllerTest < ActionController::TestCase
 
       post :update_lockable_state, params: {updates: updates}
       user_level = UserLevel.find_by(user_level_data)
-      assert_equal false, user_level.submitted?
+      assert_equal true, user_level.submitted?
       assert_equal true, user_level.readonly_answers?
       assert_not_nil user_level.unlocked_at
       assert_equal expected_updated_at, user_level.updated_at
@@ -494,7 +494,7 @@ class ApiControllerTest < ActionController::TestCase
 
       post :update_lockable_state, params: {updates: updates}
       user_level = UserLevel.find_by(user_level_data)
-      assert_equal true, user_level.submitted?
+      assert_equal false, user_level.submitted?
       assert_equal false, user_level.readonly_answers?
       assert_not_nil user_level.unlocked_at
       assert_equal expected_updated_at, user_level.updated_at

dashboard/test/ui/features/learning_platform/level_types/level_group_multi_page_dots.feature

@@ -49,20 +49,20 @@ Scenario: Submit three pages as... 1. some, 2. none, 3. all questions answered.
   # Verify the three dots in the header are 1. some, 2. none, 3. all questions answered.
   And I verify progress in the header of the current page is "perfect_assessment" for level 2
   And I verify progress in the header of the current page is "not_tried" for level 3
-  And I verify progress in the header of the current page is "attempted_assessment" for level 4
+  And I verify progress in the header of the current page is "attempted" for level 4
 
   # Open the dropdown and verify the same three dots.
   Then I open the progress drop down of the current page
   And I verify progress in the drop down of the current page is "perfect_assessment" for stage 23 level 2
   And I verify progress in the drop down of the current page is "not_tried" for stage 23 level 3
-  And I verify progress in the drop down of the current page is "attempted_assessment" for stage 23 level 4
+  And I verify progress in the drop down of the current page is "attempted" for stage 23 level 4
 
   # Go to the course page and verify the same three dots.
   Then I navigate to the course page for "allthethings"
   And I wait until jQuery Ajax requests are finished
   And I verify progress for stage 23 level 2 is "perfect_assessment"
   And I verify progress for stage 23 level 3 is "not_tried"
-  And I verify progress for stage 23 level 4 is "attempted_assessment"
+  And I verify progress for stage 23 level 4 is "attempted"
 
   # Submit the assessment.
   When I am on "http://studio.code.org/s/allthethings/stage/23/puzzle/2/page/3?noautoplay=true"
@@ -72,7 +72,7 @@ Scenario: Submit three pages as... 1. some, 2. none, 3. all questions answered.
   And I am on "http://studio.code.org/s/allthethings/stage/23/puzzle/1?noautoplay=true"
   And I wait to see ".react_stage"
 
-  # Verify the three dots in the header all reflect the submission.
+  # Verify the three dots in the header all appear submitted.
   And I verify progress in the header of the current page is "perfect_assessment" for level 2
   And I verify progress in the header of the current page is "perfect_assessment" for level 3
   And I verify progress in the header of the current page is "perfect_assessment" for level 4
@@ -121,7 +121,7 @@ Scenario: optional free play level
 
   # Verify the bubble status and submit dialog message show incomplete
 
-  Then I verify progress in the header of the current page is "attempted_assessment" for level 3
+  Then I verify progress in the header of the current page is "attempted" for level 3
 
   When I press ".submitButton" using jQuery
   And I wait to see a dialog titled "Submit your assessment"
@@ -138,7 +138,7 @@ Scenario: optional free play level
   And I wait to see ".level-group-content"
   And check that the URL contains "/page/3"
 
-  # Verify the bubble status and submit dialog contents are accurate
+  # Verify the bubble status and submit dialog contents are now complete
   Then I verify progress in the header of the current page is "perfect_assessment" for level 2
   Then I verify progress in the header of the current page is "perfect_assessment" for level 3
   Then I verify progress in the header of the current page is "perfect_assessment" for level 4