diff --git a/app/models/user.rb b/app/models/user.rb index 0315e761..686456f4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -13,7 +13,7 @@ class User < ApplicationRecord has_many :user_proxy_exercise_exercises, as: :user has_many :user_exercise_interventions, as: :user has_many :interventions, through: :user_exercise_interventions - has_one :codeharbor_link + has_one :codeharbor_link, dependent: :destroy accepts_nested_attributes_for :user_proxy_exercise_exercises diff --git a/app/policies/codeharbor_link_policy.rb b/app/policies/codeharbor_link_policy.rb index 21b1dab2..3dd4ff54 100644 --- a/app/policies/codeharbor_link_policy.rb +++ b/app/policies/codeharbor_link_policy.rb @@ -8,11 +8,11 @@ class CodeharborLinkPolicy < ApplicationPolicy end def new? - teacher? + teacher? || admin? end def create? - teacher? + teacher? || admin? end def edit? diff --git a/app/services/proforma_service/import.rb b/app/services/proforma_service/import.rb index 4fa0676e..9864b31e 100644 --- a/app/services/proforma_service/import.rb +++ b/app/services/proforma_service/import.rb @@ -28,13 +28,9 @@ module ProformaService def base_exercise exercise = Exercise.find_by(uuid: @task.uuid) - if exercise - return exercise if ExercisePolicy.new(@user, exercise).update? + return exercise if exercise && ExercisePolicy.new(@user, exercise).update? - return Exercise.new(uuid: SecureRandom.uuid, unpublished: true) - end - - Exercise.new(uuid: @task.uuid || SecureRandom.uuid, unpublished: true) + Exercise.new(uuid: @task.uuid, unpublished: true) end def import_multi diff --git a/spec/policies/codeharbor_link_policy_spec.rb b/spec/policies/codeharbor_link_policy_spec.rb index 9569dd90..dcb6ace3 100644 --- a/spec/policies/codeharbor_link_policy_spec.rb +++ b/spec/policies/codeharbor_link_policy_spec.rb @@ -18,13 +18,13 @@ describe CodeharborLinkPolicy do %i[new? create?].each do |action| permissions(action) do it 'grants access to teachers' do - expect(policy).to permit(FactoryBot.create(:teacher), codeharbor_link) + %i[teacher admin].each do |factory_name| + expect(policy).to permit(FactoryBot.create(factory_name), codeharbor_link) + end end it 'does not grant access to all other users' do - %i[external_user admin].each do |factory_name| - expect(policy).not_to permit(FactoryBot.create(factory_name), codeharbor_link) - end + expect(policy).not_to permit(FactoryBot.create(:external_user), codeharbor_link) end end end diff --git a/spec/services/proforma_service/import_spec.rb b/spec/services/proforma_service/import_spec.rb index 118284f1..6f83c4bf 100644 --- a/spec/services/proforma_service/import_spec.rb +++ b/spec/services/proforma_service/import_spec.rb @@ -128,42 +128,14 @@ describe ProformaService::Import do it { is_expected.to be_an_equal_exercise_as exercise } end - # context 'when zip contains multiple tasks' do - # let(:exporter) { ProformaService::ExportTasks.call(exercises: [exercise, exercise2]).string } - - # let(:exercise2) do - # FactoryBot.create(:dummy, - # instruction: 'instruction2', - # execution_environment: execution_environment, - # exercise_files: [], - # tests: [], - # user: user) - # end - - # it 'imports the exercises from zip containing multiple zips' do - # expect(import_service).to all be_an(Exercise) - # end - - # it 'imports the zip exactly how they were exported' do - # expect(import_service).to all be_an_equal_exercise_as(exercise).or be_an_equal_exercise_as(exercise2) - # end - - # context 'when a exercise has files and tests' do - # let(:files) { [FactoryBot.build(:file), FactoryBot.build(:file, role: 'regular_file')] } - # let(:tests) { FactoryBot.build_list(:codeharbor_test, 2) } - - # it 'imports the zip exactly how the were exported' do - # expect(import_service).to all be_an_equal_exercise_as(exercise).or be_an_equal_exercise_as(exercise2) - # end - # end - # end - context 'when task in zip has a different uuid' do let(:uuid) { SecureRandom.uuid } let(:new_uuid) { SecureRandom.uuid } + let(:imported_exercise) { import_service } before do exercise.update(uuid: new_uuid) + imported_exercise.save! end it 'creates a new Exercise' do @@ -173,16 +145,18 @@ describe ProformaService::Import do context 'when task in zip has the same uuid and nothing has changed' do let(:uuid) { SecureRandom.uuid } + let(:imported_exercise) { import_service } it 'updates the old Exercise' do - expect(import_service.id).to be exercise.id + imported_exercise.save! + expect(imported_exercise.id).to be exercise.id end context 'when another user imports the exercise' do let(:import_user) { FactoryBot.create(:teacher) } - it 'creates a new Exercise' do - expect(import_service.id).not_to be exercise.id + it 'raises a validation error' do + expect { imported_exercise.save! } .to raise_error ActiveRecord::RecordInvalid end end end