Merge branch 'master' into refactor_proforma_import_export
This commit is contained in:
@ -25,6 +25,8 @@ describe FileParameters do
|
||||
|
||||
it 'new file' do
|
||||
submission = create(:submission, exercise: hello_world, id: 1337)
|
||||
controller.instance_variable_set(:@current_user, submission.user)
|
||||
|
||||
new_file = create(:file, context: submission)
|
||||
expect(file_accepted?(new_file)).to be true
|
||||
end
|
||||
@ -42,16 +44,27 @@ describe FileParameters do
|
||||
expect(file_accepted?(hidden_file)).to be false
|
||||
end
|
||||
|
||||
it 'read only file' do
|
||||
it 'read-only file' do
|
||||
read_only_file = create(:file, context: hello_world, read_only: true)
|
||||
expect(file_accepted?(read_only_file)).to be false
|
||||
end
|
||||
|
||||
it 'non existent file' do
|
||||
it 'non-existent file' do
|
||||
# Ensure to use an invalid id for the file.
|
||||
non_existent_file = build(:file, context: hello_world, id: -1)
|
||||
expect(file_accepted?(non_existent_file)).to be false
|
||||
end
|
||||
|
||||
it 'file of another submission' do
|
||||
learner1 = create(:learner)
|
||||
learner2 = create(:learner)
|
||||
submission_learner1 = create(:submission, exercise: hello_world, user: learner1)
|
||||
_submission_learner2 = create(:submission, exercise: hello_world, user: learner2)
|
||||
|
||||
controller.instance_variable_set(:@current_user, learner2)
|
||||
other_submissions_file = create(:file, context: submission_learner1)
|
||||
expect(file_accepted?(other_submissions_file)).to be false
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
@ -7,6 +7,26 @@ describe CodeOcean::FilesController do
|
||||
|
||||
before { allow(controller).to receive(:current_user).and_return(user) }
|
||||
|
||||
describe 'GET #show_protected_upload' do
|
||||
context 'with a valid filename' do
|
||||
let(:submission) { create(:submission, exercise: create(:audio_video)) }
|
||||
|
||||
before { get :show_protected_upload, params: {filename: file.name_with_extension, id: file.id} }
|
||||
|
||||
context 'with a binary file' do
|
||||
let(:file) { submission.collect_files.detect {|file| file.file_type.file_extension == '.mp4' } }
|
||||
|
||||
expect_assigns(file: :file)
|
||||
expect_content_type('video/mp4')
|
||||
expect_http_status(:ok)
|
||||
|
||||
it 'sets the correct filename' do
|
||||
expect(response.headers['Content-Disposition']).to include("attachment; filename=\"#{file.name_with_extension}\"")
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'POST #create' do
|
||||
let(:submission) { create(:submission, user: user) }
|
||||
|
||||
|
@ -6,7 +6,10 @@ describe ExercisesController do
|
||||
let(:exercise) { create(:dummy) }
|
||||
let(:user) { create(:admin) }
|
||||
|
||||
before { allow(controller).to receive(:current_user).and_return(user) }
|
||||
before do
|
||||
create(:test_file, context: exercise)
|
||||
allow(controller).to receive(:current_user).and_return(user)
|
||||
end
|
||||
|
||||
describe 'PUT #batch_update' do
|
||||
let(:attributes) { {public: 'true'} }
|
||||
@ -184,6 +187,17 @@ describe ExercisesController do
|
||||
expect_flash_message(:alert, :'exercises.implement.no_files')
|
||||
expect_redirect(:exercise)
|
||||
end
|
||||
|
||||
context 'with other users accessing an unpublished exercise' do
|
||||
let(:exercise) { create(:fibonacci, unpublished: true) }
|
||||
let(:user) { create(:teacher) }
|
||||
|
||||
before { perform_request.call }
|
||||
|
||||
expect_assigns(exercise: :exercise)
|
||||
expect_flash_message(:alert, :'exercises.implement.unpublished')
|
||||
expect_redirect(:exercise)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'GET #index' do
|
||||
@ -220,6 +234,8 @@ describe ExercisesController do
|
||||
|
||||
describe 'GET #reload' do
|
||||
context 'when being anyone' do
|
||||
let(:exercise) { create(:fibonacci) }
|
||||
|
||||
before { get :reload, format: :json, params: {id: exercise.id} }
|
||||
|
||||
expect_assigns(exercise: :exercise)
|
||||
|
@ -74,11 +74,9 @@ describe SubmissionsController do
|
||||
|
||||
expect_assigns(file: :file)
|
||||
expect_assigns(submission: :submission)
|
||||
expect_content_type('video/mp4')
|
||||
expect_http_status(:ok)
|
||||
|
||||
it 'sets the correct filename' do
|
||||
expect(response.headers['Content-Disposition']).to include("attachment; filename=\"#{file.name_with_extension}\"")
|
||||
it 'sets the correct redirect' do
|
||||
expect(response.location).to eq protected_upload_url(id: file, filename: file.name_with_extension)
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -78,6 +78,25 @@ describe 'Authentication' do
|
||||
expect(page).to have_content(I18n.t('application.not_authorized'))
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the authentication token is used to login' do
|
||||
let(:token) { create(:authentication_token, user: user) }
|
||||
|
||||
it 'invalidates the token on login' do
|
||||
mail.deliver_now
|
||||
visit(rfc_link)
|
||||
expect(token.reload.expire_at).to be_within(10.seconds).of(Time.zone.now)
|
||||
end
|
||||
|
||||
it 'does not allow a second login' do
|
||||
mail.deliver_now
|
||||
visit(rfc_link)
|
||||
expect(page).to have_current_path(rfc_link)
|
||||
visit(sign_out_path)
|
||||
visit(rfc_link)
|
||||
expect(page).to have_current_path(root_path)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -23,6 +23,7 @@ describe 'Editor', js: true do
|
||||
}]
|
||||
end
|
||||
let(:user) { create(:teacher) }
|
||||
let(:exercise_without_test) { create(:tdd) }
|
||||
|
||||
before do
|
||||
visit(sign_in_path)
|
||||
@ -93,12 +94,28 @@ describe 'Editor', js: true do
|
||||
end
|
||||
end
|
||||
|
||||
context 'when an exercise has one or more teacher-defined assessments' do
|
||||
it 'displays the score button' do
|
||||
visit(implement_exercise_path(exercise))
|
||||
expect(page).to have_content(exercise.title)
|
||||
expect(page).to have_content(I18n.t('exercises.editor.score'))
|
||||
end
|
||||
end
|
||||
|
||||
context 'when an exercise has no teacher-defined assessment' do
|
||||
it 'disables the score button' do
|
||||
visit(implement_exercise_path(exercise_without_test))
|
||||
expect(page).to have_content(exercise_without_test.title)
|
||||
expect(page).not_to have_content(I18n.t('exercises.editor.score'))
|
||||
end
|
||||
end
|
||||
|
||||
it 'contains a button for submitting the exercise' do
|
||||
submission = build(:submission, user: user, exercise: exercise)
|
||||
allow(submission).to receive(:calculate_score).and_return(scoring_response)
|
||||
allow(Submission).to receive(:find).and_return(submission)
|
||||
click_button(I18n.t('exercises.editor.score'))
|
||||
expect(page).not_to have_css('#submit_outdated')
|
||||
expect(page).to have_css('#submit')
|
||||
expect(page).not_to have_content(I18n.t('exercises.editor.tooltips.exercise_deadline_passed'))
|
||||
expect(page).to have_content(I18n.t('exercises.editor.submit'))
|
||||
end
|
||||
end
|
||||
|
@ -93,6 +93,25 @@ describe UserMailer do
|
||||
# A five minute tolerance is allowed to account for the time difference between `now` and the creation timestamp of the token.
|
||||
expect(token.expire_at - Time.zone.now).to be_within(5.minutes).of(7.days)
|
||||
end
|
||||
|
||||
it 'sets the correct comment' do
|
||||
expect(mail.body).to include(request_for_comment.comments.first.text)
|
||||
end
|
||||
|
||||
context 'with an HTML comment' do
|
||||
let(:html_comment) { '<b>test</b>' }
|
||||
let(:escaped_comment) { '<b>test</b>' }
|
||||
|
||||
before { request_for_comment.comments.first.update(text: html_comment) }
|
||||
|
||||
it 'does not include the HTML tags' do
|
||||
expect(mail.body).not_to include(html_comment)
|
||||
end
|
||||
|
||||
it 'includes escaped HTML tags' do
|
||||
expect(mail.body).to include(escaped_comment)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#got_new_comment_for_subscription' do
|
||||
@ -128,21 +147,40 @@ describe UserMailer do
|
||||
# A five minute tolerance is allowed to account for the time difference between `now` and the creation timestamp of the token.
|
||||
expect(token.expire_at - Time.zone.now).to be_within(5.minutes).of(7.days)
|
||||
end
|
||||
|
||||
it 'sets the correct comment' do
|
||||
expect(mail.body).to include(request_for_comment.comments.first.text)
|
||||
end
|
||||
|
||||
context 'with an HTML comment' do
|
||||
let(:html_comment) { '<b>test</b>' }
|
||||
let(:escaped_comment) { '<b>test</b>' }
|
||||
|
||||
before { request_for_comment.comments.first.update(text: html_comment) }
|
||||
|
||||
it 'does not include the HTML tags' do
|
||||
expect(mail.body).not_to include(html_comment)
|
||||
end
|
||||
|
||||
it 'includes escaped HTML tags' do
|
||||
expect(mail.body).to include(escaped_comment)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#send_thank_you_note' do
|
||||
let(:user) { create(:learner) }
|
||||
let(:token) { AuthenticationToken.find_by(user: user) }
|
||||
let(:request_for_comments) { create(:rfc_with_comment, user: user) }
|
||||
let(:receiver) { InternalUser.create(attributes_for(:teacher)) }
|
||||
let(:mail) { described_class.send_thank_you_note(request_for_comments, receiver).deliver_now }
|
||||
let(:receiver) { create(:teacher) }
|
||||
let(:token) { AuthenticationToken.find_by(user: receiver) }
|
||||
let(:request_for_comment) { create(:rfc_with_comment, user: user) }
|
||||
let(:mail) { described_class.send_thank_you_note(request_for_comment, receiver).deliver_now }
|
||||
|
||||
it 'sets the correct sender' do
|
||||
expect(mail.from).to include('codeocean@hpi.de')
|
||||
end
|
||||
|
||||
it 'sets the correct subject' do
|
||||
expect(mail.subject).to eq(I18n.t('mailers.user_mailer.send_thank_you_note.subject', author: request_for_comments.user.displayname))
|
||||
expect(mail.subject).to eq(I18n.t('mailers.user_mailer.send_thank_you_note.subject', author: request_for_comment.user.displayname))
|
||||
end
|
||||
|
||||
it 'sets the correct receiver' do
|
||||
@ -150,7 +188,7 @@ describe UserMailer do
|
||||
end
|
||||
|
||||
it 'includes the correct URL' do
|
||||
expect(mail.body).to include(request_for_comment_url(request_for_comments, token: token.shared_secret))
|
||||
expect(mail.body).to include(request_for_comment_url(request_for_comment, token: token.shared_secret))
|
||||
end
|
||||
|
||||
it 'creates a new authentication token' do
|
||||
@ -162,5 +200,24 @@ describe UserMailer do
|
||||
# A five minute tolerance is allowed to account for the time difference between `now` and the creation timestamp of the token.
|
||||
expect(token.expire_at - Time.zone.now).to be_within(5.minutes).of(7.days)
|
||||
end
|
||||
|
||||
it 'sets the correct thank_you_note' do
|
||||
expect(mail.body).to include(request_for_comment.thank_you_note)
|
||||
end
|
||||
|
||||
context 'with an HTML comment' do
|
||||
let(:html_comment) { '<b>test</b>' }
|
||||
let(:escaped_comment) { '<b>test</b>' }
|
||||
|
||||
before { request_for_comment.update(thank_you_note: html_comment) }
|
||||
|
||||
it 'does not include the HTML tags' do
|
||||
expect(mail.body).not_to include(html_comment)
|
||||
end
|
||||
|
||||
it 'includes escaped HTML tags' do
|
||||
expect(mail.body).to include(escaped_comment)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
@ -69,7 +69,27 @@ describe CodeOcean::File do
|
||||
end
|
||||
|
||||
context 'when the path has been modified' do
|
||||
before { file.update(native_file: '../../../../secrets.yml') }
|
||||
before do
|
||||
file.update_column(:native_file, '../../../../secrets.yml') # rubocop:disable Rails/SkipsModelValidations
|
||||
file.reload
|
||||
end
|
||||
|
||||
it 'does not read the native file' do
|
||||
expect(file.read).not_to be_present
|
||||
end
|
||||
end
|
||||
|
||||
context 'when a symlink is used' do
|
||||
let(:fake_upload_location) { File.join(CarrierWave::Uploader::Base.new.root, 'uploads', 'files', 'secrets.yml') }
|
||||
|
||||
before do
|
||||
FileUtils.touch Rails.root.join('config/secrets.yml')
|
||||
File.symlink Rails.root.join('config/secrets.yml'), fake_upload_location
|
||||
file.update_column(:native_file, '../secrets.yml') # rubocop:disable Rails/SkipsModelValidations
|
||||
file.reload
|
||||
end
|
||||
|
||||
after { File.delete(fake_upload_location) }
|
||||
|
||||
it 'does not read the native file' do
|
||||
expect(file.read).not_to be_present
|
||||
|
@ -124,4 +124,41 @@ describe Exercise do
|
||||
expect(exercise.duplicate).to be_a(described_class)
|
||||
end
|
||||
end
|
||||
|
||||
describe '#teacher_defined_assessment?' do
|
||||
let(:exercise) { create(:dummy) }
|
||||
|
||||
context 'when no assessment is defined' do
|
||||
it 'returns false' do
|
||||
expect(exercise).not_to be_teacher_defined_assessment
|
||||
end
|
||||
end
|
||||
|
||||
context 'when unit tests are defined' do
|
||||
before { create(:test_file, context: exercise) }
|
||||
|
||||
it 'returns true' do
|
||||
expect(exercise).to be_teacher_defined_assessment
|
||||
end
|
||||
end
|
||||
|
||||
context 'when linter tests are defined' do
|
||||
before { create(:test_file, context: exercise, role: 'teacher_defined_linter') }
|
||||
|
||||
it 'returns true' do
|
||||
expect(exercise).to be_teacher_defined_assessment
|
||||
end
|
||||
end
|
||||
|
||||
context 'when unit and linter tests are defined' do
|
||||
before do
|
||||
create(:test_file, context: exercise)
|
||||
create(:test_file, context: exercise, role: 'teacher_defined_linter')
|
||||
end
|
||||
|
||||
it 'returns true' do
|
||||
expect(exercise).to be_teacher_defined_assessment
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
@ -54,7 +54,7 @@ describe CodeharborLinkPolicy do
|
||||
end
|
||||
end
|
||||
|
||||
permissions(:enabled?) do
|
||||
permissions :enabled? do
|
||||
it 'reflects the config option' do
|
||||
%i[external_user admin teacher].each do |factory_name|
|
||||
expect(policy).to permit(create(factory_name), codeharbor_link)
|
||||
@ -72,7 +72,7 @@ describe CodeharborLinkPolicy do
|
||||
allow(codeocean_config).to receive(:read).and_return(codeharbor_config)
|
||||
end
|
||||
|
||||
permissions(:enabled?) do
|
||||
permissions :enabled? do
|
||||
it 'reflects the config option' do
|
||||
%i[external_user admin teacher].each do |factory_name|
|
||||
expect(policy).not_to permit(create(factory_name), codeharbor_link)
|
||||
|
@ -7,19 +7,17 @@ describe ExecutionEnvironmentPolicy do
|
||||
|
||||
let(:execution_environment) { build(:ruby) }
|
||||
|
||||
[:index?].each do |action|
|
||||
permissions(action) do
|
||||
it 'grants access to admins' do
|
||||
expect(policy).to permit(build(:admin), execution_environment)
|
||||
end
|
||||
permissions :index? do
|
||||
it 'grants access to admins' do
|
||||
expect(policy).to permit(build(:admin), execution_environment)
|
||||
end
|
||||
|
||||
it 'grants access to teachers' do
|
||||
expect(policy).to permit(build(:teacher), execution_environment)
|
||||
end
|
||||
it 'grants access to teachers' do
|
||||
expect(policy).to permit(build(:teacher), execution_environment)
|
||||
end
|
||||
|
||||
it 'does not grant access to external users' do
|
||||
expect(policy).not_to permit(build(:external_user), execution_environment)
|
||||
end
|
||||
it 'does not grant access to external users' do
|
||||
expect(policy).not_to permit(build(:external_user), execution_environment)
|
||||
end
|
||||
end
|
||||
|
||||
@ -59,7 +57,7 @@ describe ExecutionEnvironmentPolicy do
|
||||
end
|
||||
end
|
||||
|
||||
permissions(:sync_all_to_runner_management?) do
|
||||
permissions :sync_all_to_runner_management? do
|
||||
it 'grants access to the admin' do
|
||||
expect(policy).to permit(build(:admin))
|
||||
end
|
||||
|
@ -104,19 +104,79 @@ describe ExercisePolicy do
|
||||
end
|
||||
end
|
||||
|
||||
[:show?].each do |action|
|
||||
permissions :show? do
|
||||
it 'not grants access to external users' do
|
||||
expect(policy).not_to permit(build(:external_user), exercise)
|
||||
end
|
||||
end
|
||||
|
||||
%i[implement? working_times? intervention? search? reload?].each do |action|
|
||||
permissions(action) do
|
||||
it 'not grants access to external users' do
|
||||
expect(policy).not_to permit(build(:external_user), exercise)
|
||||
context 'when the exercise has no visible files' do
|
||||
let(:exercise) { create(:dummy) }
|
||||
|
||||
it 'does not grant access to anyone' do
|
||||
%i[admin external_user teacher].each do |factory_name|
|
||||
expect(policy).not_to permit(build(factory_name), exercise)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the exercise has visible files' do
|
||||
let(:exercise) { create(:fibonacci) }
|
||||
|
||||
it 'grants access to anyone' do
|
||||
%i[admin external_user teacher].each do |factory_name|
|
||||
expect(policy).to permit(build(factory_name), exercise)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the exercise is published' do
|
||||
let(:exercise) { create(:fibonacci, unpublished: false) }
|
||||
|
||||
it 'grants access to anyone' do
|
||||
%i[admin external_user teacher].each do |factory_name|
|
||||
expect(policy).to permit(build(factory_name), exercise)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the exercise is unpublished' do
|
||||
let(:exercise) { create(:fibonacci, unpublished: true) }
|
||||
|
||||
it 'grants access to admins' do
|
||||
expect(policy).to permit(build(:admin), exercise)
|
||||
end
|
||||
|
||||
it 'grants access to the author' do
|
||||
expect(policy).to permit(exercise.author, exercise)
|
||||
end
|
||||
|
||||
it 'does not grant access to everyone' do
|
||||
%i[external_user teacher].each do |factory_name|
|
||||
expect(policy).not_to permit(build(factory_name), exercise)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
%i[implement? submit?].each do |action|
|
||||
permissions(action) do
|
||||
permissions :submit? do
|
||||
context 'when teacher-defined assessments are available' do
|
||||
before { create(:test_file, context: exercise) }
|
||||
|
||||
it 'grants access to anyone' do
|
||||
%i[admin external_user teacher].each do |factory_name|
|
||||
expect(policy).to permit(build(factory_name), Exercise.new)
|
||||
expect(policy).to permit(build(factory_name), exercise)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when teacher-defined assessments are not available' do
|
||||
it 'does not grant access to anyone' do
|
||||
%i[admin external_user teacher].each do |factory_name|
|
||||
expect(policy).not_to permit(build(factory_name), exercise)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
@ -16,14 +16,12 @@ describe ExternalUserPolicy do
|
||||
end
|
||||
end
|
||||
|
||||
[:index?].each do |action|
|
||||
permissions(action) do
|
||||
it 'grants access to admins and teachers only' do
|
||||
expect(policy).to permit(build(:admin), ExternalUser.new)
|
||||
expect(policy).to permit(build(:teacher), ExternalUser.new)
|
||||
[:external_user].each do |factory_name|
|
||||
expect(policy).not_to permit(build(factory_name), ExternalUser.new)
|
||||
end
|
||||
permissions :index? do
|
||||
it 'grants access to admins and teachers only' do
|
||||
expect(policy).to permit(build(:admin), ExternalUser.new)
|
||||
expect(policy).to permit(build(:teacher), ExternalUser.new)
|
||||
[:external_user].each do |factory_name|
|
||||
expect(policy).not_to permit(build(factory_name), ExternalUser.new)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
Reference in New Issue
Block a user