From c2995c96f0e0e8b6294ff74fc9cae1e999911473 Mon Sep 17 00:00:00 2001 From: kiragrammel Date: Thu, 24 Aug 2023 16:34:30 +0200 Subject: [PATCH] Remove clear_lti_session_data as it is no longer needed * no lti session data anymore included in the function * decided not to delete the pg_id here as it is handled in create_through_lti and implement * the function is only called once without an exercise id so the values are now directly deleted there --- app/controllers/concerns/lti.rb | 18 ------------------ app/controllers/concerns/redirect_behavior.rb | 5 ----- app/controllers/sessions_controller.rb | 7 ++++--- spec/concerns/lti_spec.rb | 10 ---------- spec/controllers/sessions_controller_spec.rb | 10 ++++------ 5 files changed, 8 insertions(+), 42 deletions(-) diff --git a/app/controllers/concerns/lti.rb b/app/controllers/concerns/lti.rb index e33af02b..579e3f32 100644 --- a/app/controllers/concerns/lti.rb +++ b/app/controllers/concerns/lti.rb @@ -19,24 +19,6 @@ module Lti private :build_tool_provider - # exercise_id.nil? ==> the user has logged out. All session data is to be destroyed - # exercise_id.exists? ==> the user has submitted the results of an exercise to the consumer. - # Only the assignment of the programming group is deleted. - def clear_lti_session_data(exercise_id = nil) - if exercise_id.nil? - session.delete(:external_user_id) - session.delete(:study_group_id) - session.delete(:embed_options) - end - session.delete(:pg_id) - - # We allow reusing the LTI credentials and don't remove them on purpose. - # This allows users to jump between remote and web evaluation with the same behavior. - # Also it prevents the user from deleting the lti_parameters for their programming team members. - end - - private :clear_lti_session_data - def consumer_return_url(provider, options = {}) consumer_return_url = provider.try(:launch_presentation_return_url) || params[:launch_presentation_return_url] consumer_return_url += "?#{options.to_query}" if consumer_return_url && options.present? diff --git a/app/controllers/concerns/redirect_behavior.rb b/app/controllers/concerns/redirect_behavior.rb index f9fc1035..da1b80bb 100644 --- a/app/controllers/concerns/redirect_behavior.rb +++ b/app/controllers/concerns/redirect_behavior.rb @@ -16,7 +16,6 @@ module RedirectBehavior # redirect 10 percent pseudorandomly to the feedback page if current_user.respond_to? :external_id if @submission.redirect_to_feedback? && !@embed_options[:disable_redirect_to_feedback] - clear_lti_session_data(@submission.exercise_id) redirect_to_user_feedback return end @@ -27,7 +26,6 @@ module RedirectBehavior flash[:notice] = I18n.t('exercises.submit.full_score_redirect_to_own_rfc') flash.keep(:notice) - clear_lti_session_data(@submission.exercise_id) respond_to do |format| format.html { redirect_to(rfc) } format.json { render(json: {redirect: url_for(rfc)}) } @@ -45,7 +43,6 @@ module RedirectBehavior # increase counter 'times_featured' in rfc rfc.increment(:times_featured) - clear_lti_session_data(@submission.exercise_id) respond_to do |format| format.html { redirect_to(rfc) } format.json { render(json: {redirect: url_for(rfc)}) } @@ -56,7 +53,6 @@ module RedirectBehavior else # redirect to feedback page if score is less than 100 percent if @exercise.needs_more_feedback?(@submission) && !@embed_options[:disable_redirect_to_feedback] - clear_lti_session_data(@submission.exercise_id) redirect_to_user_feedback else redirect_to_lti_return_path @@ -128,7 +124,6 @@ module RedirectBehavior ) path = lti_return_path(submission_id: @submission.id) - clear_lti_session_data(@submission.exercise_id) respond_to do |format| format.html { redirect_to(path) } format.json { render(json: {redirect: path}) } diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index d7a30519..285acf61 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -47,13 +47,14 @@ class SessionsController < ApplicationController authorize(@submission, :show?) lti_parameter = current_user.lti_parameters.find_by(exercise: @submission.exercise, study_group_id: current_user.current_study_group_id) @url = consumer_return_url(build_tool_provider(consumer: current_user.consumer, parameters: lti_parameter&.lti_parameters)) - - clear_lti_session_data(@submission.exercise_id) end def destroy if current_user&.external_user? - clear_lti_session_data + session.delete(:external_user_id) + session.delete(:study_group_id) + session.delete(:embed_options) + session.delete(:pg_id) # In case we have another session as an internal user, we set the study group for this one internal_user = find_or_login_current_user diff --git a/spec/concerns/lti_spec.rb b/spec/concerns/lti_spec.rb index ba1f73c0..1f72f35e 100644 --- a/spec/concerns/lti_spec.rb +++ b/spec/concerns/lti_spec.rb @@ -17,16 +17,6 @@ describe Lti do end end - describe '#clear_lti_session_data' do - it 'clears the session' do - expect(controller.session).to receive(:delete).with(:external_user_id) - expect(controller.session).to receive(:delete).with(:study_group_id) - expect(controller.session).to receive(:delete).with(:embed_options) - expect(controller.session).to receive(:delete).with(:pg_id) - controller.send(:clear_lti_session_data) - end - end - describe '#external_user_name' do let(:first_name) { 'Jane' } let(:last_name) { 'Doe' } diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 943221a2..3c887c24 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -190,7 +190,10 @@ describe SessionsController do end it 'clears the session' do - expect(controller).to receive(:clear_lti_session_data) + expect(controller.session).to receive(:delete).with(:external_user_id) + expect(controller.session).to receive(:delete).with(:study_group_id) + expect(controller.session).to receive(:delete).with(:embed_options) + expect(controller.session).to receive(:delete).with(:pg_id) delete :destroy end @@ -210,11 +213,6 @@ describe SessionsController do perform_request.call end - it 'clears the session' do - expect(controller).to receive(:clear_lti_session_data) - perform_request.call - end - expect_http_status(:ok) expect_template(:destroy_through_lti) end