From 069cc4377262e59cc20f0b9dea85d80b8390ebf9 Mon Sep 17 00:00:00 2001 From: Jamie Benstead Date: Mon, 19 May 2025 17:36:44 +0100 Subject: [PATCH 1/9] Start with creating remix from lesson functionality --- app/controllers/api/lessons_controller.rb | 25 ++++++++++- app/models/ability.rb | 2 + config/routes.rb | 1 + .../lesson/operations/create_remix.rb | 44 +++++++++++++++++++ .../project/operations/create_remix.rb | 10 +++-- 5 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 lib/concepts/lesson/operations/create_remix.rb diff --git a/app/controllers/api/lessons_controller.rb b/app/controllers/api/lessons_controller.rb index 305b2a632..b167e3d1b 100644 --- a/app/controllers/api/lessons_controller.rb +++ b/app/controllers/api/lessons_controller.rb @@ -4,7 +4,7 @@ module Api class LessonsController < ApiController before_action :authorize_user, except: %i[index show] before_action :verify_school_class_belongs_to_school, only: :create - load_and_authorize_resource :lesson + load_and_authorize_resource :lesson, except: [:create_lesson_with_remix] def index archive_scope = params[:include_archived] == 'true' ? Lesson : Lesson.unarchived @@ -41,6 +41,21 @@ def create_copy end end + def create_lesson_with_remix + remix_origin = request.origin || request.referer + + puts("lesson_params: #{lesson_params}") + + result = Lesson::CreateRemix.call(lesson_params: lesson_params, remix_origin:) + + if result.success? + @lesson_with_user = result[:lesson].with_user + render :show, formats: [:json], status: :created + else + render json: { error: result[:error] }, status: :unprocessable_entity + end + end + def update # TODO: Consider removing user_id from the lesson_params for update so users can update other users' lessons without changing ownership # OR consider dropping user_id on lessons and using teacher id/ids on the class instead @@ -75,9 +90,16 @@ def verify_school_class_belongs_to_school end def lesson_params + puts("base_params: #{base_params}") base_params.merge(user_id: current_user.id) end + def remix_lesson_params + lesson_params.merge(params.fetch(:lesson, {}).permit( + { project_attributes: [:identifier] } + )) + end + def base_params params.fetch(:lesson, {}).permit( :school_id, @@ -86,6 +108,7 @@ def base_params :description, :visibility, :due_date, + :project_identifier, { project_attributes: [ :name, diff --git a/app/models/ability.rb b/app/models/ability.rb index bca1f2dfa..aa682d3e7 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -67,6 +67,7 @@ def define_school_owner_abilities(school:) can(%i[read create create_batch update destroy], :school_student) can(%i[create create_copy], Lesson, school_id: school.id) can(%i[read update destroy], Lesson, school_id: school.id, visibility: %w[teachers students public]) + can(%i[remix], Lesson) end def define_school_teacher_abilities(user:, school:) @@ -81,6 +82,7 @@ def define_school_teacher_abilities(user:, school:) can(%i[create update destroy], Lesson) do |lesson| school_teacher_can_manage_lesson?(user:, school:, lesson:) end + can(%i[remix], Lesson) can(%i[read create_copy], Lesson, school_id: school.id, visibility: %w[teachers students]) can(%i[create], Project) do |project| school_teacher_can_manage_project?(user:, school:, project:) diff --git a/config/routes.rb b/config/routes.rb index f48edf7f1..7dad0c4aa 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -61,6 +61,7 @@ resources :lessons, only: %i[index create show update destroy] do post :copy, on: :member, to: 'lessons#create_copy' + post :remix, on: :collection, to: 'lessons#create_lesson_with_remix' end resources :teacher_invitations, param: :token, only: :show do diff --git a/lib/concepts/lesson/operations/create_remix.rb b/lib/concepts/lesson/operations/create_remix.rb new file mode 100644 index 000000000..54f305260 --- /dev/null +++ b/lib/concepts/lesson/operations/create_remix.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +class Lesson + class CreateRemix + class << self + def call(lesson_params:, remix_origin:) + ActiveRecord::Base.transaction do + response = OperationResponse.new + response[:lesson] = build_remix(lesson_params, remix_origin) + response[:lesson].save! + response + rescue StandardError => e + Sentry.capture_exception(e) + pp(e) + errors = response[:lesson].errors.full_messages.join(',') + response[:error] = "Error creating remix of lesson: #{errors}" + response + end + end + + private + + def build_remix(lesson_params, remix_origin) + original_project = Project.find_by(identifier: lesson_params[:project_identifier]) + lesson_copy = Lesson.new(name: original_project.name) + filtered_params = lesson_params.except(:project_identifier) + lesson_copy.assign_attributes(filtered_params) + lesson_copy.project = build_project_remix(original_project, lesson_params, remix_origin) + + lesson_copy + end + + def build_project_remix(original_project, lesson_params, remix_origin) + response = Project::CreateRemix.call( + params: {school_id: lesson_params[:school_id]}, + user_id: lesson_params[:user_id], + original_project: original_project, + remix_origin: remix_origin + ) + response[:project] + end + end + end +end diff --git a/lib/concepts/project/operations/create_remix.rb b/lib/concepts/project/operations/create_remix.rb index ef42defc3..f250f5f47 100644 --- a/lib/concepts/project/operations/create_remix.rb +++ b/lib/concepts/project/operations/create_remix.rb @@ -18,7 +18,7 @@ def call(params:, user_id:, original_project:, remix_origin:) private def validate_params(response, params, user_id, original_project, remix_origin) - valid = params[:identifier].present? && user_id.present? && original_project.present? && remix_origin.present? + valid = (params[:identifier].present? || original_project.identifier.present?) && user_id.present? && original_project.present? && remix_origin.present? response[:error] = I18n.t('errors.project.remixing.invalid_params') unless valid end @@ -30,7 +30,7 @@ def remix_project(response, params, user_id, original_project, remix_origin) def create_remix(original_project, params, user_id, remix_origin) remix = format_project(original_project, params, user_id, remix_origin) - + puts("remix: #{remix}") original_project.images.each do |image| remix.images.attach(image.blob) end @@ -43,9 +43,10 @@ def create_remix(original_project, params, user_id, remix_origin) remix.audio.attach(audio_file.blob) end - params[:components].each do |x| + (params[:components] || original_project.components).each do |x| remix.components.build(x.slice(:name, :extension, :content)) end + pp(remix) remix end @@ -54,11 +55,12 @@ def format_project(original_project, params, user_id, remix_origin) original_project.dup.tap do |proj| proj.identifier = PhraseIdentifier.generate proj.locale = nil - proj.name = params[:name] + proj.name = params[:name] || original_project.name proj.user_id = user_id proj.remixed_from_id = original_project.id proj.remix_origin = remix_origin proj.lesson_id = nil # Only the original can have a lesson id + proj.school_id = params[:school_id] end end end From ed2e0f6640096a1a2f3a645de5261fabc4826315 Mon Sep 17 00:00:00 2001 From: Jamie Benstead Date: Fri, 23 May 2025 13:02:37 +0100 Subject: [PATCH 2/9] Add or condition, comment out project.finished for testing --- app/views/api/lessons/index.json.jbuilder | 2 +- lib/concepts/project/operations/create_remix.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/api/lessons/index.json.jbuilder b/app/views/api/lessons/index.json.jbuilder index 6429df43c..2f0fb6b43 100644 --- a/app/views/api/lessons/index.json.jbuilder +++ b/app/views/api/lessons/index.json.jbuilder @@ -23,7 +23,7 @@ json.array!(@lessons_with_users) do |lesson, user| :identifier, :project_type ) - json.project.finished(lesson.project.finished) if lesson.project.remixed_from_id.present? + # json.project.finished(lesson.project.finished) if lesson.project.remixed_from_id.present? end json.user_name(user&.name) diff --git a/lib/concepts/project/operations/create_remix.rb b/lib/concepts/project/operations/create_remix.rb index f250f5f47..6df631ec5 100644 --- a/lib/concepts/project/operations/create_remix.rb +++ b/lib/concepts/project/operations/create_remix.rb @@ -60,7 +60,7 @@ def format_project(original_project, params, user_id, remix_origin) proj.remixed_from_id = original_project.id proj.remix_origin = remix_origin proj.lesson_id = nil # Only the original can have a lesson id - proj.school_id = params[:school_id] + proj.school_id = params[:school_id] || original_project.school_id end end end From f9dc9526e7ee407382047e4da28010e4b8d4629a Mon Sep 17 00:00:00 2001 From: Lois Wells Date: Thu, 29 May 2025 11:04:47 +0100 Subject: [PATCH 3/9] fixing permissions around teachers reading remixed projects --- app/models/ability.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index aa682d3e7..a5a030b57 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -89,7 +89,7 @@ def define_school_teacher_abilities(user:, school:) end can(%i[read update show_context], Project, school_id: school.id, lesson: { visibility: %w[teachers students] }) can(%i[read], Project, - remixed_from_id: Project.where(school_id: school.id, remixed_from_id: nil, lesson_id: Lesson.where(school_class_id: ClassTeacher.where(teacher_id: user.id).select(:school_class_id))).pluck(:id)) + remixed_from_id: Project.where(school_id: school.id, lesson_id: Lesson.where(school_class_id: ClassTeacher.where(teacher_id: user.id).select(:school_class_id))).pluck(:id)) end def define_school_student_abilities(user:, school:) From 69ef9266f5e4ed9ee26db5b54fda140bb28913ac Mon Sep 17 00:00:00 2001 From: Lois Wells Date: Tue, 3 Jun 2025 14:44:47 +0100 Subject: [PATCH 4/9] wip abilities fiddling --- app/controllers/api/lessons_controller.rb | 14 ++++++++------ app/models/ability.rb | 12 ++++++++++-- config/routes.rb | 2 +- db/schema.rb | 3 ++- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/app/controllers/api/lessons_controller.rb b/app/controllers/api/lessons_controller.rb index b167e3d1b..8502c9dce 100644 --- a/app/controllers/api/lessons_controller.rb +++ b/app/controllers/api/lessons_controller.rb @@ -3,8 +3,8 @@ module Api class LessonsController < ApiController before_action :authorize_user, except: %i[index show] - before_action :verify_school_class_belongs_to_school, only: :create - load_and_authorize_resource :lesson, except: [:create_lesson_with_remix] + before_action :verify_school_class_belongs_to_school, only: %i[create remix] + load_and_authorize_resource :lesson def index archive_scope = params[:include_archived] == 'true' ? Lesson : Lesson.unarchived @@ -41,15 +41,17 @@ def create_copy end end - def create_lesson_with_remix + def remix remix_origin = request.origin || request.referer - - puts("lesson_params: #{lesson_params}") + # project = Project.find_by(identifier: lesson_params[:project_identifier]) + # authorize! project result = Lesson::CreateRemix.call(lesson_params: lesson_params, remix_origin:) if result.success? - @lesson_with_user = result[:lesson].with_user + @lesson = result[:lesson] + # authorize! :remix, @lesson + @lesson_with_user = @lesson.with_user render :show, formats: [:json], status: :created else render json: { error: result[:error] }, status: :unprocessable_entity diff --git a/app/models/ability.rb b/app/models/ability.rb index a5a030b57..837733aab 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -67,7 +67,6 @@ def define_school_owner_abilities(school:) can(%i[read create create_batch update destroy], :school_student) can(%i[create create_copy], Lesson, school_id: school.id) can(%i[read update destroy], Lesson, school_id: school.id, visibility: %w[teachers students public]) - can(%i[remix], Lesson) end def define_school_teacher_abilities(user:, school:) @@ -82,7 +81,10 @@ def define_school_teacher_abilities(user:, school:) can(%i[create update destroy], Lesson) do |lesson| school_teacher_can_manage_lesson?(user:, school:, lesson:) end - can(%i[remix], Lesson) + can(%i[remix], Lesson) do |lesson| + pp 'checking user can remix lesson' + school_teacher_can_remix_lesson?(user:, school:, lesson:) + end can(%i[read create_copy], Lesson, school_id: school.id, visibility: %w[teachers students]) can(%i[create], Project) do |project| school_teacher_can_manage_project?(user:, school:, project:) @@ -115,4 +117,10 @@ def school_teacher_can_manage_project?(user:, school:, project:) is_my_project && (is_my_lesson || !project.lesson) end + + def school_teacher_can_remix_lesson?(user:, school:, lesson:) + pp 'the original project is ', Project.find(lesson.project.remixed_from_id).identifier + original_project_is_public = Project.find(lesson.project.remixed_from_id).user_id.nil? + school_teacher_can_manage_lesson?(user:, school:, lesson:) && original_project_is_public + end end diff --git a/config/routes.rb b/config/routes.rb index 7dad0c4aa..b57c05934 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -61,7 +61,7 @@ resources :lessons, only: %i[index create show update destroy] do post :copy, on: :member, to: 'lessons#create_copy' - post :remix, on: :collection, to: 'lessons#create_lesson_with_remix' + post :remix, on: :collection, to: 'lessons#remix' end resources :teacher_invitations, param: :token, only: :show do diff --git a/db/schema.rb b/db/schema.rb index 9aefee6f7..b9237c649 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[7.1].define(version: 2025_04_15_133546) do +ActiveRecord::Schema[7.1].define(version: 2025_05_15_081023) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -270,6 +270,7 @@ t.string "code" t.boolean "creator_agree_to_ux_contact", default: false t.boolean "creator_agree_responsible_safeguarding", default: true + t.integer "user_origin", default: 0 t.index ["code"], name: "index_schools_on_code", unique: true t.index ["creator_id"], name: "index_schools_on_creator_id", unique: true t.index ["reference"], name: "index_schools_on_reference", unique: true From a9d83a5c78e099cff2d9e12ea87f592b93ee8ee2 Mon Sep 17 00:00:00 2001 From: Lois Wells Date: Tue, 10 Jun 2025 13:41:48 +0100 Subject: [PATCH 5/9] fixing permissions --- app/controllers/api/lessons_controller.rb | 10 +++++++--- app/models/ability.rb | 1 - app/models/lesson.rb | 10 ++++++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/lessons_controller.rb b/app/controllers/api/lessons_controller.rb index 8502c9dce..4ef86c8de 100644 --- a/app/controllers/api/lessons_controller.rb +++ b/app/controllers/api/lessons_controller.rb @@ -43,14 +43,18 @@ def create_copy def remix remix_origin = request.origin || request.referer - # project = Project.find_by(identifier: lesson_params[:project_identifier]) - # authorize! project + project = Project.find_by(identifier: lesson_params[:project_identifier]) + authorize! :show, project + + lesson = Lesson.new(lesson_params.except(:project_identifier)) + lesson.project = Project.new(remixed_from_id: project.id) if project + + authorize! :remix, lesson result = Lesson::CreateRemix.call(lesson_params: lesson_params, remix_origin:) if result.success? @lesson = result[:lesson] - # authorize! :remix, @lesson @lesson_with_user = @lesson.with_user render :show, formats: [:json], status: :created else diff --git a/app/models/ability.rb b/app/models/ability.rb index 837733aab..2d568e74d 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -82,7 +82,6 @@ def define_school_teacher_abilities(user:, school:) school_teacher_can_manage_lesson?(user:, school:, lesson:) end can(%i[remix], Lesson) do |lesson| - pp 'checking user can remix lesson' school_teacher_can_remix_lesson?(user:, school:, lesson:) end can(%i[read create_copy], Lesson, school_id: school.id, visibility: %w[teachers students]) diff --git a/app/models/lesson.rb b/app/models/lesson.rb index ae0f0dfde..55926fe41 100644 --- a/app/models/lesson.rb +++ b/app/models/lesson.rb @@ -17,6 +17,7 @@ class Lesson < ApplicationRecord validate :user_has_the_school_owner_or_school_teacher_role_for_the_school validate :user_is_the_school_teacher_for_the_school_class + validate :remixed_lesson_projects_come_from_public_projects scope :archived, -> { where.not(archived_at: nil) } scope :unarchived, -> { where(archived_at: nil) } @@ -74,4 +75,13 @@ def user_is_the_school_teacher_for_the_school_class errors.add(:user, "'#{user_id}' is not the 'school-teacher' for school_class '#{school_class.id}'") end + + def remixed_lesson_projects_come_from_public_projects + return if !project || !project.remixed_from_id + + original_project = Project.find_by(id: project.remixed_from_id) + return if original_project&.user_id.nil? + + errors.add(:project, "remixed project '#{original_project.id}' is not public") + end end From 3b608a4a6aee2ff4aa125a5739e4445eb82d9839 Mon Sep 17 00:00:00 2001 From: Lois Wells Date: Tue, 10 Jun 2025 13:43:51 +0100 Subject: [PATCH 6/9] permissions tweak --- app/controllers/api/lessons_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/api/lessons_controller.rb b/app/controllers/api/lessons_controller.rb index 4ef86c8de..0e715ca00 100644 --- a/app/controllers/api/lessons_controller.rb +++ b/app/controllers/api/lessons_controller.rb @@ -4,7 +4,7 @@ module Api class LessonsController < ApiController before_action :authorize_user, except: %i[index show] before_action :verify_school_class_belongs_to_school, only: %i[create remix] - load_and_authorize_resource :lesson + load_and_authorize_resource :lesson, except: :remix def index archive_scope = params[:include_archived] == 'true' ? Lesson : Lesson.unarchived @@ -48,7 +48,6 @@ def remix lesson = Lesson.new(lesson_params.except(:project_identifier)) lesson.project = Project.new(remixed_from_id: project.id) if project - authorize! :remix, lesson result = Lesson::CreateRemix.call(lesson_params: lesson_params, remix_origin:) From bc3a865c812f0b55a2e9d363ba6f2ab3d67d5150 Mon Sep 17 00:00:00 2001 From: Lois Wells Date: Tue, 10 Jun 2025 14:01:53 +0100 Subject: [PATCH 7/9] fixing linting --- app/controllers/api/lessons_controller.rb | 5 ++--- app/models/ability.rb | 1 - lib/concepts/lesson/operations/create_remix.rb | 7 +++---- lib/concepts/project/operations/create_remix.rb | 2 -- 4 files changed, 5 insertions(+), 10 deletions(-) diff --git a/app/controllers/api/lessons_controller.rb b/app/controllers/api/lessons_controller.rb index 0e715ca00..5936cfdae 100644 --- a/app/controllers/api/lessons_controller.rb +++ b/app/controllers/api/lessons_controller.rb @@ -46,11 +46,11 @@ def remix project = Project.find_by(identifier: lesson_params[:project_identifier]) authorize! :show, project - lesson = Lesson.new(lesson_params.except(:project_identifier)) + lesson = Lesson.new(lesson_params.except(:project_identifier)) lesson.project = Project.new(remixed_from_id: project.id) if project authorize! :remix, lesson - result = Lesson::CreateRemix.call(lesson_params: lesson_params, remix_origin:) + result = Lesson::CreateRemix.call(lesson_params:, remix_origin:) if result.success? @lesson = result[:lesson] @@ -95,7 +95,6 @@ def verify_school_class_belongs_to_school end def lesson_params - puts("base_params: #{base_params}") base_params.merge(user_id: current_user.id) end diff --git a/app/models/ability.rb b/app/models/ability.rb index d91ab2322..f5de1b450 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -126,7 +126,6 @@ def school_teacher_can_manage_project?(user:, school:, project:) end def school_teacher_can_remix_lesson?(user:, school:, lesson:) - pp 'the original project is ', Project.find(lesson.project.remixed_from_id).identifier original_project_is_public = Project.find(lesson.project.remixed_from_id).user_id.nil? school_teacher_can_manage_lesson?(user:, school:, lesson:) && original_project_is_public end diff --git a/lib/concepts/lesson/operations/create_remix.rb b/lib/concepts/lesson/operations/create_remix.rb index 54f305260..21c81998a 100644 --- a/lib/concepts/lesson/operations/create_remix.rb +++ b/lib/concepts/lesson/operations/create_remix.rb @@ -11,7 +11,6 @@ def call(lesson_params:, remix_origin:) response rescue StandardError => e Sentry.capture_exception(e) - pp(e) errors = response[:lesson].errors.full_messages.join(',') response[:error] = "Error creating remix of lesson: #{errors}" response @@ -32,10 +31,10 @@ def build_remix(lesson_params, remix_origin) def build_project_remix(original_project, lesson_params, remix_origin) response = Project::CreateRemix.call( - params: {school_id: lesson_params[:school_id]}, + params: { school_id: lesson_params[:school_id] }, user_id: lesson_params[:user_id], - original_project: original_project, - remix_origin: remix_origin + original_project:, + remix_origin: ) response[:project] end diff --git a/lib/concepts/project/operations/create_remix.rb b/lib/concepts/project/operations/create_remix.rb index 6df631ec5..d2293e24d 100644 --- a/lib/concepts/project/operations/create_remix.rb +++ b/lib/concepts/project/operations/create_remix.rb @@ -30,7 +30,6 @@ def remix_project(response, params, user_id, original_project, remix_origin) def create_remix(original_project, params, user_id, remix_origin) remix = format_project(original_project, params, user_id, remix_origin) - puts("remix: #{remix}") original_project.images.each do |image| remix.images.attach(image.blob) end @@ -46,7 +45,6 @@ def create_remix(original_project, params, user_id, remix_origin) (params[:components] || original_project.components).each do |x| remix.components.build(x.slice(:name, :extension, :content)) end - pp(remix) remix end From ef5beee4646e4f17efbc816232062faf00486b1e Mon Sep 17 00:00:00 2001 From: Lois Wells Date: Tue, 10 Jun 2025 16:52:19 +0100 Subject: [PATCH 8/9] initial tests WIP --- .../project/operations/create_remix.rb | 1 + spec/concepts/project/create_remix_spec.rb | 30 +++++++++++++++++++ spec/models/lesson_spec.rb | 14 +++++++++ 3 files changed, 45 insertions(+) diff --git a/lib/concepts/project/operations/create_remix.rb b/lib/concepts/project/operations/create_remix.rb index d2293e24d..31d8e3d40 100644 --- a/lib/concepts/project/operations/create_remix.rb +++ b/lib/concepts/project/operations/create_remix.rb @@ -43,6 +43,7 @@ def create_remix(original_project, params, user_id, remix_origin) end (params[:components] || original_project.components).each do |x| + pp 'using original project components' if params[:components].nil? remix.components.build(x.slice(:name, :extension, :content)) end diff --git a/spec/concepts/project/create_remix_spec.rb b/spec/concepts/project/create_remix_spec.rb index 035cf65b9..5b1dd8c24 100644 --- a/spec/concepts/project/create_remix_spec.rb +++ b/spec/concepts/project/create_remix_spec.rb @@ -180,6 +180,36 @@ expect(create_remix[:error]).to eq(I18n.t('errors.project.remixing.cannot_save')) end end + + context 'when the remix params contain only a school' do + let(:school) { create(:school) } + let!(:remix_params) { { school_id: school.id } } + subject!(:create_remix) { described_class.call(params: remix_params, user_id: user_id, original_project: original_project, remix_origin: remix_origin) } + + it 'sets the name to the original project name' do + remixed_project = create_remix[:project] + expect(remixed_project.name).to eq(original_project.name) + end + + it 'sets the school_id to the one provided by the remix params' do + remixed_project = create_remix[:project] + expect(remixed_project.school_id).to eq(school.id) + end + + fit 'creates components from the original project' do + pp original_project.components.count + pp remix_params + expect { create_remix }.to change(Component, :count).by(original_project.components.count) + # expect { create_remix }.to change(Project, :count).by(1) + end + + fit 'copies the components from the original project' do + remixed_project = create_remix[:project] + expect(remixed_project.components.map { |c| component_props(c) }).to match_array( + original_project.components.map { |c| component_props(c) } + ) + end + end end def component_props(component) diff --git a/spec/models/lesson_spec.rb b/spec/models/lesson_spec.rb index 4eba9d729..9545f3c7a 100644 --- a/spec/models/lesson_spec.rb +++ b/spec/models/lesson_spec.rb @@ -110,6 +110,20 @@ lesson.visibility = 'invalid' expect(lesson).to be_invalid end + + it 'requires original project to be public if project is a remix' do + original_project = create(:project, user_id: SecureRandom.uuid) + lesson.project.remixed_from_id = original_project.id + + expect(lesson).to be_invalid + end + + it 'is valid with a remixed project from a public project' do + original_project = create(:project, user_id: nil) + lesson.project.remixed_from_id = original_project.id + + expect(lesson).to be_valid + end end describe '.archived' do From 60f55524693cf9bb74344b7cbc7a7da1f03eac24 Mon Sep 17 00:00:00 2001 From: Lois Wells Date: Wed, 11 Jun 2025 10:06:40 +0100 Subject: [PATCH 9/9] trying to fix test --- lib/concepts/project/operations/create_remix.rb | 2 +- spec/concepts/project/create_remix_spec.rb | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/concepts/project/operations/create_remix.rb b/lib/concepts/project/operations/create_remix.rb index 31d8e3d40..bc70315c6 100644 --- a/lib/concepts/project/operations/create_remix.rb +++ b/lib/concepts/project/operations/create_remix.rb @@ -11,7 +11,7 @@ def call(params:, user_id:, original_project:, remix_origin:) response rescue StandardError => e Sentry.capture_exception(e) - response[:error] = I18n.t('errors.project.remixing.cannot_save') + response[:error] = "#{I18n.t('errors.project.remixing.cannot_save')}: #{e.message}" response end diff --git a/spec/concepts/project/create_remix_spec.rb b/spec/concepts/project/create_remix_spec.rb index 5b1dd8c24..9a1e5d6d8 100644 --- a/spec/concepts/project/create_remix_spec.rb +++ b/spec/concepts/project/create_remix_spec.rb @@ -183,8 +183,9 @@ context 'when the remix params contain only a school' do let(:school) { create(:school) } + let(:teacher) { create(:teacher, school:) } let!(:remix_params) { { school_id: school.id } } - subject!(:create_remix) { described_class.call(params: remix_params, user_id: user_id, original_project: original_project, remix_origin: remix_origin) } + subject!(:create_remix) { described_class.call(params: remix_params, user_id: teacher.id, original_project: original_project, remix_origin: remix_origin) } it 'sets the name to the original project name' do remixed_project = create_remix[:project] @@ -196,14 +197,14 @@ expect(remixed_project.school_id).to eq(school.id) end - fit 'creates components from the original project' do - pp original_project.components.count - pp remix_params - expect { create_remix }.to change(Component, :count).by(original_project.components.count) - # expect { create_remix }.to change(Project, :count).by(1) - end + # fit 'creates components from the original project' do + # pp original_project.components.count + # pp remix_params + # # expect { create_remix }.to change(Component, :count).by(original_project.components.count) + # expect { create_remix }.to change(Project, :count).by(1) + # end - fit 'copies the components from the original project' do + it 'copies the components from the original project' do remixed_project = create_remix[:project] expect(remixed_project.components.map { |c| component_props(c) }).to match_array( original_project.components.map { |c| component_props(c) }