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

Remove blank objectives #43710

Merged
merged 2 commits into from Nov 19, 2021
Merged

Remove blank objectives #43710

merged 2 commits into from Nov 19, 2021

Conversation

davidsbailey
Copy link
Member

@davidsbailey davidsbailey commented Nov 19, 2021

Starts PLAT-1390.

  • Step 1 (this PR): remove existing blank objectives, and prevent more from being added via levelbuilder UI
  • Step 2 (make objective description required #43711): add validation preventing blank objectives from being added more generally

What i did in this step:

  • Objective.all.reject(&:description).count returned 1 in development, levelbuilder and production
  • used lesson edit page locally to remove the one problem objective, and added the resulting .script_json changes to this PR
  • updated the lesson save path to never add blank objectives.
    • I couldn't provide a default value of '' when storing the description in the database, because that gets transformed to nil somewhere, possibly in serialized_properties.rb

Testing story

  • new unit test
  • manually verified that after entering a blank objective and saving the lesson edit page and then reloading it, the blank objective is gone

Deployment strategy

  • merge step 1
  • wait for it to reach levelbuilder and production
  • check that levelbuilder and production do not contain blank objectives
  • merge step 2

@davidsbailey davidsbailey merged commit a86a0cf into staging Nov 19, 2021
@davidsbailey davidsbailey deleted the remove-blank-objectives branch November 19, 2021 18:08
snickell pushed a commit that referenced this pull request Feb 3, 2024
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