From 5618e2323055ab7b4abbe9431416701bd8e045f9 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Wed, 21 May 2025 16:44:29 +0100
Subject: [PATCH 01/31] Add Api::PublicProjectsController#create

We want to allow experience-cs admins to create "public" projects via
experience-cs administrate UI [1]. When I say "public" projects, I mean
those that are accessible to all users, because `Project#user_id` is set
to `nil`.

Public projects for Python & HTML are currently added via the
`GithubWebhooksController#github_push` [2] action and/or via the
(possibly defunct) `projects:create_all` rake task [3] both of which
make use of `ProjectImporter` [4]. However, these mechanisms are not
suitable for our use case.

This commit introduces a new `Api::PublicProjectsController` which is
somewhat based on `Api::ProjectsController`. However, it differs in a
couple of key respects: only experience-cs admin users should be
authorized to use it; it should not be possible to set
`Project#user_id`; and we want some sensible validation to restrict the
values entered into the administrate UI and provide sensible error
messages to the user. Conceivably this could be achieved by enhancing
the existing `Api::ProjectsController`, but this is already pretty
complicated and I wanted to minimize the chances of breaking existing
behaviour.

I've tried to roughly follow the existing conventions however much I
disagree with them, e.g. I've introduced a `PublicProject::Create` class
in `lib/concepts/public_project/operations/create.rb` along the same
lines as `Project::Create` in `lib/concepts/project/operations/create.rb`.

This is just a skeletal starting point - I plan to flesh it out in
subsequent commits.

[1]: https://github.com/RaspberryPiFoundation/experience-cs/issues/405
[2]: https://github.com/RaspberryPiFoundation/editor-api/blob/f348dfc92fdd8b19fbfd5434e7751340f33b4093/app/controllers/github_webhooks_controller.rb#L6-L8
[3]: https://github.com/RaspberryPiFoundation/editor-api/blob/f348dfc92fdd8b19fbfd5434e7751340f33b4093/lib/tasks/projects.rake#L4-L7
[4]: https://github.com/RaspberryPiFoundation/editor-api/blob/f348dfc92fdd8b19fbfd5434e7751340f33b4093/lib/project_importer.rb
---
 .../api/public_projects_controller.rb         | 24 +++++++++
 config/routes.rb                              |  2 +
 .../public_project/operations/create.rb       | 21 ++++++++
 spec/concepts/public_project/create_spec.rb   | 52 +++++++++++++++++++
 .../creating_a_public_project_spec.rb         | 51 ++++++++++++++++++
 spec/requests/public_projects/create_spec.rb  | 52 +++++++++++++++++++
 6 files changed, 202 insertions(+)
 create mode 100644 app/controllers/api/public_projects_controller.rb
 create mode 100644 lib/concepts/public_project/operations/create.rb
 create mode 100644 spec/concepts/public_project/create_spec.rb
 create mode 100644 spec/features/public_project/creating_a_public_project_spec.rb
 create mode 100644 spec/requests/public_projects/create_spec.rb

diff --git a/app/controllers/api/public_projects_controller.rb b/app/controllers/api/public_projects_controller.rb
new file mode 100644
index 000000000..20af124ed
--- /dev/null
+++ b/app/controllers/api/public_projects_controller.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module Api
+  class PublicProjectsController < ApiController
+    before_action :authorize_user
+
+    def create
+      result = PublicProject::Create.call(project_hash: project_params)
+
+      if result.success?
+        @project = result[:project]
+        render 'api/projects/show', formats: [:json], status: :created
+      else
+        render json: { error: result[:error] }, status: :unprocessable_entity
+      end
+    end
+
+    private
+
+    def project_params
+      params.fetch(:project, {}).permit(:identifier, :locale, :project_type, :name)
+    end
+  end
+end
diff --git a/config/routes.rb b/config/routes.rb
index f48edf7f1..7e47ef4a4 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -41,6 +41,8 @@
       resource :images, only: %i[show create], controller: 'projects/images'
     end
 
+    resources :public_projects, only: %i[create]
+
     resource :project_errors, only: %i[create]
 
     resource :school, only: [:show], controller: 'my_school'
diff --git a/lib/concepts/public_project/operations/create.rb b/lib/concepts/public_project/operations/create.rb
new file mode 100644
index 000000000..06a49734c
--- /dev/null
+++ b/lib/concepts/public_project/operations/create.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module PublicProject
+  class Create
+    class << self
+      def call(project_hash:)
+        response = OperationResponse.new
+
+        project = Project.new(project_hash)
+        project.save!
+
+        response[:project] = project
+        response
+      rescue StandardError => e
+        Sentry.capture_exception(e)
+        response[:error] = "Error creating project: #{e}"
+        response
+      end
+    end
+  end
+end
diff --git a/spec/concepts/public_project/create_spec.rb b/spec/concepts/public_project/create_spec.rb
new file mode 100644
index 000000000..1d9ccc9e9
--- /dev/null
+++ b/spec/concepts/public_project/create_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe PublicProject::Create, type: :unit do
+  describe '.call' do
+    subject(:create_project) { described_class.call(project_hash:) }
+
+    let(:identifier) { 'foo-bar-baz' }
+    let(:project_hash) do
+      {
+        identifier:,
+        locale: 'en',
+        project_type: Project::Types::SCRATCH,
+        name: 'Foo bar baz'
+      }
+    end
+
+    context 'with valid content' do
+      it 'returns success' do
+        expect(create_project.success?).to be(true)
+      end
+
+      it 'returns project with identifier' do
+        new_project = create_project[:project]
+        expect(new_project.identifier).to eq(identifier)
+      end
+    end
+
+    context 'when creation fails' do
+      before do
+        mock_project = instance_double(Project)
+        allow(mock_project).to receive(:save!).and_raise('Some error')
+        allow(Project).to receive(:new).and_return(mock_project)
+        allow(Sentry).to receive(:capture_exception)
+      end
+
+      it 'returns failure' do
+        expect(create_project.failure?).to be(true)
+      end
+
+      it 'returns error message' do
+        expect(create_project[:error]).to eq('Error creating project: Some error')
+      end
+
+      it 'sent the exception to Sentry' do
+        create_project
+        expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
+      end
+    end
+  end
+end
diff --git a/spec/features/public_project/creating_a_public_project_spec.rb b/spec/features/public_project/creating_a_public_project_spec.rb
new file mode 100644
index 000000000..095abdb26
--- /dev/null
+++ b/spec/features/public_project/creating_a_public_project_spec.rb
@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Creating a public project', type: :request do
+  let(:creator) { build(:user) }
+  let(:headers) { { Authorization: UserProfileMock::TOKEN } }
+  let(:params) do
+    {
+      project: {
+        identifier: 'test-project',
+        locale: 'en',
+        project_type: Project::Types::SCRATCH,
+        name: 'Test Project'
+      }
+    }
+  end
+
+  before do
+    authenticated_in_hydra_as(creator)
+  end
+
+  it 'responds 201 Created' do
+    post('/api/public_projects', headers:, params:)
+    expect(response).to have_http_status(:created)
+  end
+
+  it 'responds with the project JSON' do
+    post('/api/public_projects', headers:, params:)
+    data = JSON.parse(response.body, symbolize_names: true)
+
+    expect(data).to include(
+      {
+        identifier: 'test-project',
+        locale: 'en',
+        project_type: Project::Types::SCRATCH,
+        name: 'Test Project'
+      }
+    )
+  end
+
+  it 'responds 422 Unprocessable Entity when params are invalid' do
+    post('/api/public_projects', headers:, params: { project: {} })
+    expect(response).to have_http_status(:unprocessable_entity)
+  end
+
+  it 'responds 401 Unauthorized when no token is given' do
+    post('/api/public_projects', params:)
+    expect(response).to have_http_status(:unauthorized)
+  end
+end
diff --git a/spec/requests/public_projects/create_spec.rb b/spec/requests/public_projects/create_spec.rb
new file mode 100644
index 000000000..8d837384c
--- /dev/null
+++ b/spec/requests/public_projects/create_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Create public project requests' do
+  let(:project) { create(:project) }
+  let(:creator) { build(:user) }
+
+  context 'when auth is correct' do
+    let(:headers) { { Authorization: UserProfileMock::TOKEN } }
+
+    context 'when creating project is successful' do
+      before do
+        authenticated_in_hydra_as(creator)
+
+        response = OperationResponse.new
+        response[:project] = project
+        allow(PublicProject::Create).to receive(:call).and_return(response)
+      end
+
+      it 'returns success' do
+        post('/api/public_projects', headers:)
+
+        expect(response).to have_http_status(:created)
+      end
+    end
+
+    context 'when creating project fails' do
+      before do
+        authenticated_in_hydra_as(creator)
+
+        response = OperationResponse.new
+        response[:error] = 'Error creating project'
+        allow(PublicProject::Create).to receive(:call).and_return(response)
+      end
+
+      it 'returns error' do
+        post('/api/public_projects', headers:)
+
+        expect(response).to have_http_status(:unprocessable_entity)
+      end
+    end
+  end
+
+  context 'when no token is given' do
+    it 'returns unauthorized' do
+      post('/api/public_projects')
+
+      expect(response).to have_http_status(:unauthorized)
+    end
+  end
+end

From 0ca3b3d35fecc3e1a52783bf91ed7ea0913fe666 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Wed, 21 May 2025 16:57:48 +0100
Subject: [PATCH 02/31] Strict param parsing for Api::PublicProjectsController

The original version was copied from
`Api::ProjectsController#base_params`, but that seems to be an
abberation introduced [1] for no obviously discernable reason. Using
`ActionController::Parameters#require` [2] is more idiomatic Rails.

[1]: https://github.com/RaspberryPiFoundation/editor-api/pull/86
[2]: https://api.rubyonrails.org/v7.1.3.4/classes/ActionController/Parameters.html#method-i-require
---
 app/controllers/api/public_projects_controller.rb          | 2 +-
 .../public_project/creating_a_public_project_spec.rb       | 7 ++++++-
 spec/requests/public_projects/create_spec.rb               | 7 ++++---
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/app/controllers/api/public_projects_controller.rb b/app/controllers/api/public_projects_controller.rb
index 20af124ed..f817e2e23 100644
--- a/app/controllers/api/public_projects_controller.rb
+++ b/app/controllers/api/public_projects_controller.rb
@@ -18,7 +18,7 @@ def create
     private
 
     def project_params
-      params.fetch(:project, {}).permit(:identifier, :locale, :project_type, :name)
+      params.require(:project).permit(:identifier, :locale, :project_type, :name)
     end
   end
 end
diff --git a/spec/features/public_project/creating_a_public_project_spec.rb b/spec/features/public_project/creating_a_public_project_spec.rb
index 095abdb26..8e5f70202 100644
--- a/spec/features/public_project/creating_a_public_project_spec.rb
+++ b/spec/features/public_project/creating_a_public_project_spec.rb
@@ -39,8 +39,13 @@
     )
   end
 
-  it 'responds 422 Unprocessable Entity when params are invalid' do
+  it 'responds 400 Bad Request when params are malformed' do
     post('/api/public_projects', headers:, params: { project: {} })
+    expect(response).to have_http_status(:bad_request)
+  end
+
+  it 'responds 422 Unprocessable Entity when params are invalid' do
+    post('/api/public_projects', headers:, params: { project: { identifier: 'not-empty' } })
     expect(response).to have_http_status(:unprocessable_entity)
   end
 
diff --git a/spec/requests/public_projects/create_spec.rb b/spec/requests/public_projects/create_spec.rb
index 8d837384c..e44c20702 100644
--- a/spec/requests/public_projects/create_spec.rb
+++ b/spec/requests/public_projects/create_spec.rb
@@ -5,6 +5,7 @@
 RSpec.describe 'Create public project requests' do
   let(:project) { create(:project) }
   let(:creator) { build(:user) }
+  let(:params) { { project: { identifier: 'not-blank' } } }
 
   context 'when auth is correct' do
     let(:headers) { { Authorization: UserProfileMock::TOKEN } }
@@ -19,7 +20,7 @@
       end
 
       it 'returns success' do
-        post('/api/public_projects', headers:)
+        post('/api/public_projects', headers:, params:)
 
         expect(response).to have_http_status(:created)
       end
@@ -35,7 +36,7 @@
       end
 
       it 'returns error' do
-        post('/api/public_projects', headers:)
+        post('/api/public_projects', headers:, params:)
 
         expect(response).to have_http_status(:unprocessable_entity)
       end
@@ -44,7 +45,7 @@
 
   context 'when no token is given' do
     it 'returns unauthorized' do
-      post('/api/public_projects')
+      post('/api/public_projects', params:)
 
       expect(response).to have_http_status(:unauthorized)
     end

From bb435d61f0efbbb9db73c7ba012c381f803bbae5 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Wed, 21 May 2025 17:22:04 +0100
Subject: [PATCH 03/31] Extract User#parsed_roles method

I'm planning to introduce another role-related predicate method in a
subsequent commit. Extracting this method first will make that change
easier.

I'm pretty convinced the call to `#to_s` was made redundant when the
safe navigation operator was introduced in this commit [1]. However,
I suppose it's theoretically possible for the parsed JSON returned from
`HydraPublicApiClient.fetch_oauth_user` via `User.from_token` to contain
non-String values, so I'm going to leave it in place for now.

[1]: https://github.com/RaspberryPiFoundation/editor-api/commit/9a1fdb1b725b9379b940c9cc2f8628ea8fb6e0b4
---
 app/models/user.rb       |  6 +++++-
 spec/models/user_spec.rb | 32 ++++++++++++++++++++++----------
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index a7d90a6be..3b101845d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -51,7 +51,11 @@ def student?
   end
 
   def admin?
-    (roles&.to_s&.split(',')&.map(&:strip) || []).include?('editor-admin')
+    parsed_roles.include?('editor-admin')
+  end
+
+  def parsed_roles
+    roles&.to_s&.split(',')&.map(&:strip) || []
   end
 
   def ==(other)
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 103e90ada..c17c0e4b5 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -277,24 +277,36 @@
     end
   end
 
-  describe '#admin?' do
-    it 'returns true if the user has the editor-admin role in Hydra' do
-      user = build(:user, roles: 'editor-admin')
-      expect(user).to be_admin
+  describe '#parsed_roles' do
+    it 'returns array of role names when roles is set to comma-separated string' do
+      user = build(:user, roles: 'role-1,role-2')
+      expect(user.parsed_roles).to eq(%w[role-1 role-2])
     end
 
-    it 'returns false if the user does not have the editor-admin role in Hydra' do
-      user = build(:user, roles: 'another-editor-admin')
-      expect(user).not_to be_admin
+    it 'strips leading & trailing spaces from role names' do
+      user = build(:user, roles: ' role-1 , role-2 ')
+      expect(user.parsed_roles).to eq(%w[role-1 role-2])
     end
 
-    it 'returns false if roles are empty in Hydra' do
+    it 'returns empty array when roles is set to empty string' do
       user = build(:user, roles: '')
-      expect(user).not_to be_admin
+      expect(user.parsed_roles).to eq([])
     end
 
-    it 'returns false if roles are nil in Hydra' do
+    it 'returns empty array when roles is set to nil' do
       user = build(:user, roles: nil)
+      expect(user.parsed_roles).to eq([])
+    end
+  end
+
+  describe '#admin?' do
+    it 'returns true if the user has the editor-admin role in Hydra' do
+      user = build(:user, roles: 'editor-admin')
+      expect(user).to be_admin
+    end
+
+    it 'returns false if the user does not have the editor-admin role in Hydra' do
+      user = build(:user, roles: 'another-editor-admin')
       expect(user).not_to be_admin
     end
   end

From 260e08a567d7fdd2c6e3ed229040b8ca0a3758d4 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Wed, 21 May 2025 17:44:38 +0100
Subject: [PATCH 04/31] Introduce User#experience_cs_admin? method

I'm planning to make use of this to grant access to
`Api::PublicProjectsController#create` so that admin users on the
`experience-cs` website can create public Scratch projects via the
`editor-api` API.
---
 app/models/user.rb       |  4 ++++
 spec/models/user_spec.rb | 12 ++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/app/models/user.rb b/app/models/user.rb
index 3b101845d..386ab2615 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -54,6 +54,10 @@ def admin?
     parsed_roles.include?('editor-admin')
   end
 
+  def experience_cs_admin?
+    parsed_roles.include?('experience-cs-admin')
+  end
+
   def parsed_roles
     roles&.to_s&.split(',')&.map(&:strip) || []
   end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index c17c0e4b5..7cc065aa6 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -311,6 +311,18 @@
     end
   end
 
+  describe '#experience_cs_admin?' do
+    it 'returns true if the user has the experience-cs-admin role in Hydra' do
+      user = build(:user, roles: 'experience-cs-admin')
+      expect(user).to be_experience_cs_admin
+    end
+
+    it 'returns false if the user does not have the experience-cs-admin role in Hydra' do
+      user = build(:user, roles: 'another-admin')
+      expect(user).not_to be_experience_cs_admin
+    end
+  end
+
   describe '#school_roles' do
     subject(:user) { build(:user) }
 

From 88c836d751b16e7f043c3d8be888151faacced62 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Wed, 21 May 2025 17:54:47 +0100
Subject: [PATCH 05/31] Introduce permission for creating public projects

I plan to use this to restrict creating public projects to experience-cs
admin users.
---
 app/models/ability.rb       |  6 ++++++
 spec/factories/user.rb      |  4 ++++
 spec/models/ability_spec.rb | 16 ++++++++++++++++
 3 files changed, 26 insertions(+)

diff --git a/app/models/ability.rb b/app/models/ability.rb
index bca1f2dfa..1660892d9 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -14,6 +14,8 @@ def initialize(user)
       define_school_teacher_abilities(user:, school:) if user.school_teacher?(school)
       define_school_owner_abilities(school:) if user.school_owner?(school)
     end
+
+    define_experience_cs_admin_abilities(user)
   end
 
   private
@@ -100,6 +102,10 @@ def define_school_student_abilities(user:, school:)
     can(%i[show_finished set_finished], SchoolProject, project: { user_id: user.id, lesson_id: nil }, school_id: school.id)
   end
 
+  def define_experience_cs_admin_abilities(user)
+    can :create, :public_project if user.experience_cs_admin?
+  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/spec/factories/user.rb b/spec/factories/user.rb
index 473db5ec7..d7470f672 100644
--- a/spec/factories/user.rb
+++ b/spec/factories/user.rb
@@ -11,6 +11,10 @@
       roles { 'editor-admin' }
     end
 
+    factory :experience_cs_admin_user do
+      roles { 'experience-cs-admin' }
+    end
+
     factory :student do
       email { nil }
       username { Faker::Internet.username }
diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb
index 39624d64e..9d7a40aea 100644
--- a/spec/models/ability_spec.rb
+++ b/spec/models/ability_spec.rb
@@ -29,6 +29,8 @@
         it { is_expected.not_to be_able_to(:update, project) }
         it { is_expected.not_to be_able_to(:destroy, project) }
       end
+
+      it { is_expected.not_to be_able_to(:create, :public_project) }
     end
 
     context 'with a standard user' do
@@ -56,6 +58,8 @@
         it { is_expected.not_to be_able_to(:update, another_project) }
         it { is_expected.not_to be_able_to(:destroy, another_project) }
       end
+
+      it { is_expected.not_to be_able_to(:create, :public_project) }
     end
 
     context 'with a teacher' do
@@ -83,6 +87,8 @@
         it { is_expected.not_to be_able_to(:update, another_project) }
         it { is_expected.not_to be_able_to(:destroy, another_project) }
       end
+
+      it { is_expected.not_to be_able_to(:create, :public_project) }
     end
 
     context 'with an owner' do
@@ -110,6 +116,8 @@
         it { is_expected.not_to be_able_to(:update, another_project) }
         it { is_expected.not_to be_able_to(:destroy, another_project) }
       end
+
+      it { is_expected.not_to be_able_to(:create, :public_project) }
     end
 
     context 'with a student' do
@@ -137,6 +145,14 @@
         it { is_expected.not_to be_able_to(:update, another_project) }
         it { is_expected.not_to be_able_to(:destroy, another_project) }
       end
+
+      it { is_expected.not_to be_able_to(:create, :public_project) }
+    end
+
+    context 'with an experience-cs admin' do
+      let(:user) { build(:experience_cs_admin_user) }
+
+      it { is_expected.to be_able_to(:create, :public_project) }
     end
 
     # rubocop:disable RSpec/MultipleMemoizedHelpers

From d715c7aebe236a25498a904862cadde95dc59bf6 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Wed, 21 May 2025 17:54:47 +0100
Subject: [PATCH 06/31] Only experience-cs admins can create public projects

For the moment, we only want to allow experience-cs admin users to be
able to create public projects.
---
 app/controllers/api/public_projects_controller.rb     |  1 +
 .../public_project/creating_a_public_project_spec.rb  | 11 ++++++++++-
 spec/requests/public_projects/create_spec.rb          |  2 +-
 spec/support/user_profile_mock.rb                     |  3 ++-
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/app/controllers/api/public_projects_controller.rb b/app/controllers/api/public_projects_controller.rb
index f817e2e23..6e81d9e83 100644
--- a/app/controllers/api/public_projects_controller.rb
+++ b/app/controllers/api/public_projects_controller.rb
@@ -5,6 +5,7 @@ class PublicProjectsController < ApiController
     before_action :authorize_user
 
     def create
+      authorize! :create, :public_project
       result = PublicProject::Create.call(project_hash: project_params)
 
       if result.success?
diff --git a/spec/features/public_project/creating_a_public_project_spec.rb b/spec/features/public_project/creating_a_public_project_spec.rb
index 8e5f70202..d0759f416 100644
--- a/spec/features/public_project/creating_a_public_project_spec.rb
+++ b/spec/features/public_project/creating_a_public_project_spec.rb
@@ -3,7 +3,7 @@
 require 'rails_helper'
 
 RSpec.describe 'Creating a public project', type: :request do
-  let(:creator) { build(:user) }
+  let(:creator) { build(:experience_cs_admin_user) }
   let(:headers) { { Authorization: UserProfileMock::TOKEN } }
   let(:params) do
     {
@@ -39,6 +39,15 @@
     )
   end
 
+  context 'when creator is not an experience-cs admin' do
+    let(:creator) { build(:user) }
+
+    it 'responds 403 Forbidden' do
+      post('/api/public_projects', headers:, params:)
+      expect(response).to have_http_status(:forbidden)
+    end
+  end
+
   it 'responds 400 Bad Request when params are malformed' do
     post('/api/public_projects', headers:, params: { project: {} })
     expect(response).to have_http_status(:bad_request)
diff --git a/spec/requests/public_projects/create_spec.rb b/spec/requests/public_projects/create_spec.rb
index e44c20702..be8dd0034 100644
--- a/spec/requests/public_projects/create_spec.rb
+++ b/spec/requests/public_projects/create_spec.rb
@@ -4,7 +4,7 @@
 
 RSpec.describe 'Create public project requests' do
   let(:project) { create(:project) }
-  let(:creator) { build(:user) }
+  let(:creator) { build(:experience_cs_admin_user) }
   let(:params) { { project: { identifier: 'not-blank' } } }
 
   context 'when auth is correct' do
diff --git a/spec/support/user_profile_mock.rb b/spec/support/user_profile_mock.rb
index 4f56e353a..c694d158c 100644
--- a/spec/support/user_profile_mock.rb
+++ b/spec/support/user_profile_mock.rb
@@ -31,7 +31,8 @@ def user_to_hash(user, user_type, id_field = :id)
       id_field => user_type ? "#{user_type}:#{user.id}" : user.id,
       name: user.name,
       email: user.email,
-      username: user.username
+      username: user.username,
+      roles: user.roles
     }
   end
 

From 37761e605a34d6fa9b896ffc6e5f8770a782e5f2 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Wed, 21 May 2025 22:06:45 +0100
Subject: [PATCH 07/31] Rename Project#check_unique_not_null ->
 generate_identifier

I found the previous name confusing.
---
 app/models/project.rb       | 4 ++--
 spec/models/project_spec.rb | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/models/project.rb b/app/models/project.rb
index ebe956559..c3d3a98f6 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -20,7 +20,7 @@ module Types
 
   accepts_nested_attributes_for :components
 
-  before_validation :check_unique_not_null, on: :create
+  before_validation :generate_identifier, on: :create
   before_validation :create_school_project_if_needed
 
   validates :identifier, presence: true, uniqueness: { scope: :locale }
@@ -81,7 +81,7 @@ def media
 
   private
 
-  def check_unique_not_null
+  def generate_identifier
     self.identifier ||= PhraseIdentifier.generate
   end
 
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 6102ac091..37bcb4077 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -154,7 +154,7 @@
     end
   end
 
-  describe 'check_unique_not_null' do
+  describe 'generate_identifier' do
     let(:saved_project) { create(:project) }
 
     it 'generates an identifier if nil' do

From 8ddea4d9125fddae5ed24cfc7fa0070b153dc63e Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Thu, 22 May 2025 09:06:17 +0100
Subject: [PATCH 08/31] Simplify example for Project#generate_identifier

The `saved_project` wasn't really being used and it's sufficient to
assert the final state of `Project#identifier` rather than aserting that
it's changed.
---
 spec/models/project_spec.rb | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 37bcb4077..51f9b01dc 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -155,11 +155,10 @@
   end
 
   describe 'generate_identifier' do
-    let(:saved_project) { create(:project) }
-
     it 'generates an identifier if nil' do
-      unsaved_project = build(:project, identifier: nil)
-      expect { unsaved_project.valid? }.to change { unsaved_project.identifier.nil? }.from(true).to(false)
+      project = build(:project, identifier: nil)
+      project.valid?
+      expect(project.identifier).not_to be_nil
     end
   end
 

From 97d246048d793aa281ac1230c2ae89dca38389f0 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Thu, 22 May 2025 09:07:46 +0100
Subject: [PATCH 09/31] Allow skipping of identifier generation

When creating a public project we want the admin user to be able to
specify the identifier and apply validation rules against it. However,
prior to this change, if the supplied identifier was `nil`, the
`before_validation` callback, `Project#generate_identifier`, would kick
in and generate an identifier. Thus the presence validation on
`Project#identifier` never failed.

The new `Project#skip_identifier_generation` attribute gives us the
ability in specific contexts to prevent the identifier being generated
and thus allow the presence validation to fail.
---
 app/models/project.rb       | 4 +++-
 spec/models/project_spec.rb | 8 ++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/app/models/project.rb b/app/models/project.rb
index c3d3a98f6..3b7f589c5 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -7,6 +7,8 @@ module Types
     SCRATCH = 'scratch'
   end
 
+  attr_accessor :skip_identifier_generation
+
   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
@@ -20,7 +22,7 @@ module Types
 
   accepts_nested_attributes_for :components
 
-  before_validation :generate_identifier, on: :create
+  before_validation :generate_identifier, on: :create, unless: :skip_identifier_generation
   before_validation :create_school_project_if_needed
 
   validates :identifier, presence: true, uniqueness: { scope: :locale }
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 51f9b01dc..ab56c8471 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -160,6 +160,14 @@
       project.valid?
       expect(project.identifier).not_to be_nil
     end
+
+    context 'when skip_identifier_generation is true' do
+      it 'does not generate an identifier if nil' do
+        project = build(:project, identifier: nil, skip_identifier_generation: true)
+        project.valid?
+        expect(project.identifier).to be_nil
+      end
+    end
   end
 
   describe 'create_school_project_if_needed' do

From dfa62149fe82085bbcf4cbd4a6d4269b399d7569 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Thu, 22 May 2025 09:13:14 +0100
Subject: [PATCH 10/31] Require identifier when creating public project

This makes use of the recently added
`Project#skip_identifier_generation` attribute to ensure the presence
validation on `Project#identifier` can fail. In this case we want to
report this problem back to the user so they can correct it rather than
just generating a fallback identifier and continuing.
---
 .../public_project/operations/create.rb       |  4 +++-
 spec/concepts/public_project/create_spec.rb   | 21 ++++++++++++-------
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/lib/concepts/public_project/operations/create.rb b/lib/concepts/public_project/operations/create.rb
index 06a49734c..6f451f630 100644
--- a/lib/concepts/public_project/operations/create.rb
+++ b/lib/concepts/public_project/operations/create.rb
@@ -6,7 +6,9 @@ class << self
       def call(project_hash:)
         response = OperationResponse.new
 
-        project = Project.new(project_hash)
+        project = Project.new(project_hash).tap do |p|
+          p.skip_identifier_generation = true
+        end
         project.save!
 
         response[:project] = project
diff --git a/spec/concepts/public_project/create_spec.rb b/spec/concepts/public_project/create_spec.rb
index 1d9ccc9e9..b8896dda6 100644
--- a/spec/concepts/public_project/create_spec.rb
+++ b/spec/concepts/public_project/create_spec.rb
@@ -28,10 +28,9 @@
     end
 
     context 'when creation fails' do
+      let(:project_hash) { {} }
+
       before do
-        mock_project = instance_double(Project)
-        allow(mock_project).to receive(:save!).and_raise('Some error')
-        allow(Project).to receive(:new).and_return(mock_project)
         allow(Sentry).to receive(:capture_exception)
       end
 
@@ -39,14 +38,22 @@
         expect(create_project.failure?).to be(true)
       end
 
-      it 'returns error message' do
-        expect(create_project[:error]).to eq('Error creating project: Some error')
-      end
-
       it 'sent the exception to Sentry' do
         create_project
         expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
       end
     end
+
+    context 'when identifier is blank' do
+      let(:identifier) { nil }
+
+      it 'returns failure' do
+        expect(create_project.failure?).to be(true)
+      end
+
+      it 'returns error message' do
+        expect(create_project[:error]).to eq("Error creating project: Validation failed: Identifier can't be blank")
+      end
+    end
   end
 end

From f509f017f95cbcdfa1eee75d7e1e7d231e4922d0 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Thu, 22 May 2025 09:30:45 +0100
Subject: [PATCH 11/31] Forbid creation of non-Scratch public projects

Accepting the `product_type` param means that this controller can
legitimately be named more generically, i.e. `PublicProjectsController`
vs e.g. `ScratchPublicProjectsController`. However, the immediate
requirement is for admin users in the experience-cs app to be able to
create public Scratch projects and so I've only implemented the
functionality necessary to support that use case.

If we want to expand the capability of this controller to support the
creation of HTML and/or Python projects, we'll need to support other
params, e.g. `components`.
---
 app/controllers/api/public_projects_controller.rb         | 8 ++++++++
 .../public_project/creating_a_public_project_spec.rb      | 7 ++++++-
 spec/requests/public_projects/create_spec.rb              | 2 +-
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/app/controllers/api/public_projects_controller.rb b/app/controllers/api/public_projects_controller.rb
index 6e81d9e83..f7c575a43 100644
--- a/app/controllers/api/public_projects_controller.rb
+++ b/app/controllers/api/public_projects_controller.rb
@@ -3,6 +3,7 @@
 module Api
   class PublicProjectsController < ApiController
     before_action :authorize_user
+    before_action :restrict_project_type
 
     def create
       authorize! :create, :public_project
@@ -21,5 +22,12 @@ def create
     def project_params
       params.require(:project).permit(:identifier, :locale, :project_type, :name)
     end
+
+    def restrict_project_type
+      project_type = project_params[:project_type]
+      return if project_type == Project::Types::SCRATCH
+
+      raise CanCan::AccessDenied.new("#{project_type} not yet supported", :create, :public_project)
+    end
   end
 end
diff --git a/spec/features/public_project/creating_a_public_project_spec.rb b/spec/features/public_project/creating_a_public_project_spec.rb
index d0759f416..fa2a28f35 100644
--- a/spec/features/public_project/creating_a_public_project_spec.rb
+++ b/spec/features/public_project/creating_a_public_project_spec.rb
@@ -48,13 +48,18 @@
     end
   end
 
+  it 'responds 403 Forbidden when project type is not scratch' do
+    post('/api/public_projects', headers:, params: { project: { project_type: Project::Types::PYTHON } })
+    expect(response).to have_http_status(:forbidden)
+  end
+
   it 'responds 400 Bad Request when params are malformed' do
     post('/api/public_projects', headers:, params: { project: {} })
     expect(response).to have_http_status(:bad_request)
   end
 
   it 'responds 422 Unprocessable Entity when params are invalid' do
-    post('/api/public_projects', headers:, params: { project: { identifier: 'not-empty' } })
+    post('/api/public_projects', headers:, params: { project: { project_type: Project::Types::SCRATCH } })
     expect(response).to have_http_status(:unprocessable_entity)
   end
 
diff --git a/spec/requests/public_projects/create_spec.rb b/spec/requests/public_projects/create_spec.rb
index be8dd0034..6a9825822 100644
--- a/spec/requests/public_projects/create_spec.rb
+++ b/spec/requests/public_projects/create_spec.rb
@@ -5,7 +5,7 @@
 RSpec.describe 'Create public project requests' do
   let(:project) { create(:project) }
   let(:creator) { build(:experience_cs_admin_user) }
-  let(:params) { { project: { identifier: 'not-blank' } } }
+  let(:params) { { project: { project_type: Project::Types::SCRATCH } } }
 
   context 'when auth is correct' do
     let(:headers) { { Authorization: UserProfileMock::TOKEN } }

From cfb532d2aabb6840bd62095b149174a77fef1586 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Thu, 22 May 2025 10:54:09 +0100
Subject: [PATCH 12/31] Introduce PhraseIdentifier::PATTERN

The `PhraseIdentifier.generate` method is used to generate an identifier
for projects created by teachers and students. However, public projects,
e.g. those created via the `GithubWebhooksController#github_push` [1]
action and those created via the (possibly defunct)
`projects:create_all` rake task [2], have identifiers specified in their
project config via `ProjectImporter` [3].

While there is no *explicit* validation to enforce it, the identifiers
for these public projects seems to follow the pattern described by the
regular expression I have defined in `PhraseIdentifier::PATTERN`, i.e.
kebab-case with digits allowed, at least in the 2nd word [4]. The
pattern could probably be stricter, but since I don't have access to the
production data, it seems safer for it to be more relaxed.

We want admin users in experience-cs to be able to create public scratch
projects via the editor-api API and I think it makes sense to restrict
them to using identifiers following a similar pattern. I plan to add
some validation to enforce that and so we can provide feedback to the
admin user if they choose an identifier with the wrong format. I'll be
able to use this `PhraseIdentifier::PATTERN` in that validation.

[1]: https://github.com/RaspberryPiFoundation/editor-api/blob/f348dfc92fdd8b19fbfd5434e7751340f33b4093/app/controllers/github_webhooks_controller.rb#L6-L8
[2]: https://github.com/RaspberryPiFoundation/editor-api/blob/f348dfc92fdd8b19fbfd5434e7751340f33b4093/lib/tasks/projects.rake#L4-L7
[3]: https://github.com/RaspberryPiFoundation/editor-api/blob/f348dfc92fdd8b19fbfd5434e7751340f33b4093/lib/project_importer.rb
[4]: https://github.com/RaspberryPiFoundation/editor-api/blob/f348dfc92fdd8b19fbfd5434e7751340f33b4093/lib/tasks/project_components/top_5_emoji_list/project_config.yml#L2
---
 lib/phrase_identifier.rb           |  2 ++
 spec/lib/phrase_identifier_spec.rb | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/lib/phrase_identifier.rb b/lib/phrase_identifier.rb
index 8cab707d4..cb7c45916 100644
--- a/lib/phrase_identifier.rb
+++ b/lib/phrase_identifier.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class PhraseIdentifier
+  PATTERN = /\A[a-z0-9]+(-[a-z0-9]+)*\z/
+
   class Error < RuntimeError
   end
 
diff --git a/spec/lib/phrase_identifier_spec.rb b/spec/lib/phrase_identifier_spec.rb
index 6da694d1f..f6a078542 100644
--- a/spec/lib/phrase_identifier_spec.rb
+++ b/spec/lib/phrase_identifier_spec.rb
@@ -4,6 +4,24 @@
 require 'phrase_identifier'
 
 RSpec.describe PhraseIdentifier do
+  describe 'PATTERN' do
+    subject(:pattern) { described_class::PATTERN }
+
+    it { is_expected.to match('abc-def-ghi') }
+    it { is_expected.to match('123-456-789') }
+    it { is_expected.to match('a2c-d5f-g8i') }
+
+    it { is_expected.not_to match('Abc-def-ghi') }
+    it { is_expected.not_to match('Abc-def-GHI') }
+    it { is_expected.not_to match('abc--def-ghi') }
+    it { is_expected.not_to match('-abc-def-ghi') }
+    it { is_expected.not_to match('abc-def-ghi-') }
+    it { is_expected.not_to match('abc def-ghi') }
+    it { is_expected.not_to match(' abc-def-ghi') }
+    it { is_expected.not_to match('abc-def-ghi ') }
+    it { is_expected.not_to match('abc_def_ghi') }
+  end
+
   describe '#generate' do
     subject(:generate) { described_class.generate }
 
@@ -18,6 +36,14 @@
       it { is_expected.to match phrase_regex }
     end
 
+    context 'when using the default words.txt file' do
+      it 'returns identifiers conforming to the expected pattern' do
+        10.times do
+          expect(generate).to match(described_class::PATTERN)
+        end
+      end
+    end
+
     context 'when there are no available combinations' do
       let(:identifier) { Faker::Verb.base }
 

From 490e59a0a8fa8caffbeb27c83c62e7d3b6999b85 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Thu, 22 May 2025 14:30:12 +0100
Subject: [PATCH 13/31] Introduce PublicProject model

I want to add some validations to the `Project` model to provide some
sensible constraints on the values that an experience-cs admin user
enters into the administrate UI so that I can provide sensible error
messages.

However, since I don't have access [1] to staging or production data for
`editor-api`, I can't be sure that some existing projects would become
invalid. So as a workaround I've implemented this `PublicProject`
wrapper class which allows me to add validations for use only in the
context of the `Api::PublicProjectsController`.

To start with, I've just added a validation to check the format of
`Project#identifier`, although this isn't yet being used in the
controller action.

Hopefully implementing these validations as ActiveModel validations will
make it easier to move them onto the `Project` model once it's deemed
safe to do so.

[1]: https://raspberrypifoundation.slack.com/archives/C08831T17H6/p1747900618263229
---
 app/models/public_project.rb                  |  39 ++++++
 .../public_project/operations/create.rb       |   2 +-
 spec/models/public_project_spec.rb            | 116 ++++++++++++++++++
 3 files changed, 156 insertions(+), 1 deletion(-)
 create mode 100644 app/models/public_project.rb
 create mode 100644 spec/models/public_project_spec.rb

diff --git a/app/models/public_project.rb b/app/models/public_project.rb
new file mode 100644
index 000000000..b80517aa9
--- /dev/null
+++ b/app/models/public_project.rb
@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+class PublicProject
+  include ActiveModel::Validations
+
+  attr_reader :project
+
+  delegate :identifier, to: :project
+
+  validates :identifier, format: { with: PhraseIdentifier::PATTERN, allow_blank: true }
+
+  def initialize(project)
+    @project = project
+  end
+
+  def valid?
+    valid = super
+    project_valid = project.valid?
+    errors.merge!(project.errors)
+    valid && project_valid
+  end
+
+  def save!
+    raise_validation_error unless valid?
+
+    project.save!
+  end
+
+  private
+
+  def merge_errors
+    project.errors.details.each do |attribute, details_array|
+      details_array.each_with_index do |details, index|
+        message = project.errors[attribute][index]
+        errors.add(attribute, message, **details)
+      end
+    end
+  end
+end
diff --git a/lib/concepts/public_project/operations/create.rb b/lib/concepts/public_project/operations/create.rb
index 6f451f630..cbe7e5847 100644
--- a/lib/concepts/public_project/operations/create.rb
+++ b/lib/concepts/public_project/operations/create.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-module PublicProject
+class PublicProject
   class Create
     class << self
       def call(project_hash:)
diff --git a/spec/models/public_project_spec.rb b/spec/models/public_project_spec.rb
new file mode 100644
index 000000000..38e9a3a36
--- /dev/null
+++ b/spec/models/public_project_spec.rb
@@ -0,0 +1,116 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe PublicProject do
+  subject(:public_project) { described_class.new(project) }
+
+  let(:project) { build(:project, skip_identifier_generation: true, identifier: 'valid-identifier') }
+
+  describe 'validation' do
+    context 'when both project and public project are valid' do
+      it { is_expected.to be_valid }
+
+      it 'has no errors' do
+        expect(public_project.errors).to be_empty
+      end
+
+      it 'persists project on save!' do
+        public_project.save!
+        expect(project).to be_persisted
+      end
+    end
+
+    context 'when project is not valid' do
+      before do
+        project.locale = nil
+        project.user_id = nil
+      end
+
+      it { is_expected.not_to be_valid }
+
+      it 'has the project error' do
+        public_project.valid?
+        expect(public_project.errors.full_messages).to include("Locale can't be blank")
+      end
+
+      it 'raises validation error on save!' do
+        expect do
+          public_project.save!
+        end.to raise_error(
+          an_instance_of(ActiveModel::ValidationError)
+            .and(having_attributes(message: "Validation failed: Locale can't be blank"))
+        )
+      end
+
+      it 'does not persist project on save!' do
+        public_project.save!
+      rescue ActiveModel::ValidationError
+        expect(project).not_to be_persisted
+      end
+    end
+
+    context 'when public project is not valid' do
+      before do
+        project.identifier = 'InvalidIdentifier'
+      end
+
+      it { is_expected.not_to be_valid }
+
+      it 'has the public project error' do
+        public_project.valid?
+        expect(public_project.errors.full_messages).to include('Identifier is invalid')
+      end
+
+      it 'raises validation error on save!' do
+        expect do
+          public_project.save!
+        end.to raise_error(
+          an_instance_of(ActiveModel::ValidationError)
+            .and(having_attributes(message: 'Validation failed: Identifier is invalid'))
+        )
+      end
+
+      it 'does not persist project on save!' do
+        public_project.save!
+      rescue ActiveModel::ValidationError
+        expect(project).not_to be_persisted
+      end
+    end
+
+    context 'when both project and public project are not valid' do
+      before do
+        project.locale = nil
+        project.user_id = nil
+        project.identifier = 'InvalidIdentifier'
+      end
+
+      it { is_expected.not_to be_valid }
+
+      it 'has the project error' do
+        public_project.valid?
+        expect(public_project.errors.full_messages).to include("Locale can't be blank")
+      end
+
+      it 'has the public project error' do
+        public_project.valid?
+        expect(public_project.errors.full_messages).to include('Identifier is invalid')
+      end
+
+      it 'raises validation error including both errors on save!' do
+        expect do
+          public_project.save!
+        end.to raise_error(
+          an_instance_of(ActiveModel::ValidationError)
+            .and(having_attributes(message: "Validation failed: Identifier is invalid, Locale can't be blank"))
+        )
+      end
+
+      it 'does not persist project on save!' do
+        public_project.save!
+      rescue ActiveModel::ValidationError
+        expect(project).not_to be_persisted
+      end
+    end
+  end
+end

From 06ac4fad902a3e8405de403c92391df9387fdf56 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Thu, 22 May 2025 14:37:42 +0100
Subject: [PATCH 14/31] Validate format of identifier for public project

This uses the recently added `PublicProject` wrapper class to add the
validation just in the context of the `Api::PublicProjectsController`.

The validation is based on the regular expression in
`PhaseIdentifier::PATTERN`, i.e. kebab-case.
---
 lib/concepts/public_project/operations/create.rb | 12 +++++++-----
 spec/concepts/public_project/create_spec.rb      | 12 ++++++++++++
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/lib/concepts/public_project/operations/create.rb b/lib/concepts/public_project/operations/create.rb
index cbe7e5847..419b3895d 100644
--- a/lib/concepts/public_project/operations/create.rb
+++ b/lib/concepts/public_project/operations/create.rb
@@ -6,12 +6,14 @@ class << self
       def call(project_hash:)
         response = OperationResponse.new
 
-        project = Project.new(project_hash).tap do |p|
-          p.skip_identifier_generation = true
-        end
-        project.save!
+        public_project = PublicProject.new(
+          Project.new(project_hash).tap do |p|
+            p.skip_identifier_generation = true
+          end
+        )
+        public_project.save!
 
-        response[:project] = project
+        response[:project] = public_project.project
         response
       rescue StandardError => e
         Sentry.capture_exception(e)
diff --git a/spec/concepts/public_project/create_spec.rb b/spec/concepts/public_project/create_spec.rb
index b8896dda6..b2856d9f5 100644
--- a/spec/concepts/public_project/create_spec.rb
+++ b/spec/concepts/public_project/create_spec.rb
@@ -55,5 +55,17 @@
         expect(create_project[:error]).to eq("Error creating project: Validation failed: Identifier can't be blank")
       end
     end
+
+    context 'when identifier is in invalid format' do
+      let(:identifier) { 'FooBarBaz' }
+
+      it 'returns failure' do
+        expect(create_project.failure?).to be(true)
+      end
+
+      it 'returns error message' do
+        expect(create_project[:error]).to eq('Error creating project: Validation failed: Identifier is invalid')
+      end
+    end
   end
 end

From 288aeee559280d8e88c87d10807a006828450ea2 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Thu, 22 May 2025 14:37:42 +0100
Subject: [PATCH 15/31] Validate presence of name for public project

Somewhat surprisingly there is currently no validation for the presence
of `name` on the `Project` model itself, although I suspect the name is
probably implicitly guaranteed not to be blank by the clients of
editor-api.

Since admin users in experience-cs are going to be entering the name
manually in the administrate UI, I want to protect against a blank name
and I want to be able to provide a sensible error message if none is
set.

Ideally I'd add a validation to the `Project` model itself. However,
since I don't have access [1] to staging or production data for
`editor-api`, I can't be sure that some existing projects would become
invalid. So as before I've added the validation to the `PublicProject`
wrapper class which means the validation will only be used in the
context of the `Api::PublicProjectsController`.

[1]: https://raspberrypifoundation.slack.com/archives/C08831T17H6/p1747900618263229
---
 app/models/public_project.rb                |  3 ++-
 spec/concepts/public_project/create_spec.rb | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/app/models/public_project.rb b/app/models/public_project.rb
index b80517aa9..ee9a8c986 100644
--- a/app/models/public_project.rb
+++ b/app/models/public_project.rb
@@ -5,9 +5,10 @@ class PublicProject
 
   attr_reader :project
 
-  delegate :identifier, to: :project
+  delegate :identifier, :name, to: :project
 
   validates :identifier, format: { with: PhraseIdentifier::PATTERN, allow_blank: true }
+  validates :name, presence: true
 
   def initialize(project)
     @project = project
diff --git a/spec/concepts/public_project/create_spec.rb b/spec/concepts/public_project/create_spec.rb
index b2856d9f5..9d852c7d2 100644
--- a/spec/concepts/public_project/create_spec.rb
+++ b/spec/concepts/public_project/create_spec.rb
@@ -7,12 +7,13 @@
     subject(:create_project) { described_class.call(project_hash:) }
 
     let(:identifier) { 'foo-bar-baz' }
+    let(:name) { 'Foo bar baz' }
     let(:project_hash) do
       {
         identifier:,
         locale: 'en',
         project_type: Project::Types::SCRATCH,
-        name: 'Foo bar baz'
+        name:
       }
     end
 
@@ -67,5 +68,17 @@
         expect(create_project[:error]).to eq('Error creating project: Validation failed: Identifier is invalid')
       end
     end
+
+    context 'when name is blank' do
+      let(:name) { '' }
+
+      it 'returns failure' do
+        expect(create_project.failure?).to be(true)
+      end
+
+      it 'returns error message' do
+        expect(create_project[:error]).to eq("Error creating project: Validation failed: Name can't be blank")
+      end
+    end
   end
 end

From 963d8faee42ddd398f9011f064e5dcd27acfd87e Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Thu, 22 May 2025 14:56:10 +0100
Subject: [PATCH 16/31] Ensure Project#user_id is not set on public project

A public project, i.e. one that is accessible to all users, should not
have `Project#user_id` set. This is already implicitly enforced by
`Api::PublicProjectsController#project_params` which excludes the
`user_id` param. However, I thought it was worth adding an example to
guarantee it remains the case.
---
 .../public_project/creating_a_public_project_spec.rb     | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/spec/features/public_project/creating_a_public_project_spec.rb b/spec/features/public_project/creating_a_public_project_spec.rb
index fa2a28f35..21fa47f1b 100644
--- a/spec/features/public_project/creating_a_public_project_spec.rb
+++ b/spec/features/public_project/creating_a_public_project_spec.rb
@@ -39,6 +39,15 @@
     )
   end
 
+  it 'does not set user_id on project even if one is supplied' do
+    user_id = SecureRandom.uuid
+
+    post('/api/public_projects', headers:, params: params.merge(user_id:))
+    data = JSON.parse(response.body, symbolize_names: true)
+
+    expect(data).to include(user_id: nil)
+  end
+
   context 'when creator is not an experience-cs admin' do
     let(:creator) { build(:user) }
 

From 6f20092ed82e24512ce4bfad7019ecbcb0484232 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 09:07:23 +0100
Subject: [PATCH 17/31] Rename project_params in PublicProjectsController

I'm about to add an update action which needs different params, so
renaming `project_params` to `create_params` will make that change
simpler.
---
 app/controllers/api/public_projects_controller.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/controllers/api/public_projects_controller.rb b/app/controllers/api/public_projects_controller.rb
index f7c575a43..9301f6dc3 100644
--- a/app/controllers/api/public_projects_controller.rb
+++ b/app/controllers/api/public_projects_controller.rb
@@ -7,7 +7,7 @@ class PublicProjectsController < ApiController
 
     def create
       authorize! :create, :public_project
-      result = PublicProject::Create.call(project_hash: project_params)
+      result = PublicProject::Create.call(project_hash: create_params)
 
       if result.success?
         @project = result[:project]
@@ -19,12 +19,12 @@ def create
 
     private
 
-    def project_params
+    def create_params
       params.require(:project).permit(:identifier, :locale, :project_type, :name)
     end
 
     def restrict_project_type
-      project_type = project_params[:project_type]
+      project_type = create_params[:project_type]
       return if project_type == Project::Types::SCRATCH
 
       raise CanCan::AccessDenied.new("#{project_type} not yet supported", :create, :public_project)

From 29478e1b5bc55740fed1cba4a6d6f5c42eb5be0c Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 09:28:57 +0100
Subject: [PATCH 18/31] Add Api::PublicProjectsController#update

We want to allow experience-cs admins to update "public" projects via
experience-cs administrate UI [1]. When I say "public" projects, I mean
those that are accessible to all users, because `Project#user_id` is set
to `nil`.

Public projects for Python & HTML are currently updated via the
`GithubWebhooksController#github_push` [2] action and/or via the
(possibly defunct) `projects:create_all` rake task [3] both of which
make use of `ProjectImporter` [4]. However, these mechanisms are not
suitable for our use case.

This commit introduces a new `Api::PublicProjectsController#update`
action which is somewhat based on `Api::ProjectsController#update`.

I've tried to roughly follow the existing conventions however much I
disagree with them, e.g. I've introduced a `PublicProject::Update` class
in `lib/concepts/public_project/operations/update.rb` along the same
lines as `Project::Update` in
`lib/concepts/project/operations/update.rb`.

This is just a skeletal starting point - I plan to flesh it out in
subsequent commits.

Note that I've had to limit the `restrict_project_type` before action to
just the create action, because it relies on the `project.project_type`
param which will not be accepted by the `update` action.

[1]: https://github.com/RaspberryPiFoundation/experience-cs/issues/405
[2]: https://github.com/RaspberryPiFoundation/editor-api/blob/f348dfc92fdd8b19fbfd5434e7751340f33b4093/app/controllers/github_webhooks_controller.rb#L6-L8
[3]: https://github.com/RaspberryPiFoundation/editor-api/blob/f348dfc92fdd8b19fbfd5434e7751340f33b4093/lib/tasks/projects.rake#L4-L7
[4]: https://github.com/RaspberryPiFoundation/editor-api/blob/f348dfc92fdd8b19fbfd5434e7751340f33b4093/lib/project_importer.rb
---
 .../api/public_projects_controller.rb         | 22 +++++++-
 config/routes.rb                              |  2 +-
 .../public_project/operations/update.rb       | 21 ++++++++
 spec/concepts/public_project/update_spec.rb   | 47 ++++++++++++++++
 .../updating_a_public_project_spec.rb         | 41 ++++++++++++++
 spec/requests/public_projects/update_spec.rb  | 53 +++++++++++++++++++
 6 files changed, 184 insertions(+), 2 deletions(-)
 create mode 100644 lib/concepts/public_project/operations/update.rb
 create mode 100644 spec/concepts/public_project/update_spec.rb
 create mode 100644 spec/features/public_project/updating_a_public_project_spec.rb
 create mode 100644 spec/requests/public_projects/update_spec.rb

diff --git a/app/controllers/api/public_projects_controller.rb b/app/controllers/api/public_projects_controller.rb
index 9301f6dc3..150dd8c14 100644
--- a/app/controllers/api/public_projects_controller.rb
+++ b/app/controllers/api/public_projects_controller.rb
@@ -3,7 +3,8 @@
 module Api
   class PublicProjectsController < ApiController
     before_action :authorize_user
-    before_action :restrict_project_type
+    before_action :restrict_project_type, only: %i[create]
+    before_action :load_project, only: %i[update]
 
     def create
       authorize! :create, :public_project
@@ -17,12 +18,31 @@ def create
       end
     end
 
+    def update
+      result = PublicProject::Update.call(project: @project, update_hash: update_params)
+
+      if result.success?
+        @project = result[:project]
+        render 'api/projects/show', formats: [:json]
+      else
+        render json: { error: result[:error] }, status: :unprocessable_entity
+      end
+    end
+
     private
 
+    def load_project
+      @project = Project.find_by!(identifier: params[:id])
+    end
+
     def create_params
       params.require(:project).permit(:identifier, :locale, :project_type, :name)
     end
 
+    def update_params
+      params.require(:project).permit(:identifier, :name)
+    end
+
     def restrict_project_type
       project_type = create_params[:project_type]
       return if project_type == Project::Types::SCRATCH
diff --git a/config/routes.rb b/config/routes.rb
index 7e47ef4a4..e8254e01c 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -41,7 +41,7 @@
       resource :images, only: %i[show create], controller: 'projects/images'
     end
 
-    resources :public_projects, only: %i[create]
+    resources :public_projects, only: %i[create update]
 
     resource :project_errors, only: %i[create]
 
diff --git a/lib/concepts/public_project/operations/update.rb b/lib/concepts/public_project/operations/update.rb
new file mode 100644
index 000000000..a88c9f84e
--- /dev/null
+++ b/lib/concepts/public_project/operations/update.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+class PublicProject
+  class Update
+    class << self
+      def call(project:, update_hash:)
+        response = OperationResponse.new
+
+        project.assign_attributes(update_hash)
+        project.save!
+
+        response[:project] = project
+        response
+      rescue StandardError => e
+        Sentry.capture_exception(e)
+        response[:error] = "Error updating project: #{e}"
+        response
+      end
+    end
+  end
+end
diff --git a/spec/concepts/public_project/update_spec.rb b/spec/concepts/public_project/update_spec.rb
new file mode 100644
index 000000000..47a14218b
--- /dev/null
+++ b/spec/concepts/public_project/update_spec.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe PublicProject::Update, type: :unit do
+  describe '.call' do
+    subject(:update_project) { described_class.call(project:, update_hash:) }
+
+    let(:identifier) { 'foo-bar-baz' }
+    let(:new_identifier) { 'new-identifier' }
+    let(:new_name) { 'New name' }
+    let!(:project) { create(:project, identifier:) }
+    let(:update_hash) { { identifier: new_identifier, name: new_name } }
+
+    context 'with valid content' do
+      it 'returns success' do
+        expect(update_project.success?).to be(true)
+      end
+
+      it 'returns project with new identifier' do
+        updated_project = update_project[:project]
+        expect(updated_project.identifier).to eq(new_identifier)
+      end
+
+      it 'returns project with new name' do
+        updated_project = update_project[:project]
+        expect(updated_project.name).to eq(new_name)
+      end
+    end
+
+    context 'when update fails' do
+      before do
+        allow(project).to receive(:save!).and_raise('Some error')
+        allow(Sentry).to receive(:capture_exception)
+      end
+
+      it 'returns failure' do
+        expect(update_project.failure?).to be(true)
+      end
+
+      it 'sent the exception to Sentry' do
+        update_project
+        expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
+      end
+    end
+  end
+end
diff --git a/spec/features/public_project/updating_a_public_project_spec.rb b/spec/features/public_project/updating_a_public_project_spec.rb
new file mode 100644
index 000000000..980b746c6
--- /dev/null
+++ b/spec/features/public_project/updating_a_public_project_spec.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Updating a public project', type: :request do
+  let(:creator) { build(:user) }
+  let(:project) { create(:project) }
+  let(:headers) { { Authorization: UserProfileMock::TOKEN } }
+  let(:params) { { project: { identifier: 'new-identifier', name: 'New name' } } }
+
+  before do
+    authenticated_in_hydra_as(creator)
+  end
+
+  it 'responds 200 OK' do
+    put("/api/public_projects/#{project.identifier}", headers:, params:)
+    expect(response).to have_http_status(:success)
+  end
+
+  it 'responds with the project JSON' do
+    put("/api/public_projects/#{project.identifier}", headers:, params:)
+    data = JSON.parse(response.body, symbolize_names: true)
+
+    expect(data).to include(identifier: 'new-identifier', name: 'New name')
+  end
+
+  it 'responds 400 Bad Request when params are malformed' do
+    put("/api/public_projects/#{project.identifier}", headers:, params: {})
+    expect(response).to have_http_status(:bad_request)
+  end
+
+  it 'responds 401 Unauthorized when no token is given' do
+    put("/api/public_projects/#{project.identifier}", params:)
+    expect(response).to have_http_status(:unauthorized)
+  end
+
+  it 'responds 404 Not Found when project is not found' do
+    put('/api/public_projects/another-identifier', headers:, params:)
+    expect(response).to have_http_status(:not_found)
+  end
+end
diff --git a/spec/requests/public_projects/update_spec.rb b/spec/requests/public_projects/update_spec.rb
new file mode 100644
index 000000000..2f0ed3829
--- /dev/null
+++ b/spec/requests/public_projects/update_spec.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Update public project requests' do
+  let(:project) { create(:project) }
+  let(:creator) { build(:user) }
+  let(:params) { { project: { identifier: 'new-identifier', name: 'New name' } } }
+
+  context 'when auth is correct' do
+    let(:headers) { { Authorization: UserProfileMock::TOKEN } }
+
+    context 'when updating project is successful' do
+      before do
+        authenticated_in_hydra_as(creator)
+
+        response = OperationResponse.new
+        response[:project] = project
+        allow(PublicProject::Update).to receive(:call).and_return(response)
+      end
+
+      it 'returns success' do
+        put("/api/public_projects/#{project.identifier}", headers:, params:)
+
+        expect(response).to have_http_status(:success)
+      end
+    end
+
+    context 'when updating project fails' do
+      before do
+        authenticated_in_hydra_as(creator)
+
+        response = OperationResponse.new
+        response[:error] = 'Error updating project'
+        allow(PublicProject::Update).to receive(:call).and_return(response)
+      end
+
+      it 'returns error' do
+        put("/api/public_projects/#{project.identifier}", headers:, params:)
+
+        expect(response).to have_http_status(:unprocessable_entity)
+      end
+    end
+  end
+
+  context 'when no token is given' do
+    it 'returns unauthorized' do
+      put("/api/public_projects/#{project.identifier}", headers:, params:)
+
+      expect(response).to have_http_status(:unauthorized)
+    end
+  end
+end

From 626b2e0e2da11d0a8ea5eedc7c843b97f36954f3 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 09:46:00 +0100
Subject: [PATCH 19/31] Use ProjectLoader in Api::PublicProjectsController

This is closely based on how `Api::ProjectsController` works although
it's not obvious that its behaviour is tested. However, there is a spec
for `ProjectLoader` itself, so I've just added examples to make sure
it's wired up to the controller correctly.

One thing I've added is to raise `ActiveRecord::RecordNotFound` if
`ProjectLoader#load` returns `nil` which seems possible. This will
result in a 404 Not Found response which is what I was already asserting
in `spec/features/public_project/updating_a_public_project_spec.rb`. As
far as I can tell, `Api::ProjectsController` actions which make use of
`ProjectLoader#load` (e.g. the `update` action) do not check for the
project being `nil` and so will probably respond with a 500 Internal
Server Error when the code (e.g. in `Project::Update`) calls a method on
the `nil` project. This doesn't seem great!
---
 .../api/public_projects_controller.rb         |  4 +++-
 .../updating_a_public_project_spec.rb         |  2 +-
 spec/requests/public_projects/update_spec.rb  | 19 ++++++++++++++++++-
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/app/controllers/api/public_projects_controller.rb b/app/controllers/api/public_projects_controller.rb
index 150dd8c14..477525f94 100644
--- a/app/controllers/api/public_projects_controller.rb
+++ b/app/controllers/api/public_projects_controller.rb
@@ -32,7 +32,9 @@ def update
     private
 
     def load_project
-      @project = Project.find_by!(identifier: params[:id])
+      loader = ProjectLoader.new(params[:id], [params[:locale]])
+      @project = loader.load
+      raise ActiveRecord::RecordNotFound if @project.blank?
     end
 
     def create_params
diff --git a/spec/features/public_project/updating_a_public_project_spec.rb b/spec/features/public_project/updating_a_public_project_spec.rb
index 980b746c6..2bcf5f727 100644
--- a/spec/features/public_project/updating_a_public_project_spec.rb
+++ b/spec/features/public_project/updating_a_public_project_spec.rb
@@ -4,7 +4,7 @@
 
 RSpec.describe 'Updating a public project', type: :request do
   let(:creator) { build(:user) }
-  let(:project) { create(:project) }
+  let(:project) { create(:project, locale: 'en') }
   let(:headers) { { Authorization: UserProfileMock::TOKEN } }
   let(:params) { { project: { identifier: 'new-identifier', name: 'New name' } } }
 
diff --git a/spec/requests/public_projects/update_spec.rb b/spec/requests/public_projects/update_spec.rb
index 2f0ed3829..0342c92f1 100644
--- a/spec/requests/public_projects/update_spec.rb
+++ b/spec/requests/public_projects/update_spec.rb
@@ -3,7 +3,9 @@
 require 'rails_helper'
 
 RSpec.describe 'Update public project requests' do
-  let(:project) { create(:project) }
+  let(:locale) { 'fr' }
+  let(:project_loader) { instance_double(ProjectLoader) }
+  let(:project) { create(:project, locale: 'en') }
   let(:creator) { build(:user) }
   let(:params) { { project: { identifier: 'new-identifier', name: 'New name' } } }
 
@@ -14,11 +16,26 @@
       before do
         authenticated_in_hydra_as(creator)
 
+        allow(ProjectLoader).to receive(:new).and_return(project_loader)
+        allow(project_loader).to receive(:load).and_return(project)
+
         response = OperationResponse.new
         response[:project] = project
         allow(PublicProject::Update).to receive(:call).and_return(response)
       end
 
+      it 'builds ProjectLoader with identifier & locale' do
+        put("/api/public_projects/#{project.identifier}?locale=#{locale}", headers:, params:)
+
+        expect(ProjectLoader).to have_received(:new).with(project.identifier, [locale])
+      end
+
+      it 'uses ProjectLoader#load to find the project based on identifier & locale' do
+        put("/api/public_projects/#{project.identifier}?locale=#{locale}", headers:, params:)
+
+        expect(project_loader).to have_received(:load)
+      end
+
       it 'returns success' do
         put("/api/public_projects/#{project.identifier}", headers:, params:)
 

From 7a52343e4189c8246190cab66113867e9953c592 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 10:53:38 +0100
Subject: [PATCH 20/31] Make updating public projects specs more realistic

The immediate use case for `Api::PublicProjectsController#update` is for
experience-cs admin users to change the name of a public project. Since
experience-cs only deals with Scratch projects, it makes sense to use
projects of this type in the specs. However, since we're using
`ProjectLoader` to find the project and the default scope is still in
place to hide Scratch projects, we need to make use of the
`ProjectScoping` concern to use the `project_type` param to set
`Current.project_scope` to include Scratch projects.
---
 .../public_project/updating_a_public_project_spec.rb | 12 ++++++------
 spec/requests/public_projects/update_spec.rb         | 12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/spec/features/public_project/updating_a_public_project_spec.rb b/spec/features/public_project/updating_a_public_project_spec.rb
index 2bcf5f727..378f6c006 100644
--- a/spec/features/public_project/updating_a_public_project_spec.rb
+++ b/spec/features/public_project/updating_a_public_project_spec.rb
@@ -4,7 +4,7 @@
 
 RSpec.describe 'Updating a public project', type: :request do
   let(:creator) { build(:user) }
-  let(:project) { create(:project, locale: 'en') }
+  let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH) }
   let(:headers) { { Authorization: UserProfileMock::TOKEN } }
   let(:params) { { project: { identifier: 'new-identifier', name: 'New name' } } }
 
@@ -13,29 +13,29 @@
   end
 
   it 'responds 200 OK' do
-    put("/api/public_projects/#{project.identifier}", headers:, params:)
+    put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:)
     expect(response).to have_http_status(:success)
   end
 
   it 'responds with the project JSON' do
-    put("/api/public_projects/#{project.identifier}", headers:, params:)
+    put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:)
     data = JSON.parse(response.body, symbolize_names: true)
 
     expect(data).to include(identifier: 'new-identifier', name: 'New name')
   end
 
   it 'responds 400 Bad Request when params are malformed' do
-    put("/api/public_projects/#{project.identifier}", headers:, params: {})
+    put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params: {})
     expect(response).to have_http_status(:bad_request)
   end
 
   it 'responds 401 Unauthorized when no token is given' do
-    put("/api/public_projects/#{project.identifier}", params:)
+    put("/api/public_projects/#{project.identifier}?project_type=scratch", params:)
     expect(response).to have_http_status(:unauthorized)
   end
 
   it 'responds 404 Not Found when project is not found' do
-    put('/api/public_projects/another-identifier', headers:, params:)
+    put('/api/public_projects/another-identifier?project_type=scratch', headers:, params:)
     expect(response).to have_http_status(:not_found)
   end
 end
diff --git a/spec/requests/public_projects/update_spec.rb b/spec/requests/public_projects/update_spec.rb
index 0342c92f1..4566ac58f 100644
--- a/spec/requests/public_projects/update_spec.rb
+++ b/spec/requests/public_projects/update_spec.rb
@@ -5,7 +5,7 @@
 RSpec.describe 'Update public project requests' do
   let(:locale) { 'fr' }
   let(:project_loader) { instance_double(ProjectLoader) }
-  let(:project) { create(:project, locale: 'en') }
+  let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH) }
   let(:creator) { build(:user) }
   let(:params) { { project: { identifier: 'new-identifier', name: 'New name' } } }
 
@@ -25,19 +25,19 @@
       end
 
       it 'builds ProjectLoader with identifier & locale' do
-        put("/api/public_projects/#{project.identifier}?locale=#{locale}", headers:, params:)
+        put("/api/public_projects/#{project.identifier}?project_type=scratch&locale=#{locale}", headers:, params:)
 
         expect(ProjectLoader).to have_received(:new).with(project.identifier, [locale])
       end
 
       it 'uses ProjectLoader#load to find the project based on identifier & locale' do
-        put("/api/public_projects/#{project.identifier}?locale=#{locale}", headers:, params:)
+        put("/api/public_projects/#{project.identifier}?project_type=scratch&locale=#{locale}", headers:, params:)
 
         expect(project_loader).to have_received(:load)
       end
 
       it 'returns success' do
-        put("/api/public_projects/#{project.identifier}", headers:, params:)
+        put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:)
 
         expect(response).to have_http_status(:success)
       end
@@ -53,7 +53,7 @@
       end
 
       it 'returns error' do
-        put("/api/public_projects/#{project.identifier}", headers:, params:)
+        put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:)
 
         expect(response).to have_http_status(:unprocessable_entity)
       end
@@ -62,7 +62,7 @@
 
   context 'when no token is given' do
     it 'returns unauthorized' do
-      put("/api/public_projects/#{project.identifier}", headers:, params:)
+      put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:)
 
       expect(response).to have_http_status(:unauthorized)
     end

From f802d9bf625a2df3ad2aecbdffee04f9134324b6 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 11:04:42 +0100
Subject: [PATCH 21/31] Introduce permission for updating public projects

I plan to use this to restrict updating public projects to experience-cs
admin users.
---
 app/models/ability.rb       | 5 ++++-
 spec/models/ability_spec.rb | 6 ++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/app/models/ability.rb b/app/models/ability.rb
index 1660892d9..9810671d7 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -103,7 +103,10 @@ def define_school_student_abilities(user:, school:)
   end
 
   def define_experience_cs_admin_abilities(user)
-    can :create, :public_project if user.experience_cs_admin?
+    return unless user.experience_cs_admin?
+
+    can :create, :public_project
+    can :update, :public_project
   end
 
   def school_teacher_can_manage_lesson?(user:, school:, lesson:)
diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb
index 9d7a40aea..fb6c3fa3f 100644
--- a/spec/models/ability_spec.rb
+++ b/spec/models/ability_spec.rb
@@ -31,6 +31,7 @@
       end
 
       it { is_expected.not_to be_able_to(:create, :public_project) }
+      it { is_expected.not_to be_able_to(:update, :public_project) }
     end
 
     context 'with a standard user' do
@@ -60,6 +61,7 @@
       end
 
       it { is_expected.not_to be_able_to(:create, :public_project) }
+      it { is_expected.not_to be_able_to(:update, :public_project) }
     end
 
     context 'with a teacher' do
@@ -89,6 +91,7 @@
       end
 
       it { is_expected.not_to be_able_to(:create, :public_project) }
+      it { is_expected.not_to be_able_to(:update, :public_project) }
     end
 
     context 'with an owner' do
@@ -118,6 +121,7 @@
       end
 
       it { is_expected.not_to be_able_to(:create, :public_project) }
+      it { is_expected.not_to be_able_to(:update, :public_project) }
     end
 
     context 'with a student' do
@@ -147,12 +151,14 @@
       end
 
       it { is_expected.not_to be_able_to(:create, :public_project) }
+      it { is_expected.not_to be_able_to(:update, :public_project) }
     end
 
     context 'with an experience-cs admin' do
       let(:user) { build(:experience_cs_admin_user) }
 
       it { is_expected.to be_able_to(:create, :public_project) }
+      it { is_expected.to be_able_to(:update, :public_project) }
     end
 
     # rubocop:disable RSpec/MultipleMemoizedHelpers

From 8a7adeed89c14796d7b3551a6df9a38d1d1c8eb2 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 11:10:08 +0100
Subject: [PATCH 22/31] Only experience-cs admins can update public projects

For the moment, we only want to allow experience-cs admin users to be
able to update public projects.
---
 app/controllers/api/public_projects_controller.rb     |  1 +
 .../public_project/updating_a_public_project_spec.rb  | 11 ++++++++++-
 spec/requests/public_projects/update_spec.rb          |  2 +-
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/app/controllers/api/public_projects_controller.rb b/app/controllers/api/public_projects_controller.rb
index 477525f94..a62ce66be 100644
--- a/app/controllers/api/public_projects_controller.rb
+++ b/app/controllers/api/public_projects_controller.rb
@@ -19,6 +19,7 @@ def create
     end
 
     def update
+      authorize! :update, :public_project
       result = PublicProject::Update.call(project: @project, update_hash: update_params)
 
       if result.success?
diff --git a/spec/features/public_project/updating_a_public_project_spec.rb b/spec/features/public_project/updating_a_public_project_spec.rb
index 378f6c006..12962bff7 100644
--- a/spec/features/public_project/updating_a_public_project_spec.rb
+++ b/spec/features/public_project/updating_a_public_project_spec.rb
@@ -3,7 +3,7 @@
 require 'rails_helper'
 
 RSpec.describe 'Updating a public project', type: :request do
-  let(:creator) { build(:user) }
+  let(:creator) { build(:experience_cs_admin_user) }
   let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH) }
   let(:headers) { { Authorization: UserProfileMock::TOKEN } }
   let(:params) { { project: { identifier: 'new-identifier', name: 'New name' } } }
@@ -24,6 +24,15 @@
     expect(data).to include(identifier: 'new-identifier', name: 'New name')
   end
 
+  context 'when creator is not an experience-cs admin' do
+    let(:creator) { build(:user) }
+
+    it 'responds 403 Forbidden' do
+      put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:)
+      expect(response).to have_http_status(:forbidden)
+    end
+  end
+
   it 'responds 400 Bad Request when params are malformed' do
     put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params: {})
     expect(response).to have_http_status(:bad_request)
diff --git a/spec/requests/public_projects/update_spec.rb b/spec/requests/public_projects/update_spec.rb
index 4566ac58f..74d7293dc 100644
--- a/spec/requests/public_projects/update_spec.rb
+++ b/spec/requests/public_projects/update_spec.rb
@@ -6,7 +6,7 @@
   let(:locale) { 'fr' }
   let(:project_loader) { instance_double(ProjectLoader) }
   let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH) }
-  let(:creator) { build(:user) }
+  let(:creator) { build(:experience_cs_admin_user) }
   let(:params) { { project: { identifier: 'new-identifier', name: 'New name' } } }
 
   context 'when auth is correct' do

From 2b6878c090d2e84be84d124161b9506e0ee48061 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 11:42:33 +0100
Subject: [PATCH 23/31] Forbid updating of non-public projects

This `Api::PublicProjectsController` is focussed on managing "public"
projects, i.e. those that have no `Project#user_id` set. I think it's
sensible to protect against the `update` action being used to update a
non-public project, because it makes the code easier to reason about.
---
 app/controllers/api/public_projects_controller.rb    |  7 +++++++
 .../public_project/updating_a_public_project_spec.rb | 12 +++++++++++-
 spec/requests/public_projects/update_spec.rb         |  2 +-
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/app/controllers/api/public_projects_controller.rb b/app/controllers/api/public_projects_controller.rb
index a62ce66be..345b5b2ff 100644
--- a/app/controllers/api/public_projects_controller.rb
+++ b/app/controllers/api/public_projects_controller.rb
@@ -5,6 +5,7 @@ class PublicProjectsController < ApiController
     before_action :authorize_user
     before_action :restrict_project_type, only: %i[create]
     before_action :load_project, only: %i[update]
+    before_action :restrict_to_public_projects, only: %i[update]
 
     def create
       authorize! :create, :public_project
@@ -52,5 +53,11 @@ def restrict_project_type
 
       raise CanCan::AccessDenied.new("#{project_type} not yet supported", :create, :public_project)
     end
+
+    def restrict_to_public_projects
+      return if @project.user_id.blank?
+
+      raise CanCan::AccessDenied.new('Cannot update non-public project', :update, :public_project)
+    end
   end
 end
diff --git a/spec/features/public_project/updating_a_public_project_spec.rb b/spec/features/public_project/updating_a_public_project_spec.rb
index 12962bff7..86edb8e62 100644
--- a/spec/features/public_project/updating_a_public_project_spec.rb
+++ b/spec/features/public_project/updating_a_public_project_spec.rb
@@ -4,7 +4,8 @@
 
 RSpec.describe 'Updating a public project', type: :request do
   let(:creator) { build(:experience_cs_admin_user) }
-  let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH) }
+  let(:user_id) { nil }
+  let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH, user_id:) }
   let(:headers) { { Authorization: UserProfileMock::TOKEN } }
   let(:params) { { project: { identifier: 'new-identifier', name: 'New name' } } }
 
@@ -43,6 +44,15 @@
     expect(response).to have_http_status(:unauthorized)
   end
 
+  context 'when project is not public' do
+    let(:user_id) { SecureRandom.uuid }
+
+    it 'responds 403 Forbidden' do
+      put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:)
+      expect(response).to have_http_status(:forbidden)
+    end
+  end
+
   it 'responds 404 Not Found when project is not found' do
     put('/api/public_projects/another-identifier?project_type=scratch', headers:, params:)
     expect(response).to have_http_status(:not_found)
diff --git a/spec/requests/public_projects/update_spec.rb b/spec/requests/public_projects/update_spec.rb
index 74d7293dc..ef591c13f 100644
--- a/spec/requests/public_projects/update_spec.rb
+++ b/spec/requests/public_projects/update_spec.rb
@@ -5,7 +5,7 @@
 RSpec.describe 'Update public project requests' do
   let(:locale) { 'fr' }
   let(:project_loader) { instance_double(ProjectLoader) }
-  let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH) }
+  let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH, user_id: nil) }
   let(:creator) { build(:experience_cs_admin_user) }
   let(:params) { { project: { identifier: 'new-identifier', name: 'New name' } } }
 

From 2651eddc9b2855e3d53110461ea881838174f7ac Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 14:33:30 +0100
Subject: [PATCH 24/31] Public project update cannot change certain attributes

There's currently no need to be able to *update* the `locale` or
`project_type` from experience-cs, but since we are supplying them for
the `create` action, I think it's useful to highlight that they're not
accepted by the `update` action.

It doesn't make much sense to allow the `user_id` to be updated via this
controller that specifically relates to "public" projects, so I think
it's useful to add an example to highlight that `user_id` is not
accepted by the `update` action.

This behaviour is currently effectively implemented by omitting these
attibute keys from the permitted params in
`Api::PublicProjectsController#update_params`.
---
 .../updating_a_public_project_spec.rb         | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/spec/features/public_project/updating_a_public_project_spec.rb b/spec/features/public_project/updating_a_public_project_spec.rb
index 86edb8e62..05bdb65e6 100644
--- a/spec/features/public_project/updating_a_public_project_spec.rb
+++ b/spec/features/public_project/updating_a_public_project_spec.rb
@@ -25,6 +25,33 @@
     expect(data).to include(identifier: 'new-identifier', name: 'New name')
   end
 
+  it 'does not change locale on project even if one is supplied' do
+    locale = 'fr'
+
+    put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params: params.merge(locale:))
+    data = JSON.parse(response.body, symbolize_names: true)
+
+    expect(data).to include(locale: 'en')
+  end
+
+  it 'does not change project_type on project even if one is supplied' do
+    project_type = Project::Types::PYTHON
+
+    put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params: params.merge(project_type:))
+    data = JSON.parse(response.body, symbolize_names: true)
+
+    expect(data).to include(project_type: Project::Types::SCRATCH)
+  end
+
+  it 'does not set user_id on project even if one is supplied' do
+    user_id = SecureRandom.uuid
+
+    put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params: params.merge(user_id:))
+    data = JSON.parse(response.body, symbolize_names: true)
+
+    expect(data).to include(user_id: nil)
+  end
+
   context 'when creator is not an experience-cs admin' do
     let(:creator) { build(:user) }
 

From 1c75c8c78bbf47579c2088622e4a25c259d905d2 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 14:45:58 +0100
Subject: [PATCH 25/31] Use PublicProject validations in PublicProject:Update

We can re-use the validations in the `PublicProject` wrapper class to
ensure that, in `Api::PublicProjectsController#update`, the project
identifier is updated to a non-blank value in the correct format and the
project name is updated to a non-blank value.
---
 .../public_project/operations/update.rb       |  5 +--
 spec/concepts/public_project/update_spec.rb   | 36 +++++++++++++++++++
 .../updating_a_public_project_spec.rb         |  5 +++
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/lib/concepts/public_project/operations/update.rb b/lib/concepts/public_project/operations/update.rb
index a88c9f84e..b1b4d143a 100644
--- a/lib/concepts/public_project/operations/update.rb
+++ b/lib/concepts/public_project/operations/update.rb
@@ -7,9 +7,10 @@ def call(project:, update_hash:)
         response = OperationResponse.new
 
         project.assign_attributes(update_hash)
-        project.save!
+        public_project = PublicProject.new(project)
+        public_project.save!
 
-        response[:project] = project
+        response[:project] = public_project.project
         response
       rescue StandardError => e
         Sentry.capture_exception(e)
diff --git a/spec/concepts/public_project/update_spec.rb b/spec/concepts/public_project/update_spec.rb
index 47a14218b..6cbfa1625 100644
--- a/spec/concepts/public_project/update_spec.rb
+++ b/spec/concepts/public_project/update_spec.rb
@@ -43,5 +43,41 @@
         expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
       end
     end
+
+    context 'when identifier is blank' do
+      let(:new_identifier) { nil }
+
+      it 'returns failure' do
+        expect(update_project.failure?).to be(true)
+      end
+
+      it 'returns error message' do
+        expect(update_project[:error]).to eq("Error updating project: Validation failed: Identifier can't be blank")
+      end
+    end
+
+    context 'when identifier is in invalid format' do
+      let(:new_identifier) { 'FooBarBaz' }
+
+      it 'returns failure' do
+        expect(update_project.failure?).to be(true)
+      end
+
+      it 'returns error message' do
+        expect(update_project[:error]).to eq('Error updating project: Validation failed: Identifier is invalid')
+      end
+    end
+
+    context 'when name is blank' do
+      let(:new_name) { '' }
+
+      it 'returns failure' do
+        expect(update_project.failure?).to be(true)
+      end
+
+      it 'returns error message' do
+        expect(update_project[:error]).to eq("Error updating project: Validation failed: Name can't be blank")
+      end
+    end
   end
 end
diff --git a/spec/features/public_project/updating_a_public_project_spec.rb b/spec/features/public_project/updating_a_public_project_spec.rb
index 05bdb65e6..3dd93d304 100644
--- a/spec/features/public_project/updating_a_public_project_spec.rb
+++ b/spec/features/public_project/updating_a_public_project_spec.rb
@@ -84,4 +84,9 @@
     put('/api/public_projects/another-identifier?project_type=scratch', headers:, params:)
     expect(response).to have_http_status(:not_found)
   end
+
+  it 'responds 422 Unprocessable Entity when params are invalid' do
+    put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params: { project: { name: '' } })
+    expect(response).to have_http_status(:unprocessable_entity)
+  end
 end

From dd4a00997f73ecc7bb07f3d4a95f75d247d73831 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 15:36:31 +0100
Subject: [PATCH 26/31] Add Api::PublicProjectsController#destroy

We want to allow experience-cs admins to update "public" projects via
experience-cs administrate UI [1]. When I say "public" projects, I mean
those that are accessible to all users, because `Project#user_id` is set
to `nil`.

Public projects for Python & HTML are currently updated via the
`GithubWebhooksController#github_push` [2] action and/or via the
(possibly defunct) `projects:create_all` rake task [3] both of which
make use of `ProjectImporter` [4]. However, it doesn't look as if
there's a way to delete an existing project through that mechanism and
in any case it's not very suitable for our use case.

This commit introduces a new `Api::PublicProjectsController#destroy`
action which is somewhat based on `Api::ProjectsController#destroy`.

As before I've tried to roughly follow the existing conventions however
much I disagree with them.

Unlike in `Api::ProjectsController#destroy`, the action responds with
422 Unprocessable Entity if `Project#destroy` returns `false`.

This is just a skeletal starting point - I plan to flesh it out in
subsequent commits.

However, I have already re-used the `ProjectLoader` for finding the
project by identifier and locale.

[1]: https://github.com/RaspberryPiFoundation/experience-cs/issues/405
[2]: https://github.com/RaspberryPiFoundation/editor-api/blob/f348dfc92fdd8b19fbfd5434e7751340f33b4093/app/controllers/github_webhooks_controller.rb#L6-L8
[3]: https://github.com/RaspberryPiFoundation/editor-api/blob/f348dfc92fdd8b19fbfd5434e7751340f33b4093/lib/tasks/projects.rake#L4-L7
[4]: https://github.com/RaspberryPiFoundation/editor-api/blob/f348dfc92fdd8b19fbfd5434e7751340f33b4093/lib/project_importer.rb
---
 .../api/public_projects_controller.rb         | 10 ++-
 config/routes.rb                              |  2 +-
 .../destroying_a_public_project_spec.rb       | 33 +++++++++
 spec/requests/public_projects/destroy_spec.rb | 67 +++++++++++++++++++
 4 files changed, 110 insertions(+), 2 deletions(-)
 create mode 100644 spec/features/public_project/destroying_a_public_project_spec.rb
 create mode 100644 spec/requests/public_projects/destroy_spec.rb

diff --git a/app/controllers/api/public_projects_controller.rb b/app/controllers/api/public_projects_controller.rb
index 345b5b2ff..f4a7ee736 100644
--- a/app/controllers/api/public_projects_controller.rb
+++ b/app/controllers/api/public_projects_controller.rb
@@ -4,7 +4,7 @@ module Api
   class PublicProjectsController < ApiController
     before_action :authorize_user
     before_action :restrict_project_type, only: %i[create]
-    before_action :load_project, only: %i[update]
+    before_action :load_project, only: %i[update destroy]
     before_action :restrict_to_public_projects, only: %i[update]
 
     def create
@@ -31,6 +31,14 @@ def update
       end
     end
 
+    def destroy
+      if @project.destroy
+        head :ok
+      else
+        head :unprocessable_entity
+      end
+    end
+
     private
 
     def load_project
diff --git a/config/routes.rb b/config/routes.rb
index e8254e01c..815a6289a 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -41,7 +41,7 @@
       resource :images, only: %i[show create], controller: 'projects/images'
     end
 
-    resources :public_projects, only: %i[create update]
+    resources :public_projects, only: %i[create update destroy]
 
     resource :project_errors, only: %i[create]
 
diff --git a/spec/features/public_project/destroying_a_public_project_spec.rb b/spec/features/public_project/destroying_a_public_project_spec.rb
new file mode 100644
index 000000000..24f267875
--- /dev/null
+++ b/spec/features/public_project/destroying_a_public_project_spec.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Destroying a public project', type: :request do
+  let(:destroyer) { build(:user) }
+  let(:project) { create(:project, locale: 'en') }
+  let(:headers) { { Authorization: UserProfileMock::TOKEN } }
+
+  before do
+    authenticated_in_hydra_as(destroyer)
+  end
+
+  it 'responds 200 OK' do
+    delete("/api/public_projects/#{project.identifier}", headers:)
+    expect(response).to have_http_status(:success)
+  end
+
+  it 'deletes the project' do
+    delete("/api/public_projects/#{project.identifier}", headers:)
+    expect(Project).not_to exist(identifier: project.identifier)
+  end
+
+  it 'responds 401 Unauthorized when no token is given' do
+    delete("/api/public_projects/#{project.identifier}")
+    expect(response).to have_http_status(:unauthorized)
+  end
+
+  it 'responds 404 Not Found when project is not found' do
+    delete('/api/public_projects/another-identifier', headers:)
+    expect(response).to have_http_status(:not_found)
+  end
+end
diff --git a/spec/requests/public_projects/destroy_spec.rb b/spec/requests/public_projects/destroy_spec.rb
new file mode 100644
index 000000000..2ac5a6c04
--- /dev/null
+++ b/spec/requests/public_projects/destroy_spec.rb
@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Destroy public project requests' do
+  let(:locale) { 'fr' }
+  let(:project_loader) { instance_double(ProjectLoader) }
+  let(:project) { create(:project, locale: 'en') }
+  let(:destroyer) { build(:user) }
+
+  context 'when auth is correct' do
+    let(:headers) { { Authorization: UserProfileMock::TOKEN } }
+
+    before do
+      allow(ProjectLoader).to receive(:new).and_return(project_loader)
+      allow(project_loader).to receive(:load).and_return(project)
+    end
+
+    context 'when destroying project is successful' do
+      before do
+        authenticated_in_hydra_as(destroyer)
+
+        allow(project).to receive(:destroy).and_return(true)
+      end
+
+      it 'builds ProjectLoader with identifier & locale' do
+        delete("/api/public_projects/#{project.identifier}?locale=#{locale}", headers:)
+
+        expect(ProjectLoader).to have_received(:new).with(project.identifier, [locale])
+      end
+
+      it 'uses ProjectLoader#load to find the project based on identifier & locale' do
+        delete("/api/public_projects/#{project.identifier}?locale=#{locale}", headers:)
+
+        expect(project_loader).to have_received(:load)
+      end
+
+      it 'returns success' do
+        delete("/api/public_projects/#{project.identifier}", headers:)
+
+        expect(response).to have_http_status(:success)
+      end
+    end
+
+    context 'when destroying project fails' do
+      before do
+        authenticated_in_hydra_as(destroyer)
+
+        allow(project).to receive(:destroy).and_return(false)
+      end
+
+      it 'returns error' do
+        delete("/api/public_projects/#{project.identifier}", headers:)
+
+        expect(response).to have_http_status(:unprocessable_entity)
+      end
+    end
+  end
+
+  context 'when no token is given' do
+    it 'returns unauthorized' do
+      delete("/api/public_projects/#{project.identifier}")
+
+      expect(response).to have_http_status(:unauthorized)
+    end
+  end
+end

From 4d86773e868214f4cc29916f21896b1d955cb1d6 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 15:52:44 +0100
Subject: [PATCH 27/31] Introduce permission for destroying public projects

I plan to use this to restrict destroying public projects to
experience-cs admin users.
---
 app/models/ability.rb       | 1 +
 spec/models/ability_spec.rb | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/app/models/ability.rb b/app/models/ability.rb
index 9810671d7..bd6cc7b5d 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -107,6 +107,7 @@ def define_experience_cs_admin_abilities(user)
 
     can :create, :public_project
     can :update, :public_project
+    can :destroy, :public_project
   end
 
   def school_teacher_can_manage_lesson?(user:, school:, lesson:)
diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb
index fb6c3fa3f..1e8d89908 100644
--- a/spec/models/ability_spec.rb
+++ b/spec/models/ability_spec.rb
@@ -32,6 +32,7 @@
 
       it { is_expected.not_to be_able_to(:create, :public_project) }
       it { is_expected.not_to be_able_to(:update, :public_project) }
+      it { is_expected.not_to be_able_to(:destroy, :public_project) }
     end
 
     context 'with a standard user' do
@@ -62,6 +63,7 @@
 
       it { is_expected.not_to be_able_to(:create, :public_project) }
       it { is_expected.not_to be_able_to(:update, :public_project) }
+      it { is_expected.not_to be_able_to(:destroy, :public_project) }
     end
 
     context 'with a teacher' do
@@ -92,6 +94,7 @@
 
       it { is_expected.not_to be_able_to(:create, :public_project) }
       it { is_expected.not_to be_able_to(:update, :public_project) }
+      it { is_expected.not_to be_able_to(:destroy, :public_project) }
     end
 
     context 'with an owner' do
@@ -122,6 +125,7 @@
 
       it { is_expected.not_to be_able_to(:create, :public_project) }
       it { is_expected.not_to be_able_to(:update, :public_project) }
+      it { is_expected.not_to be_able_to(:destroy, :public_project) }
     end
 
     context 'with a student' do
@@ -152,6 +156,7 @@
 
       it { is_expected.not_to be_able_to(:create, :public_project) }
       it { is_expected.not_to be_able_to(:update, :public_project) }
+      it { is_expected.not_to be_able_to(:destroy, :public_project) }
     end
 
     context 'with an experience-cs admin' do
@@ -159,6 +164,7 @@
 
       it { is_expected.to be_able_to(:create, :public_project) }
       it { is_expected.to be_able_to(:update, :public_project) }
+      it { is_expected.to be_able_to(:destroy, :public_project) }
     end
 
     # rubocop:disable RSpec/MultipleMemoizedHelpers

From 0b5c42ea175a946856097828c379c92f75a9ff00 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 15:56:08 +0100
Subject: [PATCH 28/31] Only experience-cs admins can destroy public projects

For the moment, we only want to allow experience-cs admin users to be
able to destroy public projects.
---
 app/controllers/api/public_projects_controller.rb     |  2 ++
 .../destroying_a_public_project_spec.rb               | 11 ++++++++++-
 spec/requests/public_projects/destroy_spec.rb         |  2 +-
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/app/controllers/api/public_projects_controller.rb b/app/controllers/api/public_projects_controller.rb
index f4a7ee736..3a19abf4b 100644
--- a/app/controllers/api/public_projects_controller.rb
+++ b/app/controllers/api/public_projects_controller.rb
@@ -32,6 +32,8 @@ def update
     end
 
     def destroy
+      authorize! :update, :public_project
+
       if @project.destroy
         head :ok
       else
diff --git a/spec/features/public_project/destroying_a_public_project_spec.rb b/spec/features/public_project/destroying_a_public_project_spec.rb
index 24f267875..9e71f29ad 100644
--- a/spec/features/public_project/destroying_a_public_project_spec.rb
+++ b/spec/features/public_project/destroying_a_public_project_spec.rb
@@ -3,7 +3,7 @@
 require 'rails_helper'
 
 RSpec.describe 'Destroying a public project', type: :request do
-  let(:destroyer) { build(:user) }
+  let(:destroyer) { build(:experience_cs_admin_user) }
   let(:project) { create(:project, locale: 'en') }
   let(:headers) { { Authorization: UserProfileMock::TOKEN } }
 
@@ -26,6 +26,15 @@
     expect(response).to have_http_status(:unauthorized)
   end
 
+  context 'when destroyer is not an experience-cs admin' do
+    let(:destroyer) { build(:user) }
+
+    it 'responds 403 Forbidden' do
+      delete("/api/public_projects/#{project.identifier}", headers:)
+      expect(response).to have_http_status(:forbidden)
+    end
+  end
+
   it 'responds 404 Not Found when project is not found' do
     delete('/api/public_projects/another-identifier', headers:)
     expect(response).to have_http_status(:not_found)
diff --git a/spec/requests/public_projects/destroy_spec.rb b/spec/requests/public_projects/destroy_spec.rb
index 2ac5a6c04..7021a651d 100644
--- a/spec/requests/public_projects/destroy_spec.rb
+++ b/spec/requests/public_projects/destroy_spec.rb
@@ -6,7 +6,7 @@
   let(:locale) { 'fr' }
   let(:project_loader) { instance_double(ProjectLoader) }
   let(:project) { create(:project, locale: 'en') }
-  let(:destroyer) { build(:user) }
+  let(:destroyer) { build(:experience_cs_admin_user) }
 
   context 'when auth is correct' do
     let(:headers) { { Authorization: UserProfileMock::TOKEN } }

From 0eb38804879fa17b676c8d82ed2066dec1209658 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 16:05:31 +0100
Subject: [PATCH 29/31] Make destroying public projects specs more realistic

The immediate use case for `Api::PublicProjectsController#destroy` is
for experience-cs admin users to destroy a public project.  Since
experience-cs only deals with Scratch projects, it makes sense to use
projects of this type in the specs. However, since we're using
`ProjectLoader` to find the project and the default scope is still in
place to hide Scratch projects, we need to make use of the
`ProjectScoping` concern to use the `project_type` param to set
`Current.project_scope` to include Scratch projects.
---
 .../destroying_a_public_project_spec.rb              | 12 ++++++------
 spec/requests/public_projects/destroy_spec.rb        | 12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/spec/features/public_project/destroying_a_public_project_spec.rb b/spec/features/public_project/destroying_a_public_project_spec.rb
index 9e71f29ad..6b0780fb4 100644
--- a/spec/features/public_project/destroying_a_public_project_spec.rb
+++ b/spec/features/public_project/destroying_a_public_project_spec.rb
@@ -4,7 +4,7 @@
 
 RSpec.describe 'Destroying a public project', type: :request do
   let(:destroyer) { build(:experience_cs_admin_user) }
-  let(:project) { create(:project, locale: 'en') }
+  let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH) }
   let(:headers) { { Authorization: UserProfileMock::TOKEN } }
 
   before do
@@ -12,17 +12,17 @@
   end
 
   it 'responds 200 OK' do
-    delete("/api/public_projects/#{project.identifier}", headers:)
+    delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:)
     expect(response).to have_http_status(:success)
   end
 
   it 'deletes the project' do
-    delete("/api/public_projects/#{project.identifier}", headers:)
+    delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:)
     expect(Project).not_to exist(identifier: project.identifier)
   end
 
   it 'responds 401 Unauthorized when no token is given' do
-    delete("/api/public_projects/#{project.identifier}")
+    delete("/api/public_projects/#{project.identifier}?project_type=scratch")
     expect(response).to have_http_status(:unauthorized)
   end
 
@@ -30,13 +30,13 @@
     let(:destroyer) { build(:user) }
 
     it 'responds 403 Forbidden' do
-      delete("/api/public_projects/#{project.identifier}", headers:)
+      delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:)
       expect(response).to have_http_status(:forbidden)
     end
   end
 
   it 'responds 404 Not Found when project is not found' do
-    delete('/api/public_projects/another-identifier', headers:)
+    delete('/api/public_projects/another-identifier?project_type=scratch', headers:)
     expect(response).to have_http_status(:not_found)
   end
 end
diff --git a/spec/requests/public_projects/destroy_spec.rb b/spec/requests/public_projects/destroy_spec.rb
index 7021a651d..f34d2a59c 100644
--- a/spec/requests/public_projects/destroy_spec.rb
+++ b/spec/requests/public_projects/destroy_spec.rb
@@ -5,7 +5,7 @@
 RSpec.describe 'Destroy public project requests' do
   let(:locale) { 'fr' }
   let(:project_loader) { instance_double(ProjectLoader) }
-  let(:project) { create(:project, locale: 'en') }
+  let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH) }
   let(:destroyer) { build(:experience_cs_admin_user) }
 
   context 'when auth is correct' do
@@ -24,19 +24,19 @@
       end
 
       it 'builds ProjectLoader with identifier & locale' do
-        delete("/api/public_projects/#{project.identifier}?locale=#{locale}", headers:)
+        delete("/api/public_projects/#{project.identifier}?project_type=scratch&locale=#{locale}", headers:)
 
         expect(ProjectLoader).to have_received(:new).with(project.identifier, [locale])
       end
 
       it 'uses ProjectLoader#load to find the project based on identifier & locale' do
-        delete("/api/public_projects/#{project.identifier}?locale=#{locale}", headers:)
+        delete("/api/public_projects/#{project.identifier}?project_type=scratch&locale=#{locale}", headers:)
 
         expect(project_loader).to have_received(:load)
       end
 
       it 'returns success' do
-        delete("/api/public_projects/#{project.identifier}", headers:)
+        delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:)
 
         expect(response).to have_http_status(:success)
       end
@@ -50,7 +50,7 @@
       end
 
       it 'returns error' do
-        delete("/api/public_projects/#{project.identifier}", headers:)
+        delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:)
 
         expect(response).to have_http_status(:unprocessable_entity)
       end
@@ -59,7 +59,7 @@
 
   context 'when no token is given' do
     it 'returns unauthorized' do
-      delete("/api/public_projects/#{project.identifier}")
+      delete("/api/public_projects/#{project.identifier}?project_type=scratch")
 
       expect(response).to have_http_status(:unauthorized)
     end

From adea633f3190620511bf8a11168c959ea99151ef Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 17:13:04 +0100
Subject: [PATCH 30/31] Forbid destroying of non-public projects

This `Api::PublicProjectsController` is focussed on managing "public"
projects, i.e. those that have no `Project#user_id` set. I think it's
sensible to protect against the `destroy` action being used to update a
non-public project, because it makes the code easier to reason about.
---
 app/controllers/api/public_projects_controller.rb    |  2 +-
 .../destroying_a_public_project_spec.rb              | 12 +++++++++++-
 spec/requests/public_projects/destroy_spec.rb        |  2 +-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/app/controllers/api/public_projects_controller.rb b/app/controllers/api/public_projects_controller.rb
index 3a19abf4b..59c1ada37 100644
--- a/app/controllers/api/public_projects_controller.rb
+++ b/app/controllers/api/public_projects_controller.rb
@@ -5,7 +5,7 @@ class PublicProjectsController < ApiController
     before_action :authorize_user
     before_action :restrict_project_type, only: %i[create]
     before_action :load_project, only: %i[update destroy]
-    before_action :restrict_to_public_projects, only: %i[update]
+    before_action :restrict_to_public_projects, only: %i[update destroy]
 
     def create
       authorize! :create, :public_project
diff --git a/spec/features/public_project/destroying_a_public_project_spec.rb b/spec/features/public_project/destroying_a_public_project_spec.rb
index 6b0780fb4..34e3e8d2e 100644
--- a/spec/features/public_project/destroying_a_public_project_spec.rb
+++ b/spec/features/public_project/destroying_a_public_project_spec.rb
@@ -4,7 +4,8 @@
 
 RSpec.describe 'Destroying a public project', type: :request do
   let(:destroyer) { build(:experience_cs_admin_user) }
-  let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH) }
+  let(:user_id) { nil }
+  let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH, user_id:) }
   let(:headers) { { Authorization: UserProfileMock::TOKEN } }
 
   before do
@@ -35,6 +36,15 @@
     end
   end
 
+  context 'when project is not public' do
+    let(:user_id) { SecureRandom.uuid }
+
+    it 'responds 403 Forbidden' do
+      delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:)
+      expect(response).to have_http_status(:forbidden)
+    end
+  end
+
   it 'responds 404 Not Found when project is not found' do
     delete('/api/public_projects/another-identifier?project_type=scratch', headers:)
     expect(response).to have_http_status(:not_found)
diff --git a/spec/requests/public_projects/destroy_spec.rb b/spec/requests/public_projects/destroy_spec.rb
index f34d2a59c..3b50e5404 100644
--- a/spec/requests/public_projects/destroy_spec.rb
+++ b/spec/requests/public_projects/destroy_spec.rb
@@ -5,7 +5,7 @@
 RSpec.describe 'Destroy public project requests' do
   let(:locale) { 'fr' }
   let(:project_loader) { instance_double(ProjectLoader) }
-  let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH) }
+  let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH, user_id: nil) }
   let(:destroyer) { build(:experience_cs_admin_user) }
 
   context 'when auth is correct' do

From 1b08bbd085f34ad8d8397caf89277099ec18f7dd Mon Sep 17 00:00:00 2001
From: James Mead <james.mead+rpf@gofreerange.com>
Date: Fri, 23 May 2025 17:18:45 +0100
Subject: [PATCH 31/31] Prevent destruction of public project with remixes

The only use case for `Api::PublicProjectsController#destroy` is
currently to allow experience-cs admins to delete a project. Although
the `Project` model allows such a deletion, it's not obvious that it's
entirely sensible. And in any case, while the Learning Team is using
the experience-cs admin UI to create projects for the curriculum,
there's little danger that they will have remixes until they're
associated with lessons, etc. So this seems like a safer option for now.
We can always relax it later if it becomes a problem.
---
 app/controllers/api/public_projects_controller.rb     |  7 +++++++
 .../destroying_a_public_project_spec.rb               | 11 +++++++++++
 2 files changed, 18 insertions(+)

diff --git a/app/controllers/api/public_projects_controller.rb b/app/controllers/api/public_projects_controller.rb
index 59c1ada37..f3eebdf68 100644
--- a/app/controllers/api/public_projects_controller.rb
+++ b/app/controllers/api/public_projects_controller.rb
@@ -6,6 +6,7 @@ class PublicProjectsController < ApiController
     before_action :restrict_project_type, only: %i[create]
     before_action :load_project, only: %i[update destroy]
     before_action :restrict_to_public_projects, only: %i[update destroy]
+    before_action :prevent_destruction_of_public_project_with_remixes, only: %i[destroy]
 
     def create
       authorize! :create, :public_project
@@ -69,5 +70,11 @@ def restrict_to_public_projects
 
       raise CanCan::AccessDenied.new('Cannot update non-public project', :update, :public_project)
     end
+
+    def prevent_destruction_of_public_project_with_remixes
+      return if @project.remixes.none?
+
+      raise CanCan::AccessDenied.new('Cannot destroy public project with remixes', :update, :public_project)
+    end
   end
 end
diff --git a/spec/features/public_project/destroying_a_public_project_spec.rb b/spec/features/public_project/destroying_a_public_project_spec.rb
index 34e3e8d2e..d580ba681 100644
--- a/spec/features/public_project/destroying_a_public_project_spec.rb
+++ b/spec/features/public_project/destroying_a_public_project_spec.rb
@@ -45,6 +45,17 @@
     end
   end
 
+  context 'when project has one or more remixes' do
+    before do
+      project.remixes.create!(attributes_for(:project))
+    end
+
+    it 'responds 403 Forbidden' do
+      delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:)
+      expect(response).to have_http_status(:forbidden)
+    end
+  end
+
   it 'responds 404 Not Found when project is not found' do
     delete('/api/public_projects/another-identifier?project_type=scratch', headers:)
     expect(response).to have_http_status(:not_found)