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..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,14 +106,16 @@ def base_params :name, :project_type, :locale, - :instructions, { 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/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/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/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..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,21 +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 = response[:project].instructions != update_hash[:instructions] - is_school_project && user_is_student && instructions_updated - 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/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..dc184ddc3 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 @@ -197,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