From d1e3830899e15efede1eec093d8f83bcfbf07054 Mon Sep 17 00:00:00 2001 From: Chris Zetter <253059100+zetter-rpf@users.noreply.github.com> Date: Wed, 9 Sep 2026 17:11:19 +0100 Subject: [PATCH] Allow Scratch projects to be created by ExCS client Currently if a school doesn't have the blocks editor enabled they are unable to add migrated ExCS projects. This allows any client to create scratch projects, even if the School does not have Scratch enabled. I think this is fine as schools without this enabled still won't see the button to add Scratch projects. Even if we did restrict it to ExCS projects in some way, clients could add those projects and then edit them however they want making the restriction useless. Since we plan on removing the beta flag soon anyway I think it's simpler to loosen the restriction. --- .../api/lessons/batch_controller.rb | 10 ----- app/controllers/api/lessons_controller.rb | 5 --- app/controllers/concerns/lesson_creation.rb | 13 ------- .../creating_a_batch_of_lessons_spec.rb | 29 +-------------- .../features/lesson/creating_a_lesson_spec.rb | 37 +------------------ 5 files changed, 3 insertions(+), 91 deletions(-) diff --git a/app/controllers/api/lessons/batch_controller.rb b/app/controllers/api/lessons/batch_controller.rb index 6562389b3..6e445a3ea 100644 --- a/app/controllers/api/lessons/batch_controller.rb +++ b/app/controllers/api/lessons/batch_controller.rb @@ -8,7 +8,6 @@ class BatchController < ApiController before_action :authorize_user 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! @@ -33,15 +32,6 @@ def verify_school_class_belongs_to_school params[:lesson_projects].each { |lesson_params| verify_lesson_school_class!(lesson_params) } end - def verify_can_create_scratch_projects - return unless lesson_projects? - - 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 diff --git a/app/controllers/api/lessons_controller.rb b/app/controllers/api/lessons_controller.rb index c84fcf98a..dd41894ed 100644 --- a/app/controllers/api/lessons_controller.rb +++ b/app/controllers/api/lessons_controller.rb @@ -7,7 +7,6 @@ class LessonsController < ApiController before_action :authorize_user, except: %i[index show] before_action :verify_school_class_belongs_to_school, only: :create - before_action :verify_can_create_scratch_projects, only: %i[create create_copy] load_and_authorize_resource :lesson def index @@ -86,10 +85,6 @@ def verify_school_class_belongs_to_school verify_lesson_school_class!(create_params) end - def verify_can_create_scratch_projects - verify_lesson_scratch!(create_params, source_project:) - end - def user_remixes(lessons) lessons.map { |lesson| user_remix(lesson) } end diff --git a/app/controllers/concerns/lesson_creation.rb b/app/controllers/concerns/lesson_creation.rb index f8df4ae0c..79c99439b 100644 --- a/app/controllers/concerns/lesson_creation.rb +++ b/app/controllers/concerns/lesson_creation.rb @@ -32,19 +32,6 @@ def verify_lesson_school_class!(lesson_params) raise ParameterError, 'school_class_id does not correspond to school_id' end - def verify_lesson_scratch!(lesson_params, source_project: nil) - return unless scratch_project?(lesson_params) || source_project&.scratch_project? - - school = School.find_by(id: lesson_params[:school_id]) - return if school&.scratch_enabled? - - render json: { error: 'Forbidden' }, status: :forbidden - end - - 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? 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 f3123ae42..a02a29c17 100644 --- a/spec/features/lesson/creating_a_batch_of_lessons_spec.rb +++ b/spec/features/lesson/creating_a_batch_of_lessons_spec.rb @@ -20,8 +20,7 @@ end let(:teacher) { create(:teacher, school:) } - let(:school) { create(:school, scratch_enabled:) } - let(:scratch_enabled) { true } + let(:school) { create(:school) } let(:batch_path) { '/api/lessons/batch' } let(:lesson_projects) { lesson_project_params } @@ -176,7 +175,7 @@ end context 'when the user does not belong to the school' do - let(:other_school) { create(:school, scratch_enabled: true) } + let(:other_school) { create(:school) } let(:lesson_project_params) do [ { @@ -201,18 +200,6 @@ end end - context 'when the school does not have Scratch enabled' do - let(:scratch_enabled) { false } - - it 'returns 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 - context 'when there lesson projects is an empty array' do let(:lesson_project_params) { [] } @@ -317,17 +304,5 @@ 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 diff --git a/spec/features/lesson/creating_a_lesson_spec.rb b/spec/features/lesson/creating_a_lesson_spec.rb index 4757f1d8a..c45a10267 100644 --- a/spec/features/lesson/creating_a_lesson_spec.rb +++ b/spec/features/lesson/creating_a_lesson_spec.rb @@ -226,8 +226,7 @@ } end - it 'creates a lesson with a scratch component when school has Scratch enabled' do - school.update!(scratch_enabled: true) + it 'creates a lesson with a scratch component' do post('/api/lessons', headers:, params:) expect(response).to have_http_status(:created) @@ -239,12 +238,6 @@ expect(project.project_type).to eq(Project::Types::CODE_EDITOR_SCRATCH) expect(project.scratch_component.content).to eq({ 'example_data' => 'true' }) end - - it 'returns forbidden when school does not have Scratch enabled' do - school.update!(scratch_enabled: false) - post('/api/lessons', headers:, params:) - expect(response).to have_http_status(:forbidden) - end end # #create resolves `lesson[:source_project_identifier]` through @@ -278,7 +271,6 @@ context 'when the source project is a shared Experience CS project' do before do - school.update!(scratch_enabled: true) post('/api/lessons', headers:, params:) end @@ -312,32 +304,5 @@ expect(lesson_project.origin).to eq(source_project.origin) end end - - context 'when the school does not have Scratch enabled' do - # The request carries no project_attributes[:project_type], so the scratch gate has to - # look at the source project to see that a Scratch project is about to be created. - let(:params) do - { - lesson: { - name: 'Test Lesson', - school_id: school.id, - source_project_identifier: source_project.identifier, - project_attributes: { - name: 'My digital canvas', - locale: 'en' - } - } - } - end - - before do - school.update!(scratch_enabled: false) - end - - it 'responds 403 Forbidden when only source_project_identifier points at a scratch project' do - post('/api/lessons', headers:, params:) - expect(response).to have_http_status(:forbidden) - end - end end end