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

Move the lessons to a temporary lesson group so that moving lesson groups works #39353

Merged
merged 2 commits into from
Mar 5, 2021

Conversation

dmcavoy
Copy link
Contributor

@dmcavoy dmcavoy commented Mar 4, 2021

We got a report that moving the last lesson group up was giving errors about legacy script levels not being allowed in a bunch of lessons. The issue is a follow up issue to the fix in this PR: #39255.

Because the fix in the previous PR moved all the lessons to the last lesson group when you moved the last lesson group up it deleted all the lessons that had not been set up in previous lesson groups and that resulted in the chain of issues listed in that previous PR.

To fix this we add a temporary lesson group to hold all the lessons and put it at the end of the script. This temp lesson group will get deleted by these lines in add_script once everything else in the script has been set up.

temp_lgs = LessonGroup.add_lesson_groups(raw_lesson_groups, script, new_suffix, editor_experiment)
script.reload
script.lesson_groups = temp_lgs
script.save!

Testing story

Added a test to make sure you could move the last lesson group in a migrated script up without lessons getting deleted.

Follow-up work

We still want to move off of this system going forward but after our April deadline.

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • 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

@dmcavoy dmcavoy requested review from davidsbailey and a team March 4, 2021 23:02
@dmcavoy dmcavoy merged commit 264e322 into staging Mar 5, 2021
@dmcavoy dmcavoy deleted the lesson-group-issue-part-2 branch March 5, 2021 00:51
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

2 participants