From 4e0fa87f07589fb39ad73f544cc131bb742cb3bb Mon Sep 17 00:00:00 2001
From: Chris Zetter <253059100+zetter-rpf@users.noreply.github.com>
Date: Fri, 7 Aug 2026 14:04:51 +0100
Subject: [PATCH 1/2] Support instruction steps format for project instructions

Previously project instructions could only be a single markdown
string, with no way to break guidance into discrete, orderable
steps.

This change adds an instruction_steps jsonb column and stores the
new array-of-steps format there, leaving the existing instructions
text column untouched - it never needs to change type, so there is
no risk of a running server caching a stale column type against it.
Project#instructions reads instruction_steps when present and falls
back to the legacy text column otherwise; the controller permits
either a plain string or an array of {markdown_content} steps under
the same instructions param, so no other call site needs to know
about the split.

Existing rows keep instruction_steps nil until a project is saved
through the new format - no backfill needed.
---
 .../api/projects/remixes_controller.rb         |  3 ++-
 app/controllers/api/projects_controller.rb     |  3 ++-
 app/models/project.rb                          | 12 ++++++++++++
 ...120000_add_instruction_steps_to_projects.rb |  7 +++++++
 db/schema.rb                                   |  3 ++-
 lib/concepts/project/operations/update.rb      |  9 ++++++++-
 spec/models/project_spec.rb                    | 18 ++++++++++++++++++
 spec/requests/projects/show_spec.rb            | 10 ++++++++++
 spec/requests/projects/update_spec.rb          |  8 ++++++++
 9 files changed, 69 insertions(+), 4 deletions(-)
 create mode 100644 db/migrate/20260807120000_add_instruction_steps_to_projects.rb

diff --git a/app/controllers/api/projects/remixes_controller.rb b/app/controllers/api/projects/remixes_controller.rb
index acfcec769..797a10d82 100644
--- a/app/controllers/api/projects/remixes_controller.rb
+++ b/app/controllers/api/projects/remixes_controller.rb
@@ -82,7 +82,8 @@ def remix_params
                             :instructions,
                             {
                               image_list: [],
-                              components: [%i[id name extension content index]]
+                              components: [%i[id name extension content index]],
+                              instructions: [[:markdown_content]]
                             }])
       end
     end
diff --git a/app/controllers/api/projects_controller.rb b/app/controllers/api/projects_controller.rb
index b6897fc32..d9708f0aa 100644
--- a/app/controllers/api/projects_controller.rb
+++ b/app/controllers/api/projects_controller.rb
@@ -104,7 +104,8 @@ def base_params
         :locale,
         :instructions,
         {
-          components: %i[id name extension content index default]
+          components: %i[id name extension content index default],
+          instructions: [:markdown_content]
         },
         scratch_component: {},
         parent: {},
diff --git a/app/models/project.rb b/app/models/project.rb
index 52b8631a4..6d58e4701 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -74,6 +74,18 @@ def scratch_component=(value)
     super(value.is_a?(Hash) ? ScratchComponent.new(value) : value)
   end
 
+  def instructions
+    self[:instruction_steps].nil? ? self[:instructions] : self[:instruction_steps]
+  end
+
+  def instructions=(value)
+    if value.is_a?(Array)
+      self[:instruction_steps] = value
+    else
+      self[:instructions] = value
+    end
+  end
+
   def last_edited_at
     # datetime that the project or one of its components was last updated
     [updated_at, components.maximum(:updated_at)].compact.max
diff --git a/db/migrate/20260807120000_add_instruction_steps_to_projects.rb b/db/migrate/20260807120000_add_instruction_steps_to_projects.rb
new file mode 100644
index 000000000..75de32002
--- /dev/null
+++ b/db/migrate/20260807120000_add_instruction_steps_to_projects.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddInstructionStepsToProjects < ActiveRecord::Migration[8.1]
+  def change
+    add_column :projects, :instruction_steps, :jsonb
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 74dc5e40b..0403b6037 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.1].define(version: 2026_07_03_151018) do
+ActiveRecord::Schema[8.1].define(version: 2026_08_07_120000) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_catalog.plpgsql"
   enable_extension "pgcrypto"
@@ -235,6 +235,7 @@
   create_table "projects", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
     t.datetime "created_at", null: false
     t.string "identifier", null: false
+    t.jsonb "instruction_steps"
     t.text "instructions"
     t.uuid "lesson_id"
     t.string "locale"
diff --git a/lib/concepts/project/operations/update.rb b/lib/concepts/project/operations/update.rb
index 860f3cb0f..3c40cea6b 100644
--- a/lib/concepts/project/operations/update.rb
+++ b/lib/concepts/project/operations/update.rb
@@ -45,10 +45,17 @@ def validate_deletions(response)
       def student_project_instructions_updated?(response, update_hash, current_user)
         is_school_project = response[:project].school.present?
         user_is_student = current_user.student?
-        instructions_updated = response[:project].instructions != update_hash[:instructions]
+        instructions_updated = normalize_instructions(response[:project].instructions) != normalize_instructions(update_hash[:instructions])
         is_school_project && user_is_student && instructions_updated
       end
 
+      # update_hash[:instructions] may be plain Ruby data or an ActionController::Parameters
+      # array/hash (for the instruction steps format); round-trip through JSON so both sides
+      # of the comparison are in the same plain, comparable shape.
+      def normalize_instructions(value)
+        ActiveSupport::JSON.decode(ActiveSupport::JSON.encode(value))
+      end
+
       def validate_update(response, update_hash, current_user)
         return unless student_project_instructions_updated?(response, update_hash, current_user)
 
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 22bc48fda..814463e93 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -344,6 +344,24 @@
     end
   end
 
+  describe '#instructions' do
+    let(:project) { create(:project, :with_instructions, school:, user_id: create(:teacher, school:).id) }
+
+    it 'falls back to the legacy text column when instruction_steps has never been set' do
+      expect(project.instructions).to eq(project[:instructions])
+    end
+
+    it 'returns instruction_steps once set, without touching the legacy column' do
+      project.update!(instructions: [{ markdown_content: 'step 1' }])
+      expect(project.instructions).to eq([{ 'markdown_content' => 'step 1' }])
+    end
+
+    it 'does not fall back to stale legacy text once instruction_steps is set to an empty array' do
+      project.update!(instructions: [])
+      expect(project.instructions).to eq([])
+    end
+  end
+
   describe 'auditing' do
     let(:school) { create(:school) }
     let(:teacher) { create(:teacher, school:) }
diff --git a/spec/requests/projects/show_spec.rb b/spec/requests/projects/show_spec.rb
index 3cb701861..ac400d33f 100644
--- a/spec/requests/projects/show_spec.rb
+++ b/spec/requests/projects/show_spec.rb
@@ -54,6 +54,16 @@
         get("/api/projects/#{project.identifier}", headers:)
         expect(response.parsed_body).not_to have_key('finished')
       end
+
+      it 'returns instructions in the instruction steps format when saved that way' do
+        project.update!(instructions: [{ markdown_content: 'step 1' }, { markdown_content: 'step 2' }])
+
+        get("/api/projects/#{project.identifier}", headers:)
+
+        expect(response.parsed_body['instructions']).to eq(
+          [{ 'markdown_content' => 'step 1' }, { 'markdown_content' => 'step 2' }]
+        )
+      end
     end
 
     context 'when loading a student\'s project' do
diff --git a/spec/requests/projects/update_spec.rb b/spec/requests/projects/update_spec.rb
index 5cf4b3664..d7464627a 100644
--- a/spec/requests/projects/update_spec.rb
+++ b/spec/requests/projects/update_spec.rb
@@ -124,6 +124,14 @@
       put("/api/projects/#{project.identifier}", params:, headers:)
       expect(response.body).to include('updated instructions')
     end
+
+    it 'saves and returns instructions in the instruction steps format' do
+      params[:project][:instructions] = [{ markdown_content: 'step 1' }, { markdown_content: 'step 2' }]
+      put("/api/projects/#{project.identifier}", params:, headers:)
+
+      expect(project.reload.instructions).to eq([{ 'markdown_content' => 'step 1' }, { 'markdown_content' => 'step 2' }])
+      expect(response.parsed_body['instructions']).to eq([{ 'markdown_content' => 'step 1' }, { 'markdown_content' => 'step 2' }])
+    end
   end
 
   context 'when authed user is a teacher updating a class project' do

From 1562b289af43d4f9a3db679b59c824aa7b39a3b5 Mon Sep 17 00:00:00 2001
From: Chris Zetter <253059100+zetter-rpf@users.noreply.github.com>
Date: Fri, 7 Aug 2026 10:40:46 +0100
Subject: [PATCH 2/2] Move student instructions restriction into strong params

Previously Project::Update compared old and new instructions to
detect and reject a student trying to change them, needing a
current_user argument and a JSON-normalising comparison just to
tell Parameters and plain Ruby values apart.

This change drops :instructions from the permitted params entirely
when the current user is a student, so their attempt is filtered
out the same way Rails treats any other unpermitted param. The
request now succeeds with the change silently ignored, instead of
returning a 422. Project::Update no longer needs to know who the
current user is.
---
 app/controllers/api/projects_controller.rb    | 17 ++++++----
 config/locales/en.yml                         |  1 -
 lib/concepts/project/operations/update.rb     | 27 ++--------------
 .../project/update_default_component_spec.rb  |  3 +-
 .../project/update_delete_components_spec.rb  |  3 +-
 spec/concepts/project/update_invalid_spec.rb  |  3 +-
 spec/concepts/project/update_spec.rb          | 31 ++-----------------
 spec/requests/projects/update_spec.rb         |  6 ++--
 8 files changed, 24 insertions(+), 67 deletions(-)

diff --git a/app/controllers/api/projects_controller.rb b/app/controllers/api/projects_controller.rb
index d9708f0aa..e9919976a 100644
--- a/app/controllers/api/projects_controller.rb
+++ b/app/controllers/api/projects_controller.rb
@@ -41,7 +41,7 @@ def create
     end
 
     def update
-      result = Project::Update.call(project: @project, update_hash: project_params, current_user:)
+      result = Project::Update.call(project: @project, update_hash: project_params)
 
       if result.success?
         track_project_event('Project - Saved', @project)
@@ -94,7 +94,11 @@ def project_params
     end
 
     def base_params
-      params.fetch(:project, {}).permit(
+      params.fetch(:project, {}).permit(*permitted_project_attributes)
+    end
+
+    def permitted_project_attributes
+      attributes = [
         :school_id,
         :lesson_id,
         :user_id,
@@ -102,15 +106,16 @@ def base_params
         :name,
         :project_type,
         :locale,
-        :instructions,
         {
-          components: %i[id name extension content index default],
-          instructions: [:markdown_content]
+          components: %i[id name extension content index default]
         },
         scratch_component: {},
         parent: {},
         image_list: []
-      )
+      ]
+      return attributes if current_user&.student?
+
+      attributes + [:instructions, { instructions: [:markdown_content] }]
     end
 
     def school_owner?
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 664fc5670..7c9c88e49 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -9,7 +9,6 @@ en:
         delete_default_component: "Cannot delete default file"
         change_default_name: "Cannot amend default file name"
         change_default_extension: "Cannot amend default file extension"
-        student_update_instructions: "Student cannot update project instructions"
       remixing:
         invalid_params: "Invalid parameters"
         cannot_save: "Cannot create project remix"
diff --git a/lib/concepts/project/operations/update.rb b/lib/concepts/project/operations/update.rb
index 3c40cea6b..1e0e64166 100644
--- a/lib/concepts/project/operations/update.rb
+++ b/lib/concepts/project/operations/update.rb
@@ -3,11 +3,11 @@
 class Project
   class Update
     class << self
-      def call(project:, update_hash:, current_user:)
+      def call(project:, update_hash:)
         response = setup_response(project)
 
         setup_deletions(response, update_hash)
-        update_project_attributes(response, update_hash, current_user)
+        update_project_attributes(response, update_hash)
         update_component_attributes(response, update_hash)
         persist_changes(response)
         response
@@ -42,28 +42,7 @@ def validate_deletions(response)
         response[:error] = I18n.t 'errors.project.editing.delete_default_component'
       end
 
-      def student_project_instructions_updated?(response, update_hash, current_user)
-        is_school_project = response[:project].school.present?
-        user_is_student = current_user.student?
-        instructions_updated = normalize_instructions(response[:project].instructions) != normalize_instructions(update_hash[:instructions])
-        is_school_project && user_is_student && instructions_updated
-      end
-
-      # update_hash[:instructions] may be plain Ruby data or an ActionController::Parameters
-      # array/hash (for the instruction steps format); round-trip through JSON so both sides
-      # of the comparison are in the same plain, comparable shape.
-      def normalize_instructions(value)
-        ActiveSupport::JSON.decode(ActiveSupport::JSON.encode(value))
-      end
-
-      def validate_update(response, update_hash, current_user)
-        return unless student_project_instructions_updated?(response, update_hash, current_user)
-
-        response[:error] = I18n.t 'errors.project.editing.student_update_instructions'
-      end
-
-      def update_project_attributes(response, update_hash, current_user)
-        validate_update(response, update_hash, current_user)
+      def update_project_attributes(response, update_hash)
         return if response.failure?
 
         response[:project].assign_attributes(update_hash.slice(:name, :instructions))
diff --git a/spec/concepts/project/update_default_component_spec.rb b/spec/concepts/project/update_default_component_spec.rb
index 6685977ca..5960fcc61 100644
--- a/spec/concepts/project/update_default_component_spec.rb
+++ b/spec/concepts/project/update_default_component_spec.rb
@@ -3,9 +3,8 @@
 require 'rails_helper'
 
 RSpec.describe Project::Update, type: :unit do
-  subject(:update) { described_class.call(project:, update_hash:, current_user:) }
+  subject(:update) { described_class.call(project:, update_hash:) }
 
-  let(:current_user) { create(:user) }
   let!(:project) { create(:project, :with_default_component) }
   let(:default_component) { project.components.first }
 
diff --git a/spec/concepts/project/update_delete_components_spec.rb b/spec/concepts/project/update_delete_components_spec.rb
index 0fb64af55..18a1aadb0 100644
--- a/spec/concepts/project/update_delete_components_spec.rb
+++ b/spec/concepts/project/update_delete_components_spec.rb
@@ -3,9 +3,8 @@
 require 'rails_helper'
 
 RSpec.describe Project::Update, type: :unit do
-  subject(:update) { described_class.call(project:, update_hash:, current_user:) }
+  subject(:update) { described_class.call(project:, update_hash:) }
 
-  let(:current_user) { create(:user) }
   let!(:project) { create(:project, :with_default_component, :with_components) }
   let(:component_to_delete) { project.components.last }
   let(:default_component) { project.components.first }
diff --git a/spec/concepts/project/update_invalid_spec.rb b/spec/concepts/project/update_invalid_spec.rb
index 84a825140..7c2560f56 100644
--- a/spec/concepts/project/update_invalid_spec.rb
+++ b/spec/concepts/project/update_invalid_spec.rb
@@ -8,10 +8,9 @@
       name: 'updated project name',
       components: [default_component_hash, edited_component_hash, new_component_hash]
     }
-    described_class.call(project:, update_hash:, current_user:)
+    described_class.call(project:, update_hash:)
   end
 
-  let(:current_user) { create(:user) }
   let!(:project) { create(:project, :with_default_component, :with_components, component_count: 2) }
   let(:editable_component) { project.components.last }
   let(:default_component) { project.components.first }
diff --git a/spec/concepts/project/update_spec.rb b/spec/concepts/project/update_spec.rb
index 153b68946..e7466bd57 100644
--- a/spec/concepts/project/update_spec.rb
+++ b/spec/concepts/project/update_spec.rb
@@ -9,10 +9,9 @@
       components: component_hash,
       instructions:
     }
-    described_class.call(project:, update_hash:, current_user:)
+    described_class.call(project:, update_hash:)
   end
 
-  let(:current_user) { create(:user) }
   let!(:project) { create(:project, :with_default_component, :with_components) }
   let(:editable_component) { project.components.last }
   let(:default_component) { project.components.first }
@@ -97,10 +96,9 @@
       end
     end
 
-    context 'when the instructions have changed and the current user is a teacher' do
+    context 'when the instructions have changed' do
       let(:school) { create(:school) }
-      let!(:current_user) { create(:teacher, school:) }
-      let!(:project) { create(:project, :with_instructions, school:, user_id: current_user.id) }
+      let!(:project) { create(:project, :with_instructions, school:, user_id: create(:teacher, school:).id) }
       let(:instructions) { 'new instructions' }
 
       it 'returns success? true' do
@@ -111,29 +109,6 @@
         expect { update }.to change { project.reload.instructions }.to('new instructions')
       end
     end
-
-    context 'when the instructions have changed and the current user is a student' do
-      let(:school) { create(:school) }
-      let!(:current_user) { create(:student, school:) }
-      let!(:project) { create(:project, :with_instructions, school:, user_id: current_user.id) }
-      let(:instructions) { 'new instructions' }
-
-      it 'returns success? false' do
-        expect(update.success?).to be(false)
-      end
-
-      it 'does not update project name' do
-        expect { update }.not_to change { project.reload.name }
-      end
-
-      it 'does not update project instructions' do
-        expect { update }.not_to change { project.reload.instructions }
-      end
-
-      it 'returns an error message' do
-        expect(update[:error]).to eq('Student cannot update project instructions')
-      end
-    end
   end
 
   def component_properties_hash(component)
diff --git a/spec/requests/projects/update_spec.rb b/spec/requests/projects/update_spec.rb
index d7464627a..dc184ddc3 100644
--- a/spec/requests/projects/update_spec.rb
+++ b/spec/requests/projects/update_spec.rb
@@ -205,10 +205,12 @@
       expect(response).to have_http_status(:ok)
     end
 
-    it 'returns unprocessable entity if instructions updated' do
+    it 'ignores an attempt to update instructions' do
       params[:project][:instructions] = 'updated instructions'
       put("/api/projects/#{project.identifier}", params:, headers:)
-      expect(response).to have_http_status(:unprocessable_content)
+
+      expect(response).to have_http_status(:ok)
+      expect(project.reload.instructions).to be_nil
     end
 
     it 'records a project saved event' do