Prevent lesson editor from adding the same level to a script twice #39060
Conversation
| @@ -779,10 +751,36 @@ class LessonsControllerTest < ActionController::TestCase | |||
| script_level = section.script_levels.first | |||
| assert script_level.assessment | |||
| refute script_level.bonus | |||
| assert_equal 'progression name', script_level.progression | |||
There was a problem hiding this comment.
Why are you removing this?
There was a problem hiding this comment.
good catch... I've added this back.
dashboard/app/models/script.rb
Outdated
| h = {} | ||
| levels.map(&:key).each {|key| h[key] = (h[key] || 0) + 1} |
There was a problem hiding this comment.
I think you could use group_by to simplify this logic slightly, like duplicate_keys = levels.map(&:key).group_by {|key| key}.select {|_key, values| values.count > 1}.keys
But I don't think it makes much of a difference so up to you.
There was a problem hiding this comment.
💡 ! I have looked up how to do "count by" in ruby many times and never thought to use group_by. Thanks for this!
| @@ -779,10 +751,36 @@ class LessonsControllerTest < ActionController::TestCase | |||
| script_level = section.script_levels.first | |||
| assert script_level.assessment | |||
| refute script_level.bonus | |||
| assert_equal 'progression name', script_level.progression | |||
There was a problem hiding this comment.
good catch... I've added this back.
dashboard/app/models/script.rb
Outdated
| h = {} | ||
| levels.map(&:key).each {|key| h[key] = (h[key] || 0) + 1} |
There was a problem hiding this comment.
💡 ! I have looked up how to do "count by" in ruby many times and never thought to use group_by. Thanks for this!
Finishes PLAT-612.
It's hard to anticipate whether a level is already in the script or not before updating the lesson, so we use the existing database transaction to check that there are no duplicates after the lesson has been updated, but before the transaction is committed.
Testing story
New unit test verifies that the error case is hit on the server. I also manually verified that the error is visible to the user when adding a duplicate level via the UI:
Reviewer Checklist: