preserve encrypted flag when cloning DSLDefined levels #27066
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
davidsbailey
commented
Feb 14, 2019
| level | ||
| level_params = {} | ||
| level_params[:encrypted] = encrypted if encrypted | ||
| self.class.create_from_level_builder({dsl_text: new_dsl}, level_params) if new_dsl |
There was a problem hiding this comment.
[re-posting after adding another commit]
I refactored this to call create_from_level_builder directly in order to make this unit-testable. Otherwise, level = super(new_name) would return a level with the right value for encrypted, but that value would be omitted when update! --> assign_attributes --> create_from_level_builder --> rewrite_dsl_file was called, causing the resulting dsl file not to be encrypted.
There was a problem hiding this comment.
More generally, this closes the gap between what levelbuilders will see on the levelbuilder machine (based on what's in the local db) and what will be seen in other environments (based on what gets output to the filesystem).
davidsbailey
force-pushed
the
encrypt-cloned-dsl
branch
from
5bc1d95
to
b8fcc5d
Compare
|
@islemaster , please review! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
When scripts containing encrypted levels have been copied in the past, the new copies of those levels are not encrypted. for example:
code-dot-org/dashboard/config/scripts/u3l04_assessment2.multi
Lines 1 to 2 in 021454b
code-dot-org/dashboard/config/scripts/u3l04_assessment2_2018.multi
Lines 1 to 2 in 021454b
Generally speaking, Script copying works by calling
Script#clone_with_suffixwhich callsLevel#clone_with_suffixfor each of its levels. For more background on script copying, see these PRs:Description
This PR preserves the
encryptedflag when copying DSLDefined levels, and adds corresponding unit tests.Verification
In addition to the new unit tests, I've also manually verified that copying a script via
Script#clone_with_suffixpreserves theencryptedflag on encrypted/unencrypted multi/match levels in that script.