Redo the way exported projects are initialized #12623

pcardune · 2017-01-09T23:00:37Z

Hopefully this will fix a whole slew of bugs.

islemaster · 2017-01-09T23:32:58Z

apps/src/applab/api-entry.js

-import applabCommands from './commands';
+import {injectErrorHandler} from '../javascriptMode';
+import JavaScriptModeErrorHandler from '../JavaScriptModeErrorHandler';
+//import applabCommands from './commands';


islemaster · 2017-01-09T23:35:04Z

apps/src/applab/api-entry.js

+  "codeStudioLogo":false,
+  "hasI18n":false,
+  "callouts":[],
+  "channel":"Wc1JaBxTP04gGolQ9xuhNw",


Why is there a channel ID hard-coded into this object?

islemaster · 2017-01-09T23:36:15Z

apps/src/applab/api-entry.js

+  "hasContainedLevels":false,
+  "hideSource":true,
+  "share":true,
+  "labUserId":"x+OhD4/hmGgtPrHVlrC32TFHAdo",


Whose user ID is this?

islemaster · 2017-01-09T23:36:33Z

apps/src/applab/api-entry.js

+  "labUserId":"x+OhD4/hmGgtPrHVlrC32TFHAdo",
+  "firebaseName":"cdo-v3-dev",
+  "firebaseAuthToken":"eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ2IjowLCJpYXQiOjE0ODM5OTg4MjksImQiOnsidWlkIjoiOTIiLCJpc19kYXNoYm9hcmRfdXNlciI6dHJ1ZX19.DX8PP0Q8EDGg7UtMbhT2sT-h39LvDsuuPbA6YesXLG8",
+  "firebaseChannelIdSuffix":"-DEVELOPMENT-pcardune",


Uhh... auth token?

islemaster · 2017-01-09T23:37:38Z

apps/src/applab/api-entry.js

+    "level":"custom",
+    "shouldShowDialog":true
+  },
+  "locale":"en_us"


Hard-coded locale?

islemaster · 2017-01-09T23:38:44Z

apps/src/applab/api-entry.js

+    "level":"custom",
+    "shouldShowDialog":true
+  },
+  "locale":"en_us"
 };


I think we need to be a lot pickier about what gets included in this object...

islemaster · 2017-01-09T23:39:24Z

apps/src/applab/api-entry.js

+    "art_from_html":"Minecraft\u0026%238482;%20\u0026copy;%202017%20Microsoft.%20All%20Rights%20Reserved.%3Cbr%20/%3EStar%20Wars\u0026%238482;%20\u0026copy;%202017%20Disney%20and%20Lucasfilm.%20All%20Rights%20Reserved.%3Cbr%20/%3EFrozen\u0026%238482;%20\u0026copy;%202017%20Disney.%20All%20Rights%20Reserved.%3Cbr%20/%3EIce%20Age\u0026%238482;%20\u0026copy;%202017%2020th%20Century%20Fox.%20All%20Rights%20Reserved.%3Cbr%20/%3EAngry%20Birds\u0026%238482;%20\u0026copy;%202009-2017%20Rovio%20Entertainment%20Ltd.%20All%20Rights%20Reserved.%3Cbr%20/%3EPlants%20vs.%20Zombies\u0026%238482;%20\u0026copy;%202017%20Electronic%20Arts%20Inc.%20All%20Rights%20Reserved.%3Cbr%20/%3EThe%20Amazing%20World%20of%20Gumball%20is%20trademark%20and%20\u0026copy;%202017%20Cartoon%20Network.",
+    "code_from_html":"Code.org%20uses%20p5.play,%20which%20is%20licensed%20under%20%3Ca%20href=%22http://www.gnu.org/licenses/old-licenses/lgpl-2.1-standalone.html%22%3Ethe%20GNU%20LGPL%202.1%3C/a%3E.",
+    "powered_by_aws":"Powered by Amazon Web Services",
+    "trademark":"\u0026copy;%20Code.org,%202017.%20Code.org\u0026reg;,%20the%20CODE%20logo%20and%20Hour%20of%20Code\u0026reg;%20are%20trademarks%20of%20Code.org."


Can we get these localizations from someplace?

islemaster · 2017-01-09T23:39:56Z

apps/src/applab/api-entry.js

+const APP_OPTIONS = {
+  "levelGameName":"Applab",
+  "skinId":"applab",
+  "baseUrl":"/blockly/",


Surely we can get this URL in a DRY fashion?

islemaster · 2017-01-09T23:40:19Z

apps/src/applab/api-entry.js

+  "postFinalMilestone":true,
+  "puzzleRatingsUrl":"/puzzle_ratings",
+  "authoredHintViewRequestsUrl":"/authored_hint_view_requests.json",
+  "serverLevelId":2176,


I. CAN'T. EVEN.

islemaster · 2017-01-09T23:41:03Z

apps/src/applab/api-entry.js

+            "callback":null,
+            "sublevelCallback":null},
+  "sendToPhone":true,
+  "send_to_phone_url":"http://localhost-studio.code.org:3000/sms/send",


Should sendToPhone be a thing for exported projects?

islemaster · 2017-01-09T23:41:50Z

apps/src/applab/api-entry.js

+  "app":"applab",
+  "droplet":true,
+  "pretty":"",
+  "level":{


I wonder how much of the level object could be imported from applab/levels.js

islemaster · 2017-01-09T23:50:30Z

apps/src/applab/api-entry.js

 };

+Applab.render = function (){};


Not going to get solved here, but this indicates a need for a better division between App Lab the execution environment (interpreter, commands, student assets, etc) and App Lab the development environment (UI, editors, debugging, save/load, etc).

islemaster

…orted

islemaster · 2017-01-10T18:35:27Z

apps/src/applab/Exporter.js

+  }
+  const options = getAppOptionsAtPath(APP_OPTIONS_WHITELIST, getAppOptions());
+  return `window.APP_OPTIONS = ${JSON.stringify(options)};`;
+}


This method could use a unit test, should document the expected format of the whitelist argument, and could probably be simplified - especially if we can assume the whitelist grammar is strict (if you'll forgive the pseudo-BNF):

<whitelist> ::= Object.<string, value> <value> ::= true | <whitelist>

Couldn't the inner method reduce to something like this?

function getAppOptionsAtPath(whitelist, sourceOptions) { if (!whitelist || !sourceOptions) { return null; } return _.reduce(whitelist, (memo, value, key) => { if (value === true) { memo[key] = sourceOptions[key]; } else if (typeof value === 'object' && typeof sourceOptions[key] === 'object') { memo[key] = getAppOptionsAtPath(value, sourceOptions[key]); } return memo; }, {}); }

I was wondering if there was something like that in underscore. I like your version better :)

islemaster · 2017-01-10T18:37:08Z

apps/src/applab/Exporter.js

+        return;
+      }
+      if (typeof whitelist[key] === 'object') {
+        const subOptions = getAppOptionsAtPath(whitelist[key], sourceOptions[key]);


I think it's possible to pass null into the recursive call here, which doesn't get caught before a sourceOptions[key] call the next level down, which would cause a TypeError.

islemaster · 2017-01-10T19:56:50Z

apps/src/applab/Exporter.js

+  "isLegacyShare": true,
+  "postMilestone": true,
+  "postFinalMilestone": true,
+  "puzzleRatingsUrl": true,


At a glance, things I'd try pulling from the whitelist:

puzzleRatingsUrl

authoredHintViewRequestsUrl

serverLevelId

labUserId

firebaseName

firebaseAuthToken

firebaseChannelIdSuffix

teacherMarkdown

This helps avoid log spew in exported projects that don't use the data apis anyway.

…cardune-export-screen-switch-bug

islemaster · 2017-01-10T23:56:06Z

apps/src/applab/Exporter.js

@@ -1,14 +1,138 @@
 /* global dashboard */
 import $ from 'jquery';
-
+import _ from 'underscore';


Don't you mean lodash?

islemaster · 2017-01-10T23:59:21Z

apps/src/applab/Exporter.js

+  "locale": true,
+};
+
+export function getAppOptionsFile() {


Could still use a comment describing the correct whitelist format.

islemaster

LGTM after import change. 👍

Redo the way exported projects are initialized

0318043

Hopefully this will fix a whole slew of bugs.

pcardune assigned islemaster Jan 9, 2017

islemaster reviewed Jan 9, 2017

View reviewed changes

apps/src/applab/api-entry.js

"level":"custom",

"shouldShowDialog":true

},

"locale":"en_us"

Copy link

Contributor

islemaster Jan 9, 2017

Choose a reason for hiding this comment

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

Hard-coded locale?

islemaster reviewed Jan 9, 2017

View reviewed changes

islemaster suggested changes Jan 9, 2017

View reviewed changes

Export whatever app options were on the page when the project was exp…

afaa7cb

…orted

islemaster reviewed Jan 10, 2017

View reviewed changes

pcardune added 3 commits January 10, 2017 21:34

Write tests for getAppOptionsFile and simpify code

ff44e73

don't call data apis during initiliazation if we don't have to

850cf0d

This helps avoid log spew in exported projects that don't use the data apis anyway.

Merge branch 'staging' of github.com:code-dot-org/code-dot-org into p…

be81623

…cardune-export-screen-switch-bug

islemaster reviewed Jan 10, 2017

View reviewed changes

islemaster approved these changes Jan 11, 2017

View reviewed changes

Add descriptive comment and import lodash instead of underscore

3eda413

pcardune merged commit 6265b46 into staging Jan 11, 2017

pcardune deleted the pcardune-export-screen-switch-bug branch January 20, 2017 19:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redo the way exported projects are initialized #12623

Redo the way exported projects are initialized #12623

pcardune commented Jan 9, 2017

islemaster Jan 9, 2017

islemaster Jan 9, 2017

islemaster Jan 9, 2017

islemaster Jan 9, 2017

islemaster Jan 9, 2017

islemaster Jan 9, 2017

islemaster Jan 9, 2017

islemaster Jan 9, 2017

islemaster Jan 9, 2017

islemaster Jan 9, 2017

islemaster Jan 9, 2017

islemaster Jan 9, 2017

islemaster left a comment

islemaster Jan 10, 2017

pcardune Jan 10, 2017

islemaster Jan 10, 2017

islemaster Jan 10, 2017

islemaster Jan 10, 2017

islemaster Jan 10, 2017

islemaster left a comment

Redo the way exported projects are initialized #12623

Redo the way exported projects are initialized #12623

Conversation

pcardune commented Jan 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

islemaster left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

islemaster left a comment

Choose a reason for hiding this comment