diff --git a/app/controllers/api/lessons/batch_controller.rb b/app/controllers/api/lessons/batch_controller.rb index cd759f03b..6562389b3 100644 --- a/app/controllers/api/lessons/batch_controller.rb +++ b/app/controllers/api/lessons/batch_controller.rb @@ -10,13 +10,15 @@ class BatchController < ApiController before_action :verify_school_class_belongs_to_school before_action :verify_can_create_scratch_projects before_action :authorize_lesson_projects! + before_action :authorize_source_projects! def create_batch authorize_blank_lesson_batch! unless lesson_projects? raise ParameterError, 'lesson_projects cannot be blank' unless lesson_projects? @results = Lesson::CreateBatch.call( - lessons_params: batch_lessons_params + lessons_params: batch_lessons_params, + source_projects: batch_source_projects ) @user = current_user @results.select(&:success?).each { |result| track_project_event('Project - Created', result[:lesson].project) } @@ -34,18 +36,35 @@ def verify_school_class_belongs_to_school def verify_can_create_scratch_projects return unless lesson_projects? - scratch_project_params = params[:lesson_projects].find { |lesson_params| scratch_project?(lesson_params) } - return unless scratch_project_params - - verify_lesson_scratch!(scratch_project_params) + batch_lessons_params.each_index do |index| + verify_lesson_scratch!(batch_lessons_params[index], source_project: source_project_for(index)) + break if performed? + end end def batch_lessons_params @batch_lessons_params ||= params[:lesson_projects].map { |lesson_params| create_batch_params(lesson_params) } end + def batch_source_projects + batch_lessons_params.each_index.map { |index| source_project_for(index) } + end + + def source_project_for(index) + @source_project_by_index ||= {} + return @source_project_by_index[index] if @source_project_by_index.key?(index) + + lesson_params = batch_lessons_params[index] + @source_project_by_index[index] = find_source_project!( + lesson_params[:source_project_identifier], + lesson_params.dig(:project_attributes, :locale) + ) + end + def create_batch_params(lesson_project) - lesson_project.permit(*LESSON_ATTRIBUTES, :origin_identifier, project_attributes: PROJECT_ATTRIBUTES).merge(user_id: current_user.id) + lesson_project + .permit(*LESSON_ATTRIBUTES, :origin_identifier, :source_project_identifier, project_attributes: PROJECT_ATTRIBUTES) + .merge(user_id: current_user.id) end def lesson_projects? @@ -63,6 +82,15 @@ def authorize_lesson_projects! end end + def authorize_source_projects! + return unless lesson_projects? + + batch_lessons_params.each_index do |index| + source_project = source_project_for(index) + authorize! :show, source_project if source_project + end + end + def authorize_blank_lesson_batch! authorize! :create, Lesson.new(user_id: current_user.id) end diff --git a/app/controllers/api/lessons_controller.rb b/app/controllers/api/lessons_controller.rb index 0c1385ccb..c84fcf98a 100644 --- a/app/controllers/api/lessons_controller.rb +++ b/app/controllers/api/lessons_controller.rb @@ -136,19 +136,5 @@ def source_project def source_project_identifier params.dig(:lesson, :source_project_identifier) end - - def find_source_project!(identifier, locale) - return nil if identifier.blank? - - project = ProjectLoader.new(identifier, [locale]).load - raise ParameterError, "source project '#{identifier}' not found" if project.nil? - - # Only ExCS 'code editor' projects are remixed here; legacy scratch projects keep the stub path. - return nil unless project.scratch_project? - - raise ParameterError, 'source project must be an Experience CS project' unless project.origin == Project::Origins::EXPERIENCE_CS - - project - end end end diff --git a/app/controllers/concerns/lesson_creation.rb b/app/controllers/concerns/lesson_creation.rb index 438f461fb..f8df4ae0c 100644 --- a/app/controllers/concerns/lesson_creation.rb +++ b/app/controllers/concerns/lesson_creation.rb @@ -44,4 +44,18 @@ def verify_lesson_scratch!(lesson_params, source_project: nil) def scratch_project?(lesson_params) lesson_params.dig(:project_attributes, :project_type) == Project::Types::CODE_EDITOR_SCRATCH end + + def find_source_project!(identifier, locale) + return nil if identifier.blank? + + project = ProjectLoader.new(identifier, [locale]).load + raise ParameterError, "source project '#{identifier}' not found" if project.nil? + + # Only ExCS 'code editor' projects are remixed here; legacy scratch projects keep the stub path. + return nil unless project.scratch_project? + + raise ParameterError, 'source project must be an Experience CS project' unless project.origin == Project::Origins::EXPERIENCE_CS + + project + end end diff --git a/lib/concepts/lesson/operations/create_batch.rb b/lib/concepts/lesson/operations/create_batch.rb index d1bb50f80..674d9035e 100644 --- a/lib/concepts/lesson/operations/create_batch.rb +++ b/lib/concepts/lesson/operations/create_batch.rb @@ -3,15 +3,18 @@ class Lesson class CreateBatch class << self - def call(lessons_params:) - lessons_params.map { |lesson| create_one(lesson) } + def call(lessons_params:, source_projects: []) + lessons_params.zip(source_projects).map { |lesson_params, source_project| create_one(lesson_params, source_project) } end private - def create_one(lesson_params) + def create_one(lesson_params, source_project) origin_identifier = lesson_params[:origin_identifier] - Lesson::Create.call(lesson_params: lesson_params.except(:origin_identifier)).tap do |result| + Lesson::Create.call( + lesson_params: lesson_params.except(:origin_identifier, :source_project_identifier), + source_project: + ).tap do |result| result[:origin_identifier] = origin_identifier if origin_identifier.present? end end diff --git a/spec/concepts/lesson/create_batch_spec.rb b/spec/concepts/lesson/create_batch_spec.rb index ff8523aef..a73fdfdc7 100644 --- a/spec/concepts/lesson/create_batch_spec.rb +++ b/spec/concepts/lesson/create_batch_spec.rb @@ -58,9 +58,9 @@ it 'does not pass origin_identifier to lesson creation' do received_params = [] - allow(Lesson::Create).to receive(:call).and_wrap_original do |method, lesson_params:| + allow(Lesson::Create).to receive(:call).and_wrap_original do |method, lesson_params:, source_project: nil| received_params << lesson_params - method.call(lesson_params:) + method.call(lesson_params:, source_project:) end described_class.call(lessons_params:) @@ -75,5 +75,24 @@ it 'appends the origin_identifier to the second created lesson' do expect(result.second[:origin_identifier]).to eq('test-lesson-identifier-two') end + + context 'when a source project is given' do + let!(:source_project) do + create(:scratch_project, user_id: nil, school_id: nil, origin: Project::Origins::EXPERIENCE_CS) + end + let(:lessons_params) do + super().tap { |params| params.first[:source_project_identifier] = source_project.identifier } + end + let(:source_projects) { [source_project, nil] } + let(:result) { described_class.call(lessons_params:, source_projects:) } + + it 'builds the corresponding lesson project with the source project' do + expect(result.first[:lesson].project.source_project_id).to eq(source_project.id) + end + + it 'leaves other lessons unaffected' do + expect(result.second[:lesson].project.source_project_id).to be_nil + end + end end end diff --git a/spec/features/lesson/creating_a_batch_of_lessons_spec.rb b/spec/features/lesson/creating_a_batch_of_lessons_spec.rb index bead3a881..f3123ae42 100644 --- a/spec/features/lesson/creating_a_batch_of_lessons_spec.rb +++ b/spec/features/lesson/creating_a_batch_of_lessons_spec.rb @@ -248,4 +248,86 @@ expect(Lesson.count).to eq(0) end end + + # #create_batch resolves each row's `source_project_identifier` through + # ProjectLoader, authorizes it, and hands the Project to Lesson::CreateBatch + # so that row's lesson project is built as a remix instead of a stub. + context 'when entries reference source projects' do + let(:source_content) { { 'targets' => [{ 'name' => 'Stage' }], 'monitors' => [], 'extensions' => [], 'meta' => {} } } + + let!(:source_project) do + create(:scratch_project, identifier: 'my-digital-canvas', locale: 'en', user_id: nil, school_id: nil, + name: 'My digital canvas', origin: Project::Origins::EXPERIENCE_CS) + .tap { |project| project.scratch_component.update!(content: source_content) } + end + + let(:lesson_project_params) do + [ + { + name: 'Lesson 1', + school_id: school.id, + source_project_identifier: source_project.identifier, + project_attributes: { name: 'My digital canvas', locale: 'en' } + }, + { + name: 'Lesson 2', + school_id: school.id, + project_attributes: { name: 'Project 2', locale: 'en' } + } + ] + end + + let(:lesson_projects_json) { JSON.parse(response.body, symbolize_names: true) } + let(:lesson_project_with_source) { Lesson.find(lesson_projects_json.first[:id]).project } + let(:stub_lesson_project) { Lesson.find(lesson_projects_json.second[:id]).project } + + it 'responds 201 Created' do + expect(response).to have_http_status(:created) + end + + it 'records the source project on the lesson_project_with_source' do + expect(lesson_project_with_source.source_project_id).to eq(source_project.id) + end + + it 'sets the lesson_project_with_source locale to nil' do + expect(lesson_project_with_source.locale).to be_nil + end + + it 'leaves source_project_identifier as nil in the stub project' do + expect(stub_lesson_project.source_project_id).to be_nil + end + + context 'when the source project cannot be found' do + let(:lesson_project_params) do + [ + { + name: 'Lesson 1', + school_id: school.id, + source_project_identifier: 'does-not-exist', + project_attributes: { name: 'My digital canvas', locale: 'en' } + } + ] + end + + it 'responds 422 Unprocessable' do + expect(response).to have_http_status(:unprocessable_content) + end + + it 'does not create any lessons' do + expect(Lesson.count).to eq(0) + end + end + + context 'when a source_project_identifier points at a scratch project and the school does not have Scratch enabled' do + let(:scratch_enabled) { false } + + it 'responds 403 Forbidden' do + expect(response).to have_http_status(:forbidden) + end + + it 'does not create any lessons' do + expect(Lesson.count).to eq(0) + end + end + end end