Re-add project backed freeplay level to uncached levels list

[bethanyaconnor]

Reported by Brendan in this thread. Users who are signed out (not 100% sure being signed out is necessary) and go to these levels all get the same channel id. This in turn leads to saving errors and data loss for these users.

Repro steps:

1. In two different chrome profiles or on two different computers, go to /s/dance-2019/lessons/1/levels/1 at the same time with the dev console open.
2. Note that the channel ids are the same in the network tab
3. Make changes in both windows/computers. After a couple of changes, one of them will show "Error saving project" and force reload, losing the changes that user made.

I confirmed that I could repro with these steps locally before this change and could not repro with these steps after this change.

PR Checklist:

• Tests provide adequate coverage
• Privacy and Security impacts have been assessed
• Code is well-commented
• New features are translatable or updates will not break translations
• Relevant documentation has been added or updated
• User impact is well-understood and desirable
• Pull Request is labeled appropriately
• Follow-up work items (including potential tech debt) are tracked and linked

[sureshc]

Bummer. Definitely would love to find a fix for this scenario (signed out users) before Hour of Code. Let me know if I can help with implementing tests that detect this issue in environments that don't use CloudFront (we have logic that implements HTTP caching in that fallback scenario).

[davidsbailey]

This looks like the right list to me! I compared against #43629, which were the last changes to http_cache.rb before HOC last year.

Users who are signed out (not 100% sure being signed out is necessary) and go to these levels all get the same channel id.

I agree with the "not sure" bit. The teacher panel shows up on these cached levels now, as described in #45096, which suggests that somehow work WAS done to fix the experience for signed-in users, but it is not clear that anyone doing that work inspected the channel ids on the cached, project-backed levels.

[davidsbailey]

Bummer. Definitely would love to find a fix for this scenario (signed out users) before Hour of Code. Let me know if I can help with implementing tests that detect this issue in environments that don't use CloudFront (we have logic that implements HTTP caching in that fallback scenario).

huge thanks to the infra team (past and present) for making the http caching rules work without cloudfront! My initial take is that this gives us the ability to iterate on a fix for this quickly in local development. I am not sure I see a way to add an automated test to catch this kind of problem, but it could be possible.

One saving grace is that the behavior of showing an error and reloading the page is pretty aggressive and user-visible, which means it is likely that users notice and report this kind of problem. this is a big step up from when we just silently let users continue editing only to find later that their work was lost. Based on that, I would argue that automated end-to-end testing of a fix for this is not 100% required.

[davidsbailey]

I am not sure I see a way to add an automated test to catch this kind of problem, but it could be possible.

@sureshc if you believe this is possible then we'd love your help with it!

[bethanyaconnor]

Also not sure how to write a test for this but happy to if I can get a pointer as to how. I'd like to merge this PR ASAP (unfortunately, Drone decided to start slowly on this PR so not quite as ASAP as I'd have liked) so it might be done in a follow up PR.

I did create a task to fix this: https://codedotorg.atlassian.net/browse/LP-2433

huge thanks to the infra team (past and present) for making the http caching rules work without cloudfront!

💯

[davidsbailey]

unfortunately it looks like dashboard tests did not run in drone:

[test] Files affecting dashboard    tests unmodified from origin/staging. Skipping tests.

and now this test is failing in my drone runs:

==�[0m�[1000D�[K FAIL["test_dance_session_cookie_and_cache_headers", "RoutesTest", 367.90421924600014]
 test_dance_session_cookie_and_cache_headers#RoutesTest (367.90s)
        Expected /(^|,)\s*public\s*(,|$)/ to match "must-revalidate, no-store, private, max-age=0".
        test/test_helper.rb:330:in `block in assert_cache_control_match'
        test/test_helper.rb:329:in `each'
        test/test_helper.rb:329:in `assert_cache_control_match'
        test/test_helper.rb:349:in `assert_caching_enabled'
        test/integration/routes_test.rb:20:in `test_dance_session_cookie_and_cache_headers'
        test/testing/setup_all_and_teardown_all.rb:36:in `run'

@bethanyaconnor would you be willing to own the fix?

[bethanyaconnor]

oh shoot, sorry didn't notice that. Yeah, I can own