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

Bug Fix: Student has not started level message showing when not appropriate #31704

Merged
merged 4 commits into from Nov 6, 2019

Conversation

dmcavoy
Copy link
Contributor

@dmcavoy dmcavoy commented Nov 5, 2019

Description

There were two different bugs with the "Student has not started this level" banner

Bug 1: Contained levels

The banner would always show on contained levels because the progress for a contained level is actually on the containee level instead of the container level. For now, this hides the banner for all contained levels as it should be obvious from the state of the level if it has been started or not (either they answered the question or not).

Before After
Screen Shot 2019-11-06 at 10 53 38 AM Screen Shot 2019-11-06 at 10 49 02 AM

Bug 2: Dance party levels

The banner was showing on dance party levels because its a channel backed level type but it wasn't in the list of channel backed levels we were checking.

Before After
Screen Shot 2019-11-06 at 10 53 29 AM Screen Shot 2019-11-06 at 10 48 17 AM

Links

Testing story

I added a UI test for checking that the banner does not show on a contained level.

I did not add a test for dance levels as I think there would be too many cases if we had a UI test for each level type. Instead, right now there is a test for just one channel backed level type and one non-channel backed level type.

Reviewer Checklist:

  • Tests provide adequate coverage
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

Copy link
Contributor

@Erin007 Erin007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great sleuthing!

@dmcavoy dmcavoy merged commit 530f307 into staging Nov 6, 2019
@dmcavoy dmcavoy deleted the student-not-started-bug branch November 6, 2019 18:22
if (config.hasContainedLevels) {
return false;
} else if (
['Gamelab', 'Applab', 'Weblab', 'Spritelab', 'Dance'].includes(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late comment, but I think we would also need some logic for artist and playlab levels which are project-backed, for example. These levels have a project_template_level_name. I checked on https://studio.code.org/s/express-2019/stage/7/puzzle/4 and I do not see the warning message regardless of whether the student has progress. It works fine on non-project-backed artist levels though. Opening as https://codedotorg.atlassian.net/browse/LP-983

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice find Dave! Thanks! I'm going to put that in next sprint as a follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants