Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make redirects to latest versions of CB curriculum use 302 status code #35716

Merged
merged 1 commit into from Jul 8, 2020

Conversation

davidsbailey
Copy link
Member

Background

Hannah discovered that the 404 page on curriculum builder links to https://curriculum.code.org/csp-current/, which was redirecting to /csp-19/ rather than /csp-20/ (see discussion in slack). The problem is that we are using a 301 Moved Permanently redirect, which some browsers cache indefinitely, causing some users to continue to be redirected to the old version of the curriculum after a new one is marked as current.

Description

Use a 302 Moved Temporarily redirect from the urls listed below to the latest version of the specified curriculum, so that browsers will not cache it.

This fix may be moot, since we hope to be off of curriculum builder by this time next year, which is the next time these links would change. But the fix seems straightforward, so it seems better to do it than not to.

Testing Story

Ran the script locally, then verified each of the following links to go the right place with 302 status code:

Also verified that the updated config looks correct in S3:
Screen Shot 2020-07-08 at 11 33 40 AM

Reviewer Checklist:

  • Tests provide adequate coverage
  • 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

Copy link

@uponthesun uponthesun left a comment

Choose a reason for hiding this comment

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

It sounds like this has already been applied in production when you ran it? Is that the normal process for applying these changes?

@davidsbailey
Copy link
Member Author

What I did was

  1. test out the change by modifying the S3 config in 1 of the 6 routes manually. then confirmed the site behavior updated as expected.
  2. test it out again by making the change in the script in 1 of the 6 routes, then running the script. then confirmed that the S3 config and site behavior updated as expected.
  3. made the change in the script in the remaining routes. ran the script, confirmed S3 config + site behavior

I'm not sure if there is a better practice here

@davidsbailey davidsbailey merged commit 184ef15 into staging Jul 8, 2020
@davidsbailey davidsbailey deleted the cb-302-redirects branch July 8, 2020 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants