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

Prevent lesson editor from adding the same level to a script twice #39060

Merged
merged 6 commits into from
Feb 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion dashboard/app/controllers/lessons_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,12 @@ def update
msg = "The last level in a lockable lesson must be a LevelGroup and an assessment."
raise msg unless @lesson.script_levels.last.assessment && @lesson.script_levels.last.level.type == 'LevelGroup'
end

@lesson.script.prevent_duplicate_levels
@lesson.script.fix_lesson_positions
end

if Rails.application.config.levelbuilder_mode
@lesson.script.fix_lesson_positions
@lesson.script.reload

# This endpoint will only be hit from the lesson edit page, which is only
Expand Down
10 changes: 10 additions & 0 deletions dashboard/app/models/script.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,16 @@ def hide_pilot_scripts

validate :set_is_migrated_only_for_migrated_scripts

def prevent_duplicate_levels
reload

unless levels.count == levels.uniq.count
levels_by_key = levels.map(&:key).group_by {|key| key}
duplicate_keys = levels_by_key.select {|_key, values| values.count > 1}.keys
raise "duplicate levels detected: #{duplicate_keys.to_json}"
end
end

include SerializedProperties

after_save :generate_plc_objects
Expand Down
81 changes: 41 additions & 40 deletions dashboard/test/controllers/lessons_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -733,50 +733,24 @@ class LessonsControllerTest < ActionController::TestCase

test 'add script level via lesson update' do
sign_in @levelbuilder

activity = @lesson.lesson_activities.create(
name: 'activity name',
position: 1,
key: 'activity-key'
)
section = activity.activity_sections.create(
name: 'section name',
position: 1,
key: 'section-key'
)

activity = create :lesson_activity, lesson: @lesson
section = create :activity_section, lesson_activity: activity
level_to_add = create :maze, name: 'level-to-add'

@update_params['activities'] = [
{
id: activity.id,
name: 'activity name',
position: 1,
activitySections: [
{
id: section.id,
name: 'section name',
position: 1,
duration: 10,
progressionName: 'progression name',
scriptLevels: [
activitySectionPosition: 1,
activeId: level_to_add.id,
assessment: true,
levels: [
{
id: level_to_add.id,
name: level_to_add.name
}
]
]
}
]
}
].to_json
@lesson.reload
activities_data = @lesson.summarize_for_lesson_edit[:activities]
section_data = activities_data.first[:activitySections].first
section_data[:progressionName] = 'progression name'
section_data[:scriptLevels].push(
activitySectionPosition: 1,
activeId: level_to_add.id,
assessment: true,
levels: [{id: level_to_add.id, name: level_to_add.name}]
)

@update_params['activities'] = activities_data.to_json
put :update, params: @update_params

assert_response :success
@lesson.reload

assert_equal activity, @lesson.lesson_activities.first
Expand All @@ -790,6 +764,33 @@ class LessonsControllerTest < ActionController::TestCase
assert_equal ['level-to-add'], script_level.levels.map(&:name)
end

test 'cannot add duplicate level via lesson update' do
sign_in @levelbuilder
activity = create :lesson_activity, lesson: @lesson
create :activity_section, lesson_activity: activity

activity2 = create :lesson_activity, lesson: @lesson2
section2 = create :activity_section, lesson_activity: activity2
existing_level = create :maze, name: 'existing-level'
create :script_level, activity_section: section2, activity_section_position: 1, lesson: @lesson2, script: @script, levels: [existing_level]

@lesson.reload
activities_data = @lesson.summarize_for_lesson_edit[:activities]
activities_data.first[:activitySections].first[:scriptLevels].push(
activitySectionPosition: 1,
activeId: existing_level.id,
assessment: true,
levels: [{id: existing_level.id, name: existing_level.name}]
)

@update_params['activities'] = activities_data.to_json

error = assert_raises do
put :update, params: @update_params
end
assert_includes error.message, 'duplicate levels detected'
end

test 'add anonymous survey level via lesson update' do
sign_in @levelbuilder

Expand Down