Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
3 changes: 2 additions & 1 deletion app/controllers/api/lessons_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ def base_params
:name,
:project_type,
:locale,
{ components: %i[id name extension content index default] }
{ components: %i[id name extension content index default] },
{ scratch_component: {} }
Comment thread
mwtrew marked this conversation as resolved.
]
}
)
Expand Down
1 change: 1 addition & 0 deletions app/controllers/api/projects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ def base_params
{
components: %i[id name extension content index default]
},
scratch_component: {},
parent: {},
Comment thread
mwtrew marked this conversation as resolved.
image_list: []
)
Expand Down
5 changes: 4 additions & 1 deletion app/controllers/api/scratch/projects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ module Scratch
class ProjectsController < ScratchController
skip_before_action :authorize_user, only: [:show]
skip_before_action :check_scratch_feature, only: [:show]
before_action :load_project, only: %i[show update]

def show
render :show, formats: [:json]
render json: @project.scratch_component&.content
end
Comment thread
mwtrew marked this conversation as resolved.
Comment thread
mwtrew marked this conversation as resolved.

def update
@project.scratch_component&.content = params
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

@project.scratch_component&.content = params will persist framework/routing keys like controller, action, id, and potentially other untrusted request keys into content, which will then be echoed back by show. Extract and persist only the Scratch project payload (e.g., params.require(:content) or a permitted subset like targets/monitors/extensions/meta) and convert to a plain Hash before assignment.

Suggested change
@project.scratch_component&.content = params
scratch_content = params.require(:content)
@project.scratch_component&.content = scratch_content.to_unsafe_h

Copilot uses AI. Check for mistakes.
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.

Just to note that I can't see any authorization checks on the project to make sure the user is allowed to write to it.

This doesn't have to be done as part of this PR, but we should make sure there's an issue for it if not.

We can't add auth to the show action yet (but I am coming up with a proposal for that)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, expecting this to be handled in a separate ticket.

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.

Could you please make sure it's covered by an issue (by adding it to one or creating a new one) - I don't want it to be lost.

@project.save
Comment thread
mwtrew marked this conversation as resolved.
Outdated
render json: { status: 'ok' }, status: :ok
end
end
Expand Down
5 changes: 5 additions & 0 deletions app/controllers/api/scratch/scratch_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ def check_scratch_feature

raise ActiveRecord::RecordNotFound, 'Not Found'
end

def load_project
project_loader = ProjectLoader.new(params[:id], [params[:locale]])
@project = project_loader.load
Comment thread
mwtrew marked this conversation as resolved.
end
end
end
end
3 changes: 2 additions & 1 deletion app/graphql/mutations/create_project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ class CreateProject < BaseMutation
def resolve(**input)
project_hash = input.merge(
user_id: context[:current_user]&.id,
components: input[:components]&.map(&:to_h)
components: input[:components]&.map(&:to_h),
scratch_component: input[:scratch_component]&.to_h
)

response = Project::Create.call(project_hash:, current_user: context[:current_user])
Expand Down
6 changes: 6 additions & 0 deletions app/models/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ module Types
belongs_to :parent, optional: true, class_name: :Project, foreign_key: :remixed_from_id, inverse_of: :remixes
has_many :remixes, dependent: :nullify, class_name: :Project, foreign_key: :remixed_from_id, inverse_of: :parent
has_many :components, -> { order(default: :desc, name: :asc) }, dependent: :destroy, inverse_of: :project
has_one :scratch_component, dependent: :destroy, inverse_of: :project, required: false
has_many :project_errors, dependent: :nullify
has_many_attached :images
has_many_attached :videos
has_many_attached :audio
has_one :school_project, dependent: :destroy

accepts_nested_attributes_for :components
accepts_nested_attributes_for :scratch_component

before_validation :check_unique_not_null, on: :create
before_validation :create_school_project_if_needed
Expand Down Expand Up @@ -67,6 +69,10 @@ def components=(array)
super(array.map { |o| o.is_a?(Hash) ? Component.new(o) : o })
end

def scratch_component=(value)
super(value.is_a?(Hash) ? ScratchComponent.new(value) : value)
end

def last_edited_at
# datetime that the project or one of its components was last updated
[updated_at, components.maximum(:updated_at)].compact.max
Comment thread
mwtrew marked this conversation as resolved.
Expand Down
5 changes: 5 additions & 0 deletions app/models/scratch_component.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# frozen_string_literal: true

class ScratchComponent < ApplicationRecord
belongs_to :project
end
10 changes: 10 additions & 0 deletions db/migrate/20260309104851_create_scratch_components.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class CreateScratchComponents < ActiveRecord::Migration[7.2]
def change
create_table :scratch_components, id: :uuid do |t|
t.jsonb :content
Comment thread
mwtrew marked this conversation as resolved.
t.references :project, null: false, foreign_key: true, type: :uuid, index: { unique: true }

t.timestamps
end
end
end
11 changes: 10 additions & 1 deletion db/schema.rb

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

3 changes: 2 additions & 1 deletion lib/concepts/project/operations/create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ def call(project_hash:, current_user:)

def build_project(project_hash, current_user)
project_hash[:identifier] = PhraseIdentifier.generate unless current_user&.experience_cs_admin?
new_project = Project.new(project_hash.except(:components))
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])
Comment thread
mwtrew marked this conversation as resolved.
Outdated
new_project
end
end
Expand Down
8 changes: 8 additions & 0 deletions spec/factories/scratch_components.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# frozen_string_literal: true

FactoryBot.define do
factory :scratch_component do
content { { targets: [], monitors: [], extensions: [], meta: {} } }
project
end
end
9 changes: 8 additions & 1 deletion spec/features/scratch/showing_a_scratch_project_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,14 @@

RSpec.describe 'Showing a Scratch project', type: :request do
it 'returns scratch project JSON' do
get '/api/scratch/projects/any-identifier'
project = create(
:project,
project_type: Project::Types::CODE_EDITOR_SCRATCH,
locale: 'en'
)
create(:scratch_component, project: project)

get "/api/scratch/projects/#{project.identifier}"

expect(response).to have_http_status(:ok)

Expand Down
15 changes: 13 additions & 2 deletions spec/features/scratch/updating_a_scratch_project_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,31 @@
it 'responds 404 Not Found when cat_mode is not enabled' do
authenticated_in_hydra_as(teacher)

put '/api/scratch/projects/any-identifier', params: { project: { targets: [] } }, headers: cookie_headers
put '/api/scratch/projects/any-identifier', params: { content: { targets: [] } }, headers: cookie_headers

expect(response).to have_http_status(:not_found)
end

it 'updates a project when cat_mode is enabled and a cookie is provided' do
# Arrange
authenticated_in_hydra_as(teacher)
Flipper.enable_actor :cat_mode, school
project = create(
:project,
project_type: Project::Types::CODE_EDITOR_SCRATCH,
locale: 'en'
Comment thread
mwtrew marked this conversation as resolved.
)
create(:scratch_component, project: project)

put '/api/scratch/projects/any-identifier', params: { project: { targets: [] } }, headers: cookie_headers
# Act
put "/api/scratch/projects/#{project.identifier}", params: { targets: ['some update'] }, headers: cookie_headers

# Assert
expect(response).to have_http_status(:ok)

data = JSON.parse(response.body, symbolize_names: true)
expect(data[:status]).to eq('ok')

expect(project.reload.scratch_component.content.to_h['targets']).to eq(['some update'])
end
end
Loading