Web Lab: Correct share links in different environments #19625

islemaster · 2017-12-13T17:45:18Z

Reverts #19623, restoring #19535 with fixes for sharing from CSD/CSP script project levels.

I added tests that catch the bad case, fixed the bad case, then totally reorganized the tests to reduce duplication, make the test names useful for spot-checking behavior, and easier to extend in the future. I've also added a UI test covering the regression, since in general unit test coverage over project stuff is not great and I still might have missed something.

Here's the new set of tests for getShareUrl():

    project.js
      project.getShareUrl
        on https://studio.code.org
          artist projects share to https://studio.code.org/projects/artist/<project-id>
            ✔ from project edit page
            ✔ from a script level
          playlab projects share to https://studio.code.org/projects/playlab/<project-id>
            ✔ from project edit page
            ✔ from a script level
          applab projects share to https://studio.code.org/projects/applab/<project-id>
            ✔ from project edit page
            ✔ from a script level
          gamelab projects share to https://studio.code.org/projects/gamelab/<project-id>
            ✔ from project edit page
            ✔ from a script level
          weblab projects share to https://codeprojects.org/<project-id>
            ✔ from project edit page
            ✔ from project view page
            ✔ from a script level
        on https://test-studio.code.org
          artist projects share to https://test-studio.code.org/projects/artist/<project-id>
            ✔ from project edit page
            ✔ from a script level
          playlab projects share to https://test-studio.code.org/projects/playlab/<project-id>
            ✔ from project edit page
            ✔ from a script level
          applab projects share to https://test-studio.code.org/projects/applab/<project-id>
            ✔ from project edit page
            ✔ from a script level
          gamelab projects share to https://test-studio.code.org/projects/gamelab/<project-id>
            ✔ from project edit page
            ✔ from a script level
          weblab projects share to https://test.codeprojects.org/<project-id>
            ✔ from project edit page
            ✔ from project view page
            ✔ from a script level
        on https://staging-studio.code.org
          artist projects share to https://staging-studio.code.org/projects/artist/<project-id>
            ✔ from project edit page
            ✔ from a script level
          playlab projects share to https://staging-studio.code.org/projects/playlab/<project-id>
            ✔ from project edit page
            ✔ from a script level
          applab projects share to https://staging-studio.code.org/projects/applab/<project-id>
            ✔ from project edit page
            ✔ from a script level
          gamelab projects share to https://staging-studio.code.org/projects/gamelab/<project-id>
            ✔ from project edit page
            ✔ from a script level
          weblab projects share to https://staging.codeprojects.org/<project-id>
            ✔ from project edit page
            ✔ from project view page
            ✔ from a script level
        on http://localhost-studio.code.org:3000
          artist projects share to http://localhost-studio.code.org:3000/projects/artist/<project-id>
            ✔ from project edit page
            ✔ from a script level
          playlab projects share to http://localhost-studio.code.org:3000/projects/playlab/<project-id>
            ✔ from project edit page
            ✔ from a script level
          applab projects share to http://localhost-studio.code.org:3000/projects/applab/<project-id>
            ✔ from project edit page
            ✔ from a script level
          gamelab projects share to http://localhost-studio.code.org:3000/projects/gamelab/<project-id>
            ✔ from project edit page
            ✔ from a script level
          weblab projects share to http://localhost.codeprojects.org:3000/<project-id>
            ✔ from project edit page
            ✔ from project view page
            ✔ from a script level

islemaster · 2017-12-13T22:18:05Z

dashboard/test/ui/features/step_definitions/applab.rb

+  wait_short_until do
+    share_link_input = @browser.find_element(:css, '#sharing-input')
+  end
+  expect(share_link_input.attribute('value')).to include(expected_text)


Tested that this step fails as expected when the given text is not found:

breville · 2017-12-13T22:49:21Z

apps/src/code-studio/initApp/project.js

+  },
+
+  /**
+   * This method exists to be mocked for unit tests.


This made me sad :)

islemaster · 2017-12-14T18:41:09Z

This is passing tests now that the Chrome 63 failures are worked out. Can I get a review?

breville · 2017-12-14T18:53:29Z

apps/src/code-studio/initApp/project.js

+      const re = /([-.]?studio)?\.?code.org/i;
+      const environmentKey = location.hostname.replace(re, '');
+      const subdomain = environmentKey.length > 0 ? `${environmentKey}.` : '';
+      const port = 'localhost' === environmentKey ? `:${location.port}` : '';


Gut says this kind of parsing could be more generically useful across our code.

Think this should be extracted or reused somewhere right away?

I think okay for now; but it would be good to keep an eye out for chances to use shared code going forward. The Ruby side of the system has been somewhat good at using common code to deal with our various environments.

breville · 2017-12-14T18:54:38Z

apps/test/unit/code-studio/initApp/projectTest.js

+        codeProjects: 'https://test.codeprojects.org',
+      },
+      {
+        studio:'https://test.studio.code.org',


Is this a domain we offer?

Huh - I know there was a time in the past where we didn't care whether you wrote test-studio or test.studio, but maybe that's not true anymore? This is easy enough to tear out, I just wanted to make sure this utility handled that case.

Doesn't seem to work for me, and I have a feeling some of our infrastructure changes stopped them from working a while ago. Same comment for the staging link in this file too.

Didn't get confirmation yet, agreed, it looks like we don't support these anymore. I've removed the tests.

breville · 2017-12-14T18:57:16Z

dashboard/test/ui/features/step_definitions/applab.rb

@@ -1,3 +1,7 @@
+# Which stage of allthethings.script contains the App Lab levels; this way we
+# only have to update in one place if this changes.
+APPLAB_ALLTHETHINGS_STAGE = 18


Nice. Can imagine a lot of UI tests benefitting from something like this.

breville · 2017-12-14T18:57:45Z

dashboard/test/ui/features/step_definitions/applab.rb

@@ -36,6 +40,14 @@ def add_code_to_editor(code)
  STEPS
 end

+Given /^I am on the (\d+)(?:st|nd|rd|th)? App ?Lab test level$/ do |level_index|


I look forward to testing the 3th level :)

breville

Not particularly familiar part of the code base, but looks good to me!

islemaster commented Dec 13, 2017

View reviewed changes

islemaster added the bug-fix label Dec 13, 2017

islemaster requested review from cpirich and breville December 13, 2017 22:20

breville reviewed Dec 13, 2017

View reviewed changes

apps/src/code-studio/initApp/project.js

},

/**

* This method exists to be mocked for unit tests.

Copy link

Member

breville Dec 13, 2017

Choose a reason for hiding this comment

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

This made me sad :)

islemaster and others added 7 commits December 14, 2017 09:10

Revert "Revert 19535 weblab correct share links"

e803911

Test generating share URL from a script level

decbbf7

Generate correct project URL from script levels

1f59c34

Correct doc comment

048bb28

Reorganize tests

93a608c

Remove unneeded line-wrap

5af8945

Add UI test covering regression

93e4775

islemaster force-pushed the revert-19623-revert-19535-weblab-correct-share-links branch from db585f0 to 93e4775 Compare December 14, 2017 17:10

breville reviewed Dec 14, 2017

View reviewed changes

breville approved these changes Dec 14, 2017

View reviewed changes

Remove tests for unsupported subdomains

9582a8c

islemaster changed the title ~~Unrevert Web Lab correct share links~~ Web Lab: Correct share links in different environments Dec 14, 2017

islemaster merged commit f84d7ee into staging Dec 14, 2017

islemaster deleted the revert-19623-revert-19535-weblab-correct-share-links branch December 14, 2017 21:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Web Lab: Correct share links in different environments #19625

Web Lab: Correct share links in different environments #19625

islemaster commented Dec 13, 2017 •

edited

islemaster Dec 13, 2017

breville Dec 13, 2017

islemaster commented Dec 14, 2017

breville Dec 14, 2017

islemaster Dec 14, 2017

breville Dec 14, 2017 •

edited

breville Dec 14, 2017

islemaster Dec 14, 2017

breville Dec 14, 2017

islemaster Dec 14, 2017

breville Dec 14, 2017

breville Dec 14, 2017

breville left a comment

Web Lab: Correct share links in different environments #19625

Web Lab: Correct share links in different environments #19625

Conversation

islemaster commented Dec 13, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

islemaster commented Dec 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

breville Dec 14, 2017 • edited

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

breville left a comment

Choose a reason for hiding this comment

islemaster commented Dec 13, 2017 •

edited

breville Dec 14, 2017 •

edited