diff --git a/app/controllers/api/lessons_controller.rb b/app/controllers/api/lessons_controller.rb index 8289b2caa..eca32d4bd 100644 --- a/app/controllers/api/lessons_controller.rb +++ b/app/controllers/api/lessons_controller.rb @@ -6,6 +6,7 @@ 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 @@ -83,6 +84,12 @@ def verify_school_class_belongs_to_school raise ParameterError, 'school_class_id does not correspond to school_id' end + def verify_can_create_project_type + return unless scratch_project? && !school.scratch_enabled? + + render json: { error: 'Forbidden' }, status: :forbidden + end + def user_remixes(lessons) lessons.map { |lesson| user_remix(lesson) } end @@ -97,6 +104,10 @@ def user_remix(lesson) ) end + def scratch_project? + base_params.dig(:project_attributes, :project_type) == Project::Types::CODE_EDITOR_SCRATCH + end + def lesson_params base_params.merge(user_id: current_user.id) end diff --git a/app/controllers/api/schools_controller.rb b/app/controllers/api/schools_controller.rb index 52427efe9..df65a0061 100644 --- a/app/controllers/api/schools_controller.rb +++ b/app/controllers/api/schools_controller.rb @@ -16,7 +16,7 @@ def show end def create - result = School::Create.call(school_params:, creator_id: current_user.id, token: current_user.token) + result = School::Create.call(school_params: school_create_params, creator_id: current_user.id, token: current_user.token) if result.success? @school = result[:school] @@ -31,7 +31,7 @@ def create def update school = School.find(params[:id]) - result = School::Update.call(school:, school_params:) + result = School::Update.call(school:, school_params: school_update_params) if result.success? @school = result[:school] @@ -76,7 +76,7 @@ def import private - def school_params + def school_create_params params.expect( school: %i[name website @@ -99,5 +99,11 @@ def school_params user_origin] ) end + + def school_update_params + params.expect( + school: %i[scratch_enabled] + ) + end end end diff --git a/app/controllers/api/scratch/assets_controller.rb b/app/controllers/api/scratch/assets_controller.rb index 142845b6c..f41a0878a 100644 --- a/app/controllers/api/scratch/assets_controller.rb +++ b/app/controllers/api/scratch/assets_controller.rb @@ -5,7 +5,6 @@ module Scratch class AssetsController < ScratchController include ActiveStorage::SetCurrent - skip_before_action :authorize_user, only: [:show] prepend_before_action :load_project_from_header, only: %i[show create] def show diff --git a/app/controllers/api/scratch/projects_controller.rb b/app/controllers/api/scratch/projects_controller.rb index 14f73b638..1abedb13c 100644 --- a/app/controllers/api/scratch/projects_controller.rb +++ b/app/controllers/api/scratch/projects_controller.rb @@ -5,10 +5,7 @@ module Scratch class ProjectsController < ScratchController include RemixSelection - skip_before_action :authorize_user, only: [:show] - skip_before_action :check_scratch_feature, only: [:show] before_action :load_project, only: %i[show update] - before_action :ensure_create_is_a_remix, only: %i[create] def show diff --git a/app/controllers/api/scratch/scratch_controller.rb b/app/controllers/api/scratch/scratch_controller.rb index 863ab9928..5bed4bfa7 100644 --- a/app/controllers/api/scratch/scratch_controller.rb +++ b/app/controllers/api/scratch/scratch_controller.rb @@ -4,13 +4,10 @@ module Api module Scratch class ScratchController < ApiController before_action :authorize_user - before_action :check_scratch_feature + before_action :can_use_scratch? - def check_scratch_feature - return if current_user.nil? - - school = current_user&.schools&.first - return if Flipper.enabled?(:cat_mode, school) + def can_use_scratch? + return true if current_user.schools.any? raise ActiveRecord::RecordNotFound, 'Not Found' end diff --git a/app/views/api/schools/_school.json.jbuilder b/app/views/api/schools/_school.json.jbuilder index 5aee45f78..86f78342e 100644 --- a/app/views/api/schools/_school.json.jbuilder +++ b/app/views/api/schools/_school.json.jbuilder @@ -17,7 +17,8 @@ json.call( :country_code, :verified_at, :created_at, - :updated_at + :updated_at, + :scratch_enabled, ) include_roles = local_assigns.fetch(:roles, false) diff --git a/db/migrate/20260528141937_add_scratch_enabled_to_school.rb b/db/migrate/20260528141937_add_scratch_enabled_to_school.rb new file mode 100644 index 000000000..1c3f07f7f --- /dev/null +++ b/db/migrate/20260528141937_add_scratch_enabled_to_school.rb @@ -0,0 +1,5 @@ +class AddScratchEnabledToSchool < ActiveRecord::Migration[8.1] + def change + add_column :schools, :scratch_enabled, :boolean, default: false, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 36ac84ff4..497d2d7fe 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_04_29_120000) do +ActiveRecord::Schema[8.1].define(version: 2026_05_28_141937) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" enable_extension "pgcrypto" @@ -335,6 +335,7 @@ t.string "reference" t.datetime "rejected_at" t.string "school_roll_number" + t.boolean "scratch_enabled", default: false, null: false t.datetime "updated_at", null: false t.integer "user_origin", default: 0 t.datetime "verified_at" diff --git a/spec/features/lesson/creating_a_lesson_spec.rb b/spec/features/lesson/creating_a_lesson_spec.rb index 33d80c73e..0acffea98 100644 --- a/spec/features/lesson/creating_a_lesson_spec.rb +++ b/spec/features/lesson/creating_a_lesson_spec.rb @@ -186,4 +186,44 @@ expect(response).to have_http_status(:unprocessable_content) end end + + describe 'working with Scratch projects' do + let(:params) do + { + lesson: { + name: 'Test Lesson', + school_id: school.id, + project_attributes: { + name: 'Hello Scratch project', + project_type: Project::Types::CODE_EDITOR_SCRATCH, + scratch_component: { + content: { + example_data: 'true' + } + } + } + } + } + end + + it 'creates a lesson with a scratch component when school has Scratch enabled' do + school.update!(scratch_enabled: true) + post('/api/lessons', headers:, params:) + expect(response).to have_http_status(:created) + + data = JSON.parse(response.body, symbolize_names: true) + + lesson_id = data[:id] + + project = Lesson.find(lesson_id).project + 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 end diff --git a/spec/features/school/updating_a_school_spec.rb b/spec/features/school/updating_a_school_spec.rb index 92f18442f..5773b7e6d 100644 --- a/spec/features/school/updating_a_school_spec.rb +++ b/spec/features/school/updating_a_school_spec.rb @@ -7,14 +7,14 @@ authenticated_in_hydra_as(owner) end - let!(:school) { create(:school) } + let!(:school) { create(:school, scratch_enabled: false) } let(:headers) { { Authorization: UserProfileMock::TOKEN } } let(:owner) { create(:owner, school:) } let(:params) do { school: { - name: 'New Name' + scratch_enabled: true } } end @@ -28,7 +28,7 @@ put("/api/schools/#{school.id}", headers:, params:) data = JSON.parse(response.body, symbolize_names: true) - expect(data[:name]).to eq('New Name') + expect(data[:scratch_enabled]).to eq(true) end it 'responds 404 Not Found when no school exists' do @@ -41,11 +41,6 @@ expect(response).to have_http_status(:bad_request) end - it 'responds 422 Unprocessable Entity when params are invalid' do - put("/api/schools/#{school.id}", headers:, params: { school: { name: ' ' } }) - expect(response).to have_http_status(:unprocessable_content) - end - it 'responds 401 Unauthorized when no token is given' do put "/api/schools/#{school.id}" expect(response).to have_http_status(:unauthorized) diff --git a/spec/features/scratch/creating_a_scratch_asset_spec.rb b/spec/features/scratch/creating_a_scratch_asset_spec.rb index dedc1d182..bd66d5fa3 100644 --- a/spec/features/scratch/creating_a_scratch_asset_spec.rb +++ b/spec/features/scratch/creating_a_scratch_asset_spec.rb @@ -10,12 +10,7 @@ create(:scratch_component, project: scratch_project) end end - let(:auth_headers) { { 'Authorization' => UserProfileMock::TOKEN } } - let(:project_headers) { auth_headers.merge('X-Project-ID' => project.identifier) } - - before do - Flipper.enable_actor :cat_mode, school - end + let(:headers) { { 'Authorization' => UserProfileMock::TOKEN, 'X-Project-ID' => project.identifier } } it 'responds 401 Unauthorized when no Authorization header is provided' do post '/api/scratch/assets/example.svg', headers: { 'X-Project-ID' => project.identifier } @@ -23,20 +18,19 @@ expect(response).to have_http_status(:unauthorized) end - it 'responds 404 Not Found when cat_mode is not enabled' do - authenticated_in_hydra_as(teacher) - Flipper.disable :cat_mode - Flipper.disable_actor :cat_mode, school + it 'responds 404 Not Found when user is not part of a school' do + user = create(:user) + authenticated_in_hydra_as(user) - post '/api/scratch/assets/example.svg', headers: project_headers + post '/api/scratch/assets/example.svg', headers: headers expect(response).to have_http_status(:not_found) end - it 'creates an asset when cat_mode is enabled and the required headers are provided' do + it 'creates an asset when user is part of a school and the required headers are provided' do authenticated_in_hydra_as(teacher) - post '/api/scratch/assets/example.svg', headers: project_headers + post '/api/scratch/assets/example.svg', headers: headers expect(response).to have_http_status(:created) diff --git a/spec/features/scratch/creating_a_scratch_project_spec.rb b/spec/features/scratch/creating_a_scratch_project_spec.rb index 7df984067..a9909a1fa 100644 --- a/spec/features/scratch/creating_a_scratch_project_spec.rb +++ b/spec/features/scratch/creating_a_scratch_project_spec.rb @@ -36,9 +36,6 @@ before do mock_phrase_generation('new-project-id') create(:scratch_component, project: original_project) - - Flipper.disable :cat_mode - Flipper.disable_actor :cat_mode, school end def make_request(query: request_query, request_headers: headers, request_params: scratch_project) @@ -56,18 +53,18 @@ def make_request(query: request_query, request_headers: headers, request_params: expect(response).to have_http_status(:unauthorized) end - it 'responds 404 Not Found when cat_mode is not enabled' do - authenticated_in_hydra_as(teacher) + it 'responds 404 Not Found when user is not part of a school' do + user = create(:user) + authenticated_in_hydra_as(user) make_request expect(response).to have_http_status(:not_found) end - context 'when authenticated and cat_mode is enabled' do + context 'when authenticated and part of a school' do before do authenticated_in_hydra_as(teacher) - Flipper.enable_actor :cat_mode, school end it 'responds 403 Forbidden when not remixing' do diff --git a/spec/features/scratch/creating_and_showing_a_scratch_asset_spec.rb b/spec/features/scratch/creating_and_showing_a_scratch_asset_spec.rb index 03f05ac7e..69d4678fa 100644 --- a/spec/features/scratch/creating_and_showing_a_scratch_asset_spec.rb +++ b/spec/features/scratch/creating_and_showing_a_scratch_asset_spec.rb @@ -13,10 +13,6 @@ let(:auth_headers) { { 'Authorization' => UserProfileMock::TOKEN } } let(:project_headers) { auth_headers.merge('X-Project-ID' => project.identifier) } - before do - Flipper.enable_actor :cat_mode, school - end - describe 'GET #show' do it 'responds 400 Bad Request when X-Project-ID is not provided' do get '/api/scratch/assets/internalapi/asset/test_image_1.png/get/', headers: auth_headers @@ -384,18 +380,10 @@ end end - context 'when user is logged in and cat_mode is disabled' do - before do - authenticated_in_hydra_as(teacher) - Flipper.disable :cat_mode - Flipper.disable_actor :cat_mode, school - end - - it 'responds 404 Not Found when cat_mode is not enabled' do - post '/api/scratch/assets/example.svg', headers: request_headers + it 'responds 401 unauthorized when user is not part of a school' do + post '/api/scratch/assets/example.svg', headers: { 'X-Project-ID' => project.identifier } - expect(response).to have_http_status(:not_found) - end + expect(response).to have_http_status(:unauthorized) end end diff --git a/spec/features/scratch/showing_a_scratch_project_spec.rb b/spec/features/scratch/showing_a_scratch_project_spec.rb index 2ab60579a..c3e99ef94 100644 --- a/spec/features/scratch/showing_a_scratch_project_spec.rb +++ b/spec/features/scratch/showing_a_scratch_project_spec.rb @@ -3,7 +3,12 @@ require 'rails_helper' RSpec.describe 'Showing a Scratch project', type: :request do + let(:school) { create(:school) } + let(:teacher) { create(:teacher, school:) } + let(:headers) { { 'Authorization' => UserProfileMock::TOKEN } } + it 'returns scratch project JSON' do + authenticated_in_hydra_as(teacher) project = create( :project, project_type: Project::Types::CODE_EDITOR_SCRATCH, @@ -11,7 +16,7 @@ ) create(:scratch_component, project: project) - get "/api/scratch/projects/#{project.identifier}" + get "/api/scratch/projects/#{project.identifier}", headers: headers expect(response).to have_http_status(:ok) @@ -20,6 +25,7 @@ end it 'returns the stage target first when stored targets are out of order' do + authenticated_in_hydra_as(teacher) project = create( :project, project_type: Project::Types::CODE_EDITOR_SCRATCH, @@ -37,23 +43,32 @@ } ) - get "/api/scratch/projects/#{project.identifier}" + get "/api/scratch/projects/#{project.identifier}", headers: headers expect(response).to have_http_status(:ok) expect(response.parsed_body.fetch('targets').pluck('name')).to eq(%w[Stage Sprite1 Sprite2]) end it 'returns a 404 if project does not exist' do - get '/api/scratch/projects/non_existent_project' + authenticated_in_hydra_as(teacher) + get '/api/scratch/projects/non_existent_project', headers: headers expect(response).to have_http_status(:not_found) end it 'returns a 404 if project is not a scratch project' do + authenticated_in_hydra_as(teacher) project = create(:project, project_type: Project::Types::PYTHON, locale: 'en') - get "/api/scratch/projects/#{project.identifier}" + get "/api/scratch/projects/#{project.identifier}", headers: headers expect(response).to have_http_status(:not_found) end + + it 'returns a 401 unauthorized if not logged in' do + project = create(:project, project_type: Project::Types::PYTHON, locale: 'en') + get "/api/scratch/projects/#{project.identifier}" + + expect(response).to have_http_status(:unauthorized) + end end diff --git a/spec/features/scratch/updating_a_scratch_project_spec.rb b/spec/features/scratch/updating_a_scratch_project_spec.rb index 5c32a885e..eb56d9800 100644 --- a/spec/features/scratch/updating_a_scratch_project_spec.rb +++ b/spec/features/scratch/updating_a_scratch_project_spec.rb @@ -7,28 +7,14 @@ let(:teacher) { create(:teacher, school:) } let(:auth_headers) { { 'Authorization' => UserProfileMock::TOKEN } } - before do - Flipper.disable :cat_mode - Flipper.disable_actor :cat_mode, school - end - it 'responds 401 Unauthorized when no Authorization header is provided' do put '/api/scratch/projects/any-identifier', params: { project: { targets: [] } } expect(response).to have_http_status(:unauthorized) end - it 'responds 404 Not Found when cat_mode is not enabled' do - authenticated_in_hydra_as(teacher) - - put '/api/scratch/projects/any-identifier', params: { content: { targets: [] } }, headers: auth_headers - - expect(response).to have_http_status(:not_found) - end - - it 'updates a project when cat_mode is enabled and an Authorization header is provided' do + it 'updates a project when an Authorization header is provided' do authenticated_in_hydra_as(teacher) - Flipper.enable_actor :cat_mode, school project = create( :project, project_type: Project::Types::CODE_EDITOR_SCRATCH,