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

Collect strings from BubbleChoice sub-levels for translation #38629

Merged
merged 3 commits into from Jan 25, 2021

Conversation

hacodeorg
Copy link
Contributor

@hacodeorg hacodeorg commented Jan 20, 2021

FND-1250
Collect strings from BubbleChoice sub-levels for translation. This is the first step in making BubbleChoice sub-levels translatable. (The next step is to display translations properly in the Code studio.)

Example

The last lesson of Course D - 2020 is a end-of-course project. It is a BubbleChoice level, which allow students to choose from 1 of 4 types of project. Each specific project is a sub-level, for example this basketball project. Currently, i18n strings from a sub-level project such as its display name and instructions are not collected by the sync-in process.

Screen Shot 2021-01-19 at 5 13 02 PM

Testing story

Test the get_i18n_strings function from the Rails console

load "../bin/i18n/sync-in.rb";
bc = Level.find_by_name('courseD_EOC_2020');  # a BubbleChoice level at https://levelbuilder-studio.code.org/s/coursed-2020/stage/19/puzzle/2
get_i18n_strings bc   # extract i18n strings from the BubbleChoice level and its sub-levels
# Finally, verify that the results include strings from both the BubbleChoice level and its sub-levels (such as https://levelbuilder-studio.code.org/s/coursed-2020/stage/19/puzzle/2/sublevel/1)

Test the sync-in process:

  • From the repo root, run bin/i18n/sync-in.rb
  • Verify that i18n/locales/source/course_content/2020/coursed-2020.json contains strings from all BubbleChoice levels and their sub-levels. The result looks like this
    {
      "https://studio.code.org/s/coursed-2020/stage/19/puzzle/2": {
        "dsls": {
          "description": "You have reached the last lesson of Course D! Choose a project to create.",
          "display_name": "End of Course Project"
        },
        "sublevels": {
          "CourseD_EOC_bounce1": {
            "display_name": "Create a basketball game",
            "short_instructions": "Create your own basketball game!",
            "long_instructions": "Create your own basketball game!"
          },
          "CourseD_EOC_starwars": {
            "display_name": "Create a Star Wars game",
            "short_instructions": "Create your own Star Wars game!",
            "long_instructions": "Create your own Star Wars game!",
          },
          "CourseD_EOC_dance": {
            "display_name": "Create a dance",
            "short_instructions": "Create your own dance!",
            "long_instructions": "Create your own dance!",
          },
          "CourseD_EOC_artist": {
            "display_name": "Create a drawing",
            "short_instructions": "Create your own drawing!",
            "long_instructions": "Create your own drawing!",
          }
        }
      }
    }

@hacodeorg hacodeorg changed the title Collect strings for translation from BubbleChoice sublevels Collect strings from BubbleChoice sub-levels for translation Jan 20, 2021
@hacodeorg hacodeorg marked this pull request as ready for review January 20, 2021 01:24
@hacodeorg hacodeorg requested a review from a team as a code owner January 20, 2021 01:24
@hacodeorg
Copy link
Contributor Author

@Hamms I'd appreciate it if you could take a quick look at this PR :)

Copy link
Contributor

@Hamms Hamms left a comment

Choose a reason for hiding this comment

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

LGTM!

if level.is_a? BubbleChoice
i18n_strings["sublevels"] = {}
level.sublevels.map do |sublevel|
i18n_strings["sublevels"][sublevel.name] = get_i18n_strings sublevel
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@daynew This will address your concern that translations should not depend on order of the sublevels. I've updated the PR description with a new output.

Btw, Level.name is unique because of this validation:

validates_uniqueness_of :name, case_sensitive: false, conditions: -> {where.not(user_id: nil)}

Copy link
Member

@daynew daynew left a comment

Choose a reason for hiding this comment

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

Looks good as long as name isn't something which changes often.

@hacodeorg hacodeorg merged commit f5cff88 into staging Jan 25, 2021
@hacodeorg hacodeorg deleted the ha/sync-in-bubble-choice branch January 25, 2021 21:19
@Hamms
Copy link
Contributor

Hamms commented Jan 25, 2021

Looks good as long as name isn't something which changes often.

We're totally safe to depend on level names to be unique and immutable; many more critical things than the i18n sync would break if that assumption were to be violated

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