diff --git a/app/controllers/concerns/file_parameters.rb b/app/controllers/concerns/file_parameters.rb index c847f03a..8298dabc 100644 --- a/app/controllers/concerns/file_parameters.rb +++ b/app/controllers/concerns/file_parameters.rb @@ -3,13 +3,20 @@ module FileParameters def reject_illegal_file_attributes(exercise, params) if exercise && params + # We only want to load the files once, to avoid multiple database queries. + # Further, we use `unscope` to avoid that the `order` scope is applied + files = CodeOcean::File.unscope(:order).where(id: params.values.pluck(:file_id)) + params.reject do |_, file_attributes| - file = CodeOcean::File.find_by(id: file_attributes[:file_id]) + # This mechanism seems cumbersome, but we cannot use an index here. + # The ordering of the files is not guaranteed to be the same as the ordering of the file attributes. + file = files.find {|f| f.id == file_attributes[:file_id].to_i } + next true if file.nil? || file.hidden || file.read_only # avoid that public files from other contexts can be created # `next` is similar to an early return and will proceed with the next iteration of the loop - next true if file.context_type == 'Exercise' && file.context != exercise - next true if file.context_type == 'Submission' && file.context.user != current_user + next true if file.context_type == 'Exercise' && file.context_id != exercise.id + next true if file.context_type == 'Submission' && (file.context.user_id != current_user.id || file.context.user_type != current_user.class.name) next true if file.context_type == 'CommunitySolution' && controller_name != 'community_solutions' false diff --git a/app/controllers/concerns/submission_parameters.rb b/app/controllers/concerns/submission_parameters.rb index c2c7de6f..fb565c65 100644 --- a/app/controllers/concerns/submission_parameters.rb +++ b/app/controllers/concerns/submission_parameters.rb @@ -12,8 +12,9 @@ module SubmissionParameters end submission_params = merge_user(submission_params) files_attributes = submission_params[:files_attributes] - exercise = Exercise.find_by(id: submission_params[:exercise_id]) + exercise = @exercise || Exercise.find_by(id: submission_params[:exercise_id]) submission_params[:files_attributes] = reject_illegal_file_attributes(exercise, files_attributes) + submission_params[:exercise] = exercise submission_params end private :submission_params diff --git a/app/models/exercise.rb b/app/models/exercise.rb index 2359485e..426746b3 100644 --- a/app/models/exercise.rb +++ b/app/models/exercise.rb @@ -517,7 +517,7 @@ class Exercise < ApplicationRecord 0 end else - files.teacher_defined_assessments.sum(:weight) + @maximum_score ||= files.teacher_defined_assessments.sum(:weight) end end diff --git a/spec/concerns/file_parameters_spec.rb b/spec/concerns/file_parameters_spec.rb index 1a4bc595..e148e4a8 100644 --- a/spec/concerns/file_parameters_spec.rb +++ b/spec/concerns/file_parameters_spec.rb @@ -12,7 +12,7 @@ describe FileParameters do describe '#reject_illegal_file_attributes!' do def file_accepted?(file) - files = [[0, attributes_for(:file, context: hello_world, file_id: file.id)]] + files = {'0': attributes_for(:file, context: hello_world, file_id: file.id)} filtered_files = controller.send(:reject_illegal_file_attributes, hello_world, files) files.eql?(filtered_files) end