From bc629ee50643341af9c0b09c34aa8eb8e7434283 Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Thu, 9 Aug 2018 01:04:39 -0700 Subject: [PATCH 01/64] Instant visualization updates for artist --- apps/src/turtle/artist.js | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/apps/src/turtle/artist.js b/apps/src/turtle/artist.js index f45b6ab4c9be9..dede02587e502 100644 --- a/apps/src/turtle/artist.js +++ b/apps/src/turtle/artist.js @@ -55,6 +55,7 @@ import ArtistSkins from './skins'; import dom from '../dom'; import {SignInState} from '../code-studio/progressRedux'; import Visualization from '@code-dot-org/artist'; +import experiments from '../util/experiments'; const CANVAS_HEIGHT = 400; const CANVAS_WIDTH = 400; @@ -179,6 +180,8 @@ var Artist = function () { // these get set by init based on skin. this.speedSlider = null; + + this.autoRun = experiments.isEnabled('auto-artist'); }; module.exports = Artist; @@ -341,6 +344,19 @@ Artist.prototype.init = function (config) { if (finishButton) { dom.addClickTouchEvent(finishButton, this.checkAnswer.bind(this)); } + + if (this.autoRun) { + const changeHandler = () => this.execute(true); + if (this.studioApp_.isUsingBlockly()) { + const blocklyCanvas = Blockly.mainBlockSpace.getCanvas(); + blocklyCanvas.addEventListener('blocklyBlockSpaceChange', + changeHandler); + } else { + this.studioApp_.editor.on('change', changeHandler); + // Droplet doesn't automatically bubble up aceEditor changes + this.studioApp_.editor.aceEditor.on('change', changeHandler); + } + } } return Promise.all([ @@ -744,7 +760,7 @@ Artist.prototype.handleExecutionError = function (err, lineNumber, outputString) /** * Execute the user's code. Heaven help us... */ -Artist.prototype.execute = function () { +Artist.prototype.execute = function (instant=this.instant_) { this.api.log = []; // Reset the graphic. @@ -773,7 +789,7 @@ Artist.prototype.execute = function () { // If this is a free play level, save the code every time the run button is // clicked rather than only on finish - if (this.level.freePlay) { + if (this.level.freePlay && !instant) { this.levelComplete = true; this.testResults = TestResults.FREE_PLAY; this.report(false); @@ -792,17 +808,19 @@ Artist.prototype.execute = function () { this.studioApp_.reset(); } - this.studioApp_.playAudio('start', {loop : true}); + if (!instant) { + this.studioApp_.playAudio('start', {loop : true}); + } // animate the transcript. - if (this.instant_) { + if (instant) { while (this.animate()) {} } else { this.pid = window.setTimeout(_.bind(this.animate, this), 100); } - if (this.studioApp_.isUsingBlockly()) { + if (this.studioApp_.isUsingBlockly() && !instant) { // Disable toolbox while running Blockly.mainBlockSpaceEditor.setEnableToolbox(false); } From eb844ee6fb885aaf11100731b5ff823c36972a4a Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Thu, 9 Aug 2018 11:03:40 -0700 Subject: [PATCH 02/64] Artist auto_run config --- apps/src/turtle/artist.js | 5 +++-- dashboard/app/models/levels/artist.rb | 1 + dashboard/app/views/levels/editors/_artist.html.haml | 3 +++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/apps/src/turtle/artist.js b/apps/src/turtle/artist.js index dede02587e502..48f0ea9b5e127 100644 --- a/apps/src/turtle/artist.js +++ b/apps/src/turtle/artist.js @@ -180,8 +180,6 @@ var Artist = function () { // these get set by init based on skin. this.speedSlider = null; - - this.autoRun = experiments.isEnabled('auto-artist'); }; module.exports = Artist; @@ -300,6 +298,9 @@ Artist.prototype.init = function (config) { showDecoration: () => this.skin.id === "elsa", }); + this.autoRun = experiments.isEnabled('auto-artist') || + this.level.autoRun; + config.grayOutUndeletableBlocks = true; config.forceInsertTopBlock = 'when_run'; config.dropletConfig = dropletConfig; diff --git a/dashboard/app/models/levels/artist.rb b/dashboard/app/models/levels/artist.rb index 5475ce3d1e137..2e23698e73041 100644 --- a/dashboard/app/models/levels/artist.rb +++ b/dashboard/app/models/levels/artist.rb @@ -36,6 +36,7 @@ class Artist < Blockly shapeways_url disable_sharing solution_image_url + auto_run ) def xml_blocks diff --git a/dashboard/app/views/levels/editors/_artist.html.haml b/dashboard/app/views/levels/editors/_artist.html.haml index 852cd2cf90593..633ba3eb04fe0 100644 --- a/dashboard/app/views/levels/editors/_artist.html.haml +++ b/dashboard/app/views/levels/editors/_artist.html.haml @@ -22,6 +22,9 @@ .field = f.label :permitted_errors, 'Permitted errors (default 0)' = f.number_field :permitted_errors, :value => @level.permitted_errors +.field + = f.label :auto_run, 'Automatically rerun on block changes' + = boolean_check_box f, :auto_run .field = link_to 'Edit Predraw Blocks', level_edit_blocks_path(@level, :predraw_blocks) unless @level.new_record? = render partial: 'levels/editors/collapsible_block_editor', locals: {f: f, xml_id: 'predraw'} From bf06ca758c0fc29ae4f499b82984c8b6cce30232 Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Thu, 9 Aug 2018 14:34:40 -0700 Subject: [PATCH 03/64] Dedup change handler registration --- apps/src/StudioApp.js | 18 ++++++++++++++++++ apps/src/gamelab/GameLab.js | 11 +---------- apps/src/turtle/artist.js | 11 +---------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/apps/src/StudioApp.js b/apps/src/StudioApp.js index fd5baee112984..cd4a5d2327a54 100644 --- a/apps/src/StudioApp.js +++ b/apps/src/StudioApp.js @@ -837,6 +837,24 @@ StudioApp.prototype.reset = function (shouldPlayOpeningAnimation) { */ StudioApp.prototype.runButtonClick = function () {}; +StudioApp.prototype.addChangeHandler = function (newHandler) { + if (!this.changeHandlers) { + this.changeHandlers = [newHandler]; + const runAllHandlers = () => + this.changeHandlers.forEach(handler => handler()); + if (this.isUsingBlockly()) { + const blocklyCanvas = Blockly.mainBlockSpace.getCanvas(); + blocklyCanvas.addEventListener('blocklyBlockSpaceChange', runAllHandlers); + } else { + this.editor.on('change', runAllHandlers); + // Droplet doesn't automatically bubble up aceEditor changes + this.editor.aceEditor.on('change', runAllHandlers); + } + } else { + this.changeHandlers.push(newHandler); + } +}; + /** * Toggle whether run button or reset button is shown * @param {string} button Button to show, either "run" or "reset" diff --git a/apps/src/gamelab/GameLab.js b/apps/src/gamelab/GameLab.js index 5ebbcb365e9fa..7d129f961374f 100644 --- a/apps/src/gamelab/GameLab.js +++ b/apps/src/gamelab/GameLab.js @@ -358,16 +358,7 @@ GameLab.prototype.init = function (config) { this.setCrosshairCursorForPlaySpace(); if (this.shouldAutoRunSetup) { - const changeHandler = this.rerunSetupCode.bind(this); - if (this.studioApp_.isUsingBlockly()) { - const blocklyCanvas = Blockly.mainBlockSpace.getCanvas(); - blocklyCanvas.addEventListener('blocklyBlockSpaceChange', - changeHandler); - } else { - this.studioApp_.editor.on('change', changeHandler); - // Droplet doesn't automatically bubble up aceEditor changes - this.studioApp_.editor.aceEditor.on('change', changeHandler); - } + this.studioApp_.addChangeHandler(this.rerunSetupCode.bind(this)); } }; diff --git a/apps/src/turtle/artist.js b/apps/src/turtle/artist.js index 48f0ea9b5e127..9584297ab4db7 100644 --- a/apps/src/turtle/artist.js +++ b/apps/src/turtle/artist.js @@ -347,16 +347,7 @@ Artist.prototype.init = function (config) { } if (this.autoRun) { - const changeHandler = () => this.execute(true); - if (this.studioApp_.isUsingBlockly()) { - const blocklyCanvas = Blockly.mainBlockSpace.getCanvas(); - blocklyCanvas.addEventListener('blocklyBlockSpaceChange', - changeHandler); - } else { - this.studioApp_.editor.on('change', changeHandler); - // Droplet doesn't automatically bubble up aceEditor changes - this.studioApp_.editor.aceEditor.on('change', changeHandler); - } + this.studioApp_.addChangeHandler(() => this.execute(true)); } } From 0f39d97cbcad1ccffed5bbbf515a00be992be225 Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Fri, 10 Aug 2018 18:28:11 -0700 Subject: [PATCH 04/64] Add a "limited" autorun option --- apps/src/turtle/artist.js | 98 ++++++++++++++++++++++++++++++--------- 1 file changed, 75 insertions(+), 23 deletions(-) diff --git a/apps/src/turtle/artist.js b/apps/src/turtle/artist.js index 9584297ab4db7..163a6a95f0c9a 100644 --- a/apps/src/turtle/artist.js +++ b/apps/src/turtle/artist.js @@ -180,6 +180,8 @@ var Artist = function () { // these get set by init based on skin. this.speedSlider = null; + + this.immovableBlocks = []; }; module.exports = Artist; @@ -191,7 +193,7 @@ module.exports.Visualization = Visualization; */ Artist.prototype.injectStudioApp = function (studioApp) { this.studioApp_ = studioApp; - this.studioApp_.reset = _.bind(this.reset, this); + this.studioApp_.reset = _.bind(this.resetButtonClick, this); this.studioApp_.runButtonClick = _.bind(this.runButtonClick, this); this.studioApp_.setCheckForEmptyBlocks(true); @@ -298,8 +300,10 @@ Artist.prototype.init = function (config) { showDecoration: () => this.skin.id === "elsa", }); + this.limitedAutoRun = experiments.isEnabled('limited-auto-artist') || + this.level.limitedAutoRun; this.autoRun = experiments.isEnabled('auto-artist') || - this.level.autoRun; + this.level.autoRun || this.limitedAutoRun; config.grayOutUndeletableBlocks = true; config.forceInsertTopBlock = 'when_run'; @@ -347,7 +351,17 @@ Artist.prototype.init = function (config) { } if (this.autoRun) { - this.studioApp_.addChangeHandler(() => this.execute(true)); + this.studioApp_.addChangeHandler(() => { + if (this.limitedAutoRun) { + if (this.studioApp_.isRunning() && !this.executing) { + this.execute(); + } + } else { + if (!this.executing) { + this.execute(); + } + } + }); } } @@ -469,9 +483,11 @@ Artist.prototype.afterInject_ = function (config) { this.speedSlider.setValue(config.level.sliderSpeed); } + this.shouldAnimate_ = true; // Do not animate drawing, used for tests if (config.level.instant) { this.instant_ = true; + this.shouldAnimate_ = false; } if (this.studioApp_.isUsingBlockly()) { @@ -544,7 +560,7 @@ Artist.prototype.drawAnswer = function (canvas) { * composited over the scratch canvas. */ Artist.prototype.drawLogOnCanvas = function (log, canvas) { - this.studioApp_.reset(); + this.reset(); while (log.length) { var tuple = log.shift(); this.step(tuple[0], tuple.slice(1), {smoothAnimate: false}); @@ -684,15 +700,43 @@ Artist.prototype.reset = function (ignore) { * Click the run button. Start the program. */ Artist.prototype.runButtonClick = function () { + this.shouldAnimate_ = !this.instant_; this.studioApp_.toggleRunReset('reset'); document.getElementById('spinner').style.visibility = 'visible'; if (this.studioApp_.isUsingBlockly()) { Blockly.mainBlockSpace.traceOn(true); } this.studioApp_.attempts++; + if (this.limitedAutoRun) { + this.immovableBlocks = []; + Blockly.mainBlockSpace.getAllBlocks().forEach(block => { + if (!block.isMovable()) { + this.immovableBlocks.push(block); + } else { + block.setMovable(false); + } + }); + } this.execute(); }; +Artist.prototype.resetButtonClick = function () { + this.shouldAnimate_ = !this.instant_ && !this.autoRun; + if (this.limitedAutoRun) { + Blockly.mainBlockSpace.getAllBlocks().forEach(block => { + if (!this.immovableBlocks.includes(block)) { + block.setMovable(true); + } + }); + } + + if (this.autoRun && !this.limitedAutoRun) { + this.execute(); + } else { + this.reset(); + } +}; + Artist.prototype.evalCode = function (code) { try { CustomMarshalingInterpreter.evalWith(code, { @@ -752,11 +796,12 @@ Artist.prototype.handleExecutionError = function (err, lineNumber, outputString) /** * Execute the user's code. Heaven help us... */ -Artist.prototype.execute = function (instant=this.instant_) { +Artist.prototype.execute = function () { + this.executing = true; this.api.log = []; // Reset the graphic. - this.studioApp_.reset(); + this.reset(); if (this.studioApp_.hasUnwantedExtraTopBlocks() || this.studioApp_.hasDuplicateVariablesInForLoops()) { @@ -781,7 +826,7 @@ Artist.prototype.execute = function (instant=this.instant_) { // If this is a free play level, save the code every time the run button is // clicked rather than only on finish - if (this.level.freePlay && !instant) { + if (this.level.freePlay && this.shouldAnimate_) { this.levelComplete = true; this.testResults = TestResults.FREE_PLAY; this.report(false); @@ -797,25 +842,21 @@ Artist.prototype.execute = function (instant=this.instant_) { // Then, reset our state and draw the user's actions in a visible, animated // way - this.studioApp_.reset(); + this.reset(); } - if (!instant) { + if (this.shouldAnimate_) { this.studioApp_.playAudio('start', {loop : true}); - } - - // animate the transcript. - - if (instant) { - while (this.animate()) {} - } else { + // animate the transcript. this.pid = window.setTimeout(_.bind(this.animate, this), 100); + if (this.studioApp_.isUsingBlockly()) { + // Disable toolbox while running + Blockly.mainBlockSpaceEditor.setEnableToolbox(false); + } + } else { + while (this.animate()) {} } - if (this.studioApp_.isUsingBlockly() && !instant) { - // Disable toolbox while running - Blockly.mainBlockSpaceEditor.setEnableToolbox(false); - } }; /** @@ -853,6 +894,9 @@ Artist.prototype.checkforTurnAndMove_ = function () { */ Artist.prototype.executeTuple_ = function () { if (this.api.log.length === 0) { + if (!this.shouldAnimate_) { + this.visualization.display(); + } return false; } @@ -866,7 +910,9 @@ Artist.prototype.executeTuple_ = function () { var command = tuple[0]; var id = tuple[tuple.length-1]; - this.studioApp_.highlight(String(id)); + if (this.shouldAnimate_) { + this.studioApp_.highlight(String(id)); + } // Should we execute another tuple in this frame of animation? if (this.skin.consolidateTurnAndMove && this.checkforTurnAndMove_()) { @@ -875,7 +921,9 @@ Artist.prototype.executeTuple_ = function () { // We only smooth animate for Anna & Elsa, and only if there is not another tuple to be done. var tupleDone = this.step(command, tuple.slice(1), {smoothAnimate: this.skin.smoothAnimate && !executeSecondTuple}); - this.visualization.display(); + if (this.shouldAnimate_) { + this.visualization.display(); + } if (tupleDone) { this.api.log.shift(); @@ -904,6 +952,10 @@ Artist.prototype.finishExecution_ = function () { } else { this.checkAnswer(); } + setTimeout(() => { + this.executing = false; + this.shouldAnimate_ = !this.instant_ && !this.autoRun; + }, 0); }; /** @@ -955,7 +1007,7 @@ Artist.prototype.animate = function () { } } - if (!this.instant_) { + if (this.shouldAnimate_) { this.pid = window.setTimeout(_.bind(this.animate, this), stepSpeed); } return true; From 216bc2cf6a20d976427adaee5e632f8ec4e2ffa4 Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Fri, 10 Aug 2018 18:37:49 -0700 Subject: [PATCH 05/64] Add limited autorun to level config --- dashboard/app/models/levels/artist.rb | 1 + dashboard/app/views/levels/editors/_artist.html.haml | 3 +++ 2 files changed, 4 insertions(+) diff --git a/dashboard/app/models/levels/artist.rb b/dashboard/app/models/levels/artist.rb index 2e23698e73041..a35f86cc428ed 100644 --- a/dashboard/app/models/levels/artist.rb +++ b/dashboard/app/models/levels/artist.rb @@ -37,6 +37,7 @@ class Artist < Blockly disable_sharing solution_image_url auto_run + limited_auto_run ) def xml_blocks diff --git a/dashboard/app/views/levels/editors/_artist.html.haml b/dashboard/app/views/levels/editors/_artist.html.haml index 633ba3eb04fe0..6016a1ccbd8e9 100644 --- a/dashboard/app/views/levels/editors/_artist.html.haml +++ b/dashboard/app/views/levels/editors/_artist.html.haml @@ -25,6 +25,9 @@ .field = f.label :auto_run, 'Automatically rerun on block changes' = boolean_check_box f, :auto_run +.field + = f.label :limited_auto_run, 'Automatically rerun on block changes, but only allow field edits' + = boolean_check_box f, :limited_auto_run .field = link_to 'Edit Predraw Blocks', level_edit_blocks_path(@level, :predraw_blocks) unless @level.new_record? = render partial: 'levels/editors/collapsible_block_editor', locals: {f: f, xml_id: 'predraw'} From b859b29c00e787c4ffcfa405d2d7be428543ad02 Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Thu, 16 Aug 2018 10:11:47 -0700 Subject: [PATCH 06/64] Add unit tests for addChangeHandler --- apps/test/unit/StudioAppTest.js | 57 +++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/apps/test/unit/StudioAppTest.js b/apps/test/unit/StudioAppTest.js index 7abce96df92ce..e13d95265d942 100644 --- a/apps/test/unit/StudioAppTest.js +++ b/apps/test/unit/StudioAppTest.js @@ -1,3 +1,4 @@ +import $ from 'jquery'; import sinon from 'sinon'; import {expect} from '../util/configuredChai'; import {singleton as studioApp, stubStudioApp, restoreStudioApp, makeFooterMenuItems} from '@cdo/apps/StudioApp'; @@ -156,4 +157,60 @@ describe("StudioApp", () => { expect(itemTexts).to.include('footer.try_hour_of_code'); }); }); + + describe('addChangeHandler', () => { + beforeEach(stubStudioApp); + afterEach(restoreStudioApp); + + it('calls a handler in response to a blockly change', () => { + let changed = false; + studioApp().usingBlockly_ = true; + + studioApp().addChangeHandler(() => changed = true); + Blockly.mainBlockSpace.getCanvas() + .dispatchEvent(new Event('blocklyBlockSpaceChange')); + + expect(changed).to.be.true; + }); + + it('calls a handler in response to a droplet change', () => { + let changed = false; + studioApp().usingBlockly_ = false; + studioApp().editor = $(document.createElement('div')); + studioApp().editor.aceEditor = $(document.createElement('div')); + + studioApp().addChangeHandler(() => changed = true); + studioApp().editor.trigger('change'); + + expect(changed).to.be.true; + + studioApp().usingBlockly_ = true; + }); + + it('calls a handler in response to an aceEditor change', () => { + let changed = false; + studioApp().usingBlockly_ = false; + studioApp().editor = $(document.createElement('div')); + studioApp().editor.aceEditor = $(document.createElement('div')); + + studioApp().addChangeHandler(() => changed = true); + studioApp().editor.aceEditor.trigger('change'); + + expect(changed).to.be.true; + + studioApp().usingBlockly_ = true; + }); + + it('calls multiple handlers in response to a blockly change', () => { + let changed1 = false, changed2 = false; + + studioApp().addChangeHandler(() => changed1 = true); + studioApp().addChangeHandler(() => changed2 = true); + Blockly.mainBlockSpace.getCanvas() + .dispatchEvent(new Event('blocklyBlockSpaceChange')); + + expect(changed1).to.be.true; + expect(changed2).to.be.true; + }); + }); }); From a891fdcbb8bfec936ab8b517afee79db31240dde Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Fri, 17 Aug 2018 11:41:03 -0700 Subject: [PATCH 07/64] Don't count internal entrypoints for common js --- apps/Gruntfile.js | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/apps/Gruntfile.js b/apps/Gruntfile.js index 64667bf662cf6..fc97513469fec 100644 --- a/apps/Gruntfile.js +++ b/apps/Gruntfile.js @@ -440,19 +440,10 @@ describe('entry tests', () => { var appsEntries = _.fromPairs(appsToBuild.map(function (app) { return [app, './src/sites/studio/pages/levels-' + app + '-main.js']; })); + var codeStudioEntries = { 'blockly': './src/sites/studio/pages/blockly.js', 'code-studio': './src/sites/studio/pages/code-studio.js', - 'levelbuilder': './src/sites/studio/pages/levelbuilder.js', - 'levelbuilder_applab': './src/sites/studio/pages/levelbuilder_applab.js', - 'levelbuilder_craft': './src/sites/studio/pages/levelbuilder_craft.js', - 'levelbuilder_edit_script': './src/sites/studio/pages/levelbuilder_edit_script.js', - 'levelbuilder_gamelab': './src/sites/studio/pages/levelbuilder_gamelab.js', - 'levelbuilder_studio': './src/sites/studio/pages/levelbuilder_studio.js', - 'levelbuilder_pixelation': './src/sites/studio/pages/levelbuilder_pixelation.js', - 'blocks/edit': './src/sites/studio/pages/blocks/edit.js', - 'shared_blockly_functions/edit':'./src/sites/studio/pages/shared_blockly_functions/edit.js', - 'libraries/edit': './src/sites/studio/pages/libraries/edit.js', 'levels/contract_match': './src/sites/studio/pages/levels/contract_match.jsx', 'levels/_curriculum_reference': './src/sites/studio/pages/levels/_curriculum_reference.js', 'levels/_dialog': './src/sites/studio/pages/levels/_dialog.js', @@ -464,10 +455,6 @@ describe('entry tests', () => { 'levels/textMatch': './src/sites/studio/pages/levels/textMatch.js', 'levels/widget': './src/sites/studio/pages/levels/widget.js', 'levels/_external_link': './src/sites/studio/pages/levels/_external_link.js', - 'levels/editors/_blockly': './src/sites/studio/pages/levels/editors/_blockly.js', - 'levels/editors/_droplet': './src/sites/studio/pages/levels/editors/_droplet.js', - 'levels/editors/_all': './src/sites/studio/pages/levels/editors/_all.js', - 'levels/editors/_dsl': './src/sites/studio/pages/levels/editors/_dsl.js', 'projects/index': './src/sites/studio/pages/projects/index.js', 'projects/public': './src/sites/studio/pages/projects/public.js', 'projects/featured': './src/sites/studio/pages/projects/featured.js', @@ -485,10 +472,27 @@ describe('entry tests', () => { 'congrats/index': './src/sites/studio/pages/congrats/index.js', 'courses/index': './src/sites/studio/pages/courses/index.js', 'courses/show': './src/sites/studio/pages/courses/show.js', - 'courses/edit': './src/sites/studio/pages/courses/edit.js', 'devise/registrations/edit': './src/sites/studio/pages/devise/registrations/edit.js', }; + var internalEntries = { + 'blocks/edit': './src/sites/studio/pages/blocks/edit.js', + 'courses/edit': './src/sites/studio/pages/courses/edit.js', + 'levelbuilder': './src/sites/studio/pages/levelbuilder.js', + 'levelbuilder_applab': './src/sites/studio/pages/levelbuilder_applab.js', + 'levelbuilder_craft': './src/sites/studio/pages/levelbuilder_craft.js', + 'levelbuilder_edit_script': './src/sites/studio/pages/levelbuilder_edit_script.js', + 'levelbuilder_gamelab': './src/sites/studio/pages/levelbuilder_gamelab.js', + 'levelbuilder_pixelation': './src/sites/studio/pages/levelbuilder_pixelation.js', + 'levelbuilder_studio': './src/sites/studio/pages/levelbuilder_studio.js', + 'levels/editors/_all': './src/sites/studio/pages/levels/editors/_all.js', + 'levels/editors/_blockly': './src/sites/studio/pages/levels/editors/_blockly.js', + 'levels/editors/_droplet': './src/sites/studio/pages/levels/editors/_droplet.js', + 'levels/editors/_dsl': './src/sites/studio/pages/levels/editors/_dsl.js', + 'libraries/edit': './src/sites/studio/pages/libraries/edit.js', + 'shared_blockly_functions/edit':'./src/sites/studio/pages/shared_blockly_functions/edit.js', + }; + var otherEntries = { essential: './src/sites/studio/pages/essential.js', plc: './src/sites/studio/pages/plc.js', @@ -569,6 +573,7 @@ describe('entry tests', () => { {}, appsEntries, codeStudioEntries, + internalEntries, otherEntries ), function (val) { @@ -615,6 +620,9 @@ describe('entry tests', () => { ...(process.env.ANALYZE_BUNDLE ? [ new BundleAnalyzerPlugin({ analyzerMode: 'static', + excludeAssets: [ + ...Object.keys(internalEntries), + ], }), ] : []), ], From 6df9f80fa40bf4999a245279baa4afe8bd4a2b17 Mon Sep 17 00:00:00 2001 From: Elijah Hamovitz Date: Fri, 17 Aug 2018 17:18:47 -0700 Subject: [PATCH 08/64] isolate rendering of markdown into a single React component --- apps/src/templates/RenderedMarkdown.jsx | 26 +++++++++++++++++++ .../instructions/DialogInstructions.jsx | 7 +---- .../templates/instructions/Instructions.jsx | 12 ++++----- .../instructions/MarkdownInstructions.jsx | 14 +++++----- .../instructions/TopInstructionsCSF.jsx | 22 +++++----------- .../instructions/TopInstructionsCSP.jsx | 9 +++---- 6 files changed, 51 insertions(+), 39 deletions(-) create mode 100644 apps/src/templates/RenderedMarkdown.jsx diff --git a/apps/src/templates/RenderedMarkdown.jsx b/apps/src/templates/RenderedMarkdown.jsx new file mode 100644 index 0000000000000..2f56dc302d195 --- /dev/null +++ b/apps/src/templates/RenderedMarkdown.jsx @@ -0,0 +1,26 @@ +import React, { PropTypes } from 'react'; +import processMarkdown from 'marked'; +import renderer from "../util/StylelessRenderer"; + +/** + * Basic component for rendering a markdown string as HTML. + * + * Right now, it still uses marked; this will eventually be updated to use the + * new remark system, and possibly even support redaction. + */ +export default class RenderedMarkdown extends React.Component { + static propTypes = { + markdown: PropTypes.string.isRequired + }; + + render() { + const processedMarkdown = processMarkdown(this.props.markdown, { renderer }); + /* eslint-disable react/no-danger */ + return ( +
+ ); + /* eslint-enable react/no-danger */ + } +} diff --git a/apps/src/templates/instructions/DialogInstructions.jsx b/apps/src/templates/instructions/DialogInstructions.jsx index 75b56f175c147..d0c2baaafa15b 100644 --- a/apps/src/templates/instructions/DialogInstructions.jsx +++ b/apps/src/templates/instructions/DialogInstructions.jsx @@ -2,8 +2,6 @@ import React, {PropTypes} from 'react'; import { connect } from 'react-redux'; import Instructions from './Instructions'; import msg from '@cdo/locale'; -import processMarkdown from 'marked'; -import renderer from "../../util/StylelessRenderer"; /** * Component for displaying our instructions in the context of a modal dialog @@ -22,9 +20,6 @@ class DialogInstructions extends React.Component { }; render() { - const renderedMarkdown = this.props.longInstructions ? - processMarkdown(this.props.longInstructions, { renderer }) : undefined; - const showInstructions = !(this.props.imgOnly || this.props.hintsOnly); const showImg = !this.props.hintsOnly; return ( @@ -35,7 +30,7 @@ class DialogInstructions extends React.Component { })} shortInstructions={showInstructions ? this.props.shortInstructions : undefined} instructions2={showInstructions ? this.props.shortInstructions2 : undefined} - renderedMarkdown={showInstructions ? renderedMarkdown : undefined} + longInstructions={showInstructions ? this.props.longInstructions : undefined} imgURL={showImg ? this.props.imgURL : undefined} /> ); diff --git a/apps/src/templates/instructions/Instructions.jsx b/apps/src/templates/instructions/Instructions.jsx index 6c5d5a863d0d4..a07b70d47684f 100644 --- a/apps/src/templates/instructions/Instructions.jsx +++ b/apps/src/templates/instructions/Instructions.jsx @@ -17,7 +17,7 @@ const styles = { * A component for displaying our level instructions text, and possibly also * authored hints UI and/or an anigif. These instructions can appear in the top * pane or in a modal dialog. In the latter case, we will sometimes show just - * the hints or just the anigif (in this case instructions/renderedMarkdown + * the hints or just the anigif (in this case instructions/markdown * props will be undefined). */ class Instructions extends React.Component { @@ -25,7 +25,7 @@ class Instructions extends React.Component { puzzleTitle: PropTypes.string, shortInstructions: PropTypes.string, instructions2: PropTypes.string, - renderedMarkdown: PropTypes.string, + markdown: PropTypes.string, imgURL: PropTypes.string, authoredHints: PropTypes.element, inputOutputTable: PropTypes.arrayOf( @@ -38,7 +38,7 @@ class Instructions extends React.Component { render() { // Body logic is as follows: // - // If we have been given rendered markdown, render a div containing + // If we have been given long instructions, render a div containing // that, optionally with inline-styled margins. We don't need to // worry about the title in this case, as it is rendered by the // Dialog header @@ -48,16 +48,16 @@ class Instructions extends React.Component { // substituteInstructionImages return (
- {this.props.renderedMarkdown && + {this.props.markdown && } { /* Note: In this case props.shortInstructions might be undefined, but we still want to render NonMarkdownInstructions to get the puzzle title */ - !this.props.renderedMarkdown && + !this.props.markdown && + > + +
); } } diff --git a/apps/src/templates/instructions/TopInstructionsCSF.jsx b/apps/src/templates/instructions/TopInstructionsCSF.jsx index 670fb83728463..5906f5858e8f6 100644 --- a/apps/src/templates/instructions/TopInstructionsCSF.jsx +++ b/apps/src/templates/instructions/TopInstructionsCSF.jsx @@ -1,12 +1,8 @@ -/* eslint-disable react/no-danger */ - import $ from 'jquery'; import React, {PropTypes} from 'react'; import ReactDOM from 'react-dom'; import Radium from 'radium'; -import processMarkdown from 'marked'; import classNames from 'classnames'; -import renderer from "../../util/StylelessRenderer"; import { connect } from 'react-redux'; var instructions = require('../../redux/instructions'); import { openDialog } from '../../redux/instructionsDialog'; @@ -31,6 +27,8 @@ import LegacyButton from '../LegacyButton'; import { Z_INDEX as OVERLAY_Z_INDEX } from '../Overlay'; import msg from '@cdo/locale'; +import RenderedMarkdown from '../RenderedMarkdown'; + import { getOuterHeight, scrollTo, @@ -606,15 +604,10 @@ class TopInstructions extends React.Component { const markdown = this.shouldDisplayShortInstructions() ? this.props.shortInstructions : this.props.longInstructions; - const renderedMarkdown = processMarkdown(markdown, { renderer }); const ttsUrl = this.shouldDisplayShortInstructions() ? this.props.ttsInstructionsUrl : this.props.ttsMarkdownInstructionsUrl; - // Only used by star wars levels - const instructions2 = this.props.shortInstructions2 ? - processMarkdown(this.props.shortInstructions2, { renderer }) : undefined; - const leftColWidth = (this.getAvatar() ? PROMPT_ICON_WIDTH : 10) + (this.props.hasAuthoredHints ? AUTHORED_HINTS_EXTRA_WIDTH : 0); @@ -660,17 +653,16 @@ class TopInstructions extends React.Component { { this.instructions = c; }} - renderedMarkdown={renderedMarkdown} + markdown={markdown} onResize={this.adjustMaxNeededHeight} inputOutputTable={this.props.collapsed ? undefined : this.props.inputOutputTable} imgURL={this.props.aniGifURL} inTopPane /> - {instructions2 && -
+ {this.props.shortInstructions2 && +
+ +
} {this.props.overlayVisible &&
diff --git a/apps/src/templates/instructions/TopInstructionsCSP.jsx b/apps/src/templates/instructions/TopInstructionsCSP.jsx index db2124dec8232..694af24d7e84b 100644 --- a/apps/src/templates/instructions/TopInstructionsCSP.jsx +++ b/apps/src/templates/instructions/TopInstructionsCSP.jsx @@ -4,8 +4,6 @@ import React, {PropTypes, Component} from 'react'; import ReactDOM from 'react-dom'; import Radium from 'radium'; import {connect} from 'react-redux'; -import processMarkdown from 'marked'; -import renderer from "../../util/StylelessRenderer"; import TeacherOnlyMarkdown from './TeacherOnlyMarkdown'; import FeedbacksList from "./FeedbacksList"; import TeacherFeedback from "./TeacherFeedback"; @@ -109,7 +107,7 @@ class TopInstructions extends Component { height: PropTypes.number.isRequired, expandedHeight: PropTypes.number.isRequired, maxHeight: PropTypes.number.isRequired, - markdown: PropTypes.string, + longInstructions: PropTypes.string, collapsed: PropTypes.bool.isRequired, noVisualization: PropTypes.bool.isRequired, toggleInstructionsCollapsed: PropTypes.func.isRequired, @@ -342,8 +340,7 @@ class TopInstructions extends Component {
@@ -395,7 +392,7 @@ export default connect(state => ({ expandedHeight: state.instructions.expandedHeight, maxHeight: Math.min(state.instructions.maxAvailableHeight, state.instructions.maxNeededHeight), - markdown: state.instructions.longInstructions, + longInstructions: state.instructions.longInstructions, noVisualization: state.pageConstants.noVisualization, collapsed: state.instructions.collapsed, documentationUrl: state.pageConstants.documentationUrl, From 0bd152330dae67416a4cdd9676f95676d2a5f468 Mon Sep 17 00:00:00 2001 From: Brendan Reville Date: Mon, 20 Aug 2018 16:22:17 -0700 Subject: [PATCH 09/64] Notification: details link can now open in a new window This is a small followup to https://github.com/code-dot-org/code-dot-org/pull/24373 --- apps/src/templates/Notification.jsx | 8 +++++++- apps/src/templates/studioHomepages/CourseBlocks.jsx | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/apps/src/templates/Notification.jsx b/apps/src/templates/Notification.jsx index 2036e07af134b..8af2457b025fd 100644 --- a/apps/src/templates/Notification.jsx +++ b/apps/src/templates/Notification.jsx @@ -132,6 +132,7 @@ class Notification extends Component { details: PropTypes.string.isRequired, detailsLinkText: PropTypes.string, detailsLink: PropTypes.string, + detailsLinkNewWindow: PropTypes.bool, buttonText: PropTypes.string, buttonLink: PropTypes.string, dismissible: PropTypes.bool.isRequired, @@ -192,6 +193,7 @@ class Notification extends Component { details, detailsLinkText, detailsLink, + detailsLinkNewWindow, type, buttonText, buttonLink, @@ -241,7 +243,11 @@ class Notification extends Component { {detailsLinkText && detailsLink && (   - + {detailsLinkText} diff --git a/apps/src/templates/studioHomepages/CourseBlocks.jsx b/apps/src/templates/studioHomepages/CourseBlocks.jsx index d62354c2ad4cf..4180a02cc57ce 100644 --- a/apps/src/templates/studioHomepages/CourseBlocks.jsx +++ b/apps/src/templates/studioHomepages/CourseBlocks.jsx @@ -76,6 +76,7 @@ class CourseBlocksCsfModern extends Component { details={i18n.courseBlocksLegacyNotificationBody()} detailsLinkText={i18n.courseBlocksLegacyNotificationDetailsLinkText()} detailsLink="https://docs.google.com/document/d/1MVDfbEzr0o9DqaOYmOOYpsQPTfXUFvCx4Xs9uixrdBE/edit?usp=sharing" + detailsLinkNewWindow={true} dismissible={false} buttons={[ { From 40b167f3c6874b2d202f6969707d4e86cfcbf073 Mon Sep 17 00:00:00 2001 From: Elijah Hamovitz Date: Tue, 21 Aug 2018 13:46:44 -0700 Subject: [PATCH 10/64] s/RenderedMarkdown/UnsafeRenderedMarkdown --- .../{RenderedMarkdown.jsx => UnsafeRenderedMarkdown.jsx} | 7 ++++++- apps/src/templates/instructions/MarkdownInstructions.jsx | 4 ++-- apps/src/templates/instructions/TopInstructionsCSF.jsx | 4 ++-- 3 files changed, 10 insertions(+), 5 deletions(-) rename apps/src/templates/{RenderedMarkdown.jsx => UnsafeRenderedMarkdown.jsx} (66%) diff --git a/apps/src/templates/RenderedMarkdown.jsx b/apps/src/templates/UnsafeRenderedMarkdown.jsx similarity index 66% rename from apps/src/templates/RenderedMarkdown.jsx rename to apps/src/templates/UnsafeRenderedMarkdown.jsx index 2f56dc302d195..7e1bea736a159 100644 --- a/apps/src/templates/RenderedMarkdown.jsx +++ b/apps/src/templates/UnsafeRenderedMarkdown.jsx @@ -7,8 +7,13 @@ import renderer from "../util/StylelessRenderer"; * * Right now, it still uses marked; this will eventually be updated to use the * new remark system, and possibly even support redaction. + * + * Note that this component will render anything contained in the markdown into + * the browser, including arbitrary html and script tags. It should be + * considered unsafe to use for user-generated content until the markdown + * renderer driving this component can be made safe. */ -export default class RenderedMarkdown extends React.Component { +export default class UnsafeRenderedMarkdown extends React.Component { static propTypes = { markdown: PropTypes.string.isRequired }; diff --git a/apps/src/templates/instructions/MarkdownInstructions.jsx b/apps/src/templates/instructions/MarkdownInstructions.jsx index 7f85b41f80790..59773f1f0f117 100644 --- a/apps/src/templates/instructions/MarkdownInstructions.jsx +++ b/apps/src/templates/instructions/MarkdownInstructions.jsx @@ -7,7 +7,7 @@ import { connect } from 'react-redux'; import { convertXmlToBlockly } from './utils'; import { openDialog } from '@cdo/apps/redux/instructionsDialog'; -import RenderedMarkdown from '../RenderedMarkdown'; +import UnsafeRenderedMarkdown from '../UnsafeRenderedMarkdown'; const styles = { standard: { @@ -119,7 +119,7 @@ class MarkdownInstructions extends React.Component { inTopPane && canCollapse && styles.inTopPaneCanCollapse, ]} > - +
); } diff --git a/apps/src/templates/instructions/TopInstructionsCSF.jsx b/apps/src/templates/instructions/TopInstructionsCSF.jsx index 5906f5858e8f6..9f72258672e9a 100644 --- a/apps/src/templates/instructions/TopInstructionsCSF.jsx +++ b/apps/src/templates/instructions/TopInstructionsCSF.jsx @@ -27,7 +27,7 @@ import LegacyButton from '../LegacyButton'; import { Z_INDEX as OVERLAY_Z_INDEX } from '../Overlay'; import msg from '@cdo/locale'; -import RenderedMarkdown from '../RenderedMarkdown'; +import UnsafeRenderedMarkdown from '../UnsafeRenderedMarkdown'; import { getOuterHeight, @@ -661,7 +661,7 @@ class TopInstructions extends React.Component { /> {this.props.shortInstructions2 &&
- +
} {this.props.overlayVisible && From 2ab7531817647aac5fb260dc50820504bf3396f2 Mon Sep 17 00:00:00 2001 From: Elijah Hamovitz Date: Tue, 21 Aug 2018 13:47:33 -0700 Subject: [PATCH 11/64] update unit tests --- .../templates/instructions/DialogInstructionsTest.jsx | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/apps/test/unit/templates/instructions/DialogInstructionsTest.jsx b/apps/test/unit/templates/instructions/DialogInstructionsTest.jsx index 188fd3ec28fd1..b15f0a051b282 100644 --- a/apps/test/unit/templates/instructions/DialogInstructionsTest.jsx +++ b/apps/test/unit/templates/instructions/DialogInstructionsTest.jsx @@ -6,7 +6,6 @@ import { } from '@cdo/apps/templates/instructions/DialogInstructions'; import Instructions from '@cdo/apps/templates/instructions/Instructions'; import msg from '@cdo/locale'; -import processMarkdown from 'marked'; const TEST_PUZZLE_NUMBER = 2; const TEST_STAGE_TOTAL = 5; @@ -21,7 +20,6 @@ const SAMPLE_MARKDOWN=` - Point one - Point two `; -const SAMPLE_RENDERED_MARKDOWN = processMarkdown(SAMPLE_MARKDOWN); const SAMPLE_IMAGE_URL="example.gif"; const DEFAULT_PROPS = { @@ -44,7 +42,7 @@ describe('DialogInstructions', () => { puzzleTitle={EXPECTED_PUZZLE_TITLE} shortInstructions={TEST_INSTRUCTIONS_1} instructions2={TEST_INSTRUCTIONS_2} - renderedMarkdown={undefined} + longInstructions={undefined} imgURL={SAMPLE_IMAGE_URL} /> ); @@ -62,7 +60,7 @@ describe('DialogInstructions', () => { puzzleTitle={EXPECTED_PUZZLE_TITLE} shortInstructions={TEST_INSTRUCTIONS_1} instructions2={TEST_INSTRUCTIONS_2} - renderedMarkdown={SAMPLE_RENDERED_MARKDOWN} + longInstructions={SAMPLE_MARKDOWN} imgURL={SAMPLE_IMAGE_URL} /> ); @@ -81,7 +79,7 @@ describe('DialogInstructions', () => { puzzleTitle={EXPECTED_PUZZLE_TITLE} shortInstructions={undefined} instructions2={undefined} - renderedMarkdown={undefined} + longInstructions={undefined} imgURL={SAMPLE_IMAGE_URL} /> ); @@ -100,7 +98,7 @@ describe('DialogInstructions', () => { puzzleTitle={EXPECTED_PUZZLE_TITLE} shortInstructions={undefined} instructions2={undefined} - renderedMarkdown={undefined} + longInstructions={undefined} imgURL={undefined} /> ); From c967716f3abae98197fb1c8ab172c0bd748461cd Mon Sep 17 00:00:00 2001 From: Elijah Hamovitz Date: Wed, 22 Aug 2018 14:08:19 -0700 Subject: [PATCH 12/64] resolve a few inconsistencies between when we use 'markdown' vs 'longInstructions' --- apps/src/templates/instructions/Instructions.jsx | 10 +++++----- apps/src/templates/instructions/TopInstructionsCSF.jsx | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/src/templates/instructions/Instructions.jsx b/apps/src/templates/instructions/Instructions.jsx index a07b70d47684f..e92ffbe28228e 100644 --- a/apps/src/templates/instructions/Instructions.jsx +++ b/apps/src/templates/instructions/Instructions.jsx @@ -17,7 +17,7 @@ const styles = { * A component for displaying our level instructions text, and possibly also * authored hints UI and/or an anigif. These instructions can appear in the top * pane or in a modal dialog. In the latter case, we will sometimes show just - * the hints or just the anigif (in this case instructions/markdown + * the hints or just the anigif (in this case instructions/longInstructions * props will be undefined). */ class Instructions extends React.Component { @@ -25,7 +25,7 @@ class Instructions extends React.Component { puzzleTitle: PropTypes.string, shortInstructions: PropTypes.string, instructions2: PropTypes.string, - markdown: PropTypes.string, + longInstructions: PropTypes.string, imgURL: PropTypes.string, authoredHints: PropTypes.element, inputOutputTable: PropTypes.arrayOf( @@ -48,16 +48,16 @@ class Instructions extends React.Component { // substituteInstructionImages return (
- {this.props.markdown && + {this.props.longInstructions && } { /* Note: In this case props.shortInstructions might be undefined, but we still want to render NonMarkdownInstructions to get the puzzle title */ - !this.props.markdown && + !this.props.longInstructions && { this.instructions = c; }} - markdown={markdown} + longInstructions={markdown} onResize={this.adjustMaxNeededHeight} inputOutputTable={this.props.collapsed ? undefined : this.props.inputOutputTable} imgURL={this.props.aniGifURL} From 4ff5f90b60e4893d41c2c2f0e27621c3a6ab22ec Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Wed, 22 Aug 2018 16:04:16 -0700 Subject: [PATCH 13/64] Use email method instead of read_attribute(:email) in normalize_ and hash_email methods --- dashboard/app/models/user.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dashboard/app/models/user.rb b/dashboard/app/models/user.rb index a38073ea8df1c..995c20fb52bb8 100644 --- a/dashboard/app/models/user.rb +++ b/dashboard/app/models/user.rb @@ -507,8 +507,8 @@ def make_teachers_21 end def normalize_email - return unless read_attribute(:email).present? - self.email = read_attribute(:email).strip.downcase + return unless email.present? + self.email = email.strip.downcase end def self.hash_email(email) @@ -516,8 +516,8 @@ def self.hash_email(email) end def hash_email - return unless read_attribute(:email).present? - self.hashed_email = User.hash_email(read_attribute(:email)) + return unless email.present? + self.hashed_email = User.hash_email(email) end # @return [Boolean, nil] Whether the the list of races stored in the `races` column represents an From b845596a81b797f7c04078e8cbfcb32d57760eaf Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Fri, 24 Aug 2018 09:42:27 -0700 Subject: [PATCH 14/64] Test normalize_ and hash_email for migrated users --- dashboard/test/models/user_test.rb | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/dashboard/test/models/user_test.rb b/dashboard/test/models/user_test.rb index 3f4ace34d4fb7..4ce321aebf2a8 100644 --- a/dashboard/test/models/user_test.rb +++ b/dashboard/test/models/user_test.rb @@ -84,17 +84,32 @@ class UserTest < ActiveSupport::TestCase experiment.destroy end - test 'normalize_email' do + test 'normalize_email for non-migrated user' do teacher = create :teacher, email: 'CAPS@EXAMPLE.COM' assert_equal 'caps@example.com', teacher.email end - test 'hash_email' do + test 'normalize_email for migrated user' do + teacher = create :teacher, :with_migrated_email_authentication_option, email: 'OLD@EXAMPLE.COM' + teacher.update!(primary_contact_info: create(:authentication_option, user: teacher, email: 'NEW@EXAMPLE.COM')) + assert_equal 'new@example.com', teacher.primary_contact_info.email + assert_equal 'new@example.com', teacher.read_attribute(:email) + end + + test 'hash_email for non-migrated user' do @teacher.update!(email: 'hash_email@example.com') assert_equal User.hash_email('hash_email@example.com'), @teacher.hashed_email end + test 'hash_email for migrated user' do + teacher = create :teacher, :with_migrated_email_authentication_option, email: 'OLD@EXAMPLE.COM' + teacher.update!(primary_contact_info: create(:authentication_option, user: teacher, email: 'NEW@EXAMPLE.COM')) + hashed_email = User.hash_email('new@example.com') + assert_equal hashed_email, teacher.primary_contact_info.hashed_email + assert_equal hashed_email, teacher.read_attribute(:hashed_email) + end + test "log in with password with pepper" do assert Devise.pepper From bc55dd19d34e92435d35565c98f4200a912bec0d Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Fri, 24 Aug 2018 11:39:01 -0700 Subject: [PATCH 15/64] Add unit tests --- apps/src/StudioApp.js | 34 ++++++++++++++--------- apps/src/turtle/artist.js | 30 ++++++++++----------- apps/test/unit/StudioAppTest.js | 4 +++ apps/test/unit/turtle/turtleTest.js | 42 +++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 27 deletions(-) diff --git a/apps/src/StudioApp.js b/apps/src/StudioApp.js index cd4a5d2327a54..528ece7a27a34 100644 --- a/apps/src/StudioApp.js +++ b/apps/src/StudioApp.js @@ -839,19 +839,27 @@ StudioApp.prototype.runButtonClick = function () {}; StudioApp.prototype.addChangeHandler = function (newHandler) { if (!this.changeHandlers) { - this.changeHandlers = [newHandler]; - const runAllHandlers = () => - this.changeHandlers.forEach(handler => handler()); - if (this.isUsingBlockly()) { - const blocklyCanvas = Blockly.mainBlockSpace.getCanvas(); - blocklyCanvas.addEventListener('blocklyBlockSpaceChange', runAllHandlers); - } else { - this.editor.on('change', runAllHandlers); - // Droplet doesn't automatically bubble up aceEditor changes - this.editor.aceEditor.on('change', runAllHandlers); - } + this.changeHandlers = []; + } + this.changeHandlers.push(newHandler); +}; + +StudioApp.prototype.runChangeHandlers = function () { + if (!this.changeHandlers) { + return; + } + this.changeHandlers.forEach(handler => handler()); +}; + +StudioApp.prototype.setupChangeHandlers = function () { + const runAllHandlers = this.runChangeHandlers.bind(this); + if (this.isUsingBlockly()) { + const blocklyCanvas = Blockly.mainBlockSpace.getCanvas(); + blocklyCanvas.addEventListener('blocklyBlockSpaceChange', runAllHandlers); } else { - this.changeHandlers.push(newHandler); + this.editor.on('change', runAllHandlers); + // Droplet doesn't automatically bubble up aceEditor changes + this.editor.aceEditor.on('change', runAllHandlers); } }; @@ -2080,6 +2088,7 @@ StudioApp.prototype.handleEditCode_ = function (config) { !!config.level.textModeAtStart ), }); + this.setupChangeHandlers(); if (config.level.paletteCategoryAtStart) { this.editor.changePaletteGroup(config.level.paletteCategoryAtStart); @@ -2467,6 +2476,7 @@ StudioApp.prototype.handleUsingBlockly_ = function (config) { }); this.inject(div, options); this.onResize(); + this.setupChangeHandlers(); if (config.afterInject) { config.afterInject(); diff --git a/apps/src/turtle/artist.js b/apps/src/turtle/artist.js index 163a6a95f0c9a..7e27bbc1bde2c 100644 --- a/apps/src/turtle/artist.js +++ b/apps/src/turtle/artist.js @@ -259,7 +259,7 @@ Artist.prototype.preloadAllPatternImages = function () { }); const patternOptions = (this.skin && this.skin.lineStylePatternOptions); - if (patternOptions.length) { + if (patternOptions && patternOptions.length) { return Promise.all(patternOptions.map(loadPattern)); } else { return Promise.resolve(); @@ -343,26 +343,26 @@ Artist.prototype.init = function (config) { /> ); + if (this.autoRun) { + this.studioApp_.addChangeHandler(() => { + if (this.limitedAutoRun) { + if (this.studioApp_.isRunning() && !this.executing) { + this.execute(); + } + } else { + if (!this.executing) { + this.execute(); + } + } + }); + } + function onMount() { this.studioApp_.init(config); const finishButton = document.getElementById('finishButton'); if (finishButton) { dom.addClickTouchEvent(finishButton, this.checkAnswer.bind(this)); } - - if (this.autoRun) { - this.studioApp_.addChangeHandler(() => { - if (this.limitedAutoRun) { - if (this.studioApp_.isRunning() && !this.executing) { - this.execute(); - } - } else { - if (!this.executing) { - this.execute(); - } - } - }); - } } return Promise.all([ diff --git a/apps/test/unit/StudioAppTest.js b/apps/test/unit/StudioAppTest.js index e13d95265d942..c524f97e0c441 100644 --- a/apps/test/unit/StudioAppTest.js +++ b/apps/test/unit/StudioAppTest.js @@ -165,6 +165,7 @@ describe("StudioApp", () => { it('calls a handler in response to a blockly change', () => { let changed = false; studioApp().usingBlockly_ = true; + studioApp().setupChangeHandlers(); studioApp().addChangeHandler(() => changed = true); Blockly.mainBlockSpace.getCanvas() @@ -178,6 +179,7 @@ describe("StudioApp", () => { studioApp().usingBlockly_ = false; studioApp().editor = $(document.createElement('div')); studioApp().editor.aceEditor = $(document.createElement('div')); + studioApp().setupChangeHandlers(); studioApp().addChangeHandler(() => changed = true); studioApp().editor.trigger('change'); @@ -192,6 +194,7 @@ describe("StudioApp", () => { studioApp().usingBlockly_ = false; studioApp().editor = $(document.createElement('div')); studioApp().editor.aceEditor = $(document.createElement('div')); + studioApp().setupChangeHandlers(); studioApp().addChangeHandler(() => changed = true); studioApp().editor.aceEditor.trigger('change'); @@ -203,6 +206,7 @@ describe("StudioApp", () => { it('calls multiple handlers in response to a blockly change', () => { let changed1 = false, changed2 = false; + studioApp().setupChangeHandlers(); studioApp().addChangeHandler(() => changed1 = true); studioApp().addChangeHandler(() => changed2 = true); diff --git a/apps/test/unit/turtle/turtleTest.js b/apps/test/unit/turtle/turtleTest.js index 7e30aa2a380d5..033f530fcaac7 100644 --- a/apps/test/unit/turtle/turtleTest.js +++ b/apps/test/unit/turtle/turtleTest.js @@ -2,6 +2,7 @@ import sinon from 'sinon'; import {expect} from '../../util/configuredChai'; import {parseElement} from '@cdo/apps/xml'; import {Position} from '@cdo/apps/constants'; +import {singleton as studioAppSingleton} from '@cdo/apps/StudioApp'; const Artist = require('@cdo/apps/turtle/artist'); const SHORT_DIAGONAL = 50 * Math.sqrt(2); @@ -258,6 +259,47 @@ describe('Artist', () => { }); }); + describe('autoArtist', () => { + const studioApp = studioAppSingleton(); + + it('executes upon reset', () => { + const artist = new Artist(); + const execute = sinon.stub(artist, 'execute'); + artist.injectStudioApp(studioApp); + artist.init({ + skin: {}, + level: { + autoRun: true, + }, + }); + + artist.resetButtonClick(); + + expect(execute).to.have.been.called; + execute.restore(); + }); + + it('executes upon code changes', () => { + const artist = new Artist(); + const execute = sinon.stub(Artist.prototype, 'execute'); + const container = document.createElement('div'); + container.id = 'artistContainer'; + document.body.appendChild(container); + artist.injectStudioApp(studioApp); + artist.init({ + skin: {}, + level: { + autoRun: true, + }, + containerId: 'artistContainer', + }); + studioApp.runChangeHandlers(); + + expect(execute).to.have.been.called; + execute.restore(); + }); + }); + describe('prepareForRemix', () => { let artist, newDom, oldXml; From e143bb8e945864b297be373a6f08229cccebf8c4 Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Fri, 24 Aug 2018 14:14:20 -0700 Subject: [PATCH 16/64] Use a single dropdown instead of 2 checkboxes --- apps/script/generateSharedConstants.rb | 1 + apps/src/turtle/artist.js | 6 +++--- dashboard/app/models/levels/artist.rb | 1 - dashboard/app/views/levels/editors/_artist.html.haml | 11 +++++++---- lib/cdo/shared_constants.rb | 8 ++++++++ 5 files changed, 19 insertions(+), 8 deletions(-) diff --git a/apps/script/generateSharedConstants.rb b/apps/script/generateSharedConstants.rb index bdd4c3c4c3753..58e44b9af842d 100755 --- a/apps/script/generateSharedConstants.rb +++ b/apps/script/generateSharedConstants.rb @@ -68,6 +68,7 @@ def parse_raw(raw) def main shared_content = generate_multiple_constants %w( + ARTIST_AUTORUN_OPTIONS GAMELAB_AUTORUN_OPTIONS LEVEL_KIND LEVEL_STATUS diff --git a/apps/src/turtle/artist.js b/apps/src/turtle/artist.js index 7e27bbc1bde2c..d9a0afb147fe8 100644 --- a/apps/src/turtle/artist.js +++ b/apps/src/turtle/artist.js @@ -56,6 +56,7 @@ import dom from '../dom'; import {SignInState} from '../code-studio/progressRedux'; import Visualization from '@code-dot-org/artist'; import experiments from '../util/experiments'; +import {ArtistAutorunOptions} from '@cdo/apps/util/sharedConstants'; const CANVAS_HEIGHT = 400; const CANVAS_WIDTH = 400; @@ -301,9 +302,8 @@ Artist.prototype.init = function (config) { }); this.limitedAutoRun = experiments.isEnabled('limited-auto-artist') || - this.level.limitedAutoRun; - this.autoRun = experiments.isEnabled('auto-artist') || - this.level.autoRun || this.limitedAutoRun; + this.level.autoRun === ArtistAutorunOptions.limited_auto_run; + this.autoRun = experiments.isEnabled('auto-artist') || this.level.autoRun; config.grayOutUndeletableBlocks = true; config.forceInsertTopBlock = 'when_run'; diff --git a/dashboard/app/models/levels/artist.rb b/dashboard/app/models/levels/artist.rb index a35f86cc428ed..2e23698e73041 100644 --- a/dashboard/app/models/levels/artist.rb +++ b/dashboard/app/models/levels/artist.rb @@ -37,7 +37,6 @@ class Artist < Blockly disable_sharing solution_image_url auto_run - limited_auto_run ) def xml_blocks diff --git a/dashboard/app/views/levels/editors/_artist.html.haml b/dashboard/app/views/levels/editors/_artist.html.haml index 6016a1ccbd8e9..0584da58c16e5 100644 --- a/dashboard/app/views/levels/editors/_artist.html.haml +++ b/dashboard/app/views/levels/editors/_artist.html.haml @@ -24,10 +24,13 @@ = f.number_field :permitted_errors, :value => @level.permitted_errors .field = f.label :auto_run, 'Automatically rerun on block changes' - = boolean_check_box f, :auto_run -.field - = f.label :limited_auto_run, 'Automatically rerun on block changes, but only allow field edits' - = boolean_check_box f, :limited_auto_run + :ruby + selector = f.select :auto_run, options_for_select([ + ['No', nil], + ['Yes, but only for field changes', SharedConstants::ARTIST_AUTORUN_OPTIONS.limited_auto_run], + ['Yes, for all code changes', SharedConstants::ARTIST_AUTORUN_OPTIONS.full_auto_run], + ], selected: @level.auto_run) + = selector .field = link_to 'Edit Predraw Blocks', level_edit_blocks_path(@level, :predraw_blocks) unless @level.new_record? = render partial: 'levels/editors/collapsible_block_editor', locals: {f: f, xml_id: 'predraw'} diff --git a/lib/cdo/shared_constants.rb b/lib/cdo/shared_constants.rb index e868e3590fc7e..9bd8f5d256773 100644 --- a/lib/cdo/shared_constants.rb +++ b/lib/cdo/shared_constants.rb @@ -49,6 +49,14 @@ module SharedConstants } ) + # The set of artist autorun options + ARTIST_AUTORUN_OPTIONS = OpenStruct.new( + { + limited_auto_run: 'LIMITED_AUTO_RUN', + full_auto_run: 'FULL_AUTO_RUN', + } + ).freeze + # The set of gamelab autorun options GAMELAB_AUTORUN_OPTIONS = OpenStruct.new( { From f361a80757092fc4b076cb33522be45c760912d6 Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Fri, 24 Aug 2018 14:31:03 -0700 Subject: [PATCH 17/64] Use (un)lockMovement() instead of setMovable() --- apps/src/turtle/artist.js | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/apps/src/turtle/artist.js b/apps/src/turtle/artist.js index d9a0afb147fe8..c1dbf8d44fea6 100644 --- a/apps/src/turtle/artist.js +++ b/apps/src/turtle/artist.js @@ -708,14 +708,7 @@ Artist.prototype.runButtonClick = function () { } this.studioApp_.attempts++; if (this.limitedAutoRun) { - this.immovableBlocks = []; - Blockly.mainBlockSpace.getAllBlocks().forEach(block => { - if (!block.isMovable()) { - this.immovableBlocks.push(block); - } else { - block.setMovable(false); - } - }); + Blockly.mainBlockSpace.blockSpaceEditor.lockMovement(); } this.execute(); }; @@ -723,11 +716,7 @@ Artist.prototype.runButtonClick = function () { Artist.prototype.resetButtonClick = function () { this.shouldAnimate_ = !this.instant_ && !this.autoRun; if (this.limitedAutoRun) { - Blockly.mainBlockSpace.getAllBlocks().forEach(block => { - if (!this.immovableBlocks.includes(block)) { - block.setMovable(true); - } - }); + Blockly.mainBlockSpace.blockSpaceEditor.unlockMovement(); } if (this.autoRun && !this.limitedAutoRun) { From 84c10f8d5dc581864c382176734d37cc48666369 Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Fri, 24 Aug 2018 15:49:32 -0700 Subject: [PATCH 18/64] Add an eyes test for artist autorun --- apps/src/turtle/artist.js | 4 +- dashboard/config/scripts/allthethings.script | 1 + .../scripts/levels/Artist Autorun Test.level | 126 ++++++++++++++++++ .../test/ui/features/artist_autorun.feature | 9 ++ 4 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 dashboard/config/scripts/levels/Artist Autorun Test.level create mode 100644 dashboard/test/ui/features/artist_autorun.feature diff --git a/apps/src/turtle/artist.js b/apps/src/turtle/artist.js index c1dbf8d44fea6..5568cd622e74e 100644 --- a/apps/src/turtle/artist.js +++ b/apps/src/turtle/artist.js @@ -939,7 +939,9 @@ Artist.prototype.finishExecution_ = function () { if (this.level.freePlay) { window.dispatchEvent(utils.createEvent('artistDrawingComplete')); } else { - this.checkAnswer(); + if (this.shouldAnimate_) { + this.checkAnswer(); + } } setTimeout(() => { this.executing = false; diff --git a/dashboard/config/scripts/allthethings.script b/dashboard/config/scripts/allthethings.script index 0b7cdbfacdacf..c98befea23729 100644 --- a/dashboard/config/scripts/allthethings.script +++ b/dashboard/config/scripts/allthethings.script @@ -22,6 +22,7 @@ level 'NEW Course 4 Artist Functions 3' level 'Auto Open Function Editor' level 'Angle Helper Test' level 'allthethings-artist-project-backed' +level 'Artist Autorun Test' stage 'Bee' level 'K-1 Bee 1' diff --git a/dashboard/config/scripts/levels/Artist Autorun Test.level b/dashboard/config/scripts/levels/Artist Autorun Test.level new file mode 100644 index 0000000000000..76da2f87fdced --- /dev/null +++ b/dashboard/config/scripts/levels/Artist Autorun Test.level @@ -0,0 +1,126 @@ + + + + + + + + + + + draw a square + + + 4 + + + moveForward + 100 + + + turnRight + 90 + + + + + + + + + + + + + moveForward + 100 + + + turnRight + 90 + + + turnLeft + 90 + + + jumpForward + 100 + + + ??? + + + + + + + + #ff0000 + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/dashboard/test/ui/features/artist_autorun.feature b/dashboard/test/ui/features/artist_autorun.feature new file mode 100644 index 0000000000000..c35ff0660d3fb --- /dev/null +++ b/dashboard/test/ui/features/artist_autorun.feature @@ -0,0 +1,9 @@ +@eyes +Feature: Artist Autorun + +Scenario: Autorun Eyes Test + Given I am on "http://studio.code.org/s/allthethings/stage/3/puzzle/9" + And I wait to see "#runButton" + And I open my eyes to test "artist autorun" + Then I see no difference for "square already drawn" + And I close my eyes From de4717e852410946af0d1474f2e4d0e30a54c0e1 Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Fri, 24 Aug 2018 16:20:11 -0700 Subject: [PATCH 19/64] Add a step to the eyes test --- dashboard/test/ui/features/artist_autorun.feature | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dashboard/test/ui/features/artist_autorun.feature b/dashboard/test/ui/features/artist_autorun.feature index c35ff0660d3fb..aae95f9bfe8a0 100644 --- a/dashboard/test/ui/features/artist_autorun.feature +++ b/dashboard/test/ui/features/artist_autorun.feature @@ -4,6 +4,10 @@ Feature: Artist Autorun Scenario: Autorun Eyes Test Given I am on "http://studio.code.org/s/allthethings/stage/3/puzzle/9" And I wait to see "#runButton" + And I close the instructions overlay if it exists And I open my eyes to test "artist autorun" Then I see no difference for "square already drawn" + When I drag block "2" to block "12" + And I drag block "6" to block "17" + Then I see no difference for "two squares drawn" And I close my eyes From 9e6e15dd7273db08e69e996fd0a641cac098954e Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Thu, 16 Aug 2018 14:17:53 -0700 Subject: [PATCH 20/64] Import AWS.Firehose, not of all of aws-sdk --- apps/src/lib/util/firehose.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/src/lib/util/firehose.js b/apps/src/lib/util/firehose.js index 1f82612032e50..ce4ef7b321344 100644 --- a/apps/src/lib/util/firehose.js +++ b/apps/src/lib/util/firehose.js @@ -1,6 +1,8 @@ /** @file Provides clients to AWS Firehose, whose data is imported into AWS Redshift. */ -import AWS from 'aws-sdk'; +import AWS from 'aws-sdk/lib/core'; +import 'aws-sdk/lib/config'; +import Firehose from 'aws-sdk/clients/firehose'; import {createUuid, trySetLocalStorage, tryGetLocalStorage} from '@cdo/apps/utils'; import {getStore} from '@cdo/apps/redux'; @@ -255,6 +257,6 @@ class FirehoseClient { // eslint-disable-next-line const _0x12ed=['\x41\x4b\x49\x41\x4a\x41\x41\x4d\x42\x59\x4d\x36\x55\x53\x59\x54\x34\x35\x34\x51','\x78\x4e\x4e\x39\x4e\x79\x32\x61\x6d\x39\x78\x75\x4b\x79\x57\x39\x53\x2b\x4e\x76\x41\x77\x33\x67\x68\x68\x74\x68\x72\x6b\x37\x6b\x6e\x51\x59\x54\x77\x6d\x4d\x48','\x75\x73\x2d\x65\x61\x73\x74\x2d\x31','\x63\x6f\x6e\x66\x69\x67'];(function(_0xb54a92,_0x4e682a){var _0x44f3e8=function(_0x35c55a){while(--_0x35c55a){_0xb54a92['\x70\x75\x73\x68'](_0xb54a92['\x73\x68\x69\x66\x74']());}};_0x44f3e8(++_0x4e682a);}(_0x12ed,0x127));var _0xd12e=function(_0x2cedd5,_0x518781){_0x2cedd5=_0x2cedd5-0x0;var _0x4291ea=_0x12ed[_0x2cedd5];return _0x4291ea;};AWS[_0xd12e('0x0')]=new AWS['\x43\x6f\x6e\x66\x69\x67']({'\x61\x63\x63\x65\x73\x73\x4b\x65\x79\x49\x64':_0xd12e('0x1'),'\x73\x65\x63\x72\x65\x74\x41\x63\x63\x65\x73\x73\x4b\x65\x79':_0xd12e('0x2'),'\x72\x65\x67\x69\x6f\x6e':_0xd12e('0x3')}); -const FIREHOSE = new AWS.Firehose({apiVersion: '2015-08-04'}); +const FIREHOSE = new Firehose({apiVersion: '2015-08-04'}); const firehoseClient = new FirehoseClient(); export default firehoseClient; From a9336f24add2fd631cb8f982685bf34ebf48b5f7 Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Thu, 16 Aug 2018 15:56:35 -0700 Subject: [PATCH 21/64] Move maker.isEnabled/isAvailable to maker redux --- apps/src/applab/applab.js | 3 ++- apps/src/lib/kits/maker/redux.js | 4 ++++ apps/src/lib/kits/maker/toolkit.js | 21 +++------------------ apps/src/lib/ui/SettingsCog.jsx | 10 ++++++---- 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/apps/src/applab/applab.js b/apps/src/applab/applab.js index ed7fdb24db667..586f2546c6ce8 100644 --- a/apps/src/applab/applab.js +++ b/apps/src/applab/applab.js @@ -60,6 +60,7 @@ import { } from '../lib/tools/jsdebugger/redux'; import JavaScriptModeErrorHandler from '../JavaScriptModeErrorHandler'; import * as makerToolkit from '../lib/kits/maker/toolkit'; +import * as makerToolkitRedux from '../lib/kits/maker/redux'; import project from '../code-studio/initApp/project'; import * as thumbnailUtils from '../util/thumbnail'; import Sounds from '../Sounds'; @@ -1009,7 +1010,7 @@ Applab.execute = function () { } } - if (makerToolkit.isEnabled()) { + if (makerToolkitRedux.isEnabled(getStore().getState())) { makerToolkit.connect({ interpreter: Applab.JSInterpreter, onDisconnect: () => studioApp().resetButtonClick(), diff --git a/apps/src/lib/kits/maker/redux.js b/apps/src/lib/kits/maker/redux.js index 5d09bd7dbc5b5..9ec8793e88312 100644 --- a/apps/src/lib/kits/maker/redux.js +++ b/apps/src/lib/kits/maker/redux.js @@ -26,6 +26,10 @@ export function isEnabled(state) { return getRoot(state).enabled; } +export function isAvailable(state) { + return !!(state && state.maker); +} + export function isConnecting(state) { return getRoot(state).connectionState === CONNECTING; } diff --git a/apps/src/lib/kits/maker/toolkit.js b/apps/src/lib/kits/maker/toolkit.js index dab263fb43ea7..a01a857f9cc7c 100644 --- a/apps/src/lib/kits/maker/toolkit.js +++ b/apps/src/lib/kits/maker/toolkit.js @@ -30,27 +30,12 @@ let currentBoard = null; * Enable Maker Toolkit for the current level. */ export function enable() { - if (!isAvailable()) { + if (!redux.isAvailable(getStore().getState())) { throw new MakerError('Maker cannot be enabled: Its reducer was not registered.'); } getStore().dispatch(redux.enable()); } -/** - * @returns {boolean} whether Maker Toolkit is enabled for the current level - */ -export function isEnabled() { - return redux.isEnabled(getStore().getState()); -} - -/** - * @returns {boolean} whether Maker Toolkit is usable with the current app at all - */ -export function isAvailable() { - const state = getStore().getState(); - return !!(state && state.maker); -} - /** * Called when starting execution of the student app. * Looks for a connected board, sets up an appropriate board controller, @@ -66,7 +51,7 @@ export function isAvailable() { * Rejects with another error type if something unexpected happens. */ export function connect({interpreter, onDisconnect}) { - if (!isEnabled()) { + if (!redux.isEnabled(getStore().getState())) { return Promise.reject(new Error('Attempted to connect to a maker board, ' + 'but Maker Toolkit is not enabled.')); } @@ -157,7 +142,7 @@ function shouldRunWithFakeBoard() { * and puts maker UI back in a default state. */ export function reset() { - if (!isEnabled()) { + if (!redux.isEnabled(getStore().getState())) { return; } diff --git a/apps/src/lib/ui/SettingsCog.jsx b/apps/src/lib/ui/SettingsCog.jsx index e23d216ecc14a..56ee59deb95ec 100644 --- a/apps/src/lib/ui/SettingsCog.jsx +++ b/apps/src/lib/ui/SettingsCog.jsx @@ -6,9 +6,10 @@ import FontAwesome from '../../templates/FontAwesome'; import color from '../../util/color'; import * as assets from '../../code-studio/assets'; import project from '../../code-studio/initApp/project'; -import * as maker from '../kits/maker/toolkit'; +import * as makerToolkitRedux from '../kits/maker/redux'; import PopUpMenu from './PopUpMenu'; import ConfirmEnableMakerDialog from "./ConfirmEnableMakerDialog"; +import {getStore} from '../../redux'; const style = { iconContainer: { @@ -69,7 +70,7 @@ class SettingsCog extends Component { toggleMakerToolkit = () => { this.close(); - if (!maker.isEnabled()) { + if (!makerToolkitRedux.isEnabled(getStore().getState())) { // Pop a confirmation dialog when trying to enable maker, // because we've had several users do this accidentally. this.showConfirmation(); @@ -158,12 +159,13 @@ ManageAssets.propTypes = { }; export function ToggleMaker(props) { - if (!maker.isAvailable()) { + const reduxState = getStore().getState(); + if (!makerToolkitRedux.isAvailable(reduxState)) { return null; } return ( - {maker.isEnabled() ? msg.disableMaker() : msg.enableMaker()} + {makerToolkitRedux.isEnabled(reduxState) ? msg.disableMaker() : msg.enableMaker()} ); } From c9e79d2895e68c868899c2ba5ccdb21db7ea66e0 Mon Sep 17 00:00:00 2001 From: Ram Kandasamy Date: Fri, 24 Aug 2018 18:25:03 -0700 Subject: [PATCH 22/64] Fix unit tests --- apps/test/unit/turtle/turtleTest.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/test/unit/turtle/turtleTest.js b/apps/test/unit/turtle/turtleTest.js index 033f530fcaac7..a52973367e871 100644 --- a/apps/test/unit/turtle/turtleTest.js +++ b/apps/test/unit/turtle/turtleTest.js @@ -262,7 +262,7 @@ describe('Artist', () => { describe('autoArtist', () => { const studioApp = studioAppSingleton(); - it('executes upon reset', () => { + it('executes upon reset', done => { const artist = new Artist(); const execute = sinon.stub(artist, 'execute'); artist.injectStudioApp(studioApp); @@ -271,7 +271,7 @@ describe('Artist', () => { level: { autoRun: true, }, - }); + }).then(done).catch(() => done()); artist.resetButtonClick(); @@ -279,7 +279,7 @@ describe('Artist', () => { execute.restore(); }); - it('executes upon code changes', () => { + it('executes upon code changes', done => { const artist = new Artist(); const execute = sinon.stub(Artist.prototype, 'execute'); const container = document.createElement('div'); @@ -292,7 +292,7 @@ describe('Artist', () => { autoRun: true, }, containerId: 'artistContainer', - }); + }).then(done).catch(() => done()); studioApp.runChangeHandlers(); expect(execute).to.have.been.called; From 1795da310f7ed1291bf135829e11dd3b0937a3c2 Mon Sep 17 00:00:00 2001 From: Brendan Reville Date: Sat, 25 Aug 2018 18:12:06 -0700 Subject: [PATCH 23/64] School dropdown: first working attempt to fix it on sign-up The dropdown wasn't showing the selected option immediately because it needed to be passed both a value and a label. --- apps/src/sites/studio/pages/signup.js | 5 ++++- apps/src/templates/SchoolAutocompleteDropdown.jsx | 9 +++++---- .../SchoolAutocompleteDropdownWithLabel.jsx | 11 ++++++----- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/apps/src/sites/studio/pages/signup.js b/apps/src/sites/studio/pages/signup.js index 905eb93115fce..25e97fd272512 100644 --- a/apps/src/sites/studio/pages/signup.js +++ b/apps/src/sites/studio/pages/signup.js @@ -62,6 +62,7 @@ window.SignupManager = function (options) { schoolZip: '', schoolType: '', showErrorMsg: false, + schoolLabel: '' }; // Check for URL having: /users/sign_up?user%5Buser_type%5D=teacher @@ -181,8 +182,10 @@ window.SignupManager = function (options) { updateAutocompleteSchoolFields(schoolData); } + // event has a label, school, value. school has nces_id which is same as value. function onSchoolChange(_, event) { schoolData.nces = event ? event.value : ''; + schoolData.label = event ? event.label : ''; updateAutocompleteSchoolFields(schoolData); } @@ -233,7 +236,7 @@ window.SignupManager = function (options) { {isUS && SCHOOL_TYPES_HAVING_NCES_SEARCH.includes(data.schoolType) && { // Existing value? Construct the matching option for display. - if (q.length === 0 && this.props.value) { - if (this.props.value === '-1') { + if (q.length === 0 && this.props.dropdownValue) { + if (this.props.dropdownValue.value === '-1') { return Promise.resolve({options: [this.constructSchoolNotFoundOption()]}); } else { - const getUrl = `/dashboardapi/v1/schools/${this.props.value}`; + const getUrl = `/dashboardapi/v1/schools/${this.props.dropdownValue.value}`; return fetch(getUrl) .then(response => response.ok ? response.json() : []) .then(json => ({options: [this.constructSchoolOption(json)]})); diff --git a/apps/src/templates/census2017/SchoolAutocompleteDropdownWithLabel.jsx b/apps/src/templates/census2017/SchoolAutocompleteDropdownWithLabel.jsx index 633a3263c14aa..9d85a0ec0fa17 100644 --- a/apps/src/templates/census2017/SchoolAutocompleteDropdownWithLabel.jsx +++ b/apps/src/templates/census2017/SchoolAutocompleteDropdownWithLabel.jsx @@ -25,8 +25,8 @@ export default class SchoolAutocompleteDropdownWithLabel extends Component { static propTypes = { setField: PropTypes.func, showErrorMsg: PropTypes.bool, - // Value is the NCES id of the school - value: PropTypes.string, + // old: Value is the NCES id of the school + dropdownValue: PropTypes.object, fieldName: PropTypes.string, singleLineLayout: PropTypes.bool, showRequiredIndicator: PropTypes.bool, @@ -41,6 +41,7 @@ export default class SchoolAutocompleteDropdownWithLabel extends Component { }; sendToParent = (selectValue) => { + // selectValue has a label, school, value. school has nces_id which is same as value. this.props.setField("nces", selectValue); }; @@ -61,8 +62,8 @@ export default class SchoolAutocompleteDropdownWithLabel extends Component { const {showRequiredIndicator, singleLineLayout} = this.props; const questionStyle = {...styles.question, ...(singleLineLayout && singleLineLayoutStyles)}; const containerStyle = {...(singleLineLayout && singleLineContainerStyles)}; - const showError = this.props.showErrorMsg && !this.props.value && !this.props.schoolDropdownOption; - const schoolNotFound = !!((this.props.value === "-1") || (this.props.schoolDropdownOption && this.props.schoolDropdownOption.value === "-1")); + const showError = this.props.showErrorMsg && !this.props.dropdownValue.value && !this.props.schoolDropdownOption; + const schoolNotFound = !!((this.props.dropdownValue.value === "-1") || (this.props.schoolDropdownOption && this.props.schoolDropdownOption.value === "-1")); const errorDiv = (
{i18n.censusRequiredSelect()} @@ -81,7 +82,7 @@ export default class SchoolAutocompleteDropdownWithLabel extends Component {
Date: Sat, 25 Aug 2018 18:25:57 -0700 Subject: [PATCH 24/64] School dropdown: simplify fix for signup --- apps/src/sites/studio/pages/signup.js | 2 +- apps/src/templates/SchoolAutocompleteDropdown.jsx | 11 ++++------- .../SchoolAutocompleteDropdownWithLabel.jsx | 9 +++------ 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/apps/src/sites/studio/pages/signup.js b/apps/src/sites/studio/pages/signup.js index 25e97fd272512..b952c32387abd 100644 --- a/apps/src/sites/studio/pages/signup.js +++ b/apps/src/sites/studio/pages/signup.js @@ -236,7 +236,7 @@ window.SignupManager = function (options) { {isUS && SCHOOL_TYPES_HAVING_NCES_SEARCH.includes(data.schoolType) && { // Existing value? Construct the matching option for display. - if (q.length === 0 && this.props.dropdownValue) { - if (this.props.dropdownValue.value === '-1') { + if (q.length === 0 && this.props.schoolDropdownOption) { + if (this.props.schoolDropdownOption.value === '-1') { return Promise.resolve({options: [this.constructSchoolNotFoundOption()]}); } else { - const getUrl = `/dashboardapi/v1/schools/${this.props.dropdownValue.value}`; + const getUrl = `/dashboardapi/v1/schools/${this.props.schoolDropdownOption.value}`; return fetch(getUrl) .then(response => response.ok ? response.json() : []) .then(json => ({options: [this.constructSchoolOption(json)]})); @@ -106,7 +103,7 @@ export default class SchoolAutocompleteDropdown extends Component { loadOptions={this.getOptions} cache={false} filterOption={() => true} - value={this.props.schoolDropdownOption ? this.props.schoolDropdownOption : this.props.value} + value={this.props.schoolDropdownOption} onChange={this.props.onChange} placeholder={i18n.searchForSchool()} /> diff --git a/apps/src/templates/census2017/SchoolAutocompleteDropdownWithLabel.jsx b/apps/src/templates/census2017/SchoolAutocompleteDropdownWithLabel.jsx index 9d85a0ec0fa17..bd21b16c3fd60 100644 --- a/apps/src/templates/census2017/SchoolAutocompleteDropdownWithLabel.jsx +++ b/apps/src/templates/census2017/SchoolAutocompleteDropdownWithLabel.jsx @@ -25,8 +25,6 @@ export default class SchoolAutocompleteDropdownWithLabel extends Component { static propTypes = { setField: PropTypes.func, showErrorMsg: PropTypes.bool, - // old: Value is the NCES id of the school - dropdownValue: PropTypes.object, fieldName: PropTypes.string, singleLineLayout: PropTypes.bool, showRequiredIndicator: PropTypes.bool, @@ -62,8 +60,8 @@ export default class SchoolAutocompleteDropdownWithLabel extends Component { const {showRequiredIndicator, singleLineLayout} = this.props; const questionStyle = {...styles.question, ...(singleLineLayout && singleLineLayoutStyles)}; const containerStyle = {...(singleLineLayout && singleLineContainerStyles)}; - const showError = this.props.showErrorMsg && !this.props.dropdownValue.value && !this.props.schoolDropdownOption; - const schoolNotFound = !!((this.props.dropdownValue.value === "-1") || (this.props.schoolDropdownOption && this.props.schoolDropdownOption.value === "-1")); + const showError = this.props.showErrorMsg && !this.props.schoolDropdownOption; + const schoolNotFound = !!(this.props.schoolDropdownOption && this.props.schoolDropdownOption.value === "-1"); const errorDiv = (
{i18n.censusRequiredSelect()} @@ -82,10 +80,9 @@ export default class SchoolAutocompleteDropdownWithLabel extends Component {