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

Limit Editing on Levelbuilder to only en-US Locale #34979

Merged
merged 4 commits into from May 29, 2020

Conversation

dmcavoy
Copy link
Contributor

@dmcavoy dmcavoy commented May 27, 2020

We had an issue where a levelbuilder was in a Spanish locale on levelbuilder. They went in and edited a script and when it saved it changed the name and description of the script to the Spanish version. This change got deployed and users for confused why their course was now in Spanish when their locale was not.

To prevent this we are going to limit editing of scripts, levels, etc on levelbuilder to only the en-US locale. If you try to perform an editing type action in another locale you will get redirected to the homepage and see a flash notice (see below).

Screen Shot 2020-05-26 at 5 24 45 PM

This change will impact actions in the following controllers:
blocks_controller.rb
courses_controller.rb
datasets_controller.rb
level_assets_controller.rb
level_starter_assets_controller.rb
levels_controller.rb
libraries_controller.rb
script_levels_controller.rb
scripts_controller.rb
shared_blockly_functions_controller.rb
videos_controller.rb

Links

Testing story

I added a test to make sure you get redirected if you try to edit a script or a level in another locale besides en-US.

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
Member

@davidsbailey davidsbailey left a comment

Choose a reason for hiding this comment

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

Great solution Dani! In addition to the test coverage you added, we also have at least one UI test covering the require_levelbuilder_mode_or_test_env pathway on the script edit page.

def require_levelbuilder_mode
require_english_in_levelbuilder_mode
Copy link
Member

Choose a reason for hiding this comment

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

thank you for pushing for this more general approach, Dani! I think this is quite likely to save us from another surprise unwanted translation somewhere besides the script and level edit pages.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1000

@dmcavoy dmcavoy requested a review from davidsbailey May 29, 2020 00:33
Copy link
Member

@davidsbailey davidsbailey left a comment

Choose a reason for hiding this comment

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

new UI tests look good!

@dmcavoy dmcavoy merged commit 9ca7fba into staging May 29, 2020
@dmcavoy dmcavoy deleted the prevent-edit-levelbuilder-lang branch May 29, 2020 13:15
@Hamms
Copy link
Contributor

Hamms commented May 29, 2020

This looks great!

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