From 807a9f0b4de78253de158f406793472c2156616d Mon Sep 17 00:00:00 2001 From: Chris Zetter <253059100+zetter-rpf@users.noreply.github.com> Date: Fri, 9 Oct 2026 15:48:48 +0100 Subject: [PATCH 1/4] Stop seeding Experience CS example projects on release Previously every release ran projects:create_experience_cs_examples, re-creating a hardcoded list of Scratch example projects. When new projects were added to Experience CS, they had to be put in this list too. We no longer need this list as now projects are synced from Experience CS directly when they are created. Co-Authored-By: Claude Opus 5 (1M context) --- Procfile | 2 +- db/seeds.rb | 1 - lib/tasks/projects.rake | 335 ---------------------------------------- 3 files changed, 1 insertion(+), 337 deletions(-) diff --git a/Procfile b/Procfile index 3813fe26d..73f55236b 100644 --- a/Procfile +++ b/Procfile @@ -1,3 +1,3 @@ web: bundle exec puma -C config/puma.rb -release: bundle exec rails db:migrate && bundle exec rake projects:create_experience_cs_examples integration_tests:dispatch +release: bundle exec rails db:migrate && bundle exec rake integration_tests:dispatch worker: bundle exec good_job start --max-threads=8 diff --git a/db/seeds.rb b/db/seeds.rb index 0c0500e18..97eb33852 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -5,5 +5,4 @@ if Rails.env.development? Rake::Task['projects:create_all'].invoke Rake::Task['for_education:seed_a_school_with_lessons_and_students'].invoke - Rake::Task['projects:create_experience_cs_examples'].invoke end diff --git a/lib/tasks/projects.rake b/lib/tasks/projects.rake index 076a2eca9..93082f7a6 100644 --- a/lib/tasks/projects.rake +++ b/lib/tasks/projects.rake @@ -5,339 +5,4 @@ namespace :projects do task create_all: :environment do FilesystemProject.import_all! end - - desc "Create example Scratch projects for Experience CS (if they don't already exist)" - task create_experience_cs_examples: :environment do - projects = [ - { - identifier: 'a-familar-tune', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'A Familar Tune', - user_id: nil - }, - { - identifier: 'blank-scratch-starter', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Blank Scratch Starter', - user_id: nil - }, - { - identifier: 'broadcasting-chords', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Broadcasting Chords', - user_id: nil - }, - { - identifier: 'chord-detectives', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Chord Detectives', - user_id: nil - }, - { - identifier: 'comparing-programs', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Comparing Programs', - user_id: nil - }, - { - identifier: 'counting-with-variables', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Counting With Variables', - user_id: nil - }, - { - identifier: 'creating-a-program', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Creating A Program', - user_id: nil - }, - { - identifier: 'creating-clones', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Creating Clones', - user_id: nil - }, - { - identifier: 'creating-programs', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Creating Programs', - user_id: nil - }, - { - identifier: 'debug-it', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Debug It', - user_id: nil - }, - { - identifier: 'debugging-in-scratch', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Debugging In Scratch', - user_id: nil - }, - { - identifier: 'dialogue-in-scratch', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Dialogue In Scratch', - user_id: nil - }, - { - identifier: 'digit-dash', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Digit Dash', - user_id: nil - }, - { - identifier: 'experience-cs-example', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Experience Cs Example', - user_id: nil - }, - { - identifier: 'getting-started-1', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Getting Started 1', - user_id: nil - }, - { - identifier: 'investigating-broadcasting', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Investigating Broadcasting', - user_id: nil - }, - { - identifier: 'lets-explore-scratch', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Lets Explore Scratch', - user_id: nil - }, - { - identifier: 'lets-loop-it', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Lets Loop It', - user_id: nil - }, - { - identifier: 'ma-testing', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Ma Testing', - user_id: nil - }, - { - identifier: 'modifying-picture-graphs', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Modifying Picture Graphs', - user_id: nil - }, - { - identifier: 'modifying-programs', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Modifying Programs', - user_id: nil - }, - { - identifier: 'move-with-purpose', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Move With Purpose', - user_id: nil - }, - { - identifier: 'my-anti-app', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'My Anti App', - user_id: nil - }, - { - identifier: 'my-digital-canvas', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'My Digital Canvas', - user_id: nil - }, - { - identifier: 'my-first-function', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'My First Function', - user_id: nil - }, - { - identifier: 'my-simulation', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'My Simulation', - user_id: nil - }, - { - identifier: 'mystery-story', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Mystery Story', - user_id: nil - }, - { - identifier: 'paper-airplane-simulation', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Paper Airplane Simulation', - user_id: nil - }, - { - identifier: 'pedestrian-button', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Pedestrian Button', - user_id: nil - }, - { - identifier: 'pollination-patrol', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Pollination Patrol', - user_id: nil - }, - { - identifier: 'programming-functions', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Programming Functions', - user_id: nil - }, - { - identifier: 'programming-progressions', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Programming Progressions', - user_id: nil - }, - { - identifier: 'sensing-motion', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Sensing Motion', - user_id: nil - }, - { - identifier: 'sequence-a-melody', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Sequence A Melody', - user_id: nil - }, - { - identifier: 'sequencing-programs', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Sequencing Programs', - user_id: nil - }, - { - identifier: 'taking-a-tour', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Taking A Tour', - user_id: nil - }, - { - identifier: 'ten-block-mission', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Ten Block Mission', - user_id: nil - }, - { - identifier: 'the-me-project', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'The Me Project', - user_id: nil - }, - { - identifier: 'the-vanishing-garden', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'The Vanishing Garden', - user_id: nil - }, - { - identifier: 'time-travelers', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Time Travelers', - user_id: nil - }, - { - identifier: 'traffic-light-timer', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Traffic Light Timer', - user_id: nil - }, - { - identifier: 'transforming-sprites', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Transforming Sprites', - user_id: nil - }, - { - identifier: 'weather-data', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Weather Data', - user_id: nil - }, - { - identifier: 'word-art', - locale: 'en', - project_type: Project::Types::SCRATCH, - name: 'Word Art', - user_id: nil - } - ] - projects.each do |attributes| - identifier = attributes[:identifier] - project = Project.find_by(attributes.slice(:identifier, :locale, :project_type)) - if project.present? - puts "Scratch project with identifier '#{identifier}' already exists" - project.assign_attributes(attributes.except(:identifier, :locale)) - if project.changed? - if project.save - puts "Scratch project with identifier '#{identifier}' updated successfully" - else - puts "Scratch project with identifier '#{identifier}' update failed" - end - else - puts "Scratch project with identifier '#{identifier}' has not changed" - end - elsif Project.create(attributes) - puts "Scratch project with identifier '#{identifier}' created successfully" - else - puts "Scratch project with identifier '#{identifier}' creation failed" - end - end - end end From 63c6ba4e4526bced70d069146f09d5621bca529e Mon Sep 17 00:00:00 2001 From: Chris Zetter <253059100+zetter-rpf@users.noreply.github.com> Date: Fri, 9 Oct 2026 16:13:16 +0100 Subject: [PATCH 2/4] Remove the Experience CS project migration endpoints These were temporary endpoints used for the one-off project migration. The service account is still used for public project create/update and global Scratch asset upload. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 6 - ...rience_cs_project_migrations_controller.rb | 66 --------- .../api/scratch/assets_controller.rb | 26 +--- app/models/ability.rb | 7 - app/models/project.rb | 4 - config/routes.rb | 3 - ...eating_and_showing_a_scratch_asset_spec.rb | 66 --------- spec/models/ability_spec.rb | 19 +-- spec/models/project_spec.rb | 37 ----- .../update_spec.rb | 127 ------------------ 10 files changed, 7 insertions(+), 354 deletions(-) delete mode 100644 app/controllers/api/experience_cs_project_migrations_controller.rb delete mode 100644 spec/requests/experience_cs_project_migrations/update_spec.rb diff --git a/README.md b/README.md index dd3df052d..8b4ded637 100644 --- a/README.md +++ b/README.md @@ -166,12 +166,6 @@ assets asynchronously. Configure `EXPERIENCE_CS_API_KEY` to the same secret as Experience CS's `EDITOR_API_SYNC_API_KEY`. The corresponding request header is accepted for public project create/update and global Scratch asset upload. -`PUT /api/experience-cs/projects/:identifier/migrate` lets the service replace -an exact locale-less Experience CS user-project stub with its Markdown -instructions and Scratch content. Project-scoped migration assets use -`POST /api/experience-cs/projects/:identifier/assets/:filename` and remain -subject to the project's normal viewing permissions. - ### Code Editor for Education Editor API provides routes for managing resources such as schools, school classes and lessons, as well as for inviting teachers and managing student accounts via `profile` requests. diff --git a/app/controllers/api/experience_cs_project_migrations_controller.rb b/app/controllers/api/experience_cs_project_migrations_controller.rb deleted file mode 100644 index b3b21f42a..000000000 --- a/app/controllers/api/experience_cs_project_migrations_controller.rb +++ /dev/null @@ -1,66 +0,0 @@ -# frozen_string_literal: true - -module Api - class ExperienceCsProjectMigrationsController < ApiController - prepend_before_action :load_experience_cs_service_user - before_action :authorize_user - before_action :load_project - - def update - migrate_project! - render json: { - identifier: @project.identifier, - locale: @project.locale, - project_type: @project.project_type - } - rescue ActiveRecord::RecordInvalid, ActionController::ParameterMissing => e - render json: { error: e.message }, status: :unprocessable_content - end - - private - - def load_project - @project = Project.find_by!(identifier: params.expect(:id), locale: nil) - end - - def migrate_project! - attributes = migration_params - FeatureFlags.without_salesforce_sync do - @project.with_lock do - authorize! :migrate_from_experience_cs, @project - @project.update!( - attributes.slice(:name, :instructions).merge( - project_type: Project::Types::CODE_EDITOR_SCRATCH, - origin: Project::Origins::EXPERIENCE_CS - ) - ) - scratch_component = @project.scratch_component || @project.build_scratch_component - scratch_component.update!(attributes.require(:scratch_component).slice(:content)) - convert_finished_flag_to_complete! - end - end - end - - def convert_finished_flag_to_complete! - school_project = @project.school_project - return unless school_project&.finished? - - school_project.transaction do - school_project.update!(finished: false) - if school_project.can_transition_to?(:complete) - school_project.transition_status_to!(:complete, nil, info: 'backfilled_from_finished') - else - Rails.logger.warn("School project #{school_project.id} cannot transition to complete, in state #{school_project.status}") - end - end - end - - def migration_params - params.fetch(:project, {}).permit( - :name, - { instructions: [:markdown_content] }, - { scratch_component: { content: {} } } - ) - end - end -end diff --git a/app/controllers/api/scratch/assets_controller.rb b/app/controllers/api/scratch/assets_controller.rb index 76504a81e..249853cde 100644 --- a/app/controllers/api/scratch/assets_controller.rb +++ b/app/controllers/api/scratch/assets_controller.rb @@ -7,12 +7,10 @@ module Scratch class AssetsController < ApiController include ActiveStorage::SetCurrent - prepend_before_action :load_experience_cs_service_user, only: %i[create_global create_migration] + prepend_before_action :load_experience_cs_service_user, only: %i[create_global] before_action :authorize_user, except: %i[show] prepend_before_action :load_project_from_header, only: %i[show create] - authorize_resource :project_from_header, except: %i[create_global create_migration] - before_action :load_migration_project, only: :create_migration - before_action :authorize_migration_asset, only: :create_migration + authorize_resource :project_from_header, except: %i[create_global] def show filename_with_extension = "#{params[:id]}.#{params[:format]}" @@ -36,15 +34,6 @@ def create ) end - def create_migration - create_asset( - project: @migration_project, - uploaded_user_id: @migration_project.user_id, - filename: "#{params[:id]}.#{params[:format]}", - reject_conflicting_content: true - ) - end - def create_global authorize! :create_global, ScratchAsset @@ -131,17 +120,6 @@ def load_project_from_header project_type: Project::Types::CODE_EDITOR_SCRATCH ) end - - def load_migration_project - @migration_project = Project.find_by!( - identifier: params.expect(:project_id), - locale: nil - ) - end - - def authorize_migration_asset - authorize! :upload_migration_asset, @migration_project - end end end end diff --git a/app/models/ability.rb b/app/models/ability.rb index f8546627f..fb89ebe16 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -18,7 +18,6 @@ def initialize(user) define_editor_admin_abilities(user) define_experience_cs_admin_abilities(user) - define_experience_cs_service_abilities(user) end private @@ -161,12 +160,6 @@ def define_experience_cs_admin_abilities(user) define_school_import_abilities(user) end - def define_experience_cs_service_abilities(user) - return unless user&.experience_cs_service_account? - - can %i[migrate_from_experience_cs upload_migration_asset], Project, &:experience_cs_migration_target? - end - def school_teacher_can_manage_lesson?(user:, school:, lesson:) is_my_lesson = lesson.school_id == school.id && lesson.user_id == user.id is_my_class = lesson.school_class&.teacher_ids&.include?(user.id) diff --git a/app/models/project.rb b/app/models/project.rb index c5be1907a..09d60c29e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -111,10 +111,6 @@ def public_experience_cs_project? user_id.nil? && school_id.nil? && EXPERIENCE_CS_PROJECT_TYPES.include?(project_type) end - def experience_cs_migration_target? - user_id.present? && school_id.present? && project_type == Types::SCRATCH - end - def self_and_ancestors projects = [] current_project = self diff --git a/config/routes.rb b/config/routes.rb index 6b6577149..2548a9bd1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -37,9 +37,6 @@ mount GraphiQL::Rails::Engine, at: '/graphql', graphql_path: '/graphql#execute' unless Rails.env.production? namespace :api do - put '/experience-cs/projects/:id/migrate', to: 'experience_cs_project_migrations#update' - post '/experience-cs/projects/:project_id/assets/:id.:format', to: 'scratch/assets#create_migration' - namespace :scratch do resources :projects, only: %i[show update create] get '/assets/internalapi/asset/:id.:format/get/' => 'assets#show' 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 e0035a75e..facc57d55 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 @@ -475,72 +475,6 @@ def make_request end end - context 'when the Experience CS service uploads a migration asset' do - let(:request_headers) do - { - 'Content-Type' => 'application/octet-stream', - ExperienceCsServiceAuthenticator::HEADER => 'service-api-key' - } - end - let(:request_path) { "/api/experience-cs/projects/#{project.identifier}/assets/test_image_1.png" } - let(:project) do - create( - :project, - school:, - user_id: teacher.id, - locale: nil, - project_type: Project::Types::SCRATCH - ) - end - - before do - allow(Rails.configuration.x.experience_cs).to receive(:service_api_key).and_return('service-api-key') - end - - it 'stores the asset against the existing stub as an upload by its owner' do - expect { make_request }.to change(ScratchAsset, :count).by(1) - - asset = ScratchAsset.find_by!(filename:, project:) - expect(asset.uploaded_user_id).to eq(teacher.id) - expect(asset.file.download).to eq(upload) - expect(response).to have_http_status(:created) - end - - it 'accepts repeated uploads with identical bytes' do - make_request - - expect { make_request }.not_to change(ScratchAsset, :count) - - expect(response).to have_http_status(:created) - expect(ScratchAsset.find_by!(filename:, project:).file.download).to eq(upload) - end - - it 'rejects repeated uploads with conflicting bytes' do - existing_asset = create_uploaded_scratch_asset( - filename:, - project:, - uploaded_user_id: teacher.id, - body: 'existing bytes' - ) - - expect { make_request }.not_to change(ScratchAsset, :count) - - expect(response).to have_http_status(:conflict) - expect(response.parsed_body).to eq( - 'error' => 'Asset content conflicts with the existing project asset' - ) - expect(existing_asset.reload.file.download).to eq('existing bytes') - end - - it 'rejects uploads after the stub has been converted' do - project.update!(project_type: Project::Types::CODE_EDITOR_SCRATCH) - - expect { make_request }.not_to change(ScratchAsset, :count) - - expect(response).to have_http_status(:forbidden) - end - end - it 'responds 401 unauthorized when user is not signed in' do post '/api/scratch/assets/example.svg', headers: { 'X-Project-ID' => project.identifier } diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index a46cbf32c..4a258fb8c 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -40,28 +40,19 @@ roles: 'experience-cs-admin' ) end - let(:migratable_project) do + let(:school_project) do build( :project, school_id: SecureRandom.uuid, user_id: SecureRandom.uuid, locale: nil, - project_type: Project::Types::SCRATCH + project_type: Project::Types::CODE_EDITOR_SCRATCH ) end - it { is_expected.to be_able_to(:migrate_from_experience_cs, migratable_project) } - it { is_expected.to be_able_to(:upload_migration_asset, migratable_project) } - it { is_expected.not_to be_able_to(:update, migratable_project) } - it { is_expected.not_to be_able_to(:create, migratable_project) } - it { is_expected.not_to be_able_to(:show, migratable_project) } - - it 'cannot migrate a native Code Classroom Scratch project' do - migratable_project.project_type = Project::Types::CODE_EDITOR_SCRATCH - - expect(ability).not_to be_able_to(:migrate_from_experience_cs, migratable_project) - expect(ability).not_to be_able_to(:upload_migration_asset, migratable_project) - end + it { is_expected.not_to be_able_to(:update, school_project) } + it { is_expected.not_to be_able_to(:create, school_project) } + it { is_expected.not_to be_able_to(:show, school_project) } end describe 'Project' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c2885ac0e..c25166aa1 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -284,43 +284,6 @@ end end - describe '#experience_cs_migration_target?' do - let(:project) do - build( - :project, - school_id: SecureRandom.uuid, - user_id: SecureRandom.uuid, - locale: nil, - project_type: described_class::Types::SCRATCH - ) - end - - it 'allows a user-owned school Scratch stub' do - expect(project).to be_experience_cs_migration_target - end - - it 'rejects a Code Classroom Scratch project' do - project.project_type = described_class::Types::CODE_EDITOR_SCRATCH - - expect(project).not_to be_experience_cs_migration_target - end - - it 'rejects a non-Scratch project' do - project.project_type = described_class::Types::PYTHON - - expect(project).not_to be_experience_cs_migration_target - end - - it 'rejects public and non-school projects', :aggregate_failures do - project.user_id = nil - expect(project).not_to be_experience_cs_migration_target - - project.user_id = SecureRandom.uuid - project.school_id = nil - expect(project).not_to be_experience_cs_migration_target - end - end - describe 'create_school_project_if_needed' do let(:teacher) { create(:teacher, school:) } let(:teacher_project) { create(:project, school_id: school.id, user_id: teacher.id) } diff --git a/spec/requests/experience_cs_project_migrations/update_spec.rb b/spec/requests/experience_cs_project_migrations/update_spec.rb deleted file mode 100644 index 615f01f94..000000000 --- a/spec/requests/experience_cs_project_migrations/update_spec.rb +++ /dev/null @@ -1,127 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe 'Experience CS project migration requests' do - include ActiveJob::TestHelper - - let(:headers) { { ExperienceCsServiceAuthenticator::HEADER => 'service-api-key' } } - let(:school) { create(:school) } - let(:owner) { create(:teacher, school:) } - let(:project) do - create( - :project, - identifier: 'user-project-stub', - school:, - user_id: owner.id, - locale: nil, - project_type: Project::Types::SCRATCH, - lesson: create(:lesson, school: school, user_id: owner.id) - ) - end - let(:scratch_data) { { targets: [], monitors: [], extensions: [], meta: {} } } - let(:params) do - { - project: { - name: 'Migrated project', - instructions: [{ markdown_content: 'Make the sprite move.' }], - scratch_component: { content: scratch_data } - } - } - end - let(:path) { '/api/experience-cs/projects/user-project-stub/migrate' } - - before do - project - allow(Rails.configuration.x.experience_cs).to receive(:service_api_key).and_return('service-api-key') - end - - it 'replaces the exact stub in place' do - original_id = project.id - - put(path, params:, headers:, as: :json) - - expect(response).to have_http_status(:ok) - expect(project.reload).to have_attributes( - id: original_id, - identifier: 'user-project-stub', - locale: nil, - user_id: owner.id, - school_id: school.id, - name: 'Migrated project', - instructions: [{ 'markdown_content' => 'Make the sprite move.' }], - project_type: Project::Types::CODE_EDITOR_SCRATCH, - origin: Project::Origins::EXPERIENCE_CS - ) - expect(project.scratch_component.content.to_h).to eq(scratch_data.deep_stringify_keys) - end - - it 'converts a finished flag into a complete' do - project.school_project.update!(finished: true) - - put(path, params:, headers:, as: :json) - - expect(response).to have_http_status(:ok) - school_project = project.reload.school_project - expect(school_project).to have_attributes(finished: false, status: 'complete') - expect(school_project.school_project_transitions.order(:sort_key).last.metadata) - .to include('info' => 'backfilled_from_finished') - end - - it 'does not run salesforce sync' do - project.school_project.update!(finished: true) - - allow(Salesforce::LessonSyncJob).to receive(:perform_later) - - ClimateControl.modify(SALESFORCE_ENABLED: 'true') do - put(path, params:, headers:, as: :json) - end - - expect(Salesforce::LessonSyncJob).not_to have_received(:perform_later) - - expect(response).to have_http_status(:ok) - end - - it 'rejects a replay without overwriting Code Classroom changes' do - put(path, params:, headers:, as: :json) - code_classroom_data = scratch_data.merge(meta: { updated_in_code_classroom: true }) - project.reload.scratch_component.update!(content: code_classroom_data) - - put(path, params:, headers:, as: :json) - - expect(response).to have_http_status(:forbidden) - expect(project.reload.scratch_component.content.to_h).to eq(code_classroom_data.deep_stringify_keys) - end - - it 'does not authorize a human Experience CS admin' do - authenticated_in_hydra_as(create(:experience_cs_admin_user)) - - put(path, params:, headers: { Authorization: UserProfileMock::TOKEN }, as: :json) - - expect(response).to have_http_status(:forbidden) - end - - it 'does not overwrite a native Code Classroom project' do - project.update!(project_type: Project::Types::CODE_EDITOR_SCRATCH) - - put(path, params:, headers:, as: :json) - - expect(response).to have_http_status(:forbidden) - end - - it 'does not fall back to a public locale' do - project.destroy! - public_project = create( - :project, - identifier: 'user-project-stub', - locale: 'en', - user_id: nil, - project_type: Project::Types::SCRATCH - ) - - put(path, params:, headers:, as: :json) - - expect(response).to have_http_status(:not_found) - expect(public_project.reload.project_type).to eq(Project::Types::SCRATCH) - end -end From de32254c48abf351e656e51999b302527b773cab Mon Sep 17 00:00:00 2001 From: Chris Zetter <253059100+zetter-rpf@users.noreply.github.com> Date: Fri, 9 Oct 2026 16:15:04 +0100 Subject: [PATCH 3/4] Remove the old scratch project type This was here for Experience CS projects which have now been migrated to code_editor_scratch. Salesforce still expects 'scratch' as the teacherprojecttype__c picklist value for Experience CS lessons, so LessonSyncJob keeps sending that string via its own constant rather than deriving it from Project::Types. Co-Authored-By: Claude Opus 5 (1M context) --- app/jobs/salesforce/lesson_sync_job.rb | 4 +++- app/models/project.rb | 5 +---- app/models/scratch_asset.rb | 2 +- spec/features/project/creating_a_project_spec.rb | 10 ---------- spec/features/project/updating_a_project_spec.rb | 2 +- spec/jobs/salesforce/lesson_sync_job_spec.rb | 12 +++++------- spec/models/project_spec.rb | 16 ++++++---------- spec/requests/projects/destroy_spec.rb | 2 +- spec/requests/projects/update_spec.rb | 2 +- 9 files changed, 19 insertions(+), 36 deletions(-) diff --git a/app/jobs/salesforce/lesson_sync_job.rb b/app/jobs/salesforce/lesson_sync_job.rb index 88c6378df..f289f4d37 100644 --- a/app/jobs/salesforce/lesson_sync_job.rb +++ b/app/jobs/salesforce/lesson_sync_job.rb @@ -4,6 +4,8 @@ module Salesforce class LessonSyncJob < SalesforceSyncJob MODEL_CLASS = Salesforce::Lesson + EXPERIENCE_CS_SALESFORCE_PROJECT_TYPE = 'scratch' + FIELD_MAPPINGS = { lesson_uuid__c: :id, classroom__r__classroomuuid__c: :school_class_id, @@ -47,7 +49,7 @@ def mapped_attributes(lesson:) end def project_type_attribute(lesson) - return Project::Types::SCRATCH if lesson.project&.origin == Project::Origins::EXPERIENCE_CS + return EXPERIENCE_CS_SALESFORCE_PROJECT_TYPE if lesson.project&.origin == Project::Origins::EXPERIENCE_CS lesson.project&.project_type end diff --git a/app/models/project.rb b/app/models/project.rb index 09d60c29e..72e665578 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -6,7 +6,6 @@ class Project < ApplicationRecord module Types PYTHON = 'python' HTML = 'html' - SCRATCH = 'scratch' CODE_EDITOR_SCRATCH = 'code_editor_scratch' end @@ -14,8 +13,6 @@ module Origins EXPERIENCE_CS = 'experience_cs' end - EXPERIENCE_CS_PROJECT_TYPES = [Types::SCRATCH, Types::CODE_EDITOR_SCRATCH].freeze - belongs_to :school, optional: true belongs_to :lesson, optional: true belongs_to :parent, optional: true, class_name: :Project, foreign_key: :remixed_from_id, inverse_of: :remixes @@ -108,7 +105,7 @@ def scratch_project? end def public_experience_cs_project? - user_id.nil? && school_id.nil? && EXPERIENCE_CS_PROJECT_TYPES.include?(project_type) + user_id.nil? && school_id.nil? && scratch_project? end def self_and_ancestors diff --git a/app/models/scratch_asset.rb b/app/models/scratch_asset.rb index 6f48b6cfb..e1347089a 100644 --- a/app/models/scratch_asset.rb +++ b/app/models/scratch_asset.rb @@ -40,7 +40,7 @@ def response_content_type private def belongs_to_scratch_project - return if project.blank? || Project::EXPERIENCE_CS_PROJECT_TYPES.include?(project.project_type) + return if project.blank? || project.scratch_project? errors.add(:project, 'must be a Scratch project') end diff --git a/spec/features/project/creating_a_project_spec.rb b/spec/features/project/creating_a_project_spec.rb index 815f9bbd5..c09e19d52 100644 --- a/spec/features/project/creating_a_project_spec.rb +++ b/spec/features/project/creating_a_project_spec.rb @@ -329,16 +329,6 @@ expect(Project).to exist(identifier: 'test-project', locale: 'fr', user_id: nil) end - it 'creates a public legacy Scratch project' do - params[:project].except!(:instructions, :scratch_component) - params[:project][:project_type] = Project::Types::SCRATCH - - post('/api/projects', headers:, params:, as: :json) - - expect(response).to have_http_status(:created) - expect(Project).to exist(identifier: 'test-project', locale: 'fr', project_type: Project::Types::SCRATCH) - end - it 'does not authorize user-project creation' do params[:project][:user_id] = SecureRandom.uuid diff --git a/spec/features/project/updating_a_project_spec.rb b/spec/features/project/updating_a_project_spec.rb index b674c3236..087913bcf 100644 --- a/spec/features/project/updating_a_project_spec.rb +++ b/spec/features/project/updating_a_project_spec.rb @@ -59,7 +59,7 @@ context 'when an Experience CS admin creates a starter Scratch project' do let(:experience_cs_admin) { create(:experience_cs_admin_user) } let(:user_id) { nil } - let(:project_type) { Project::Types::SCRATCH } + let(:project_type) { Project::Types::CODE_EDITOR_SCRATCH } let(:params) { { project: { name: 'Test Project' } } } before do diff --git a/spec/jobs/salesforce/lesson_sync_job_spec.rb b/spec/jobs/salesforce/lesson_sync_job_spec.rb index 1ce96228f..6a96ff370 100644 --- a/spec/jobs/salesforce/lesson_sync_job_spec.rb +++ b/spec/jobs/salesforce/lesson_sync_job_spec.rb @@ -44,13 +44,11 @@ end context 'when the lesson project originates from Experience CS' do - Project::EXPERIENCE_CS_PROJECT_TYPES.each do |project_type| - it "syncs teacherprojecttype__c as scratch when the underlying project_type is #{project_type}" do - lesson.project.update!(origin: Project::Origins::EXPERIENCE_CS, project_type:) - perform_job - sf_lesson = Salesforce::Lesson.find_by(lesson_uuid__c: lesson.id) - expect(sf_lesson.teacherprojecttype__c).to eq(Project::Types::SCRATCH) - end + it 'syncs teacherprojecttype__c as scratch' do + lesson.project.update!(origin: Project::Origins::EXPERIENCE_CS, project_type: Project::Types::CODE_EDITOR_SCRATCH) + perform_job + sf_lesson = Salesforce::Lesson.find_by(lesson_uuid__c: lesson.id) + expect(sf_lesson.teacherprojecttype__c).to eq('scratch') end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c25166aa1..3b86214af 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -92,7 +92,7 @@ :project, instructions: '

Project instructions

', locale: 'en', - project_type: Project::Types::SCRATCH, + project_type: Project::Types::PYTHON, user_id: nil ) @@ -250,18 +250,14 @@ end describe '#public_experience_cs_project?' do - it 'returns true for public Experience CS project types', :aggregate_failures do - project_types = [described_class::Types::SCRATCH, described_class::Types::CODE_EDITOR_SCRATCH] + it 'returns true for a public Scratch project' do + project = build(:project, project_type: described_class::Types::CODE_EDITOR_SCRATCH, user_id: nil, school_id: nil) - project_types.each do |project_type| - project = build(:project, project_type:, user_id: nil, school_id: nil) - - expect(project).to be_public_experience_cs_project - end + expect(project).to be_public_experience_cs_project end it 'returns false for a user-owned project' do - project = build(:project, project_type: described_class::Types::SCRATCH) + project = build(:project, project_type: described_class::Types::CODE_EDITOR_SCRATCH) expect(project).not_to be_public_experience_cs_project end @@ -269,7 +265,7 @@ it 'returns false for a school-owned project' do project = build( :project, - project_type: described_class::Types::SCRATCH, + project_type: described_class::Types::CODE_EDITOR_SCRATCH, user_id: nil, school_id: SecureRandom.uuid ) diff --git a/spec/requests/projects/destroy_spec.rb b/spec/requests/projects/destroy_spec.rb index feef49dc3..e68dbf618 100644 --- a/spec/requests/projects/destroy_spec.rb +++ b/spec/requests/projects/destroy_spec.rb @@ -41,7 +41,7 @@ let(:project) do create( :project, { - project_type: Project::Types::SCRATCH, + project_type: Project::Types::CODE_EDITOR_SCRATCH, user_id: nil, locale: 'en' } diff --git a/spec/requests/projects/update_spec.rb b/spec/requests/projects/update_spec.rb index 249891a28..fdc06638a 100644 --- a/spec/requests/projects/update_spec.rb +++ b/spec/requests/projects/update_spec.rb @@ -141,7 +141,7 @@ :project, identifier: 'experience-cs-project', locale: 'fr', - project_type: Project::Types::SCRATCH, + project_type: Project::Types::CODE_EDITOR_SCRATCH, user_id: nil ) end From 81a47eee139aad027d6b4116714a7a76ca2173eb Mon Sep 17 00:00:00 2001 From: Chris Zetter <253059100+zetter-rpf@users.noreply.github.com> Date: Fri, 9 Oct 2026 16:21:17 +0100 Subject: [PATCH 4/4] Reject projects with an unknown project type Make sure that we only save expected project types. I'm doing this so that new scratch projects don't sneak in from experience CS api calls or the project importer. Co-Authored-By: Claude Opus 5 (1M context) --- app/models/project.rb | 3 +++ spec/models/project_spec.rb | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/app/models/project.rb b/app/models/project.rb index 72e665578..b8f52e00e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -7,6 +7,8 @@ module Types PYTHON = 'python' HTML = 'html' CODE_EDITOR_SCRATCH = 'code_editor_scratch' + + ALL = [PYTHON, HTML, CODE_EDITOR_SCRATCH].freeze end module Origins @@ -42,6 +44,7 @@ module Origins validate :project_with_instructions_must_belong_to_school validate :project_with_school_id_has_school_project validate :school_project_school_matches_project_school + validates :project_type, inclusion: { in: Types::ALL } validates :origin, inclusion: { in: [Origins::EXPERIENCE_CS], allow_nil: true } validate :origin_cannot_change, on: :update diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 3b86214af..976b73d4e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -70,6 +70,11 @@ expect(invalid_project).not_to be_valid end + it 'is invalid with an unrecognised project type' do + invalid_project = build(:project, project_type: 'scratch') + expect(invalid_project).not_to be_valid + end + it 'is valid without a source project' do valid_project = build(:project, source_project: nil) expect(valid_project).to be_valid