diff --git a/app/assets/javascripts/editor/submissions.js b/app/assets/javascripts/editor/submissions.js index 0670731c..e1d0356d 100644 --- a/app/assets/javascripts/editor/submissions.js +++ b/app/assets/javascripts/editor/submissions.js @@ -211,10 +211,13 @@ CodeOceanEditorSubmissions = { Turbolinks.visit(response.redirect); } else if (response.status === 'container_depleted') { this.showContainerDepletedMessage(); - } else if (response.message) { - $.flash.danger({ - text: response.message - }); + } else { + for (let [type, text] of Object.entries(response)) { + $.flash[type]({ + text: text, + showPermanent: true // We might display a very long text message! + }) + } } this.initializeEventHandlers(); }) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 5b971045..21a2e726 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -18,6 +18,7 @@ class ApplicationController < ActionController::Base rescue_from Pundit::NotAuthorizedError, with: :render_not_authorized rescue_from ActiveRecord::RecordNotFound, with: :render_not_found rescue_from ActionController::InvalidAuthenticityToken, with: :render_csrf_error + add_flash_types :danger, :warning, :info, :success def current_user @current_user ||= find_or_login_current_user&.store_current_study_group_id(session[:study_group_id]) diff --git a/app/controllers/concerns/lti.rb b/app/controllers/concerns/lti.rb index a863e560..22111b68 100644 --- a/app/controllers/concerns/lti.rb +++ b/app/controllers/concerns/lti.rb @@ -9,6 +9,7 @@ module Lti MAXIMUM_SCORE = 1 MAXIMUM_SESSION_AGE = 60.minutes SESSION_PARAMETERS = %w[launch_presentation_return_url lis_outcome_service_url lis_result_sourcedid].freeze + ERROR_STATUS = %w[error unsupported].freeze def build_tool_provider(options = {}) if options[:consumer] && options[:parameters] @@ -135,21 +136,27 @@ module Lti private :return_to_consumer - def send_score(submission) + def send_scores(submission) unless (0..MAXIMUM_SCORE).cover?(submission.normalized_score) raise Error.new("Score #{submission.normalized_score} must be between 0 and #{MAXIMUM_SCORE}!") end - if submission.contributor.consumer - lti_parameter = LtiParameter.where(consumers_id: submission.contributor.consumer.id, - external_users_id: submission.contributor_id, + submission.users.map {|user| send_score_for submission, user } + end + + private :send_scores + + def send_score_for(submission, user) + if user.consumer + lti_parameter = LtiParameter.where(consumers_id: user.consumer.id, + external_users_id: user.id, exercises_id: submission.exercise_id).last - provider = build_tool_provider(consumer: submission.contributor.consumer, parameters: lti_parameter.lti_parameters) + provider = build_tool_provider(consumer: user.consumer, parameters: lti_parameter&.lti_parameters) end if provider.nil? - {status: 'error'} + {status: 'error', user: user.displayname} elsif provider.outcome_service? Sentry.set_extras({ provider: provider.inspect, @@ -171,17 +178,17 @@ module Lti begin response = provider.post_replace_result!(normalized_lti_score) - {code: response.response_code, message: response.post_response.body, status: response.code_major, score_sent: normalized_lti_score} + {code: response.response_code, message: response.post_response.body, status: response.code_major, score_sent: normalized_lti_score, user: user.displayname} rescue IMS::LTI::XMLParseError, Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET, SocketError # A parsing error might happen if the LTI provider is down and doesn't return a valid XML response - {status: 'error'} + {status: 'error', user: user.displayname} end else - {status: 'unsupported'} + {status: 'unsupported', user: user.displayname} end end - private :send_score + private :send_score_for def set_current_user @current_user = ExternalUser.find_or_create_by(consumer_id: @consumer.id, external_id: @provider.user_id) diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 3aa13c9f..f223d2ab 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -536,25 +536,39 @@ class ExercisesController < ApplicationController Rails.logger.debug { "Runner error while submitting submission #{@submission.id}: #{e.message}" } respond_to do |format| format.html { redirect_to(implement_exercise_path(@submission.exercise)) } - format.json { render(json: {message: I18n.t('exercises.editor.depleted'), status: :container_depleted}) } + format.json { render(json: {danger: I18n.t('exercises.editor.depleted'), status: :container_depleted}) } end end def transmit_lti_score - response = send_score(@submission) + responses = send_scores(@submission) + messages = {} + failed_users = [] - if response[:status] == 'success' - if response[:score_sent] != @submission.normalized_score - # Score has been reduced due to the passed deadline - flash.now[:warning] = I18n.t('exercises.submit.too_late') - flash.keep(:warning) + responses.each do |response| + if Lti::ERROR_STATUS.include? response[:status] + failed_users << response[:user] + elsif response[:score_sent] != @submission.normalized_score # the score was sent successfully, but received too late + messages[:warning] = I18n.t('exercises.submit.too_late') end - redirect_after_submit + end + + if failed_users.size == responses.size # all submissions failed + messages[:danger] = I18n.t('exercises.submit.failure') + elsif failed_users.size.positive? # at least one submission failed + messages[:warning] = [[messages[:warning]], I18n.t('exercises.submit.warning_not_for_all_users_submitted', user: failed_users.join(', '))].join('

') + messages[:warning] = "#{messages[:warning]}\n\n#{I18n.t('exercises.submit.warning_not_for_all_users_submitted', user: failed_users.join(', '))}".strip else - respond_to do |format| - format.html { redirect_to(implement_exercise_path(@submission.exercise, alert: I18n.t('exercises.submit.failure'))) } - format.json { render(json: {message: I18n.t('exercises.submit.failure')}, status: :service_unavailable) } + messages.each do |type, message_text| + flash.now[type] = message_text + flash.keep(type) end + return redirect_after_submit + end + + respond_to do |format| + format.html { redirect_to(implement_exercise_path(@submission.exercise), **messages) } + format.json { render(json: messages) } # We must not change the HTTP status code here, since otherwise the custom message is not displayed. end end diff --git a/app/controllers/remote_evaluation_controller.rb b/app/controllers/remote_evaluation_controller.rb index 062fdb7e..a9eb78ad 100644 --- a/app/controllers/remote_evaluation_controller.rb +++ b/app/controllers/remote_evaluation_controller.rb @@ -36,8 +36,8 @@ class RemoteEvaluationController < ApplicationController def try_lti # TODO: Need to consider and support programming groups if !@submission.user.nil? && lti_outcome_service?(@submission.exercise_id, @submission.user.id) - lti_response = send_score(@submission) - process_lti_response(lti_response) + lti_responses = send_scores(@submission) + process_lti_response(lti_responses.first) else { message: "Your submission was successfully scored with #{@submission.normalized_score * 100}%. " \ diff --git a/app/models/submission.rb b/app/models/submission.rb index ce6485b5..1d63afed 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -72,11 +72,11 @@ class Submission < ApplicationRecord end def normalized_score - if !score.nil? && !exercise.maximum_score.nil? && exercise.maximum_score.positive? - score / exercise.maximum_score - else - 0 - end + @normalized_score ||= if !score.nil? && !exercise.maximum_score.nil? && exercise.maximum_score.positive? + score / exercise.maximum_score + else + 0 + end end def percentage diff --git a/app/views/sessions/destroy_through_lti.html.slim b/app/views/sessions/destroy_through_lti.html.slim index e95ab6b4..2a44b4e9 100644 --- a/app/views/sessions/destroy_through_lti.html.slim +++ b/app/views/sessions/destroy_through_lti.html.slim @@ -1,6 +1,6 @@ h1 = t('.headline') -- consumer = @submission.user.consumer +- consumer = current_user.consumer p = t(".success_#{consumer ? 'with' : 'without'}_outcome", consumer: consumer) diff --git a/config/locales/de.yml b/config/locales/de.yml index df031a31..b885050b 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -550,6 +550,7 @@ de: too_late: Ihre Abgabe wurde erfolgreich gespeichert, ging jedoch nach der Abgabefrist ein. full_score_redirect_to_rfc: Herzlichen Glückwunsch! Sie haben die maximale Punktzahl für diese Aufgabe an den Kurs übertragen. Ein anderer Teilnehmer hat eine Frage zu der von Ihnen gelösten Aufgabe. Er würde sich sicherlich sehr über ihre Hilfe und Kommentare freuen. full_score_redirect_to_own_rfc: Herzlichen Glückwunsch! Sie haben die maximale Punktzahl für diese Aufgabe an den Kurs übertragen. Ihre Frage ist damit wahrscheinlich gelöst? Falls ja, fügen Sie doch den entscheidenden Kniff als Antwort hinzu und markieren die Frage als gelöst, bevor sie das Fenster schließen. + warning_not_for_all_users_submitted: "Die Punkteübertragung war nur teilweise erfolgreich. Die Punkte konnten nicht für %{user} übertragen werden. Diese Person(en) sollte die Aufgabe über die e-Learning Platform erneut öffnen und anschließend die Punkte selbst übermitteln." study_group_dashboard: live_dashboard: Live Dashboard time_spent_per_learner: Verwendete Zeit pro Lerner diff --git a/config/locales/en.yml b/config/locales/en.yml index 6704839f..87f289e2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -550,6 +550,7 @@ en: too_late: Your submission was saved successfully but was received after the deadline passed. full_score_redirect_to_rfc: Congratulations! You achieved and submitted the highest possible score for this exercise. Another participant has a question concerning the exercise you just solved. Your help and comments will be greatly appreciated! full_score_redirect_to_own_rfc: Congratulations! You achieved and submitted the highest possible score for this exercise. Your question concerning the exercise is solved? If so, please share the essential insight with your fellows and mark the question as solved, before you close this window! + warning_not_for_all_users_submitted: "The transmission of points was only partially successful. The score was not transmitted for %{user}. The user(s) should reopen the exercise via the e-learning platform and then try to submit the points themselves." study_group_dashboard: live_dashboard: Live Dashboard time_spent_per_learner: Time spent per Learner diff --git a/spec/concerns/lti_spec.rb b/spec/concerns/lti_spec.rb index 2a9cd308..85655e8a 100644 --- a/spec/concerns/lti_spec.rb +++ b/spec/concerns/lti_spec.rb @@ -102,7 +102,7 @@ describe Lti do end end - describe '#send_score' do + describe '#send_scores' do let(:consumer) { create(:consumer) } let(:score) { 0.5 } let(:submission) { create(:submission) } @@ -114,7 +114,7 @@ describe Lti do context 'with an invalid score' do it 'raises an exception' do allow(submission).to receive(:normalized_score).and_return Lti::MAXIMUM_SCORE * 2 - expect { controller.send(:send_score, submission) }.to raise_error(Lti::Error) + expect { controller.send(:send_scores, submission) }.to raise_error(Lti::Error) end end @@ -124,13 +124,13 @@ describe Lti do it 'returns a corresponding status' do allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:outcome_service?).and_return(false) allow(submission).to receive(:normalized_score).and_return score - expect(controller.send(:send_score, submission)[:status]).to eq('unsupported') + expect(controller.send(:send_scores, submission).first[:status]).to eq('unsupported') end end context 'when grading is supported' do let(:response) { double } - let(:send_score) { controller.send(:send_score, submission) } + let(:send_scores) { controller.send(:send_scores, submission).first } before do allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:outcome_service?).and_return(true) @@ -144,13 +144,13 @@ describe Lti do it 'sends the score' do expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:post_replace_result!).with(score) - send_score + send_scores end it 'returns code, message, and status' do - expect(send_score[:code]).to eq(response.response_code) - expect(send_score[:message]).to eq(response.body) - expect(send_score[:status]).to eq(response.code_major) + expect(send_scores[:code]).to eq(response.response_code) + expect(send_scores[:message]).to eq(response.body) + expect(send_scores[:status]).to eq(response.code_major) end end end @@ -160,7 +160,7 @@ describe Lti do submission.contributor.consumer = nil allow(submission).to receive(:normalized_score).and_return score - expect(controller.send(:send_score, submission)[:status]).to eq('error') + expect(controller.send(:send_scores, submission).first[:status]).to eq('error') end end end diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index c32061ce..53f5f89d 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -325,7 +325,7 @@ describe ExercisesController do context 'when the score transmission succeeds' do before do - allow(controller).to receive(:send_score).and_return(status: 'success') + allow(controller).to receive(:send_scores).and_return([{status: 'success'}]) perform_request end @@ -341,7 +341,7 @@ describe ExercisesController do context 'when the score transmission fails' do before do - allow(controller).to receive(:send_score).and_return(status: 'unsupported') + allow(controller).to receive(:send_scores).and_return([{status: 'unsupported'}]) perform_request end @@ -351,8 +351,11 @@ describe ExercisesController do expect(assigns(:submission)).to be_a(Submission) end + it 'returns an error message' do + expect(response.parsed_body).to eq('danger' => I18n.t('exercises.submit.failure')) + end + expect_json - expect_http_status(:service_unavailable) end end @@ -369,7 +372,7 @@ describe ExercisesController do end it 'does not send scores' do - expect(controller).not_to receive(:send_score) + expect(controller).not_to receive(:send_scores) end expect_json diff --git a/spec/models/exercise_spec.rb b/spec/models/exercise_spec.rb index 3f11d0c3..6b2aa263 100644 --- a/spec/models/exercise_spec.rb +++ b/spec/models/exercise_spec.rb @@ -91,7 +91,7 @@ describe Exercise do context 'without submissions' do it 'returns nil' do - expect(exercise.average_score).to be 0 + expect(exercise.average_score).to be 0.0 end end