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

Restore "preserve level variants when saving script editor" #40872

Merged

Conversation

davidsbailey
Copy link
Member

@davidsbailey davidsbailey commented Jun 1, 2021

Reverts #40868, restoring #40807 . The problem was that I accidentally changed .concat to .push, because I thought the thing being passed in was a string rather than an array.

Testing story

  • manually verified that courses A and D save properly now
  • manually verified there were no changes to the stage dsl being sent to the server
  • new unit test

The new unit test is a bit contrived, but will do the job of preventing a similar regression if this code changes again before it is removed.

@davidsbailey davidsbailey changed the title Revert "Revert "preserve level variants when saving script editor"" Restore "preserve level variants when saving script editor" Jun 1, 2021
@davidsbailey davidsbailey requested a review from a team June 1, 2021 21:37
@@ -364,11 +364,12 @@ const serializeLesson = (lesson, levelKeyList) => {
s.push('variants');
level.ids.forEach(id => {
const active = id === level.activeId;
s.push(` ${serializeLevel(levelKeyList, id, level, active)}`);
const lines = serializeLevel(levelKeyList, id, level, active);
s = s.concat(lines.map(line => ` ${line}`));
});
s.push('endvariants');
Copy link
Contributor

Choose a reason for hiding this comment

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

should this line and line 364 not also be concat?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think either would work because s.push and s = s.concat do the same thing when passed a string. serializeLevel is different because it (confusingly) returns an array.

@davidsbailey davidsbailey merged commit d70e56d into staging Jun 2, 2021
@davidsbailey davidsbailey deleted the revert-40868-revert-40807-preserve-variant-script-update branch June 2, 2021 04:58
davidsbailey added a commit that referenced this pull request Jun 17, 2021
…rt-40807-preserve-variant-script-update"

This reverts commit d70e56d, reversing
changes made to 2af4071.
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