Extract ScoringChecks and rework RemoteEvaluations

This commit is contained in:
Sebastian Serth
2023-11-01 22:10:04 +01:00
committed by Sebastian Serth
parent 175c8933f3
commit fe26378387
7 changed files with 331 additions and 93 deletions

View File

@ -0,0 +1,52 @@
# frozen_string_literal: true
module ScoringChecks
def check_submission(submit_info)
lti_check = check_lti_transmission(submit_info[:users])
# If we got a `:scoring_failure` from the LTI check, we want to display this message exclusively.
return [lti_check] if lti_check.present? && lti_check[:status] == :scoring_failure
# Otherwise, the score was sent successfully for the current user,
# or it was not attempted for any user (i.e., no `lis_outcome_service` was available).
# In any way, we want to check for further conditions and return all messages.
[
lti_check,
check_scoring_too_late(submit_info),
check_full_score,
]
end
private
def check_full_score
# The submission was not scored with the full score, hence the exercise is not finished yet.
return unless @submission.full_score?
{status: :exercise_finished, url: finalize_submission_path(@submission)}
end
def check_lti_transmission(scored_users)
if scored_users[:all] == scored_users[:error] || scored_users[:error].include?(current_user)
# The score was not sent for any user or sending the score for the current user failed.
# In the latter case, we want to encourage the current user to reopen the exercise through the LMS.
# Hence, we always display the most severe error message.
{status: :scoring_failure}
elsif scored_users[:all] != scored_users[:success] && scored_users[:success].include?(current_user)
# The score was sent successfully for current user.
# However, at the same time, the transmission failed for some other users.
# This could either be due to a temporary network error, which is unlikely, or a more "permanent" error.
# Permanent errors would be that the deadline has passed on the LMS (which would then not provide a `lis_outcome_service`),
# working together with an internal user, or with someone who has never opened the exercise before.
{status: :not_for_all_users_submitted, failed_users: scored_users[:error].map(&:displayname).join(', ')}
end
end
def check_scoring_too_late(submit_info)
# The submission was either performed before any deadline or no deadline was configured at all for the current exercise.
return if %i[within_grace_period after_late_deadline].exclude? submit_info[:deadline]
# The `lis_outcome_service` was not provided by the LMS, hence we were not able to send any score.
return if submit_info[:users][:unsupported].include?(current_user)
{status: :scoring_too_late, score_sent: (submit_info[:score][:sent] * 100).to_i}
end
end

View File

@ -1,8 +1,9 @@
# frozen_string_literal: true
class RemoteEvaluationController < ApplicationController
include RemoteEvaluationParameters
include Lti
include ScoringChecks
include RemoteEvaluationParameters
skip_after_action :verify_authorized
skip_before_action :verify_authenticity_token
@ -11,66 +12,55 @@ class RemoteEvaluationController < ApplicationController
# POST /evaluate
def evaluate
result = create_and_score_submission('remoteAssess')
status = if result.is_a?(Hash) && result.key?(:status)
result[:status]
else
201
end
render json: result, status:
# For this route, we don't want to display the LTI result, but only the result of the submission.
try_lti if result.key?(:feedback)
render json: result.fetch(:feedback, result), status: result.fetch(:status, 201)
end
# POST /submit
def submit
result = create_and_score_submission('remoteSubmit')
status = 201
if @submission.present?
score_achieved_percentage = @submission.normalized_score
result = try_lti
result[:score] = score_achieved_percentage * 100 unless result[:score]
status = result[:status]
result = try_lti if result.key?(:feedback)
render json: result, status: result.fetch(:status, 201)
end
render json: result, status:
end
private
def try_lti
# TODO: Need to consider and support programming groups
if !@submission.user.nil? && lti_outcome_service?(@submission.exercise, @submission.user, @submission.study_group_id)
lti_responses = send_scores(@submission)
process_lti_response(lti_responses.first)
else
{
message: "Your submission was successfully scored with #{@submission.normalized_score * 100}%. " \
'However, your score could not be sent to the e-Learning platform. Please check ' \
'the submission deadline, reopen the exercise through the e-Learning platform and try again.',
status: 410,
}
end
end
private :try_lti
check_lti_results = check_lti_transmission(lti_responses[:users]) || {}
score = (@submission.normalized_score * 100).to_i
def process_lti_response(lti_response)
if (lti_response[:status] == 'success') && (lti_response[:score_sent] != @submission.normalized_score)
# Score has been reduced due to the passed deadline
{message: I18n.t('exercises.editor.submit_too_late', score_sent: lti_response[:score_sent] * 100), status: 207, score: lti_response[:score_sent] * 100}
elsif lti_response[:status] == 'success'
{message: I18n.t('sessions.destroy_through_lti.success_with_outcome', consumer: @submission.user.consumer.name), status: 202}
# Since we are in an API context, we only want to return a **single** JSON response.
# For simplicity, we always return the most severe error message.
if lti_responses[:users][:all] == lti_responses[:users][:unsupported]
# No LTI transmission was attempted, i.e., no `lis_outcome_service` was provided by the LMS.
{message: I18n.t('exercises.editor.submit_failure_remote', score:), status: 410, score:}
elsif check_lti_results[:status] == :scoring_failure
{message: I18n.t('exercises.editor.submit_failure_all'), status: 424, score:}
elsif check_lti_results[:status] == :not_for_all_users_submitted
{message: I18n.t('exercises.editor.submit_failure_other_users', user: check_lti_results[:failed_users]), status: 417, score:}
elsif check_scoring_too_late(lti_responses).present?
score_sent = (lti_responses[:score][:sent] * 100).to_i
{message: I18n.t('exercises.editor.submit_too_late', score_sent:), status: 207, score: score_sent}
elsif check_full_score.present?
{message: I18n.t('exercises.editor.exercise_finished_remote', consumer: current_user.consumer.name), status: 200, score:}
else
{message: I18n.t('exercises.editor.submit_failure_all'), status: 424}
{message: I18n.t('sessions.destroy_through_lti.success_with_outcome', consumer: current_user.consumer.name), status: 202, score:}
end
# TODO: Delete LTI parameters?
end
private :process_lti_response
def create_and_score_submission(cause)
validation_token = remote_evaluation_params[:validation_token]
if (remote_evaluation_mapping = RemoteEvaluationMapping.find_by(validation_token:))
@current_user = remote_evaluation_mapping.user
@submission = Submission.create(build_submission_params(cause, remote_evaluation_mapping))
@submission.calculate_score(remote_evaluation_mapping.user)
feedback = @submission.calculate_score(remote_evaluation_mapping.user)
{message: I18n.t('exercises.editor.run_success'), status: 201, feedback:}
else
# TODO: better output
# TODO: check token expired?
{message: 'No exercise found for this validation_token! Please keep out!', status: 401}
{message: I18n.t('exercises.editor.submit_no_validation_token'), status: 401}
end
rescue Runner::Error::RunnerInUse => e
Rails.logger.debug { "Scoring a submission failed because the runner was already in use: #{e.message}" }
@ -79,7 +69,6 @@ class RemoteEvaluationController < ApplicationController
Rails.logger.debug { "Runner error while scoring submission #{@submission.id}: #{e.message}" }
{message: I18n.t('exercises.editor.depleted'), status: 503}
end
private :create_and_score_submission
def build_submission_params(cause, remote_evaluation_mapping)
Sentry.set_user(
@ -98,5 +87,4 @@ class RemoteEvaluationController < ApplicationController
reject_illegal_file_attributes(remote_evaluation_mapping.exercise, files_attributes)
submission_params
end
private :build_submission_params
end

View File

@ -5,6 +5,7 @@ class SubmissionsController < ApplicationController
include FileConversion
include Lti
include RedirectBehavior
include ScoringChecks
include SubmissionParameters
include Tubesock::Hijack
@ -266,7 +267,10 @@ class SubmissionsController < ApplicationController
# To enable hints when scoring a submission, uncomment the next line:
# send_hints(client_socket, StructuredError.where(submission: @submission))
transmit_lti_score(client_socket)
# Finally, send the score to the LTI consumer and check for notifications
check_submission(send_scores(@submission)).compact.each do |notification|
client_socket&.send_data(notification&.merge(cmd: :status)&.to_json)
end
rescue Runner::Error::RunnerInUse => e
extract_durations(e)
send_and_store client_socket, {cmd: :status, status: :runner_in_use}
@ -481,53 +485,6 @@ class SubmissionsController < ApplicationController
}
end
def check_scoring_too_late(submit_info)
# The submission was either performed before any deadline or no deadline was configured at all for the current exercise.
return if %i[within_grace_period after_late_deadline].exclude? submit_info[:deadline]
# The `lis_outcome_service` was not provided by the LMS, hence we were not able to send any score.
return if submit_info[:users][:unsupported].include?(current_user)
{status: :scoring_too_late, score_sent: submit_info[:score][:sent]}
end
def check_full_score
# The submission was not scored with the full score, hence the exercise is not finished yet.
return unless @submission.full_score?
{status: :exercise_finished, url: finalize_submission_path(@submission)}
end
def transmit_lti_score(client_socket)
submit_info = send_scores(@submission)
scored_users = submit_info[:users]
notifications = []
if scored_users[:all] == scored_users[:error] || scored_users[:error].include?(current_user)
# The score was not sent for any user or sending the score for the current user failed.
# In the latter case, we want to encourage the current user to reopen the exercise through the LMS.
# Hence, we always display the most severe error message.
notifications << {status: :scoring_failure}
elsif scored_users[:all] != scored_users[:success] && scored_users[:success].include?(current_user)
# The score was sent successfully for current user.
# However, at the same time, the transmission failed for some other users.
# This could either be due to a temporary network error, which is unlikely, or a more "permanent" error.
# Permanent errors would be that the deadline has passed on the LMS (which would then not provide a `lis_outcome_service`),
# working together with an internal user, or with someone who has never opened the exercise before.
notifications << {status: :not_for_all_users_submitted, failed_users: scored_users[:error].map(&:displayname).join(', ')}
end
if notifications.empty? || notifications.first[:status] != :scoring_failure
# Either, the score was sent successfully for the current user,
# or it was not attempted for any user (i.e., no `lis_outcome_service`).
notifications << check_scoring_too_late(submit_info)
notifications << check_full_score
end
notifications.compact.each do |notification|
client_socket&.send_data(notification&.merge(cmd: :status)&.to_json)
end
end
def retrieve_message_from_output(data, stream)
parsed = JSON.parse(data)
if parsed.instance_of?(Hash) && parsed.key?('cmd')

View File

@ -391,6 +391,7 @@ de:
exercise_finished: Herzlichen Glückwunsch! Sie haben die Aufgabe vollständig gelöst. <a href="%{url}" class="alert-link">Klicken Sie hier, um die Aufgabe jetzt abzuschließen.</a>
exercise_finished_redirect_to_rfc: Herzlichen Glückwunsch! Sie haben die Aufgabe vollständig gelöst und die Punkte ü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.
exercise_finished_redirect_to_own_rfc: Herzlichen Glückwunsch! Sie haben die Aufgabe vollständig gelöst und die Punkte ü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.
exercise_finished_remote: Herzlichen Glückwunsch! Sie haben die Aufgabe vollständig gelöst und die Punkte erfolgreich an %{consumer} übertragen.
expand_action_sidebar: Aktions-Leiste Ausklappen
expand_output_sidebar: Ausgabe-Leiste Ausklappen
input: Ihre Eingabe
@ -414,7 +415,9 @@ de:
stop: Stoppen
submit_failure_all: Beim Übermitteln Ihrer Punktzahl ist ein Fehler aufgetreten. Bitte versuchen Sie es später erneut.
submit_failure_other_users: "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."
submit_too_late: Ihre Abgabe wurde erfolgreich gespeichert, ging jedoch nach der Abgabefrist ein, sodass nur %{score_sent} Punkte übertragen wurden.
submit_failure_remote: "Ihre Abgabe wurde erfolgreich mit %{score}% bewertet. Ihr Ergebnis konnte jedoch nicht an die E-Learning-Plattform gesendet werden. Bitte überprüfen Sie die Abgabefrist, öffnen Sie die Übung über die E-Learning-Plattform und versuchen Sie es erneut."
submit_no_validation_token: Keine Übung für dieses Validierungstoken gefunden!
submit_too_late: Ihre Abgabe wurde erfolgreich gespeichert, ging jedoch nach der Abgabefrist ein, sodass nur %{score_sent}% übertragen wurden.
deadline: Deadline
test: Testen
timeout: 'Ausführung gestoppt. Ihr Code hat die erlaubte Ausführungszeit von %{permitted_execution_time} Sekunden überschritten.'

View File

@ -389,6 +389,7 @@ en:
download: Download
dummy: No Action
exercise_finished: Congratulations! You have completely solved this exercise. <a href="%{url}" class="alert-link">Please click here to finish the exercise now.</a>
exercise_finished_remote: Congratulations! You have completely solved this exercise and submitted the points to %{consumer}.
exercise_finished_redirect_to_rfc: Congratulations! You have completely solved this exercise and submitted the points. Another participant has a question concerning the exercise you just solved. Your help and comments will be greatly appreciated!
exercise_finished_redirect_to_own_rfc: Congratulations! You have completely solved this exercise and submitted the points. 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!
expand_action_sidebar: Expand Action Sidebar
@ -414,7 +415,9 @@ en:
stop: Stop
submit_failure_all: An error occurred while transmitting your score. Please try again later.
submit_failure_other_users: "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."
submit_too_late: Your submission was saved successfully but was received after the deadline, so that only %{score_sent} points were transmitted.
submit_failure_remote: "Your submission was successfully scored with %{score}%. However, your score could not be sent to the e-learning platform. Please check the submission deadline, reopen the exercise through the e-learning platform, and try again."
submit_no_validation_token: No exercise found for this validation token! Please keep out!
submit_too_late: Your submission was saved successfully but was received after the deadline, so that only %{score_sent}% were transmitted.
deadline: Deadline
test: Test
timeout: 'Execution stopped. Your code exceeded the permitted execution time of %{permitted_execution_time} seconds.'

View File

@ -0,0 +1,213 @@
# frozen_string_literal: true
require 'rails_helper'
RSpec.describe RemoteEvaluationController do
let(:contributor) { create(:external_user) }
let(:exercise) { create(:hello_world) }
let(:programming_group) { nil }
let(:validation_token) { create(:remote_evaluation_mapping, user: contributor, exercise:).validation_token }
let(:files_attributes) { {'0': {file_id: exercise.files.find_by(role: 'main_file').id, content: ''}} }
let(:remote_evaluation_params) { {remote_evaluation: {validation_token:, files_attributes:}} }
let(:calculate_response) do
[{
status: :ok,
stdout: '',
stderr: '',
waiting_for_container_time: 0,
container_execution_time: 0,
file_role: :teacher_defined_test,
count: 1,
failed: 0,
error_messages: [],
passed: 1,
score:,
filename: 'exercise_spec.rb',
message: 'Well done.',
weight: 1.0,
hidden_feedback: false,
exit_code: 0,
}]
end
shared_examples 'response' do |info|
it 'returns a message and a status' do
options = {}
options[:score_sent] = (score_sent * 100).to_i if defined? score_sent
options[:score] = (score * 100).to_i if defined? score
options[:user] = users_error.map(&:displayname).join(', ') if defined? users_error
options[:consumer] = contributor.consumer.name if defined? contributor
expect(response.parsed_body.symbolize_keys[:message]).to eq(I18n.t(info[:message], **options))
expect(response.parsed_body.symbolize_keys[:status]).to eq(info[:status])
end
end
before do
allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(contributor)
end
describe '#POST submit' do
let(:perform_request) { proc { post :submit, params: remote_evaluation_params } }
let(:scoring_response) do
{
users: {all: users_success + users_error + users_unsupported, success: users_success, error: users_error, unsupported: users_unsupported},
score: {original: score, sent: score_sent},
deadline:,
detailed_results: [],
}
end
context 'when remote evaluation mapping is available' do
context 'when the scoring is successful' do
let(:score) { 1 }
let(:score_sent) { score }
let(:deadline) { :none }
before do
allow_any_instance_of(Submission).to receive_messages(calculate_score: calculate_response, score:)
allow_any_instance_of(described_class).to receive(:send_scores).and_return(scoring_response)
perform_request.call
end
context 'when no LTI transmission was attempted' do
let(:users_success) { [] }
let(:users_error) { [] }
let(:users_unsupported) { [contributor] }
it_behaves_like 'response', {message: 'exercises.editor.submit_failure_remote', status: 410}
end
context 'when transmission of points failed for all users' do
let(:users_success) { [] }
let(:users_error) { [contributor] }
let(:users_unsupported) { [] }
it_behaves_like 'response', {message: 'exercises.editor.submit_failure_all', status: 424}
end
context 'when transmission of points failed for some users' do
let(:users_success) { [contributor] }
let(:users_error) { [create(:external_user)] }
let(:users_unsupported) { [] }
it_behaves_like 'response', {message: 'exercises.editor.submit_failure_other_users', status: 417}
end
context 'when the scoring was too late' do
let(:users_success) { [contributor] }
let(:users_error) { [] }
let(:users_unsupported) { [] }
let(:deadline) { :within_grace_period }
let(:score_sent) { score * 0.8 }
it_behaves_like 'response', {message: 'exercises.editor.submit_too_late', status: 207}
it 'sends a reduced score' do
expect(response.parsed_body.symbolize_keys[:score]).to eq((score_sent * 100).to_i)
end
end
context 'when transmission of points was successful' do
let(:users_success) { [contributor] }
let(:users_error) { [] }
let(:users_unsupported) { [] }
context 'when exercise is finished' do
it_behaves_like 'response', {message: 'exercises.editor.exercise_finished_remote', status: 200}
end
context 'when exercise is not finished' do
let(:score) { 0.5 }
it_behaves_like 'response', {message: 'sessions.destroy_through_lti.success_with_outcome', status: 202}
end
end
end
context 'when the scoring was not successful' do
let(:users_success) { [contributor] }
let(:users_error) { [] }
let(:users_unsupported) { [] }
before do
allow_any_instance_of(Submission).to receive(:calculate_score).and_raise(error)
perform_request.call
end
context 'when the desired runner is already in use' do
let(:error) { Runner::Error::RunnerInUse }
it_behaves_like 'response', {message: 'exercises.editor.runner_in_use', status: 409}
end
context 'when no runner is available' do
let(:error) { Runner::Error::NotAvailable }
it_behaves_like 'response', {message: 'exercises.editor.depleted', status: 503}
end
end
end
context 'when remote evaluation mapping is not available' do
let(:validation_token) { nil }
before { perform_request.call }
it_behaves_like 'response', {message: 'exercises.editor.submit_no_validation_token', status: 401}
end
end
describe '#POST evaluate' do
let(:perform_request) { proc { post :evaluate, params: remote_evaluation_params } }
context 'when remote evaluation mapping is available' do
context 'when the scoring is successful' do
let(:score) { 1 }
before do
allow_any_instance_of(Submission).to receive_messages(calculate_score: calculate_response, score:)
perform_request.call
end
it 'returns the feedback' do
expect(response.body).to eq(calculate_response.to_json)
end
end
context 'when the scoring was not successful' do
let(:users_success) { [contributor] }
let(:users_error) { [] }
let(:users_unsupported) { [] }
before do
allow_any_instance_of(Submission).to receive(:calculate_score).and_raise(error)
perform_request.call
end
context 'when the desired runner is already in use' do
let(:error) { Runner::Error::RunnerInUse }
it_behaves_like 'response', {message: 'exercises.editor.runner_in_use', status: 409}
end
context 'when no runner is available' do
let(:error) { Runner::Error::NotAvailable }
it_behaves_like 'response', {message: 'exercises.editor.depleted', status: 503}
end
end
end
context 'when remote evaluation mapping is not available' do
let(:validation_token) { nil }
before { perform_request.call }
it_behaves_like 'response', {message: 'exercises.editor.submit_no_validation_token', status: 401}
end
end
end

View File

@ -0,0 +1,22 @@
# frozen_string_literal: true
FactoryBot.define do
factory :remote_evaluation_mapping, class: 'RemoteEvaluationMapping' do
created_by_external_user
validation_token { SecureRandom.urlsafe_base64 }
exercise factory: :math
after(:create) do |remote_evaluation_mapping|
# Do not change anything if a study group was provided explicitly or user has no study groups
unless remote_evaluation_mapping.study_group_id.present? || remote_evaluation_mapping.user.study_groups.blank?
remote_evaluation_mapping.update!(study_group_id: remote_evaluation_mapping.user.study_groups.first.id)
end
pg = remote_evaluation_mapping.user.programming_groups.find_by(exercise: remote_evaluation_mapping.exercise)
# Do not change anything if a programming group was provided explicitly or user has no programming group
unless remote_evaluation_mapping.programming_group_id.present? || pg.blank?
remote_evaluation_mapping.update!(programming_group_id: pg.id)
end
end
end
end