diff --git a/app/assets/javascripts/community_solution.js b/app/assets/javascripts/community_solution.js index 6566d45a..5007e8d8 100644 --- a/app/assets/javascripts/community_solution.js +++ b/app/assets/javascripts/community_solution.js @@ -21,8 +21,29 @@ $(document).on('turbolinks:load', function() { ) $(document).on('theme:change:ace', CodeOceanEditor.handleAceThemeChangeEvent.bind(CodeOceanEditor)); - $('#submit').one('click', CodeOceanEditorSubmissions.submitCode.bind(CodeOceanEditor)); - $('#accept').one('click', CodeOceanEditorSubmissions.submitCode.bind(CodeOceanEditor)); + $('#submit').one('click', submitCode.bind(CodeOceanEditor)); + $('#accept').one('click', submitCode.bind(CodeOceanEditor)); } - }); + +function submitCode(event) { + const button = $(event.target) || $('#submit'); + this.startSentryTransaction(button); + this.teardownEventHandlers(); + this.createSubmission(button, null, function (response) { + if (response.redirect) { + this.autosaveIfChanged(); + this.stopCode(event); + this.editors = []; + Turbolinks.clearCache(); + Turbolinks.visit(response.redirect); + } else if (response.status === 'container_depleted') { + this.showContainerDepletedMessage(); + } else if (response.message) { + $.flash.danger({ + text: response.message + }); + } + this.initializeEventHandlers(); + }) +} diff --git a/app/assets/javascripts/editor/editor.js.erb b/app/assets/javascripts/editor/editor.js.erb index 55cb56ea..45260862 100644 --- a/app/assets/javascripts/editor/editor.js.erb +++ b/app/assets/javascripts/editor/editor.js.erb @@ -472,8 +472,7 @@ var CodeOceanEditor = { }, initializeWorkspaceButtons: function () { - $('#submit').one('click', this.submitCode.bind(this)); - $('#assess').on('click', this.scoreCode.bind(this)); + $('#assess').one('click', this.scoreCode.bind(this)); $('#render').on('click', this.renderCode.bind(this)); $('#run').on('click', this.runCode.bind(this)); $('#stop').on('click', this.stopCode.bind(this)); @@ -483,7 +482,6 @@ var CodeOceanEditor = { }, teardownWorkspaceButtons: function () { - $('#submit').unbind('click'); $('#assess').unbind('click'); $('#render').unbind('click'); $('#run').unbind('click'); @@ -775,14 +773,32 @@ var CodeOceanEditor = { }, showStatus: function (output) { - if (output.status === 'timeout' || output.status === 'buffer_overflow') { - this.showTimeoutMessage(); - } else if (output.status === 'container_depleted') { - this.showContainerDepletedMessage(); - } else if (output.status === 'out_of_memory') { - this.showOutOfMemoryMessage(); - } else if (output.status === 'runner_in_use') { - this.showRunnerInUseMessage(); + switch (output.status) { + case 'timeout': + case 'buffer_overflow': + this.showTimeoutMessage(); + break; + case 'container_depleted': + this.showContainerDepletedMessage(); + break; + case 'out_of_memory': + this.showOutOfMemoryMessage(); + break; + case 'runner_in_use': + this.showRunnerInUseMessage(); + break; + case 'scoring_failure': + this.showScoringFailureMessage(); + break; + case 'not_for_all_users_submitted': + this.showNotForAllUsersSubmittedMessage(output.failed_users); + break; + case 'scoring_too_late': + this.showScoringTooLateMessage(output.score_sent); + break; + case 'exercise_finished': + this.showExerciseFinishedMessage(output.url); + break; } }, @@ -849,6 +865,35 @@ var CodeOceanEditor = { }); }, + showScoringFailureMessage: function () { + $.flash.danger({ + icon: ['fa-solid', 'fa-exclamation-circle'], + text: I18n.t('exercises.editor.submit_failure_all') + }); + }, + + showNotForAllUsersSubmittedMessage: function (failed_users) { + $.flash.warning({ + icon: ['fa-solid', 'fa-triangle-exclamation'], + text: I18n.t('exercises.editor.submit_failure_other_users', {user: failed_users}) + }); + }, + + showScoringTooLateMessage: function (score_sent) { + $.flash.info({ + icon: ['fa-solid', 'fa-circle-info'], + text: I18n.t('exercises.editor.submit_too_late', {score_sent: score_sent}) + }); + }, + + showExerciseFinishedMessage: function (url) { + $.flash.success({ + showPermanent: true, + icon: ['fa-solid', 'fa-graduation-cap'], + text: I18n.t('exercises.editor.exercise_finished', {url: url}) + }); + }, + showTimeoutMessage: function () { $.flash.info({ icon: ['fa-regular', 'fa-clock'], diff --git a/app/assets/javascripts/editor/evaluation.js b/app/assets/javascripts/editor/evaluation.js index c493f98d..0be38dc0 100644 --- a/app/assets/javascripts/editor/evaluation.js +++ b/app/assets/javascripts/editor/evaluation.js @@ -29,27 +29,6 @@ CodeOceanEditorEvaluation = { }, 0).toFixed(2); $('#score').data('score', score); this.renderScore(); - this.showSubmitButton(); - }, - - showSubmitButton: function () { - if (this.submission_deadline || this.late_submission_deadline) { - const now = new Date(); - if (now <= this.submission_deadline) { - // before_deadline - // default is btn-success, so no change in color - $('#submit').get(0).lastChild.nodeValue = I18n.t('exercises.editor.submit_on_time'); - } else if (now > this.submission_deadline && this.late_submission_deadline && now <= this.late_submission_deadline) { - // within_grace_period - $('#submit').removeClass("btn-success btn-warning").addClass("btn-warning"); - $('#submit').get(0).lastChild.nodeValue = I18n.t('exercises.editor.submit_within_grace_period'); - } else if (this.late_submission_deadline && now > this.late_submission_deadline || now > this.submission_deadline) { - // after_late_deadline - $('#submit').removeClass("btn-success btn-warning btn-danger").addClass("btn-danger"); - $('#submit').get(0).lastChild.nodeValue = I18n.t('exercises.editor.submit_after_late_deadline'); - } - } - $('#submit').removeClass("d-none"); }, printScoringResult: function (result, index) { diff --git a/app/assets/javascripts/editor/execution.js b/app/assets/javascripts/editor/execution.js index 6ded66d8..69f841b7 100644 --- a/app/assets/javascripts/editor/execution.js +++ b/app/assets/javascripts/editor/execution.js @@ -21,7 +21,7 @@ CodeOceanEditorWebsocket = { return sockURL.toString(); }, - initializeSocket: function(url) { + initializeSocket: function(url, closeCallback) { const cleanedPath = url.replace(/\/\d+\//, '/*/').replace(/\/[^\/]+$/, '/*'); const websocketHost = window.location.origin.replace(/^http/, 'ws'); const sentryDescription = `WebSocket ${websocketHost}${cleanedPath}`; @@ -33,7 +33,12 @@ CodeOceanEditorWebsocket = { ); CodeOceanEditorWebsocket.websocket = this.websocket; this.websocket.onError(this.showWebsocketError.bind(this)); - this.websocket.onClose(span?.finish?.bind(span)); + this.websocket.onClose( function(span, callback){ + span?.finish() + if(callback != null){ + callback(); + } + }.bind(this, span, closeCallback)); }, initializeSocketForTesting: function(url) { @@ -43,10 +48,13 @@ CodeOceanEditorWebsocket = { }, initializeSocketForScoring: function(url) { - this.initializeSocket(url); + this.initializeSocket(url, function() { + $('#assess').one('click', this.scoreCode.bind(this)) + }.bind(this)); this.websocket.on('default',this.handleScoringResponse.bind(this)); this.websocket.on('hint', this.showHint.bind(this)); this.websocket.on('exit', this.handleExitCommand.bind(this)); + this.websocket.on('status', this.showStatus.bind(this)); }, initializeSocketForRunning: function(url) { diff --git a/app/assets/javascripts/editor/submissions.js b/app/assets/javascripts/editor/submissions.js index 6367d8e7..893528db 100644 --- a/app/assets/javascripts/editor/submissions.js +++ b/app/assets/javascripts/editor/submissions.js @@ -205,33 +205,6 @@ CodeOceanEditorSubmissions = { } }, - submitCode: function(event) { - const button = $(event.target) || $('#submit'); - this.startSentryTransaction(button); - this.teardownEventHandlers(); - this.createSubmission(button, null, function (response) { - if (response.redirect) { - App.synchronized_editor?.disconnect(); - this.autosaveIfChanged(); - this.stopCode(event); - this.editors = []; - Turbolinks.clearCache(); - Turbolinks.visit(response.redirect); - } else if (response.status === 'container_depleted') { - this.initializeEventHandlers(); - this.showContainerDepletedMessage(); - } else { - this.initializeEventHandlers(); - for (let [type, text] of Object.entries(response)) { - $.flash[type]({ - text: text, - showPermanent: true // We might display a very long text message! - }) - } - } - }) - }, - /** * Autosave-Logic */ diff --git a/app/controllers/concerns/lti.rb b/app/controllers/concerns/lti.rb index 30735de6..9d229013 100644 --- a/app/controllers/concerns/lti.rb +++ b/app/controllers/concerns/lti.rb @@ -123,47 +123,68 @@ module Lti raise Error.new("Score #{submission.normalized_score} must be between 0 and #{MAXIMUM_SCORE}!") end - submission.users.map {|user| send_score_for submission, user } + # Prepare score to be sent + score = submission.normalized_score + deadline = :none + if submission.before_deadline? + # Keep the full score + deadline = :before_deadline + elsif submission.within_grace_period? + # Reduce score by 20% + score *= 0.8 + deadline = :within_grace_period + elsif submission.after_late_deadline? + # Reduce score by 100% + score *= 0.0 + deadline = :after_late_deadline + end + + # Actually send the score for all users + detailed_results = submission.users.map {|user| send_score_for submission, user, score } + + # Prepare return value + erroneous_results = detailed_results.filter {|result| result[:status] == 'error' } + unsupported_results = detailed_results.filter {|result| result[:status] == 'unsupported' } + statistics = { + all: detailed_results, + success: detailed_results - erroneous_results - unsupported_results, + error: erroneous_results, + unsupported: unsupported_results, + } + + { + users: statistics.transform_values {|value| value.pluck(:user) }, + score: {original: submission.normalized_score, sent: score}, + deadline:, + detailed_results:, + } end private :send_scores - def send_score_for(submission, user) - if user.external_user? && user.consumer - lti_parameter = user.lti_parameters.find_by(exercise: submission.exercise, study_group: submission.study_group) - provider = build_tool_provider(consumer: user.consumer, parameters: lti_parameter&.lti_parameters) - end + def send_score_for(submission, user, score) + return {status: 'error', user:} unless user.external_user? && user.consumer - if provider.nil? - {status: 'error', user: user.displayname} - elsif provider.outcome_service? - Sentry.set_extras({ - provider: provider.inspect, - score: submission.normalized_score, - lti_parameter: lti_parameter.inspect, - session: session.to_hash, - exercise_id: submission.exercise_id, - }) - normalized_lti_score = submission.normalized_score - if submission.before_deadline? - # Keep the full score - elsif submission.within_grace_period? - # Reduce score by 20% - normalized_lti_score *= 0.8 - elsif submission.after_late_deadline? - # Reduce score by 100% - normalized_lti_score *= 0.0 - end + lti_parameter = user.lti_parameters.find_by(exercise: submission.exercise, study_group: submission.study_group) + provider = build_tool_provider(consumer: user.consumer, parameters: lti_parameter&.lti_parameters) + return {status: 'error', user:} if provider.nil? + return {status: 'unsupported', user:} unless provider.outcome_service? - 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, user: user.displayname} - rescue IMS::LTI::XMLParseError, Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET, SocketError, EOFError - # A parsing error might happen if the LTI provider is down and doesn't return a valid XML response - {status: 'error', user: user.displayname} - end - else - {status: 'unsupported', user: user.displayname} + Sentry.set_extras({ + provider: provider.inspect, + normalized_score: submission.normalized_score, + score:, + lti_parameter: lti_parameter.inspect, + session: defined?(session) ? session.to_hash : nil, + exercise_id: submission.exercise_id, + }) + + begin + response = provider.post_replace_result!(score) + {code: response.response_code, message: response.post_response.body, status: response.code_major, user:} + rescue IMS::LTI::XMLParseError, Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET, SocketError, EOFError + # A parsing error might happen if the LTI provider is down and doesn't return a valid XML response + {status: 'error', user:} end end diff --git a/app/controllers/concerns/redirect_behavior.rb b/app/controllers/concerns/redirect_behavior.rb index 15db82bc..20cd6abc 100644 --- a/app/controllers/concerns/redirect_behavior.rb +++ b/app/controllers/concerns/redirect_behavior.rb @@ -6,60 +6,19 @@ module RedirectBehavior def redirect_after_submit Rails.logger.debug { "Redirecting user with score:s #{@submission.normalized_score}" } - if @submission.normalized_score.to_d == BigDecimal('1.0') - if redirect_to_community_solution? - redirect_to_community_solution - return - end + # Redirect to the corresponding community solution if enabled and the user is eligible. + return redirect_to_community_solution if redirect_to_community_solution? - # if user is external and has an own rfc, redirect to it and message him to clean up and accept the answer. (we need to check that the user is external, - # otherwise an internal user could be shown a false rfc here, since current_user.id is polymorphic, but only makes sense for external users when used with rfcs.) - # 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] - redirect_to_user_feedback - return - end + # Redirect 10 percent pseudo-randomly to the feedback page. + return redirect_to_user_feedback if !@embed_options[:disable_redirect_to_feedback] && @submission.redirect_to_feedback? - rfc = @submission.own_unsolved_rfc(current_user) - if rfc - # set a message that informs the user that his own RFC should be closed. - flash[:notice] = I18n.t('exercises.submit.full_score_redirect_to_own_rfc') - flash.keep(:notice) + # If the user has an own rfc, redirect to it and message them to resolve and reflect on it. + return redirect_to_unsolved_rfc(own: true) if redirect_to_own_unsolved_rfc? - respond_to do |format| - format.html { redirect_to(rfc) } - format.json { render(json: {redirect: url_for(rfc)}) } - end - return - end + # Otherwise, redirect to an unsolved rfc and ask for assistance. + return redirect_to_unsolved_rfc if redirect_to_unsolved_rfc? - # else: show open rfc for same exercise if available - rfc = @submission.unsolved_rfc(current_user) - unless rfc.nil? || @embed_options[:disable_redirect_to_rfcs] || @embed_options[:disable_rfc] - # set a message that informs the user that his score was perfect and help in RFC is greatly appreciated. - flash[:notice] = I18n.t('exercises.submit.full_score_redirect_to_rfc') - flash.keep(:notice) - - # increase counter 'times_featured' in rfc - rfc.increment(:times_featured) - - respond_to do |format| - format.html { redirect_to(rfc) } - format.json { render(json: {redirect: url_for(rfc)}) } - end - return - end - end - else - # redirect to feedback page if score is less than 100 percent - if @exercise.needs_more_feedback?(@submission) && !@embed_options[:disable_redirect_to_feedback] - redirect_to_user_feedback - else - redirect_to_lti_return_path - end - return - end + # Fallback: Show the score and allow learners to return to the LTI consumer. redirect_to_lti_return_path end @@ -74,9 +33,9 @@ module RedirectBehavior end def redirect_to_community_solution? - return false unless Java21Study.allow_redirect_to_community_solution?(current_user, @exercise) + return false unless Java21Study.allow_redirect_to_community_solution?(current_user, @submission.exercise) - @community_solution = CommunitySolution.find_by(exercise: @exercise) + @community_solution = CommunitySolution.find_by(exercise: @submission.exercise) return false if @community_solution.blank? last_contribution = CommunitySolutionContribution.where(community_solution: @community_solution).order(created_at: :asc).last @@ -100,11 +59,11 @@ module RedirectBehavior end def redirect_to_user_feedback - uef = UserExerciseFeedback.find_by(exercise: @exercise, user: current_user) + uef = UserExerciseFeedback.find_by(exercise: @submission.exercise, user: current_user) url = if uef edit_user_exercise_feedback_path(uef) else - new_user_exercise_feedback_path(user_exercise_feedback: {exercise_id: @exercise.id}) + new_user_exercise_feedback_path(user_exercise_feedback: {exercise_id: @submission.exercise.id}) end respond_to do |format| @@ -113,6 +72,32 @@ module RedirectBehavior end end + def redirect_to_unsolved_rfc(own: false) + # Set a message that informs the user that their own RFC should be closed or help in another RFC is greatly appreciated. + flash[:notice] = I18n.t("exercises.editor.exercise_finished_redirect_to_#{own ? 'own_' : ''}rfc") + flash.keep(:notice) + + # Increase counter 'times_featured' in rfc + @rfc.increment(:times_featured) unless own + + respond_to do |format| + format.html { redirect_to(@rfc) } + format.json { render(json: {redirect: url_for(@rfc)}) } + end + end + + def redirect_to_own_unsolved_rfc? + @rfc = @submission.own_unsolved_rfc(current_user) + @rfc.present? + end + + def redirect_to_unsolved_rfc? + return false if @embed_options[:disable_redirect_to_rfcs] || @embed_options[:disable_rfc] + + @rfc = @submission.unsolved_rfc(current_user) + @rfc.present? + end + def redirect_to_lti_return_path Sentry.set_extras( consumers_id: current_user.consumer_id, diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 31aa9ffd..d471e32b 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -10,7 +10,7 @@ class ExercisesController < ApplicationController before_action :handle_file_uploads, only: %i[create update] before_action :set_execution_environments, only: %i[index create edit new update] before_action :set_exercise_and_authorize, - only: MEMBER_ACTIONS + %i[clone implement working_times intervention statistics submit reload feedback + only: MEMBER_ACTIONS + %i[clone implement working_times intervention statistics reload feedback study_group_dashboard export_external_check export_external_confirm external_user_statistics] before_action :collect_set_and_unset_exercise_tags, only: MEMBER_ACTIONS @@ -548,57 +548,6 @@ class ExercisesController < ApplicationController render 'exercises/external_users/statistics' end - def submit - @submission = Submission.create(submission_params) - @submission.calculate_score(current_user) - - if @submission.users.map {|user| lti_outcome_service?(@submission.exercise, user, @submission.study_group_id) }.any? - transmit_lti_score - else - redirect_after_submit - end - rescue Runner::Error => e - 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: {danger: I18n.t('exercises.editor.depleted'), status: :container_depleted}) } - end - end - - def transmit_lti_score - responses = send_scores(@submission) - messages = {} - failed_users = [] - - 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 - 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 - 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 - - private :transmit_lti_score - def destroy destroy_and_respond(object: @exercise) end diff --git a/app/controllers/remote_evaluation_controller.rb b/app/controllers/remote_evaluation_controller.rb index caa63f66..057de1c0 100644 --- a/app/controllers/remote_evaluation_controller.rb +++ b/app/controllers/remote_evaluation_controller.rb @@ -52,11 +52,11 @@ class RemoteEvaluationController < ApplicationController 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.submit.too_late'), status: 207, score: lti_response[:score_sent] * 100} + {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} else - {message: I18n.t('exercises.submit.failure'), status: 424} + {message: I18n.t('exercises.editor.submit_failure_all'), status: 424} end # TODO: Delete LTI parameters? end diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index a93c4a95..64684b61 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -2,12 +2,13 @@ class SubmissionsController < ApplicationController include CommonBehavior - include Lti include FileConversion + include Lti + include RedirectBehavior include SubmissionParameters include Tubesock::Hijack - before_action :set_submission, only: %i[download download_file run score show statistics test] + before_action :set_submission, only: %i[download download_file run score show statistics test finalize] before_action :set_testrun, only: %i[run score test] before_action :set_files, only: %i[download show] before_action :set_files_and_specific_file, only: %i[download_file run test] @@ -72,6 +73,11 @@ class SubmissionsController < ApplicationController end end + def finalize + @submission.update!(cause: 'submit') + redirect_after_submit + end + def show; end def render_file @@ -166,7 +172,7 @@ class SubmissionsController < ApplicationController durations = @submission.run(@file) do |socket, starting_time| runner_socket = socket @testrun[:starting_time] = starting_time - client_socket.send_data JSON.dump({cmd: :status, status: :container_running}) + client_socket.send_data({cmd: :status, status: :container_running}.to_json) runner_socket.on :stdout do |data| message = retrieve_message_from_output data, :stdout @@ -256,9 +262,11 @@ class SubmissionsController < ApplicationController return true if disable_scoring # The score is stored separately, we can forward it to the client immediately - client_socket&.send_data(JSON.dump(@submission.calculate_score(current_user))) + client_socket&.send_data(@submission.calculate_score(current_user).to_json) # To enable hints when scoring a submission, uncomment the next line: # send_hints(client_socket, StructuredError.where(submission: @submission)) + + transmit_lti_score(client_socket) rescue Runner::Error::RunnerInUse => e extract_durations(e) send_and_store client_socket, {cmd: :status, status: :runner_in_use} @@ -300,7 +308,7 @@ class SubmissionsController < ApplicationController return true if @embed_options[:disable_run] # The score is stored separately, we can forward it to the client immediately - client_socket&.send_data(JSON.dump(@submission.test(@file, current_user))) + client_socket&.send_data(@submission.test(@file, current_user).to_json) rescue Runner::Error::RunnerInUse => e extract_durations(e) send_and_store client_socket, {cmd: :status, status: :runner_in_use} @@ -338,7 +346,7 @@ class SubmissionsController < ApplicationController return unless client_socket # We don't want to store this (arbitrary) exit command and redirect it ourselves - client_socket.send_data JSON.dump({cmd: :exit}) + client_socket.send_data({cmd: :exit}.to_json) client_socket.send_data nil, :close # We must not close the socket manually (with `client_socket.close`), as this would close it twice. # When the socket is closed twice, nginx registers a `Connection reset by peer` error. @@ -401,7 +409,7 @@ class SubmissionsController < ApplicationController end @testrun[:messages].push message @testrun[:status] = message[:status] if message[:status] - client_socket.send_data JSON.dump(message) + client_socket.send_data(message.to_json) end def max_output_buffer_size @@ -473,6 +481,53 @@ 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') diff --git a/app/models/exercise.rb b/app/models/exercise.rb index 2fc64cdd..4fa333b8 100644 --- a/app/models/exercise.rb +++ b/app/models/exercise.rb @@ -593,12 +593,8 @@ class Exercise < ApplicationRecord end private :valid_submission_deadlines? - def needs_more_feedback?(submission) - if submission.normalized_score.to_d == BigDecimal('1.0') - user_exercise_feedbacks.final.size <= MAX_GROUP_EXERCISE_FEEDBACKS - else - user_exercise_feedbacks.intermediate.size <= MAX_GROUP_EXERCISE_FEEDBACKS - end + def needs_more_feedback? + user_exercise_feedbacks.size <= MAX_GROUP_EXERCISE_FEEDBACKS end def last_submission_per_contributor diff --git a/app/models/submission.rb b/app/models/submission.rb index 2a57be80..a2288e0b 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -74,6 +74,10 @@ class Submission < ApplicationRecord collect_files.detect {|file| file.filepath == file_path } end + def full_score? + score == exercise.maximum_score + end + def normalized_score @normalized_score ||= if !score.nil? && !exercise.maximum_score.nil? && exercise.maximum_score.positive? score / exercise.maximum_score @@ -123,7 +127,7 @@ class Submission < ApplicationRecord def redirect_to_feedback? # Redirect 10% of users to the exercise feedback page. Ensure, that always the same # users get redirected per exercise and different users for different exercises. If - # desired, the number of feedbacks can be limited with exercise.needs_more_feedback?(submission) + # desired, the number of feedbacks can be limited with exercise.needs_more_feedback? (contributor_id + exercise.created_at.to_i) % 10 == 1 end diff --git a/app/policies/exercise_policy.rb b/app/policies/exercise_policy.rb index cb8338dc..4768aa5a 100644 --- a/app/policies/exercise_policy.rb +++ b/app/policies/exercise_policy.rb @@ -41,10 +41,6 @@ class ExercisePolicy < AdminOrAuthorPolicy admin? end - def submit? - everyone && @record.teacher_defined_assessment? - end - class Scope < Scope def resolve if @user.admin? diff --git a/app/policies/submission_policy.rb b/app/policies/submission_policy.rb index 77254a8a..0dd3e950 100644 --- a/app/policies/submission_policy.rb +++ b/app/policies/submission_policy.rb @@ -8,7 +8,7 @@ class SubmissionPolicy < ApplicationPolicy # insights? is used in the flowr_controller.rb as we use it to authorize the user for a submission # download_submission_file? is used in the live_streams_controller.rb %i[download? download_file? download_submission_file? run? score? show? statistics? stop? test? - insights?].each do |action| + insights? finalize?].each do |action| define_method(action) { admin? || author? || author_in_programming_group? } end diff --git a/app/views/exercises/_editor_output.html.slim b/app/views/exercises/_editor_output.html.slim index b1e0e75c..cff864c1 100644 --- a/app/views/exercises/_editor_output.html.slim +++ b/app/views/exercises/_editor_output.html.slim @@ -59,15 +59,11 @@ div.d-grid id='output_sidebar_uncollapsed' class='d-none col-sm-12 enforce-botto .progress .progress-bar role='progressbar' - br - - if lti_outcome_service?(@exercise, current_user) - p.text-center = render('editor_button', classes: 'btn-lg btn-success d-none', data: {:'data-url' => submit_exercise_path(@exercise)}, icon: 'fa-solid fa-paper-plane', id: 'submit', label: t('exercises.editor.submit')) - - if @exercise.submission_deadline.present? || @exercise.late_submission_deadline.present? - #deadline data-submission-deadline=@exercise.submission_deadline&.rfc2822 data-late-submission-deadline=@exercise.late_submission_deadline&.rfc2822 - h4 = t('exercises.editor.deadline') - = t('exercises.editor.hints.disclaimer') - - else - p.text-center.disabled = render('editor_button', classes: 'btn-lg btn-secondary', data: {:'data-bs-placement' => 'bottom', :'data-bs-toggle' => 'tooltip', :'data-bs-container' => 'body'}, icon: 'fa-regular fa-clock', id: 'submit_outdated', label: t('exercises.editor.exercise_deadline_passed'), title: t('exercises.editor.tooltips.exercise_deadline_passed')) + - if @exercise.submission_deadline.present? || @exercise.late_submission_deadline.present? + br + #deadline data-submission-deadline=@exercise.submission_deadline&.rfc2822 data-late-submission-deadline=@exercise.late_submission_deadline&.rfc2822 + h4 = t('exercises.editor.deadline') + = t('exercises.editor.hints.disclaimer') hr #turtlediv.enforce-big-bottom-margin.overflow-auto.d-none canvas#turtlecanvas width=400 height=400 diff --git a/app/views/submissions/show.json.jbuilder b/app/views/submissions/show.json.jbuilder index 86ef2172..b1b1e544 100644 --- a/app/views/submissions/show.json.jbuilder +++ b/app/views/submissions/show.json.jbuilder @@ -16,3 +16,4 @@ unless @embed_options[:disable_download] end json.run_url run_submission_path(@submission, 'a.', format: :json).gsub(/a\.\.json$/, '{filename}.json') json.test_url test_submission_path(@submission, 'a.', format: :json).gsub(/a\.\.json$/, '{filename}.json') +json.finalize_url finalize_submission_path(@submission) diff --git a/config/locales/de.yml b/config/locales/de.yml index 8c2a08d9..2c797885 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -383,12 +383,14 @@ de: collapse_output_sidebar: Ausgabe-Leiste Einklappen confirm_start_over: Wollen Sie in dieser Aufgabe wirklich von vorne anfangen? Ihre bisherigen Änderungen in dieser Aufgabe werden entfernt; andere Aufgaben bleiben unverändert. Diese Aktion kann nicht rückgängig gemacht werden. confirm_start_over_active_file: Wollen Sie wirklich Ihre Änderungen in der ausgewählten Datei '%{filename}' zurücksetzen? Diese Aktion kann nicht rückgängig gemacht werden. - confirm_submit: Wollen Sie Ihren Code wirklich zur Bewertung abgeben? create_file: Neue Datei depleted: Alle Ausführungsumgebungen sind momentan in Benutzung. Probiere es später nochmal. destroy_file: Datei löschen download: Herunterladen dummy: Keine Aktion + exercise_finished: Herzlichen Glückwunsch! Sie haben die Aufgabe vollständig gelöst. Klicken Sie hier, um die Aufgabe jetzt abzuschließen. + 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. expand_action_sidebar: Aktions-Leiste Ausklappen expand_output_sidebar: Ausgabe-Leiste Ausklappen input: Ihre Eingabe @@ -410,11 +412,10 @@ de: start_over_active_file: Diese Datei zurücksetzen start_video: (Video-) Chat starten stop: Stoppen - submit: Code zur Bewertung abgeben + 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. deadline: Deadline - submit_on_time: Code rechtzeitig zur Bewertung abgeben - submit_within_grace_period: Code innerhalb der Gnadenfrist zur Bewertung abgeben - submit_after_late_deadline: Code verspätet zur Bewertung abgeben test: Testen timeout: 'Ausführung gestoppt. Ihr Code hat die erlaubte Ausführungszeit von %{permitted_execution_time} Sekunden überschritten.' out_of_memory: 'Ausführung gestoppt. Ihr Code hat den erlaubten Arbeitsspeicher von %{memory_limit} MB überschritten.' @@ -424,7 +425,7 @@ de: exercise_deadline_passed: 'Entweder ist die Abgabefrist bereits abgelaufen oder Sie haben die Aufgabe nicht direkt über die E-Learning Plattform gestartet. (Möglicherweise haben Sie den Zurück Button Ihres Browsers benutzt nachdem Sie Ihre Aufgabe abgegeben haben?)' request_for_comments_sent: "Kommentaranfrage gesendet." hints: - submission_deadline: Diese Abgabe ist am %{deadline} fällig.
Klicken Sie daher rechtzeitig auf 'Abgeben', um Ihre Punktzahl an die E-Learning-Plattform zu übertragen. %{otherwise} + submission_deadline: Diese Abgabe ist am %{deadline} fällig.
Bitte schließen Sie daher die Aufgabe rechtzeitig vorher ab. %{otherwise} late_submission_deadline: Bis %{deadline} werden 80% Ihrer Punktzahl anerkannt.
Wenn Sie diese erweiterte Frist ungenutzt verstreichen lassen und Ihre Abgabe später einreichen, werden 0 Punkte übertragen. otherwise: Nach der Abgabefrist werden 0 Punkte übertragen. disclaimer: Bei Fragen zu Deadlines wenden Sie sich bitte an das Teaching Team. Die hier angezeigte Abgabefrist dient nur zur Information und Angaben auf der jeweiligen Kursseite in der E-Learning-Plattform sollen immer Vorrang haben. @@ -478,7 +479,7 @@ de: default_test_feedback: Sehr gut. Alle Tests waren erfolgreich. default_linter_feedback: Sehr gut. Der Linter hat nichts mehr zu beanstanden. error_messages: Fehlermeldungen - existing_programming_group: Sie arbeiten gerade an der Übung mit dem Titel '%{exercise}' als Teil einer Programmiergruppe. Bitte schließen Sie Ihre Arbeit dort ab, indem Sie Ihren Code bewerten und abgeben, bevor Sie mit der Bearbeitung dieser Übung beginnen. + existing_programming_group: Sie arbeiten gerade an der Übung mit dem Titel '%{exercise}' als Teil einer Programmiergruppe. Bitte schließen Sie Ihre Arbeit dort ab, bevor Sie mit der Bearbeitung dieser Übung beginnen. external_privacy_policy: Datenschutzerklärung messages: Meldungen feedback: Feedback @@ -560,12 +561,6 @@ de: external_users: Externe Nutzer programming_groups: Programmiergruppen finishing_rate: Abschlussrate - submit: - failure: Beim Übermitteln Ihrer Punktzahl ist ein Fehler aufgetreten. Bitte versuchen Sie es später erneut. - 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 @@ -876,7 +871,7 @@ de: failure: Fehlerhafte E-Mail oder Passwort. success: Sie haben sich erfolgreich angemeldet. create_through_lti: - session_with_outcome: 'Bitte beachten Sie, dass zur Gutschrift der Punkte Ihr Code nach der Bearbeitung durch Klicken auf den Button "Code zur Bewertung abgeben" eingetragen werden muss.' + session_with_outcome: 'Ihre Punkte werden durch Klicken auf "Bewerten" automatisch Ihrem Fortschritt gutgeschrieben.' session_without_outcome: 'Dies ist eine Übungs-Session. Ihre Bewertung wird nicht an %{consumer} übermittelt.' destroy: link: Abmelden diff --git a/config/locales/en.yml b/config/locales/en.yml index 6ccb59c5..5bf553d5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -383,12 +383,14 @@ en: collapse_output_sidebar: Collapse Output Sidebar confirm_start_over: Do you really want to start over? Your previous changes in this exercise will be reset; other exercises remain untouched. You cannot undo this action. confirm_start_over_active_file: Do you really want to remove any changes in the active file '%{filename}'? You cannot undo this action. - confirm_submit: Do you really want to submit your code for grading? create_file: New File depleted: All execution environments are busy. Please try again later. destroy_file: Delete File download: Download dummy: No Action + exercise_finished: Congratulations! You have completely solved this exercise. Please click here to finish the exercise now. + 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 expand_output_sidebar: Expand Output Sidebar input: Your input @@ -410,11 +412,10 @@ en: start_over_active_file: Reset this file start_video: Start (video) chat stop: Stop - submit: Submit Code For Assessment + 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. deadline: Deadline - submit_on_time: Submit Code for Assessment on Time - submit_within_grace_period: Submit Code for Assessment Within Grace Period - submit_after_late_deadline: Submit Code for Assessment After Deadline Passed test: Test timeout: 'Execution stopped. Your code exceeded the permitted execution time of %{permitted_execution_time} seconds.' out_of_memory: 'Execution stopped. Your code exceeded the permitted RAM usage of %{memory_limit} MB.' @@ -424,8 +425,8 @@ en: exercise_deadline_passed: 'Either the deadline has already passed or you did not directly access this page from the e-learning platform. (Did you use the Back button of your browser after submitting the score?)' request_for_comments_sent: "Request for comments sent." hints: - submission_deadline: This exercise is due %{deadline}.
Click 'submit' to transfer your score to the e-learning platform before this deadline passes. %{otherwise} - late_submission_deadline: Until %{deadline}, 80% of your score will be awarded.
If you miss this extended deadline and submit your score afterwards, 0 points will be transmitted. + submission_deadline: This exercise is due %{deadline}.
Please finish the exercise before this deadline passes. %{otherwise} + late_submission_deadline: Until %{deadline}, 80% of your score will be awarded.
If you miss this extended deadline and score your code afterwards, 0 points will be transmitted. otherwise: Otherwise, a score of 0 points will be transmitted. disclaimer: If unsure about a deadline, please contact a course instructor. The deadline shown here is only informational and information from the e-learning platform should always take precedence. editor_file_tree: @@ -478,7 +479,7 @@ en: default_test_feedback: Well done. All tests have been passed. default_linter_feedback: Well done. The linter is completly satisfied. error_messages: Error Messages - existing_programming_group: You are currently working on the exercise entitled '%{exercise}' as part of a programming group. Please finish your work there by scoring and submitting your code before you start implementing this exercise. + existing_programming_group: You are currently working on the exercise entitled '%{exercise}' as part of a programming group. Please finish your work there before you start implementing this exercise. external_privacy_policy: privacy policy messages: Messages feedback: Feedback @@ -560,12 +561,6 @@ en: external_users: External Users programming_groups: Programming Groups finishing_rate: Finishing Rate - submit: - failure: An error occurred while transmitting your score. Please try again later. - 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 @@ -876,7 +871,7 @@ en: failure: Invalid email or password. success: Successfully signed in. create_through_lti: - session_with_outcome: 'Please click "Submit Code for Assessment" after scoring to send your score to %{consumer}.' + session_with_outcome: 'By clicking on "Score", your points will be added automatically to your progress.' session_without_outcome: 'This is a practice session. Your grade will not be transmitted to %{consumer}.' destroy: link: Sign out diff --git a/config/routes.rb b/config/routes.rb index c8c7c2f1..d045b425 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -91,7 +91,6 @@ Rails.application.routes.draw do get :statistics get :feedback get :reload - post :submit get 'study_group_dashboard/:study_group_id', to: 'exercises#study_group_dashboard' post :export_external_check post :export_external_confirm @@ -164,6 +163,7 @@ Rails.application.routes.draw do get :score get :statistics get 'test/:filename', as: :test, constraints: {filename: FILENAME_REGEXP}, action: :test + get :finalize end end diff --git a/spec/concerns/lti_spec.rb b/spec/concerns/lti_spec.rb index 8ea55222..c0f4e7e7 100644 --- a/spec/concerns/lti_spec.rb +++ b/spec/concerns/lti_spec.rb @@ -114,17 +114,19 @@ RSpec.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_scores, submission).first[:status]).to eq('unsupported') + submit_info = controller.send(:send_scores, submission) + expect(submit_info[:users][:all]).to eq(submit_info[:users][:unsupported]) end end context 'when grading is supported' do let(:response) { double } - let(:send_scores) { controller.send(:send_scores, submission).first } + let(:send_scores) { controller.send(:send_scores, submission) } + let(:score_sent) { score } before do allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:outcome_service?).and_return(true) - allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:post_replace_result!).with(score).and_return(response) + allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:post_replace_result!).with(score_sent).and_return(response) allow(response).to receive(:response_code).at_least(:once).and_return(200) allow(response).to receive(:post_response).and_return(response) allow(response).to receive(:body).at_least(:once).and_return('') @@ -133,14 +135,83 @@ RSpec.describe Lti do end it 'sends the score' do - expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:post_replace_result!).with(score) + expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:post_replace_result!).with(score_sent) send_scores end - it 'returns code, message, and status' do - 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) + it 'returns code, message, deadline and status' do + expect(send_scores[:users][:all]).to eq(send_scores[:users][:success]) + expect(send_scores[:deadline]).to eq(:none) + expect(send_scores[:detailed_results].first[:code]).to eq(response.response_code) + expect(send_scores[:detailed_results].first[:message]).to eq(response.body) + expect(send_scores[:detailed_results].first[:status]).to eq(response.code_major) + end + + context 'when submission is before deadline' do + before do + allow(submission).to receive(:before_deadline?).and_return true + end + + it 'returns deadline' do + expect(send_scores[:deadline]).to eq(:before_deadline) + end + end + + context 'when submission is within grace period' do + let(:score_sent) { score * 0.8 } + + before do + allow(submission).to receive_messages(before_deadline?: false, within_grace_period?: true) + end + + it 'returns deadline and reduced score' do + expect(send_scores[:deadline]).to eq(:within_grace_period) + expect(send_scores[:score][:sent]).to eq(score * 0.8) + end + + it 'sends the reduced score' do + expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:post_replace_result!).with(score_sent) + send_scores + end + end + + context 'when submission is after late deadline' do + let(:score_sent) { score * 0 } + + before do + allow(submission).to receive_messages(before_deadline?: false, + within_grace_period?: false, + after_late_deadline?: true) + end + + it 'returns deadline and reduced score' do + expect(send_scores[:deadline]).to eq(:after_late_deadline) + expect(send_scores[:score][:sent]).to eq(score * 0) + end + + it 'sends the reduced score' do + expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:post_replace_result!).with(score_sent) + send_scores + end + end + end + + context 'when transmission fails' do + let(:send_scores) { controller.send(:send_scores, submission) } + let(:score_sent) { 0 } + + before do + allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:outcome_service?).and_return(true) + allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:post_replace_result!).with(score_sent).and_raise(IMS::LTI::XMLParseError) + end + + it 'does not raise an exception' do + expect { send_scores }.not_to raise_error + end + + it 'returns an error status' do + expect(send_scores[:users][:all]).to eq(send_scores[:users][:error]) + expect(send_scores[:detailed_results].first[:status]).to eq('error') end end end @@ -150,7 +221,8 @@ RSpec.describe Lti do submission.contributor.consumer = nil allow(submission).to receive(:normalized_score).and_return score - expect(controller.send(:send_scores, submission).first[:status]).to eq('error') + submit_info = controller.send(:send_scores, submission) + expect(submit_info[:users][:all]).to eq(submit_info[:users][:unsupported]) end end end diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index d591a422..ffd7a0e1 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -288,98 +288,6 @@ RSpec.describe ExercisesController do end end - describe 'POST #submit' do - let(:output) { {} } - let(:perform_request) { post :submit, format: :json, params: {id: exercise.id, submission: {cause: 'submit', exercise_id: exercise.id}} } - let(:contributor) { create(:external_user) } - let(:scoring_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: 1.0, - filename: 'index.html_spec.rb', - message: 'Well done.', - weight: 2.0, - }] - end - - before do - create(:lti_parameter, external_user: contributor, exercise:) - submission = build(:submission, exercise:, contributor:) - allow(submission).to receive_messages(normalized_score: 1, calculate_score: scoring_response, redirect_to_feedback?: false) - allow(Submission).to receive(:create).and_return(submission) - end - - context 'when LTI outcomes are supported' do - before do - allow(controller).to receive(:lti_outcome_service?).and_return(true) - end - - context 'when the score transmission succeeds' do - before do - allow(controller).to receive(:send_scores).and_return([{status: 'success'}]) - perform_request - end - - expect_assigns(exercise: :exercise) - - it 'creates a submission' do - expect(assigns(:submission)).to be_a(Submission) - end - - expect_json - expect_http_status(:ok) - end - - context 'when the score transmission fails' do - before do - allow(controller).to receive(:send_scores).and_return([{status: 'unsupported'}]) - perform_request - end - - expect_assigns(exercise: :exercise) - - it 'creates a submission' 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 - end - end - - context 'when LTI outcomes are not supported' do - before do - allow(controller).to receive(:lti_outcome_service?).and_return(false) - perform_request - end - - expect_assigns(exercise: :exercise) - - it 'creates a submission' do - expect(assigns(:submission)).to be_a(Submission) - end - - it 'does not send scores' do - expect(controller).not_to receive(:send_scores) - end - - expect_json - expect_http_status(:ok) - end - end - describe 'PUT #update' do context 'with a valid exercise' do let(:exercise_attributes) { build(:dummy).attributes } diff --git a/spec/controllers/submissions_controller_spec.rb b/spec/controllers/submissions_controller_spec.rb index 88b1e4a7..395b7293 100644 --- a/spec/controllers/submissions_controller_spec.rb +++ b/spec/controllers/submissions_controller_spec.rb @@ -6,7 +6,8 @@ RSpec.describe SubmissionsController do render_views let(:exercise) { create(:math) } - let(:submission) { create(:submission, exercise:, contributor:) } + let(:cause) { 'save' } + let(:submission) { create(:submission, exercise:, contributor:, cause:) } shared_examples 'a regular user' do |record_not_found_status_code| describe 'POST #create' do @@ -39,6 +40,15 @@ RSpec.describe SubmissionsController do end end + describe 'GET #download' do + let(:perform_request) { proc { get :download, params: {id: submission.id} } } + + before { perform_request.call } + + expect_assigns(submission: :submission) + expect_http_status(:ok) + end + describe 'GET #download_file' do context 'with an invalid filename' do before { get :download_file, params: {filename: SecureRandom.hex, id: submission.id, format: :json} } @@ -98,6 +108,84 @@ RSpec.describe SubmissionsController do end end + describe 'GET #finalize' do + let(:perform_request) { proc { get :finalize, params: {id: submission.id} } } + let(:cause) { 'assess' } + + context 'when the request is performed' do + before { perform_request.call } + + expect_assigns(submission: :submission) + expect_redirect + end + + it 'updates cause to submit' do + expect { perform_request.call && submission.reload }.to change(submission, :cause).from('assess').to('submit') + end + + context 'when contributing to a community solution is possible' do + let!(:community_solution) { CommunitySolution.create(exercise:) } + + before do + allow(Java21Study).to receive(:allow_redirect_to_community_solution?).and_return(true) + perform_request.call + end + + expect_redirect { edit_community_solution_path(community_solution, lock_id: CommunitySolutionLock.last) } + end + + context 'when sharing exercise feedback is desired' do + before do + uef&.save! + allow_any_instance_of(Submission).to receive(:redirect_to_feedback?).and_return(true) + perform_request.call + end + + context 'without any previous feedback' do + let(:uef) { nil } + + expect_redirect { new_user_exercise_feedback_path(user_exercise_feedback: {exercise_id: submission.exercise.id}) } + end + + context 'with a previous feedback for the same exercise' do + let(:uef) { create(:user_exercise_feedback, exercise:, user: current_user) } + + expect_redirect { edit_user_exercise_feedback_path(uef) } + end + end + + context 'with an RfC' do + before do + rfc.save! + allow_any_instance_of(Submission).to receive(:redirect_to_feedback?).and_return(false) + perform_request.call + end + + context 'when an own RfC is unsolved' do + let(:rfc) { create(:rfc, user: current_user, exercise:, submission:) } + + expect_flash_message(:notice, I18n.t('exercises.editor.exercise_finished_redirect_to_own_rfc')) + expect_redirect { request_for_comment_url(rfc) } + end + + context 'when another RfC is unsolved' do + let(:rfc) { create(:rfc, exercise:) } + + expect_flash_message(:notice, I18n.t('exercises.editor.exercise_finished_redirect_to_rfc')) + expect_redirect { request_for_comment_url(rfc) } + end + end + + context 'when neither a community solution, feedback nor RfC is available' do + before do + allow_any_instance_of(Submission).to receive(:redirect_to_feedback?).and_return(false) + perform_request.call + end + + expect_redirect { lti_return_path(submission_id: submission.id) } + end + end + describe 'GET #render_file' do let(:file) { submission.files.first } let(:signed_url) { AuthenticatedUrlHelper.sign(render_submission_url(submission, filename), submission) } @@ -259,8 +347,9 @@ RSpec.describe SubmissionsController do context 'with an admin user' do let(:contributor) { create(:admin) } + let(:current_user) { contributor } - before { allow(controller).to receive(:current_user).and_return(contributor) } + before { allow(controller).to receive_messages(current_user:) } describe 'GET #index' do before do @@ -280,10 +369,9 @@ RSpec.describe SubmissionsController do let(:group_author) { create(:external_user) } let(:other_group_author) { create(:external_user) } let(:contributor) { create(:programming_group, exercise:, users: [group_author, other_group_author]) } + let(:current_user) { group_author } - before do - allow(controller).to receive_messages(current_contributor: contributor, current_user: group_author) - end + before { allow(controller).to receive_messages(current_contributor: contributor, current_user:) } it_behaves_like 'a regular user', :unauthorized it_behaves_like 'denies access for regular, non-admin users' @@ -291,10 +379,9 @@ RSpec.describe SubmissionsController do context 'with a learner' do let(:contributor) { create(:external_user) } + let(:current_user) { contributor } - before do - allow(controller).to receive_messages(current_user: contributor) - end + before { allow(controller).to receive_messages(current_user:) } it_behaves_like 'a regular user', :unauthorized it_behaves_like 'denies access for regular, non-admin users' diff --git a/spec/features/editor_spec.rb b/spec/features/editor_spec.rb index 2ea6be2a..c666a843 100644 --- a/spec/features/editor_spec.rb +++ b/spec/features/editor_spec.rb @@ -109,13 +109,4 @@ RSpec.describe 'Editor', :js do expect(page).not_to have_content(I18n.t('exercises.editor.score')) end end - - it 'contains a button for submitting the exercise' do - submission = build(:submission, contributor:, exercise:) - allow(submission).to receive(:calculate_score).and_return(scoring_response) - allow(Submission).to receive(:find).and_return(submission) - click_button(I18n.t('exercises.editor.score')) - expect(page).not_to have_content(I18n.t('exercises.editor.tooltips.exercise_deadline_passed')) - expect(page).to have_content(I18n.t('exercises.editor.submit')) - end end diff --git a/spec/features/score_spec.rb b/spec/features/score_spec.rb new file mode 100644 index 00000000..baf6295c --- /dev/null +++ b/spec/features/score_spec.rb @@ -0,0 +1,317 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Score', :js do + let(:exercise) { create(:hello_world) } + let(:contributor) { create(:external_user) } + let(:submission) { create(:submission, exercise:, contributor:, score:) } + + before do + allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(contributor) + allow(Submission).to receive(:find).and_return(submission) + visit(implement_exercise_path(exercise)) + end + + shared_examples 'exercise finished notification' do + it "shows an 'exercise finished' notification" do + # Text needs to be split because it includes the embedded URL in the HTML which is not shown in the notification. + # We compare the shown notification text and the URL separately. + expect(page).to have_content(I18n.t('exercises.editor.exercise_finished').split('.').first) + expect(page).to have_link(nil, href: finalize_submission_path(submission)) + end + end + + shared_examples 'no exercise finished notification' do + it "does not show an 'exercise finished' notification" do + # Text needs to be split because it includes the embedded URL in the HTML which is not shown in the notification. + # We compare the shown notification text and the URL separately. + expect(page).not_to have_content(I18n.t('exercises.editor.exercise_finished').split('.').first) + expect(page).not_to have_link(nil, href: finalize_submission_path(submission)) + end + end + + shared_examples 'notification' do |message_key| + it "shows a '#{message_key.split('.').last}' notification" do + options = {} + options[:score_sent] = (score_sent * 100).to_i if defined? score_sent + options[:user] = users_error.map(&:displayname).join(', ') if defined? users_error + + expect(page).to have_content(I18n.t(message_key, **options)) + end + end + + shared_examples 'no notification' do |message_key| + it "does not show a '#{message_key.split('.').last}' notification" do + expect(page).not_to have_content(I18n.t(message_key)) + end + end + + context 'when scoring is successful' do + let(:lti_outcome_service?) { true } + + 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 + + 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 + + before do + allow_any_instance_of(LtiHelper).to receive(:lti_outcome_service?).and_return(lti_outcome_service?) + allow(submission).to receive(:calculate_score).and_return(calculate_response) + allow_any_instance_of(SubmissionsController).to receive(:send_scores).and_return(scoring_response) + click_button(I18n.t('exercises.editor.score')) + end + + shared_context 'when full score reached' do + let(:score) { 1 } + end + + shared_context 'when full score is not reached' do + let(:score) { 0 } + end + + shared_context 'when scored without deadline' do + let(:deadline) { :none } + let(:score_sent) { score } + end + + shared_context 'when scored before deadline' do + let(:deadline) { :before_deadline } + let(:score_sent) { score } + end + + shared_context 'when scored within grace period' do + let(:deadline) { :within_grace_period } + let(:score_sent) { score * 0.8 } + end + + shared_context 'when scored after late deadline' do + let(:deadline) { :after_late_deadline } + let(:score_sent) { score * 0 } + end + + context 'when the LTI outcome service is supported' do + describe 'LTI failure' do + let(:users_success) { [] } + let(:users_error) { [contributor] } + let(:users_unsupported) { [] } + + context 'when full score is reached' do + include_context 'when full score reached' + + %w[without_deadline before_deadline within_grace_period after_late_deadline].each do |scenario| + context "when scored #{scenario.tr('_', ' ')}" do + include_context "when scored #{scenario.tr('_', ' ')}" + + it_behaves_like 'no exercise finished notification' + it_behaves_like 'notification', 'exercises.editor.submit_failure_all' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_other_users' + it_behaves_like 'no notification', 'exercises.editor.submit_too_late' + end + end + end + + context 'when full score is not reached' do + include_context 'when full score is not reached' + + %w[without_deadline before_deadline within_grace_period after_late_deadline].each do |scenario| + context "when scored #{scenario.tr('_', ' ')}" do + include_context "when scored #{scenario.tr('_', ' ')}" + + it_behaves_like 'no exercise finished notification' + it_behaves_like 'notification', 'exercises.editor.submit_failure_all' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_other_users' + it_behaves_like 'no notification', 'exercises.editor.submit_too_late' + end + end + end + end + + describe 'LTI success' do + let(:users_success) { [contributor] } + let(:users_error) { [] } + let(:users_unsupported) { [] } + + context 'when full score is reached' do + include_context 'when full score reached' + + %w[without_deadline before_deadline].each do |scenario| + context "when scored #{scenario.tr('_', ' ')}" do + include_context "when scored #{scenario.tr('_', ' ')}" + + it_behaves_like 'exercise finished notification' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_all' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_other_users' + it_behaves_like 'no notification', 'exercises.editor.submit_too_late' + end + end + + %w[within_grace_period after_late_deadline].each do |scenario| + context "when scored #{scenario.tr('_', ' ')}" do + include_context "when scored #{scenario.tr('_', ' ')}" + + it_behaves_like 'exercise finished notification' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_all' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_other_users' + it_behaves_like 'notification', 'exercises.editor.submit_too_late' + end + end + end + + context 'when full score is not reached' do + include_context 'when full score is not reached' + + %w[within_grace_period after_late_deadline].each do |scenario| + context "when scored #{scenario.tr('_', ' ')}" do + include_context "when scored #{scenario.tr('_', ' ')}" + + it_behaves_like 'no exercise finished notification' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_all' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_other_users' + it_behaves_like 'notification', 'exercises.editor.submit_too_late' + end + end + end + end + + describe 'LTI success for current contributor and failure for other' do + let(:users_success) { [contributor] } + let(:users_error) { [create(:external_user)] } + let(:users_unsupported) { [] } + + context 'when full score is reached' do + include_context 'when full score reached' + + %w[without_deadline before_deadline].each do |scenario| + context "when scored #{scenario.tr('_', ' ')}" do + include_context "when scored #{scenario.tr('_', ' ')}" + + it_behaves_like 'exercise finished notification' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_all' + it_behaves_like 'notification', 'exercises.editor.submit_failure_other_users' + it_behaves_like 'no notification', 'exercises.editor.submit_too_late' + end + end + + %w[within_grace_period after_late_deadline].each do |scenario| + context "when scored #{scenario.tr('_', ' ')}" do + include_context "when scored #{scenario.tr('_', ' ')}" + + it_behaves_like 'exercise finished notification' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_all' + it_behaves_like 'notification', 'exercises.editor.submit_failure_other_users' + it_behaves_like 'notification', 'exercises.editor.submit_too_late' + end + end + end + + context 'when full score is not reached' do + include_context 'when full score is not reached' + + %w[within_grace_period after_late_deadline].each do |scenario| + context "when scored #{scenario.tr('_', ' ')}" do + include_context "when scored #{scenario.tr('_', ' ')}" + + it_behaves_like 'no exercise finished notification' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_all' + it_behaves_like 'notification', 'exercises.editor.submit_failure_other_users' + it_behaves_like 'notification', 'exercises.editor.submit_too_late' + end + end + end + end + end + + context 'when the LTI outcomes are not supported' do + let(:lti_outcome_service?) { false } + let(:users_success) { [] } + let(:users_error) { [] } + let(:users_unsupported) { [contributor] } + + context 'when full score is reached' do + include_context 'when full score reached' + + %w[without_deadline before_deadline within_grace_period after_late_deadline].each do |scenario| + context "when scored #{scenario.tr('_', ' ')}" do + include_context "when scored #{scenario.tr('_', ' ')}" + + it_behaves_like 'exercise finished notification' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_all' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_other_users' + it_behaves_like 'no notification', 'exercises.editor.submit_too_late' + end + end + end + + context 'when full score is not reached' do + include_context 'when full score is not reached' + + %w[without_deadline before_deadline within_grace_period after_late_deadline].each do |scenario| + context "when scored #{scenario.tr('_', ' ')}" do + include_context "when scored #{scenario.tr('_', ' ')}" + + it_behaves_like 'no exercise finished notification' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_all' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_other_users' + it_behaves_like 'no notification', 'exercises.editor.submit_too_late' + end + end + end + end + end + + context 'when scoring is not successful' do + let(:score) { 0 } + + context 'when the desired runner is already in use' do + before do + allow(submission).to receive(:calculate_score).and_raise(Runner::Error::RunnerInUse) + click_button(I18n.t('exercises.editor.score')) + end + + it_behaves_like 'notification', 'exercises.editor.runner_in_use' + it_behaves_like 'no exercise finished notification' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_all' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_other_users' + it_behaves_like 'no notification', 'exercises.editor.submit_too_late' + end + + context 'when no runner is available' do + before do + allow(submission).to receive(:calculate_score).and_raise(Runner::Error::NotAvailable) + click_button(I18n.t('exercises.editor.score')) + end + + it_behaves_like 'notification', 'exercises.editor.depleted' + it_behaves_like 'no exercise finished notification' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_all' + it_behaves_like 'no notification', 'exercises.editor.submit_failure_other_users' + it_behaves_like 'no notification', 'exercises.editor.submit_too_late' + end + end +end diff --git a/spec/policies/exercise_policy_spec.rb b/spec/policies/exercise_policy_spec.rb index 62f9f163..27e5077f 100644 --- a/spec/policies/exercise_policy_spec.rb +++ b/spec/policies/exercise_policy_spec.rb @@ -188,29 +188,6 @@ RSpec.describe ExercisePolicy do end end - permissions :submit? do - context 'when teacher-defined assessments are available' do - before do - create(:test_file, context: exercise) - exercise.reload - end - - it 'grants access to anyone' do - %i[admin external_user teacher].each do |factory_name| - expect(policy).to permit(create(factory_name), exercise) - end - end - end - - context 'when teacher-defined assessments are not available' do - it 'does not grant access to anyone' do - %i[admin external_user teacher].each do |factory_name| - expect(policy).not_to permit(create(factory_name), exercise) - end - end - end - end - describe ExercisePolicy::Scope do describe '#resolve' do let(:admin) { create(:admin) } diff --git a/spec/policies/submission_policy_spec.rb b/spec/policies/submission_policy_spec.rb index e65fd682..82150842 100644 --- a/spec/policies/submission_policy_spec.rb +++ b/spec/policies/submission_policy_spec.rb @@ -13,7 +13,7 @@ RSpec.describe SubmissionPolicy do end end - %i[download_file? render_file? run? score? show? statistics? stop? test?].each do |action| + %i[download? download_file? download_submission_file? render_file? run? score? show? statistics? stop? test? insights? finalize?].each do |action| permissions(action) do let(:exercise) { build(:math) } let(:group_author) { build(:external_user) }