Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jan 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion apps/src/templates/progress/ProgressBubble.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)
};

Expand Down
3 changes: 1 addition & 2 deletions apps/src/templates/progress/ProgressPill.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
83 changes: 24 additions & 59 deletions apps/src/templates/progress/progressStyles.js
Original file line number Diff line number Diff line change
@@ -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).
Expand Down Expand Up @@ -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.
Expand All @@ -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;
};
9 changes: 9 additions & 0 deletions dashboard/app/models/user_level.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions dashboard/test/controllers/api_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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"
Expand All @@ -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
Expand Down