Skip to content

Commit

Permalink
address code review
Browse files Browse the repository at this point in the history
  • Loading branch information
Bjvanminnen committed May 18, 2017
1 parent e3b9c54 commit ea13fb9
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 5 deletions.
2 changes: 1 addition & 1 deletion dashboard/app/controllers/courses_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def create

def update
course = Course.find_by_name!(params[:course_name])
course.update_and_persist(params[:scripts], i18n_params)
course.persist_strings_and_scripts_changes(params[:scripts], i18n_params)
redirect_to course
end

Expand Down
8 changes: 5 additions & 3 deletions dashboard/app/models/course.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ class Course < ApplicationRecord
has_one :plc_course, class_name: 'Plc::Course'
has_many :course_scripts, -> {order('position ASC')}

after_save :write_serialization

def skip_name_format_validation
!!plc_course
end
Expand Down Expand Up @@ -68,10 +70,10 @@ def serialize
# the set of scripts that are in the course, then writes out our serialization
# @param scripts [Array<String>] - Updated list of names of scripts in this course
# @param course_strings[Hash{String => String}]
def update_and_persist(scripts, course_strings)
def persist_strings_and_scripts_changes(scripts, course_strings)
Course.update_strings(name, course_strings)
update_scripts(scripts)
write_serialization
save!
end

def write_serialization
Expand All @@ -81,7 +83,7 @@ def write_serialization

# @param new_scripts [Array<String>]
def update_scripts(new_scripts)
new_scripts.reject!(&:empty?)
new_scripts = new_scripts.reject(&:empty?)
# we want to delete existing course scripts that aren't in our new list
scripts_to_delete = course_scripts.map(&:script).map(&:name) - new_scripts

Expand Down
1 change: 0 additions & 1 deletion dashboard/test/models/course_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ class UpdateScriptsTests < ActiveSupport::TestCase
name: 'this-course',
script_names: ['script1', 'script2']
}.to_json
# File.any_instance.stub(:read) { serialization }
File.stubs(:read).returns(serialization)

Course.load_from_path('file_path')
Expand Down

0 comments on commit ea13fb9

Please sign in to comment.