diff --git a/app/controllers/concerns/submission_parameters.rb b/app/controllers/concerns/submission_parameters.rb index 3a4154d6..1def6759 100644 --- a/app/controllers/concerns/submission_parameters.rb +++ b/app/controllers/concerns/submission_parameters.rb @@ -14,14 +14,10 @@ module SubmissionParameters private :submission_params def merge_user(params) - if current_user - current_user_id = current_user.id - current_user_class_name = current_user.class.name - end # The study_group_id might not be present in the session (e.g. for internal users), resulting in session[:study_group_id] = nil which is intended. params.merge( - user_id: current_user_id, - user_type: current_user_class_name, + user_id: current_user.id, + user_type: current_user.class.name, study_group_id: session[:study_group_id] ) end diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 56a9f6b3..b26ee75b 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -500,8 +500,7 @@ class ExercisesController < ApplicationController def submit @submission = Submission.create(submission_params) score_submission(@submission) - current_user = ExternalUser.find(@submission.user_id) - if !current_user.nil? && lti_outcome_service?(@submission.exercise_id, current_user.id) + if @submission.user.external_user? && lti_outcome_service?(@submission.exercise_id, @submission.user.id) transmit_lti_score else redirect_after_submit diff --git a/config/database.yml.ci b/config/database.yml.ci index 8a62703f..4b898269 100644 --- a/config/database.yml.ci +++ b/config/database.yml.ci @@ -5,3 +5,4 @@ test: host: localhost username: postgres password: postgres + reaping_frequency: 0 diff --git a/config/initializers/new_framework_defaults_6_0.rb b/config/initializers/new_framework_defaults_6_0.rb new file mode 100644 index 00000000..0c410fe9 --- /dev/null +++ b/config/initializers/new_framework_defaults_6_0.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +# Be sure to restart your server when you modify this file. +# +# This file contains migration options to ease your Rails 6.0 upgrade. +# +# Once upgraded flip defaults one by one to migrate to the new default. +# +# Read the Guide for Upgrading Ruby on Rails for more info on each option. + +# Don't force requests from old versions of IE to be UTF-8 encoded. +# Rails.application.config.action_view.default_enforce_utf8 = false + +# Embed purpose and expiry metadata inside signed and encrypted +# cookies for increased security. +# +# This option is not backwards compatible with earlier Rails versions. +# It's best enabled when your entire app is migrated and stable on 6.0. +# Rails.application.config.action_dispatch.use_cookies_with_metadata = true + +# Change the return value of `ActionDispatch::Response#content_type` to Content-Type header without modification. +# Rails.application.config.action_dispatch.return_only_media_type_on_content_type = false + +# Return false instead of self when enqueuing is aborted from a callback. +# Rails.application.config.active_job.return_false_on_aborted_enqueue = true + +# Send Active Storage analysis and purge jobs to dedicated queues. +# Rails.application.config.active_storage.queues.analysis = :active_storage_analysis +# Rails.application.config.active_storage.queues.purge = :active_storage_purge + +# When assigning to a collection of attachments declared via `has_many_attached`, replace existing +# attachments instead of appending. Use #attach to add new attachments without replacing existing ones. +# Rails.application.config.active_storage.replace_on_assign_to_many = true + +# Use ActionMailer::MailDeliveryJob for sending parameterized and normal mail. +# +# The default delivery jobs (ActionMailer::Parameterized::DeliveryJob, ActionMailer::DeliveryJob), +# will be removed in Rails 6.1. This setting is not backwards compatible with earlier Rails versions. +# If you send mail in the background, job workers need to have a copy of +# MailDeliveryJob to ensure all delivery jobs are processed properly. +# Make sure your entire app is migrated and stable on 6.0 before using this setting. +# Rails.application.config.action_mailer.delivery_job = "ActionMailer::MailDeliveryJob" + +# Enable the same cache key to be reused when the object being cached of type +# `ActiveRecord::Relation` changes by moving the volatile information (max updated at and count) +# of the relation's cache key into the cache version to support recycling cache key. +# Rails.application.config.active_record.collection_cache_versioning = true diff --git a/spec/controllers/code_ocean/files_controller_spec.rb b/spec/controllers/code_ocean/files_controller_spec.rb index bca4daaf..7525c097 100644 --- a/spec/controllers/code_ocean/files_controller_spec.rb +++ b/spec/controllers/code_ocean/files_controller_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rails_helper' describe CodeOcean::FilesController do @@ -48,6 +50,8 @@ describe CodeOcean::FilesController do expect { perform_request.call }.to change(CodeOcean::File, :count).by(-1) end - expect_redirect(:exercise) + it 'redirects to exercise path' do + expect(controller).to redirect_to(exercise) + end end end diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index 9d77ed24..afb620aa 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rails_helper' describe ExercisesController do @@ -230,8 +232,8 @@ describe ExercisesController do describe 'POST #submit' do let(:output) { {} } let(:perform_request) { post :submit, format: :json, params: { id: exercise.id, submission: {cause: 'submit', exercise_id: exercise.id} } } - let!(:external_user) { FactoryBot.create(:external_user) } - let!(:lti_parameter) { FactoryBot.create(:lti_parameter, external_user: external_user, exercise: exercise) } + let(:user) { FactoryBot.create(:external_user) } + let!(:lti_parameter) { FactoryBot.create(:lti_parameter, external_user: user, exercise: exercise) } before(:each) do allow_any_instance_of(Submission).to receive(:normalized_score).and_return(1) diff --git a/spec/support/database_cleaner.rb b/spec/support/database_cleaner.rb index a15f37ff..e283ef43 100644 --- a/spec/support/database_cleaner.rb +++ b/spec/support/database_cleaner.rb @@ -1,10 +1,48 @@ +# frozen_string_literal: true + +require 'capybara/rspec' + RSpec.configure do |config| + + config.use_transactional_fixtures = false + config.before(:suite) do - DatabaseCleaner.strategy = :truncation + if config.use_transactional_fixtures? + raise(<<-MSG) + Delete line `config.use_transactional_fixtures = true` from rails_helper.rb + (or set it to false) to prevent uncommitted transactions being used in + JavaScript-dependent specs. + + During testing, the app-under-test that the browser driver connects to + uses a different database connection to the database connection used by + the spec. The app's database connection would not be able to access + uncommitted transaction data setup over the spec's database connection. + MSG + end + DatabaseCleaner.clean_with(:truncation) end - config.around(:each) do |example| - DatabaseCleaner.cleaning { example.run } + config.before do + DatabaseCleaner.strategy = :transaction + end + + config.before(:each, type: :feature) do + # :rack_test driver's Rack app under test shares database connection + # with the specs, so continue to use transaction strategy for speed. + unless Capybara.current_driver == :rack_test + # Driver is probably for an external browser with an app + # under test that does *not* share a database connection with the + # specs, so use truncation strategy. + DatabaseCleaner.strategy = :truncation + end + end + + config.before do + DatabaseCleaner.start + end + + config.append_after do + DatabaseCleaner.clean end end