fixing tests p1

This commit is contained in:
Tom Staubitz
2017-01-12 18:12:36 +01:00
parent e4daa7ece3
commit 5fe12bfa78
11 changed files with 41 additions and 135 deletions

View File

@ -18,14 +18,13 @@ module Lti
# exercise_id.nil? ==> the user has logged out. All session data is to be destroyed # 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. # exercise_id.exists? ==> the user has submitted the results of an exercise to the consumer.
# Only the lti_parameters are deleted. # Only the lti_parameters are deleted.
def clear_lti_session_data(exercise_id = nil) def clear_lti_session_data(exercise_id = nil, user_id = nil, consumer_id = nil)
@current_user = ExternalUser.find(@submission.user_id)
if (exercise_id.nil?) if (exercise_id.nil?)
session.delete(:consumer_id) session.delete(:consumer_id)
session.delete(:external_user_id) session.delete(:external_user_id)
else else
LtiParameter.destroy_all(consumers_id: @consumer.id, LtiParameter.destroy_all(consumers_id: consumer_id,
external_user_id: @current_user.external_id, external_users_id: user_id,
exercises_id: exercise_id) exercises_id: exercise_id)
end end
end end
@ -99,12 +98,12 @@ module Lti
end end
private :return_to_consumer private :return_to_consumer
def send_score(exercise_id, score) def send_score(exercise_id, score, user_id)
::NewRelic::Agent.add_custom_parameters({ score: score, session: session }) ::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) fail(Error, "Score #{score} must be between 0 and #{MAXIMUM_SCORE}!") unless (0..MAXIMUM_SCORE).include?(score)
lti_parameter = LtiParameter.where(consumers_id: session[:consumer_id], lti_parameter = LtiParameter.where(consumers_id: session[:consumer_id],
external_user_id: @current_user.external_id, external_users_id: user_id,
exercises_id: exercise_id).first exercises_id: exercise_id).first
consumer = Consumer.find_by(id: session[:consumer_id]) consumer = Consumer.find_by(id: session[:consumer_id])
@ -123,7 +122,6 @@ module Lti
def set_current_user def set_current_user
@current_user = ExternalUser.find_or_create_by(consumer_id: @consumer.id, external_id: @provider.user_id) @current_user = ExternalUser.find_or_create_by(consumer_id: @consumer.id, external_id: @provider.user_id)
#TODO is this really necessary, and does it work at all?
@current_user.update(email: external_user_email(@provider), name: external_user_name(@provider)) @current_user.update(email: external_user_email(@provider), name: external_user_name(@provider))
end end
private :set_current_user private :set_current_user
@ -133,7 +131,7 @@ module Lti
exercise_id = exercise.id unless exercise.nil? exercise_id = exercise.id unless exercise.nil?
lti_parameters = LtiParameter.find_or_create_by(consumers_id: options[:consumer].id, lti_parameters = LtiParameter.find_or_create_by(consumers_id: options[:consumer].id,
external_user_id: options[:parameters][:user_id].to_s, external_users_id: @current_user.id, #options[:parameters][:user_id].to_s,
exercises_id: exercise_id) exercises_id: exercise_id)
lti_parameters.lti_parameters = options[:parameters].slice(*SESSION_PARAMETERS).to_json lti_parameters.lti_parameters = options[:parameters].slice(*SESSION_PARAMETERS).to_json

View File

@ -158,7 +158,7 @@ class ExercisesController < ApplicationController
def redirect_to_lti_return_path def redirect_to_lti_return_path
lti_parameter = LtiParameter.where(consumers_id: session[:consumer_id], lti_parameter = LtiParameter.where(consumers_id: session[:consumer_id],
external_user_id: @current_user.external_id, external_users_id: @current_user.id,
exercises_id: @submission.exercise_id).first exercises_id: @submission.exercise_id).first
path = lti_return_path(consumer_id: session[:consumer_id], path = lti_return_path(consumer_id: session[:consumer_id],
@ -228,7 +228,7 @@ class ExercisesController < ApplicationController
def submit def submit
@submission = Submission.create(submission_params) @submission = Submission.create(submission_params)
score_submission(@submission) score_submission(@submission)
if lti_outcome_service?(@submission.exercise_id, @current_user.external_id, @current_user.consumer_id) if lti_outcome_service?(@submission.exercise_id, @current_user.id, @current_user.consumer_id)
transmit_lti_score transmit_lti_score
else else
redirect_after_submit redirect_after_submit
@ -237,7 +237,7 @@ class ExercisesController < ApplicationController
def transmit_lti_score def transmit_lti_score
::NewRelic::Agent.add_custom_parameters({ submission: @submission.id, normalized_score: @submission.normalized_score }) ::NewRelic::Agent.add_custom_parameters({ submission: @submission.id, normalized_score: @submission.normalized_score })
response = send_score(@submission.exercise_id, @submission.normalized_score) response = send_score(@submission.exercise_id, @submission.normalized_score, @submission.user_id)
if response[:status] == 'success' if response[:status] == 'success'
redirect_after_submit redirect_after_submit

View File

@ -22,7 +22,7 @@ class SessionsController < ApplicationController
store_lti_session_data(consumer: @consumer, parameters: params) store_lti_session_data(consumer: @consumer, parameters: params)
store_nonce(params[:oauth_nonce]) store_nonce(params[:oauth_nonce])
redirect_to(implement_exercise_path(@exercise), redirect_to(implement_exercise_path(@exercise),
notice: t("sessions.create_through_lti.session_#{lti_outcome_service?(@exercise.id, @current_user.external_id , @consumer.id) ? 'with' : 'without'}_outcome", notice: t("sessions.create_through_lti.session_#{lti_outcome_service?(@exercise.id, @current_user.id , @consumer.id) ? 'with' : 'without'}_outcome",
consumer: @consumer)) consumer: @consumer))
end end
@ -36,9 +36,8 @@ class SessionsController < ApplicationController
end end
def destroy_through_lti def destroy_through_lti
@consumer = Consumer.find_by(id: params[:consumer_id])
@submission = Submission.find(params[:submission_id]) @submission = Submission.find(params[:submission_id])
clear_lti_session_data(@submission.exercise_id) clear_lti_session_data(@submission.exercise_id, @submission.user_id, params[:consumer_id])
end end
def new def new

View File

@ -3,7 +3,7 @@ module LtiHelper
return false if external_user_id == '' || consumer_id == '' return false if external_user_id == '' || consumer_id == ''
lti_parameters = LtiParameter.where(consumers_id: consumer_id, lti_parameters = LtiParameter.where(consumers_id: consumer_id,
external_user_id: external_user_id, external_users_id: external_user_id,
exercises_id: exercise_id).lis_outcome_service_url? exercises_id: exercise_id).lis_outcome_service_url?
!lti_parameters.nil? && lti_parameters.size > 0 !lti_parameters.nil? && lti_parameters.size > 0
end end

View File

@ -1,6 +1,7 @@
- external_user_id = @current_user.respond_to?(:external_id) ? @current_user.external_id : '' #'tests' #(@current_user.uuid.present? ? @current_user.uuid : '') - external_user_external_id = @current_user.respond_to?(:external_id) ? @current_user.external_id : '' #'tests' #(@current_user.uuid.present? ? @current_user.uuid : '')
- external_user_id = @current_user.respond_to?(:external_id) ? @current_user.id : '' #'tests' #(@current_user.uuid.present? ? @current_user.uuid : '')
- consumer_id = @current_user.respond_to?(:external_id) ? @current_user.consumer_id : '' #'tests' #(@current_user.uuid.present? ? @current_user.uuid : '') - consumer_id = @current_user.respond_to?(:external_id) ? @current_user.consumer_id : '' #'tests' #(@current_user.uuid.present? ? @current_user.uuid : '')
#editor.row data-exercise-id=exercise.id data-message-depleted=t('exercises.editor.depleted') data-message-timeout=t('exercises.editor.timeout', permitted_execution_time: @exercise.execution_environment.permitted_execution_time) data-errors-url=execution_environment_errors_path(exercise.execution_environment) data-submissions-url=submissions_path data-user-id=@current_user.id data-user-external-id=external_user_id #editor.row data-exercise-id=exercise.id data-message-depleted=t('exercises.editor.depleted') data-message-timeout=t('exercises.editor.timeout', permitted_execution_time: @exercise.execution_environment.permitted_execution_time) data-errors-url=execution_environment_errors_path(exercise.execution_environment) data-submissions-url=submissions_path data-user-id=@current_user.id data-user-external-id=external_user_external_id
div id="sidebar" class=(@exercise.hide_file_tree ? 'sidebar-col-collapsed' : 'sidebar-col') = render('editor_file_tree', exercise: @exercise, files: @files) div id="sidebar" class=(@exercise.hide_file_tree ? 'sidebar-col-collapsed' : 'sidebar-col') = render('editor_file_tree', exercise: @exercise, files: @files)
div id='output_sidebar' class='output-col-collapsed' = render('exercises/editor_output', external_user_id: external_user_id, consumer_id: consumer_id ) div id='output_sidebar' class='output-col-collapsed' = render('exercises/editor_output', external_user_id: external_user_id, consumer_id: consumer_id )
div id='frames' class='editor-col' div id='frames' class='editor-col'

View File

@ -1,11 +1,10 @@
class CreateLtiParameters < ActiveRecord::Migration class CreateLtiParameters < ActiveRecord::Migration
def change def change
create_table :lti_parameters do |t| create_table :lti_parameters do |t|
t.string :external_user_id t.belongs_to :external_users
t.belongs_to :consumers t.belongs_to :consumers
t.belongs_to :exercises t.belongs_to :exercises
t.jsonb :lti_parameters, null: false, default: '{}' t.jsonb :lti_parameters, null: false, default: '{}'
t.timestamps t.timestamps
end end
end end

View File

@ -485,43 +485,13 @@ CREATE SEQUENCE internal_users_id_seq
ALTER SEQUENCE internal_users_id_seq OWNED BY internal_users.id; ALTER SEQUENCE internal_users_id_seq OWNED BY internal_users.id;
--
-- Name: internal_users_teams; Type: TABLE; Schema: public; Owner: -
--
CREATE TABLE internal_users_teams (
id integer NOT NULL,
internal_user_id integer,
team_id integer
);
--
-- Name: internal_users_teams_id_seq; Type: SEQUENCE; Schema: public; Owner: -
--
CREATE SEQUENCE internal_users_teams_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
--
-- Name: internal_users_teams_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: -
--
ALTER SEQUENCE internal_users_teams_id_seq OWNED BY internal_users_teams.id;
-- --
-- Name: lti_parameters; Type: TABLE; Schema: public; Owner: - -- Name: lti_parameters; Type: TABLE; Schema: public; Owner: -
-- --
CREATE TABLE lti_parameters ( CREATE TABLE lti_parameters (
id integer NOT NULL, id integer NOT NULL,
external_user_id character varying, external_users_id integer,
consumers_id integer, consumers_id integer,
exercises_id integer, exercises_id integer,
lti_parameters jsonb DEFAULT '{}'::jsonb NOT NULL, lti_parameters jsonb DEFAULT '{}'::jsonb NOT NULL,
@ -591,7 +561,7 @@ ALTER SEQUENCE request_for_comments_id_seq OWNED BY request_for_comments.id;
-- --
CREATE TABLE schema_migrations ( CREATE TABLE schema_migrations (
version character varying NOT NULL version character varying(255) NOT NULL
); );
@ -630,37 +600,6 @@ CREATE SEQUENCE submissions_id_seq
ALTER SEQUENCE submissions_id_seq OWNED BY submissions.id; ALTER SEQUENCE submissions_id_seq OWNED BY submissions.id;
--
-- Name: teams; Type: TABLE; Schema: public; Owner: -
--
CREATE TABLE teams (
id integer NOT NULL,
name character varying,
created_at timestamp without time zone,
updated_at timestamp without time zone
);
--
-- Name: teams_id_seq; Type: SEQUENCE; Schema: public; Owner: -
--
CREATE SEQUENCE teams_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
--
-- Name: teams_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: -
--
ALTER SEQUENCE teams_id_seq OWNED BY teams.id;
-- --
-- Name: testruns; Type: TABLE; Schema: public; Owner: - -- Name: testruns; Type: TABLE; Schema: public; Owner: -
-- --
@ -779,13 +718,6 @@ ALTER TABLE ONLY hints ALTER COLUMN id SET DEFAULT nextval('hints_id_seq'::regcl
ALTER TABLE ONLY internal_users ALTER COLUMN id SET DEFAULT nextval('internal_users_id_seq'::regclass); ALTER TABLE ONLY internal_users ALTER COLUMN id SET DEFAULT nextval('internal_users_id_seq'::regclass);
--
-- Name: id; Type: DEFAULT; Schema: public; Owner: -
--
ALTER TABLE ONLY internal_users_teams ALTER COLUMN id SET DEFAULT nextval('internal_users_teams_id_seq'::regclass);
-- --
-- Name: id; Type: DEFAULT; Schema: public; Owner: - -- Name: id; Type: DEFAULT; Schema: public; Owner: -
-- --
@ -807,13 +739,6 @@ ALTER TABLE ONLY request_for_comments ALTER COLUMN id SET DEFAULT nextval('reque
ALTER TABLE ONLY submissions ALTER COLUMN id SET DEFAULT nextval('submissions_id_seq'::regclass); ALTER TABLE ONLY submissions ALTER COLUMN id SET DEFAULT nextval('submissions_id_seq'::regclass);
--
-- Name: id; Type: DEFAULT; Schema: public; Owner: -
--
ALTER TABLE ONLY teams ALTER COLUMN id SET DEFAULT nextval('teams_id_seq'::regclass);
-- --
-- Name: id; Type: DEFAULT; Schema: public; Owner: - -- Name: id; Type: DEFAULT; Schema: public; Owner: -
-- --
@ -917,14 +842,6 @@ ALTER TABLE ONLY internal_users
ADD CONSTRAINT internal_users_pkey PRIMARY KEY (id); ADD CONSTRAINT internal_users_pkey PRIMARY KEY (id);
--
-- Name: internal_users_teams_pkey; Type: CONSTRAINT; Schema: public; Owner: -
--
ALTER TABLE ONLY internal_users_teams
ADD CONSTRAINT internal_users_teams_pkey PRIMARY KEY (id);
-- --
-- Name: lti_parameters_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- Name: lti_parameters_pkey; Type: CONSTRAINT; Schema: public; Owner: -
-- --
@ -949,14 +866,6 @@ ALTER TABLE ONLY submissions
ADD CONSTRAINT submissions_pkey PRIMARY KEY (id); ADD CONSTRAINT submissions_pkey PRIMARY KEY (id);
--
-- Name: teams_pkey; Type: CONSTRAINT; Schema: public; Owner: -
--
ALTER TABLE ONLY teams
ADD CONSTRAINT teams_pkey PRIMARY KEY (id);
-- --
-- Name: testruns_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- Name: testruns_pkey; Type: CONSTRAINT; Schema: public; Owner: -
-- --
@ -1028,20 +937,6 @@ CREATE INDEX index_internal_users_on_remember_me_token ON internal_users USING b
CREATE INDEX index_internal_users_on_reset_password_token ON internal_users USING btree (reset_password_token); CREATE INDEX index_internal_users_on_reset_password_token ON internal_users USING btree (reset_password_token);
--
-- Name: index_internal_users_teams_on_internal_user_id; Type: INDEX; Schema: public; Owner: -
--
CREATE INDEX index_internal_users_teams_on_internal_user_id ON internal_users_teams USING btree (internal_user_id);
--
-- Name: index_internal_users_teams_on_team_id; Type: INDEX; Schema: public; Owner: -
--
CREATE INDEX index_internal_users_teams_on_team_id ON internal_users_teams USING btree (team_id);
-- --
-- Name: unique_schema_migrations; Type: INDEX; Schema: public; Owner: - -- Name: unique_schema_migrations; Type: INDEX; Schema: public; Owner: -
-- --
@ -1223,5 +1118,5 @@ INSERT INTO schema_migrations (version) VALUES ('20160704143402');
INSERT INTO schema_migrations (version) VALUES ('20160907123009'); INSERT INTO schema_migrations (version) VALUES ('20160907123009');
INSERT INTO schema_migrations (version) VALUES ('20161214144837'); INSERT INTO schema_migrations (version) VALUES ('20170112151637');

View File

@ -17,11 +17,8 @@ describe Lti do
describe '#clear_lti_session_data' do describe '#clear_lti_session_data' do
it 'clears the session' do it 'clears the session' do
#Todo replace session with lti_parameter
expect(controller.session).to receive(:delete).with(:consumer_id) 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(:external_user_id)
# 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) controller.send(:clear_lti_session_data)
end end
end end
@ -108,10 +105,13 @@ describe Lti do
describe '#send_score' do describe '#send_score' do
let(:consumer) { FactoryGirl.create(:consumer) } let(:consumer) { FactoryGirl.create(:consumer) }
let(:score) { 0.5 } let(:score) { 0.5 }
#let(:exercise) { FactoryGirl.create(:math) }
let(:submission) { FactoryGirl.create(:submission) }
let!(:lti_parameter) { FactoryGirl.create(:lti_parameter)}
context 'with an invalid score' do context 'with an invalid score' do
it 'raises an exception' do it 'raises an exception' do
expect { controller.send(:send_score, Lti::MAXIMUM_SCORE * 2) }.to raise_error(Lti::Error) expect { controller.send(:send_score, Lti::MAXIMUM_SCORE * 2, submission.exercise_id, submission.user_id) }.to raise_error(Lti::Error)
end end
end end
@ -168,9 +168,10 @@ describe Lti do
describe '#store_lti_session_data' do describe '#store_lti_session_data' do
#Todo replace session with lti_parameter #Todo replace session with lti_parameter
let(:parameters) { {} } let(:parameters) { {} }
before_count = LtiParameter.count
before(:each) { controller.instance_variable_set(:@current_user, FactoryGirl.create(:external_user)) } before(:each) { controller.instance_variable_set(:@current_user, FactoryGirl.create(:external_user)) }
#Todo do this with lti_parameter object #Todo do this with lti_parameter object
# after(:each) { controller.send(:store_lti_session_data, consumer: FactoryGirl.build(:consumer), parameters: parameters) } after(:each) { controller.send(:store_lti_session_data, consumer: FactoryGirl.build(:consumer), parameters: parameters) }
it 'stores data in the session' do it 'stores data in the session' do
#Todo replace session with lti_parameter #Todo replace session with lti_parameter
@ -178,6 +179,7 @@ describe Lti do
expect(controller.session).to receive(:[]=).with(:external_user_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 #Todo it creates an LtiParameter Object
expect(LtiParameter.count).to eq(before_count + 1)
end end
it 'stores only selected tuples' do it 'stores only selected tuples' do

View File

@ -0,0 +1,12 @@
FactoryGirl.define do
factory :lti_parameter do
association :consumers_id, factory: :consumer
association :exercises_id, factory: :math
association :external_users_id, factory: :external_user
trait :lti_parameters do
JSON.parse('{"lis_result_sourcedid": "c2db0c7c-4411-4b27-a52b-ddfc3dc32065", "lis_outcome_service_url": "http://172.16.54.235:3000/courses/0132156a-9afb-434d-83cc-704780104105/sections/21c6c6f4-1fb6-43b4-af3c-04fdc098879e/items/999b1fe6-d4b6-47b7-a577-ea2b4b1041ec/tool_grading", "launch_presentation_return_url": "http://172.16.54.235:3000/courses/0132156a-9afb-434d-83cc-704780104105/sections/21c6c6f4-1fb6-43b4-af3c-04fdc098879e/items/999b1fe6-d4b6-47b7-a577-ea2b4b1041ec/tool_return"}')
end
end
end

View File

@ -2,7 +2,7 @@ FactoryGirl.define do
factory :submission do factory :submission do
cause 'save' cause 'save'
created_by_external_user created_by_external_user
association :exercise, factory: :fibonacci association :exercise, factory: :math
after(:create) do |submission| after(:create) do |submission|
submission.exercise.files.editable.visible.each do |file| submission.exercise.files.editable.visible.each do |file|

View File

@ -3,8 +3,8 @@ require 'seeds_helper'
describe DockerClient, docker: true do describe DockerClient, docker: true do
let(:command) { 'whoami' } let(:command) { 'whoami' }
let(:docker_client) { described_class.new(execution_environment: FactoryGirl.build(:ruby), user: FactoryGirl.build(:admin)) } let(:docker_client) { described_class.new(execution_environment: FactoryGirl.build(:java), user: FactoryGirl.build(:admin)) }
let(:execution_environment) { FactoryGirl.build(:ruby) } let(:execution_environment) { FactoryGirl.build(:java) }
let(:image) { double } let(:image) { double }
let(:submission) { FactoryGirl.create(:submission) } let(:submission) { FactoryGirl.create(:submission) }
let(:workspace_path) { '/tmp' } let(:workspace_path) { '/tmp' }