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

Fnd 1613 add curriculum unplugged to i18n sync #41758

Merged
merged 1 commit into from
Jul 28, 2021

Conversation

daynew
Copy link
Member

@daynew daynew commented Jul 27, 2021

We want to make https://code.org/curriculum/unplugged translatable. That URL is currently rendered by pegasus/sites.v3/code.org/public/curriculum/unplugged.md.erb. This is a problem because it is an Embedded RuBy (.erb) file. Making this file available for translation would mean that translators could make our servers execute malicious ruby scripts. This fix is to instead make it a Markdown(.md) file which has all the plain text translators need. The existing ruby code we need to execute for the video thumbnails are instead hidden away in different files (partials) which will not be made available to translators. .partial is added to unplugged.md to enable these sub-templates. Ultimately we end up with pegasus/sites.v3/code.org/public/curriculum/unplugged.md.partial which should be safe for translation.

Links

Testing story

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

@daynew daynew force-pushed the fnd-1613-add-curriculum-unplugged-to-i18n-sync branch 2 times, most recently from 73cb845 to 6836bec Compare July 27, 2021 19:20
@daynew daynew requested a review from a team July 27, 2021 20:57
@daynew daynew marked this pull request as ready for review July 27, 2021 20:57
@daynew daynew requested a review from a team as a code owner July 27, 2021 20:57
Copy link
Contributor

@KatieShipley KatieShipley left a comment

Choose a reason for hiding this comment

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

LGTM!

In order to make this page translatable, we first needed to remove all the embedded ruby code because it is a security risk to allow translators to write arbitrary Ruby code which is executed on our servers.
@daynew daynew force-pushed the fnd-1613-add-curriculum-unplugged-to-i18n-sync branch from 6836bec to 8fbff28 Compare July 28, 2021 18:25
@daynew daynew merged commit 00c69a7 into staging Jul 28, 2021
@daynew daynew deleted the fnd-1613-add-curriculum-unplugged-to-i18n-sync branch July 28, 2021 18:25
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

2 participants