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/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/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/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..b8f52e00e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -6,16 +6,15 @@ class Project < ApplicationRecord module Types PYTHON = 'python' HTML = 'html' - SCRATCH = 'scratch' CODE_EDITOR_SCRATCH = 'code_editor_scratch' + + ALL = [PYTHON, HTML, CODE_EDITOR_SCRATCH].freeze end 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 @@ -45,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 @@ -108,11 +108,7 @@ def scratch_project? end 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 + 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/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/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 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/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/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/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..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 @@ -92,7 +97,7 @@ :project, instructions: '

Project instructions

', locale: 'en', - project_type: Project::Types::SCRATCH, + project_type: Project::Types::PYTHON, user_id: nil ) @@ -250,18 +255,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 +270,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 ) @@ -284,43 +285,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 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