Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions app/models/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ module Types
CODE_EDITOR_SCRATCH = 'code_editor_scratch'
end

module Origins
EXPERIENCE_CS = 'experience_cs'
end

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
Expand Down Expand Up @@ -35,6 +39,8 @@ module Types
validate :project_with_instructions_must_belong_to_school
validate :project_with_school_id_has_school_project
validate :school_project_school_matches_project_school
validates :origin, inclusion: { in: [Origins::EXPERIENCE_CS], allow_nil: true }
validate :origin_cannot_change, on: :update

scope :internal_projects, -> { where(user_id: nil) }

Expand Down Expand Up @@ -182,4 +188,12 @@ def school_project_school_matches_project_school

errors.add(:school_project, 'School project school_id must match project school_id')
end

def origin_cannot_change
return unless origin_changed?
# allow filling in origin when it's nil
return if origin_was.nil?

errors.add(:origin, 'cannot be changed once set')
end
end
8 changes: 8 additions & 0 deletions db/migrate/20260820122510_add_origin_to_projects.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# frozen_string_literal: true

class AddOriginToProjects < ActiveRecord::Migration[8.1]
def change
add_column :projects, :origin, :string
add_index :projects, :origin, where: 'origin IS NOT NULL'
end
end
4 changes: 3 additions & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion lib/concepts/project/operations/create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ def call(project_hash:, current_user:)
private

def build_project(project_hash, current_user)
project_hash[:identifier] = PhraseIdentifier.generate unless current_user&.experience_cs_admin?
if current_user&.experience_cs_admin?
project_hash[:origin] = Project::Origins::EXPERIENCE_CS
else
project_hash[:identifier] = PhraseIdentifier.generate
end

Comment on lines -20 to +25

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any the tradeoffs for setting the project origin here compared to getting the caller (experience CS) to set it and passing in the param?

new_project = Project.new(project_hash.except(:components, :scratch_component))
new_project.components.build(project_hash[:components])
new_project.build_scratch_component(project_hash[:scratch_component]) if project_hash[:scratch_component].present?
Expand Down
14 changes: 14 additions & 0 deletions spec/concepts/project/create_remix_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,20 @@
expect(remixed_project.remix_origin).to eq(remix_origin)
end

it 'does not set an origin when the original project has none' do
remixed_project = create_remix[:project]
expect(remixed_project.origin).to be_nil
end

context 'when the original project has an origin' do
let!(:original_project) { create(:project, :with_components, origin: Project::Origins::EXPERIENCE_CS) }

it 'copies the origin to the remix' do
remixed_project = create_remix[:project]
expect(remixed_project.origin).to eq(Project::Origins::EXPERIENCE_CS)
end
end

it 'links remix to attached images' do
remixed_project = create_remix[:project]
expect(remixed_project.images.length).to eq(original_project.images.length)
Expand Down
30 changes: 30 additions & 0 deletions spec/concepts/project/create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,36 @@
new_project = create_project_with_content[:project]
expect(new_project.components.first.content).to eq('print("hello world")')
end

it 'does not set the project origin' do
expect(create_project_with_content[:project].origin).to be_nil
end
end

context 'when the current user is an Experience CS admin' do
subject(:create_project_as_admin) { described_class.call(project_hash:, current_user:) }

let(:current_user) { create(:experience_cs_admin_user) }
let(:project_hash) do
{
project_type: Project::Types::PYTHON,
components: [{
name: 'main',
extension: 'py',
content: 'print("hello world")',
default: true
}],
user_id:
}
end

it 'returns success' do
expect(create_project_as_admin.success?).to be(true)
end

it 'sets the project origin to experience_cs' do
expect(create_project_as_admin[:project].origin).to eq(Project::Origins::EXPERIENCE_CS)
end
end

context 'when creation fails' do
Expand Down
6 changes: 6 additions & 0 deletions spec/features/project/creating_a_project_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -308,5 +308,11 @@
project = Project.find_by!(identifier: 'test-project', locale: 'fr')
expect(project.scratch_component.content.to_h).to eq(scratch_data.deep_stringify_keys)
end

it 'sets the project origin to experience_cs' do
post('/api/projects', headers:, params:, as: :json)

expect(Project.find_by!(identifier: 'test-project', locale: 'fr').origin).to eq(Project::Origins::EXPERIENCE_CS)
end
end
end
33 changes: 33 additions & 0 deletions spec/models/project_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,21 @@
expect(valid_project).to be_valid
end

it 'is valid without an origin' do
valid_project = build(:project, origin: nil)
expect(valid_project).to be_valid
end

it 'is valid with a known origin' do
valid_project = build(:project, origin: Project::Origins::EXPERIENCE_CS)
expect(valid_project).to be_valid
end

it 'is invalid with an unrecognised origin' do
invalid_project = build(:project, origin: 'invalid_origin')
expect(invalid_project).not_to be_valid
end

it 'allows a public Code Classroom Blocks project to have instructions' do
project = build(
:project,
Expand Down Expand Up @@ -197,6 +212,24 @@
end
end

describe 'origin_cannot_change' do
it 'allows an origin to be set on create' do
expect { create(:project, origin: Project::Origins::EXPERIENCE_CS) }.not_to raise_error
end

it 'allows an origin to be set on a project that does not have one' do
project = create(:project, origin: nil)
expect { project.update!(origin: Project::Origins::EXPERIENCE_CS) }.not_to raise_error
end

it 'does not allow an origin to be updated once set' do
project = create(:project, origin: Project::Origins::EXPERIENCE_CS)

expect(project.update(origin: nil)).to be(false)
expect(project.errors[:origin]).to include(/cannot be changed once set/)
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) }
Expand Down
Loading