diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 0aa26303..617bab02 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -10,7 +10,6 @@ class ApplicationController < ActionController::Base rescue_from Pundit::NotAuthorizedError, with: :render_not_authorized def current_user - #Todo replace session with lti_parameter @current_user ||= ExternalUser.find_by(id: session[:external_user_id]) || login_from_session || login_from_other_sources end diff --git a/app/controllers/concerns/lti.rb b/app/controllers/concerns/lti.rb index b5e53e1a..cf489b7f 100644 --- a/app/controllers/concerns/lti.rb +++ b/app/controllers/concerns/lti.rb @@ -2,6 +2,7 @@ require 'oauth/request_proxy/rack_request' module Lti extend ActiveSupport::Concern + include LtiHelper MAXIMUM_SCORE = 1 MAXIMUM_SESSION_AGE = 60.minutes @@ -14,11 +15,17 @@ module Lti end private :build_tool_provider - def clear_lti_session_data - #Todo replace session with lti_parameter + def clear_lti_session_data(exercise_id = nil) + #Todo replace session with lti_parameter /done + #TODO decide if we need to remove all LtiParameters for user/consumer + if (exercise_id.nil?) + LtiParameter.destroy_all(consumers_id: session[:consumer_id], external_user_id: session[:external_user_id]) + else #TODO: probably it does not make sense to keep the LtiParameters if the session is deleted + LtiParameter.destroy_all(consumers_id: session[:consumer_id], external_user_id: session[:external_user_id], exercises_id: exercise_id) + end session.delete(:consumer_id) session.delete(:external_user_id) - session.delete(:lti_parameters) + #session.delete(:lti_parameters) end private :clear_lti_session_data @@ -44,12 +51,6 @@ module Lti end private :external_user_name - def lti_outcome_service? - #Todo replace session with lti_parameter - session[:lti_parameters].try(:has_key?, 'lis_outcome_service_url') - end - private :lti_outcome_service? - def refuse_lti_launch(options = {}) return_to_consumer(lti_errorlog: options[:message], lti_errormsg: t('sessions.oauth.failure')) end @@ -96,11 +97,18 @@ module Lti end private :return_to_consumer - def send_score(score) + def send_score(exercise_id, score) ::NewRelic::Agent.add_custom_parameters({ score: score, session: session }) fail(Error, "Score #{score} must be between 0 and #{MAXIMUM_SCORE}!") unless (0..MAXIMUM_SCORE).include?(score) - #Todo replace session with lti_parameter - provider = build_tool_provider(consumer: Consumer.find_by(id: session[:consumer_id]), parameters: session[:lti_parameters]) + #Todo replace session with lti_parameter /done + lti_parameter = LtiParameter.where(consumers_id: session[:consumer_id], + external_user_id: session[:external_user_id], + exercises_id: exercise_id).first + lti_parameters = JSON.parse(lti_parameter.lti_parameters) + + consumer = Consumer.find_by(id: session[:consumer_id]) + provider = build_tool_provider(consumer: consumer, parameters: lti_parameters) + # provider = build_tool_provider(consumer: Consumer.find_by(id: session[:consumer_id]), parameters: session[:lti_parameters]) if provider.nil? {status: 'error'} elsif provider.outcome_service? @@ -128,6 +136,9 @@ module Lti lti_parameters.lti_parameters = options[:parameters].slice(*SESSION_PARAMETERS).to_json lti_parameters.save! + + session[:consumer_id] = options[:consumer].id + session[:external_user_id] = @current_user.external_id end private :store_lti_session_data diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index e1ada974..34bc376f 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -157,8 +157,18 @@ class ExercisesController < ApplicationController end def redirect_to_lti_return_path - #Todo replace session with lti_parameter - path = lti_return_path(consumer_id: session[:consumer_id], submission_id: @submission.id, url: consumer_return_url(build_tool_provider(consumer: Consumer.find_by(id: session[:consumer_id]), parameters: session[:lti_parameters]))) + #Todo replace session with lti_parameter /done + lti_parameter = LtiParameter.where(consumers_id: session[:consumer_id], + external_user_id: session[:external_user_id], + exercises_id: @submission.exercise_id).first + + lti_parameters = JSON.parse(lti_parameter.lti_parameters) + + path = lti_return_path(consumer_id: session[:consumer_id], + submission_id: @submission.id, + url: consumer_return_url(build_tool_provider(consumer: Consumer.find_by(id: session[:consumer_id]), + parameters: lti_parameters))) + # parameters: session[:lti_parameters]))) respond_to do |format| format.html { redirect_to(path) } format.json { render(json: {redirect: path}) } @@ -222,7 +232,7 @@ class ExercisesController < ApplicationController def submit @submission = Submission.create(submission_params) score_submission(@submission) - if lti_outcome_service? + if lti_outcome_service?(@submission.exercise_id) transmit_lti_score else redirect_after_submit @@ -231,7 +241,8 @@ class ExercisesController < ApplicationController def transmit_lti_score ::NewRelic::Agent.add_custom_parameters({ submission: @submission.id, normalized_score: @submission.normalized_score }) - response = send_score(@submission.normalized_score) + response = send_score(@submission.exercise_id, @submission.normalized_score) + if response[:status] == 'success' redirect_after_submit else diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index ce1418bb..9a9a7029 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -21,7 +21,9 @@ class SessionsController < ApplicationController set_current_user store_lti_session_data(consumer: @consumer, parameters: params) store_nonce(params[:oauth_nonce]) - redirect_to(implement_exercise_path(@exercise), notice: t("sessions.create_through_lti.session_#{lti_outcome_service? ? 'with' : 'without'}_outcome", consumer: @consumer)) + redirect_to(implement_exercise_path(@exercise), + notice: t("sessions.create_through_lti.session_#{lti_outcome_service?(@exercise.id) ? 'with' : 'without'}_outcome", + consumer: @consumer)) end def destroy @@ -36,7 +38,8 @@ class SessionsController < ApplicationController def destroy_through_lti @consumer = Consumer.find_by(id: params[:consumer_id]) @submission = Submission.find(params[:submission_id]) - clear_lti_session_data + #TODO decide if we need to remove all LtiParameters for user/consumer + clear_lti_session_data(@submission.exercise_id) end def new diff --git a/app/helpers/exercise_helper.rb b/app/helpers/exercise_helper.rb index 7dcbb6f4..190a18f3 100644 --- a/app/helpers/exercise_helper.rb +++ b/app/helpers/exercise_helper.rb @@ -1,4 +1,6 @@ module ExerciseHelper + include LtiHelper + def embedding_parameters(exercise) "locale=#{I18n.locale}&token=#{exercise.token}" end diff --git a/app/helpers/lti_helper.rb b/app/helpers/lti_helper.rb new file mode 100644 index 00000000..4edf8445 --- /dev/null +++ b/app/helpers/lti_helper.rb @@ -0,0 +1,10 @@ +module LtiHelper + def lti_outcome_service?(exercise_id) + #Todo replace session with lti_parameter /done + lti_parameters = LtiParameter.where(consumers_id: session[:consumer_id], + external_user_id: session[:external_user_id], + exercises_id: exercise_id).lis_outcome_service_url? + !lti_parameters.nil? && lti_parameters.size > 0 + # session[:lti_parameters].try(:has_key?, 'lis_outcome_service_url') + end +end \ No newline at end of file diff --git a/app/models/lti_parameter.rb b/app/models/lti_parameter.rb index 667570be..a6b1917b 100644 --- a/app/models/lti_parameter.rb +++ b/app/models/lti_parameter.rb @@ -1,2 +1,5 @@ class LtiParameter < ActiveRecord::Base -end + scope :lis_outcome_service_url?, -> { + where("lti_parameters.lti_parameters ? 'lis_outcome_service_url'") + } +end \ No newline at end of file diff --git a/app/views/exercises/_editor_output.html.slim b/app/views/exercises/_editor_output.html.slim index 645d1a1d..cc3b6b98 100644 --- a/app/views/exercises/_editor_output.html.slim +++ b/app/views/exercises/_editor_output.html.slim @@ -27,8 +27,9 @@ div id='output_sidebar_uncollapsed' class='hidden col-sm-12 enforce-bottom-margi .progress-bar role='progressbar' br - / #Todo replace session with lti_parameter - - if session[:lti_parameters].try(:has_key?, 'lis_outcome_service_url') + / #Todo replace session with lti_parameter /done + /- if session[:lti_parameters].try(:has_key?, 'lis_outcome_service_url') + - if lti_outcome_service?(@exercise.id) p.text-center = render('editor_button', classes: 'btn-lg btn-success', data: {:'data-url' => submit_exercise_path(@exercise)}, icon: 'fa fa-send', id: 'submit', label: t('exercises.editor.submit')) - else p.text-center = render('editor_button', classes: 'btn-lg btn-warning-outline', data: {:'data-placement' => 'bottom', :'data-tooltip' => true}, icon: 'fa fa-clock-o', id: 'submit_outdated', label: t('exercises.editor.exercise_deadline_passed'), title: t('exercises.editor.tooltips.exercise_deadline_passed')) diff --git a/debian_installer/setup_debian_1_install_postgres.sh b/debian_installer/setup_debian_1_install_postgres.sh index f94c7ee0..515362f1 100644 --- a/debian_installer/setup_debian_1_install_postgres.sh +++ b/debian_installer/setup_debian_1_install_postgres.sh @@ -44,7 +44,7 @@ else echo "Postgres installation failed" fi -# create database +# create development database # TODO: extract databasename to variable if ! (sudo -u postgres psql -l | grep -q codeocean-development) then @@ -57,7 +57,13 @@ then sudo -u postgres psql -d codeocean-development -U postgres -c "CREATE USER codeocean;" sudo -u postgres psql -d codeocean-development -U postgres -c 'GRANT ALL PRIVILEGES ON DATABASE "codeocean-development" to codeocean'; sudo -u postgres psql -d codeocean-development -U postgres -c 'ALTER DATABASE "codeocean-development" OWNER TO codeocean'; + sudo -u postgres psql -d codeocean-development -U postgres -c 'ALTER USER "codeocean" CREATEDB'; echo "Done" else echo "Database codeocean-development already exists" -fi \ No newline at end of file +fi + +# TODO: create test database + + + diff --git a/spec/concerns/lti_spec.rb b/spec/concerns/lti_spec.rb index ab047306..87c11d02 100644 --- a/spec/concerns/lti_spec.rb +++ b/spec/concerns/lti_spec.rb @@ -20,7 +20,8 @@ describe Lti do #Todo replace session with lti_parameter expect(controller.session).to receive(:delete).with(:consumer_id) expect(controller.session).to receive(:delete).with(:external_user_id) - expect(controller.session).to receive(:delete).with(:lti_parameters) + # expect(controller.session).to receive(:delete).with(:lti_parameters) + #Todo check that there are no more LtiParameters for this user/consumer/(exercise?) controller.send(:clear_lti_session_data) end end @@ -119,7 +120,8 @@ describe Lti do before(:each) do #Todo replace session with lti_parameter controller.session[:consumer_id] = consumer.id - controller.session[:lti_parameters] = {} + # controller.session[:lti_parameters] = {} + #Todo create empty LtiParameter instead end context 'when grading is not supported' do @@ -167,13 +169,15 @@ describe Lti do #Todo replace session with lti_parameter let(:parameters) { {} } before(:each) { controller.instance_variable_set(:@current_user, FactoryGirl.create(:external_user)) } - after(:each) { controller.send(:store_lti_session_data, consumer: FactoryGirl.build(:consumer), parameters: parameters) } + #Todo do this with lti_parameter object + # after(:each) { controller.send(:store_lti_session_data, consumer: FactoryGirl.build(:consumer), parameters: parameters) } it 'stores data in the session' do #Todo replace session with lti_parameter expect(controller.session).to receive(:[]=).with(:consumer_id, anything) expect(controller.session).to receive(:[]=).with(:external_user_id, anything) - expect(controller.session).to receive(:[]=).with(:lti_parameters, kind_of(Hash)) + # expect(controller.session).to receive(:[]=).with(:lti_parameters, kind_of(Hash)) + #Todo it creates an LtiParameter Object end it 'stores only selected tuples' do diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 9062008f..0ad4b20d 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -4,7 +4,6 @@ describe ApplicationController do describe '#current_user' do context 'with an external user' do let(:external_user) { FactoryGirl.create(:external_user) } - #Todo replace session with lti_parameter before(:each) { session[:external_user_id] = external_user.id } it 'returns the external user' do diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 06fc68dc..ffa957ea 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -78,7 +78,6 @@ describe SessionsController do it 'assigns the current user' do request expect(assigns(:current_user)).to be_an(ExternalUser) - #Todo replace session with lti_parameter expect(session[:external_user_id]).to eq(user.id) end @@ -94,7 +93,7 @@ describe SessionsController do end it 'stores LTI parameters in the session' do - #Todo replace session with lti_parameter + #Todo replace session with lti_parameter /should be done already expect(controller).to receive(:store_lti_session_data) request end @@ -161,7 +160,7 @@ describe SessionsController do end it 'clears the session' do - #Todo replace session with lti_parameter + #Todo replace session with lti_parameter /should be done already expect(controller).to receive(:clear_lti_session_data) delete :destroy end @@ -179,13 +178,14 @@ describe SessionsController do before(:each) do #Todo replace session with lti_parameter session[:consumer_id] = consumer.id - session[:lti_parameters] = {} + #Todo create LtiParameter Object + # session[:lti_parameters] = {} end before(:each) { request.call } it 'clears the session' do - #Todo replace session with lti_parameter + #Todo replace session with lti_parameter /should be done already expect(controller).to receive(:clear_lti_session_data) request.call end