From 9fc54ad4c2527fdcff94d171fbe530e7596426e0 Mon Sep 17 00:00:00 2001 From: kiragrammel Date: Mon, 21 Aug 2023 11:36:22 +0200 Subject: [PATCH] Apply suggestions --- app/assets/javascripts/community_solution.js | 27 ++++++++++++++++--- app/assets/javascripts/editor/editor.js.erb | 8 +++--- app/assets/javascripts/exercise_graphs.js | 5 ++++ .../remote_evaluation_controller.rb | 2 +- app/controllers/submissions_controller.rb | 11 +++++--- app/policies/submission_policy.rb | 2 +- app/views/submissions/show.json.jbuilder | 2 +- config/locales/de.yml | 13 +++------ config/locales/en.yml | 15 ++++------- config/routes.rb | 2 +- spec/features/editor_spec.rb | 4 +-- spec/features/score_spec.rb | 26 +++++++++++------- spec/policies/exercise_policy_spec.rb | 22 --------------- 13 files changed, 72 insertions(+), 67 deletions(-) diff --git a/app/assets/javascripts/community_solution.js b/app/assets/javascripts/community_solution.js index 6566d45a2..5007e8d81 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 249a75c2c..530727ff9 100644 --- a/app/assets/javascripts/editor/editor.js.erb +++ b/app/assets/javascripts/editor/editor.js.erb @@ -784,8 +784,8 @@ var CodeOceanEditor = { case 'scoring_failure': this.showScoringFailureMessage(); break; - case 'scoring_to_late': - this.showScoringTooLateMessage(); + case 'scoring_too_late': + this.showScoringTooLateMessage(output.score_sent); break; case 'full_score_reached': this.showScoringFullScoreMessage(output.url); @@ -856,10 +856,10 @@ var CodeOceanEditor = { }); }, - showScoringTooLateMessage: function () { + showScoringTooLateMessage: function (score_sent) { $.flash.warning({ icon: ['fa-solid', 'fa-triangle-exclamation'], - text: I18n.t('exercises.submit.too_late') + text: I18n.t('exercises.submit.too_late', {score_sent: score_sent}) }); }, diff --git a/app/assets/javascripts/exercise_graphs.js b/app/assets/javascripts/exercise_graphs.js index 9aece8bdd..a7a9eb23a 100644 --- a/app/assets/javascripts/exercise_graphs.js +++ b/app/assets/javascripts/exercise_graphs.js @@ -31,6 +31,8 @@ $(document).on('turbolinks:load', function() { if(submission.cause == "assess"){ submissionsScoreAndTimeAssess.push(submissionArray); + } else if(submission.cause == "submit"){ + submissionsScoreAndTimeSubmits.push(submissionArray); } else if(submission.cause == "run"){ submissionsScoreAndTimeRuns.push(submissionArray[1]); } else if(submission.cause == "autosave"){ @@ -121,6 +123,9 @@ $(document).on('turbolinks:load', function() { if(largestSubmittedTimeStamp.cause === "assess"){ largestArrayForRange = submissionsScoreAndTimeAssess; x.domain([0,largestArrayForRange[largestArrayForRange.length - 1][1]]).clamp(true); + } else if(largestSubmittedTimeStamp.cause == "submit"){ + largestArrayForRange = submissionsScoreAndTimeSubmits; + x.domain([0,largestArrayForRange[largestArrayForRange.length - 1][1]]).clamp(true); } else if(largestSubmittedTimeStamp.cause === "submit"){ largestArrayForRange = submissionsScoreAndTimeSubmits; x.domain([0,largestArrayForRange[largestArrayForRange.length - 1][1]]).clamp(true); diff --git a/app/controllers/remote_evaluation_controller.rb b/app/controllers/remote_evaluation_controller.rb index 63aecf234..97835c8f6 100644 --- a/app/controllers/remote_evaluation_controller.rb +++ b/app/controllers/remote_evaluation_controller.rb @@ -51,7 +51,7 @@ def try_lti 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.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 diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index ddade176c..fa3ce20d8 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -8,7 +8,7 @@ class SubmissionsController < ApplicationController include SubmissionParameters include Tubesock::Hijack - before_action :set_submission, only: %i[download download_file run score show statistics test redirect_after_submit] + 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] @@ -73,6 +73,11 @@ def download_file end end + def finalize + @submission.update(cause: 'submit') + redirect_after_submit + end + def show; end def render_file @@ -261,7 +266,7 @@ def score if response[:status] == 'success' if response[:score_sent] != @submission.normalized_score # Score has been reduced due to the passed deadline - client_socket&.send_data(JSON.dump({cmd: :status, status: :scoring_to_late})) + client_socket&.send_data(JSON.dump({cmd: :status, status: :scoring_too_late, score_sent: response[:score_sent]})) end else client_socket&.send_data(JSON.dump({cmd: :status, status: :scoring_failure})) @@ -269,7 +274,7 @@ def score end if @submission.score == @submission.exercise.maximum_score - @url = redirect_after_submit_submission_path(@submission) + @url = finalize_submission_path(@submission) client_socket&.send_data(JSON.dump({cmd: :status, status: :full_score_reached, url: @url})) end diff --git a/app/policies/submission_policy.rb b/app/policies/submission_policy.rb index 11760b231..fba03ca77 100644 --- a/app/policies/submission_policy.rb +++ b/app/policies/submission_policy.rb @@ -8,7 +8,7 @@ def create? # 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? redirect_after_submit?].each do |action| + insights? finalize?].each do |action| define_method(action) { admin? || author? } end diff --git a/app/views/submissions/show.json.jbuilder b/app/views/submissions/show.json.jbuilder index 1607bae4f..b1b1e544b 100644 --- a/app/views/submissions/show.json.jbuilder +++ b/app/views/submissions/show.json.jbuilder @@ -16,4 +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.redirect_after_submit_url redirect_after_submit_submission_path(@submission) +json.finalize_url finalize_submission_path(@submission) diff --git a/config/locales/de.yml b/config/locales/de.yml index a178976be..cd81f5641 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -360,7 +360,6 @@ 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 @@ -383,11 +382,7 @@ de: start_over: Diese Aufgabe zurücksetzen start_over_active_file: Diese Datei zurücksetzen stop: Stoppen - submit: Code zur Bewertung abgeben 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.' @@ -397,7 +392,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.
Klicken Sie daher rechtzeitig auf 'Bewerten', um Ihre Punktzahl an die E-Learning-Plattform zu übertragen. %{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. @@ -530,9 +525,9 @@ de: external_users: Externe Nutzer 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: Herzlichen Glückwunsch! Sie haben die maximale Punktzahl für diese Aufgabe erreicht. Klicken Sie hier, um die Aufgabe abzuschließen. + failure: Die Bewertung wurde erfolgreich durchgeführt, aber 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, sodass nur %{score_sent} Punkte übertragen wurden. + full_score: Herzlichen Glückwunsch! Sie haben die maximale Punktzahl für diese Aufgabe erreicht. Klicken Sie hier, um die Aufgabe abzuschließen. 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. study_group_dashboard: diff --git a/config/locales/en.yml b/config/locales/en.yml index 22b501212..ecd80a422 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -360,7 +360,6 @@ 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 @@ -383,11 +382,7 @@ en: start_over: Reset this exercise start_over_active_file: Reset this file stop: Stop - submit: Submit Code For Assessment 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.' @@ -397,8 +392,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}.
Click 'score' 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 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: @@ -530,9 +525,9 @@ en: external_users: External Users 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: Congratulations! You achieved and submitted the highest possible score for this exercise. Click here to complete the exercise. + failure: The scoring was performed successfully but 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 so that only %{score_sent} points were transmitted. + full_score: Congratulations! You have achieved the highest possible score for this exercise. Click here to complete the exercise. 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! study_group_dashboard: diff --git a/config/routes.rb b/config/routes.rb index c9e55dc89..77de3dcc4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -159,7 +159,7 @@ get :score get :statistics get 'test/:filename', as: :test, constraints: {filename: FILENAME_REGEXP}, action: :test - get :redirect_after_submit + get :finalize end end diff --git a/spec/features/editor_spec.rb b/spec/features/editor_spec.rb index e380a2b4e..ad7c0fe5c 100644 --- a/spec/features/editor_spec.rb +++ b/spec/features/editor_spec.rb @@ -95,7 +95,7 @@ end context 'when an exercise has one or more teacher-defined assessments' do - it 'displays the score and submit button' do + it 'displays the score button' do visit(implement_exercise_path(exercise)) expect(page).to have_content(exercise.title) expect(page).to have_content(I18n.t('exercises.editor.score')) @@ -103,7 +103,7 @@ end context 'when an exercise has no teacher-defined assessment' do - it 'disables the score and submit button' do + it 'disables the score button' do visit(implement_exercise_path(exercise_without_test)) expect(page).to have_content(exercise_without_test.title) expect(page).not_to have_content(I18n.t('exercises.editor.score')) diff --git a/spec/features/score_spec.rb b/spec/features/score_spec.rb index 8524540d4..a5346efea 100644 --- a/spec/features/score_spec.rb +++ b/spec/features/score_spec.rb @@ -47,21 +47,22 @@ end context 'when LTI outcomes are supported' do + let(:submission) { create(:submission, exercise:, user:, score:) } + before do allow_any_instance_of(LtiHelper).to receive(:lti_outcome_service?).and_return(true) visit(implement_exercise_path(exercise)) allow(Submission).to receive(:find).and_return(submission) allow(submission).to receive(:calculate_score).and_return(scoring_response) allow(submission).to receive(:normalized_score).and_return(normalized_score) - allow_any_instance_of(SubmissionsController).to receive(:send_score).and_return(status:, score_sent: sent_score) + allow_any_instance_of(SubmissionsController).to receive(:send_score).and_return(status:, score_sent: score) click_button(I18n.t('exercises.editor.score')) end context 'when the full score is reached' do - let(:submission) { create(:submission, exercise:, user:, score: 1.0) } let(:scoring_response) { scoring_response_full_score } + let(:score) { 1 } let(:normalized_score) { 1 } - let(:sent_score) { 1 } context 'when scoring is successful' do let(:status) { 'success' } @@ -70,21 +71,21 @@ # Text needs to be split because it includes the embedded url in Html which is not shown in the notification # We only compare the shown notification text expect(page).to have_content(I18n.t('exercises.submit.full_score').split('.').first) - expect(page).to have_link(nil, href: redirect_after_submit_submission_path(submission)) + expect(page).to have_link(nil, href: finalize_submission_path(submission)) end end - context 'when scoring is to late' do + context 'when scoring is too late' do let(:normalized_score) { 0.7 } let(:status) { 'success' } - it 'shows a scoring to late notification' do + it 'shows a scoring too late notification' do expect(page).to have_content(I18n.t('exercises.submit.too_late')) end end context 'when scoring fails' do - let(:status) { 'error' } # unsupported also works + let(:status) { 'error' } it 'shows a scoring failure notification' do expect(page).to have_content(I18n.t('exercises.submit.failure')) @@ -93,20 +94,21 @@ end context 'when full score is not reached' do - let(:submission) { create(:submission, exercise:, user:, score: 0.0) } + let(:submission) { create(:submission, exercise:, user:, score:) } let(:scoring_response) { scoring_response_zero_points } + let(:score) { 0 } let(:normalized_score) { 0 } let(:status) { 'success' } - let(:sent_score) { 0 } end end context 'when LTI outcomes are not supported' do + let(:status) { 'unsupported' } + before do allow_any_instance_of(LtiHelper).to receive(:lti_outcome_service?).and_return(false) visit(implement_exercise_path(exercise)) click_button(I18n.t('exercises.editor.score')) - sleep 10 end it 'does not send scores' do @@ -116,5 +118,9 @@ it 'shows execution environments full notification' do expect(page).to have_content(I18n.t('exercises.editor.depleted')) end + + it 'shows a scoring failure notification' do + expect(page).to have_content(I18n.t('exercises.submit.failure')) + end end end diff --git a/spec/policies/exercise_policy_spec.rb b/spec/policies/exercise_policy_spec.rb index 6db0e2a2e..6624ce95d 100644 --- a/spec/policies/exercise_policy_spec.rb +++ b/spec/policies/exercise_policy_spec.rb @@ -186,28 +186,6 @@ end end - # TODO: Remove? - permissions :submit? do - context 'when teacher-defined assessments are available' do - before { create(:test_file, context: exercise) } - - 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 - - # TODO: Remove? - 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) }