From 6b08013b423ae990c34224fdd6c358b08026e9f0 Mon Sep 17 00:00:00 2001 From: Andrew Cain Date: Tue, 16 Jul 2024 13:38:59 +1000 Subject: [PATCH] feat: check that old tii submissions upload when eula accepted --- app/helpers/turn_it_in.rb | 10 + app/models/turn_it_in/tii_action.rb | 1 + .../tii_action_upload_submission.rb | 26 ++- app/sidekiq/tii_check_progress_job.rb | 2 + test/sidekiq/tii_check_progress_job_test.rb | 215 +++++++++++++++++- 5 files changed, 251 insertions(+), 3 deletions(-) diff --git a/app/helpers/turn_it_in.rb b/app/helpers/turn_it_in.rb index 2ab737917..62df4e3f7 100644 --- a/app/helpers/turn_it_in.rb +++ b/app/helpers/turn_it_in.rb @@ -205,6 +205,16 @@ def self.tii_role_for(task, user) end end + # Check and retry any failed tii submissions, where it was due to no accepted EULA + def self.check_and_retry_submissions_with_updated_eula + TiiActionUploadSubmission + .where( + complete: false, + custom_error_message: TiiActionUploadSubmission::NO_USER_ACCEPTED_EULA_ERROR + ) + .find_each(&:attempt_retry_on_no_eula) + end + private def logger diff --git a/app/models/turn_it_in/tii_action.rb b/app/models/turn_it_in/tii_action.rb index 93a30792f..2573ddee0 100644 --- a/app/models/turn_it_in/tii_action.rb +++ b/app/models/turn_it_in/tii_action.rb @@ -124,6 +124,7 @@ def error? def save_and_reschedule(reset_retry: true) self.retries = 0 if reset_retry + self.error_code = nil # reset error code self.retry = true save end diff --git a/app/models/turn_it_in/tii_action_upload_submission.rb b/app/models/turn_it_in/tii_action_upload_submission.rb index bdd3403a0..e8281c0c1 100644 --- a/app/models/turn_it_in/tii_action_upload_submission.rb +++ b/app/models/turn_it_in/tii_action_upload_submission.rb @@ -4,6 +4,8 @@ class TiiActionUploadSubmission < TiiAction delegate :status_sym, :status, :submission_id, :submitted_by_user, :task, :idx, :similarity_pdf_id, :similarity_pdf_path, :filename, to: :entity + NO_USER_ACCEPTED_EULA_ERROR = 'None of the student, tutor, or unit lead have accepted the EULA for Turnitin'.freeze + def description "Upload #{self.filename} for #{self.task.student.username} from #{self.task.task_definition.abbreviation} (#{self.status} - #{self.next_step})" end @@ -214,7 +216,7 @@ def tii_submission_data result.submitter = submitted_by_user.username unless submitted_by_user.accepted_tii_eula? || (params.key?("accepted_tii_eula") && params["accepted_tii_eula"]) - save_and_log_custom_error "None of the student, tutor, or unit lead have accepted the EULA for Turnitin" + save_and_log_custom_error NO_USER_ACCEPTED_EULA_ERROR return nil end @@ -443,4 +445,26 @@ def fetch_tii_similarity_pdf_status result.status end end + + # If this submission is not progressing due to a user not accepting the EULA, then + # check if the user has accepted the EULA now and retry + def attempt_retry_on_no_eula + if self.retry == false && status_sym == :created && error_message == NO_USER_ACCEPTED_EULA_ERROR + # If the student has now submitted the eula... + unless entity.submitted_by.accepted_tii_eula? + # Try reassigning the submitted_by so that it checks for tutor + # or convenor eula + entity.submitted_by = entity.submitted_by_user + end + + # If we can submit from someone... + if submitted_by_user.accepted_tii_eula? + # Save any changes to the entity + entity.save + save_and_reschedule + end + + end + end + end diff --git a/app/sidekiq/tii_check_progress_job.rb b/app/sidekiq/tii_check_progress_job.rb index 5b7b02cf5..ca8084bee 100644 --- a/app/sidekiq/tii_check_progress_job.rb +++ b/app/sidekiq/tii_check_progress_job.rb @@ -9,6 +9,8 @@ class TiiCheckProgressJob def perform return unless TurnItIn.enabled? + TurnItIn.check_and_retry_submissions_with_updated_eula + run_waiting_actions TurnItIn.check_and_update_eula TurnItIn.check_and_update_features diff --git a/test/sidekiq/tii_check_progress_job_test.rb b/test/sidekiq/tii_check_progress_job_test.rb index e3b960b4c..f030a98da 100644 --- a/test/sidekiq/tii_check_progress_job_test.rb +++ b/test/sidekiq/tii_check_progress_job_test.rb @@ -4,6 +4,217 @@ class TiiCheckProgressJobTest < ActiveSupport::TestCase include TestHelpers::TiiTestHelper + def test_check_eula_change + TiiAction.delete_all + + setup_tii_eula + + # Create a task definition with two attachments + unit = FactoryBot.create(:unit, with_students: false, task_count: 0, stream_count: 0) + + task_def = FactoryBot.create(:task_definition, unit: unit, upload_requirements: [ + { + 'key' => 'file0', + 'name' => 'My document', + 'type' => 'document', + 'tii_check' => 'true', + 'tii_pct' => '10' + } + ]) + + # Setup users + convenor = unit.main_convenor_user + tutor = FactoryBot.create(:user, :tutor) + student = FactoryBot.create(:user, :student) + + # Add users to unit + tutor_unit_role = unit.employ_staff(tutor, Role.tutor) + project = unit.enrol_student(student, Campus.first) + + # Create tutorial and enrol + tutorial = FactoryBot.create(:tutorial, unit: unit, campus: Campus.first, unit_role: tutor_unit_role) + + project.enrol_in tutorial + + task = project.task_for_task_definition(task_def) + + # Create a submission + sub1 = TiiSubmission.create( + task: task, + idx: 0, + filename: 'test.doc', + status: :created, + submitted_by_user: student + ) + sub2 = TiiSubmission.create( + task: task, + idx: 0, + filename: 'test.doc', + status: :created, + submitted_by_user: student + ) + sub3 = TiiSubmission.create( + task: task, + idx: 0, + filename: 'test.doc', + status: :created, + submitted_by_user: student + ) + + action = TiiActionUploadSubmission.find_or_create_by(entity: sub1) + + # Test fail as not EULA accepted + action.perform + + assert_not action.retry + assert_not action.complete + assert_equal TiiActionUploadSubmission::NO_USER_ACCEPTED_EULA_ERROR, action.custom_error_message + + # Now have convenor accept EULA + convenor.tii_eula_date = DateTime.now + convenor.tii_eula_version = TurnItIn.eula_version + convenor.save + + # Check the convenor has accepted + assert convenor.accepted_tii_eula? + + # See if we can retry + action.attempt_retry_on_no_eula + + assert action.retry + assert_not action.complete + assert_equal convenor, sub1.submitted_by + + convenor.tii_eula_version = nil + convenor.tii_eula_date = nil + convenor.save + assert_not convenor.accepted_tii_eula? + + # Reset... to try with tutor + action = TiiActionUploadSubmission.find_or_create_by(entity: sub2) + action.perform + + # Tutor accepts eula + tutor.tii_eula_date = DateTime.now + tutor.tii_eula_version = TurnItIn.eula_version + tutor.save + + # Check the tutor has accepted + assert tutor.accepted_tii_eula? + + # See if we can retry + action.attempt_retry_on_no_eula + + assert action.retry + assert_not action.complete + assert_equal tutor, sub2.submitted_by + + tutor.tii_eula_version = nil + tutor.tii_eula_date = nil + tutor.save + assert_not tutor.accepted_tii_eula? + + # Reset... to try with student + action = TiiActionUploadSubmission.find_or_create_by(entity: sub3) + action.perform + + # Student accepts eula + student.tii_eula_date = DateTime.now + student.tii_eula_version = TurnItIn.eula_version + student.save + + # Check the student has accepted + assert student.accepted_tii_eula? + + # See if we can retry + action.attempt_retry_on_no_eula + + assert action.retry + assert_not action.complete + assert_equal student, sub3.submitted_by + ensure + unit.destroy + end + + def test_that_progress_checks_eula_change + TiiAction.delete_all + + setup_tii_eula + setup_tii_features_enabled + + # Create a task definition with two attachments + unit = FactoryBot.create(:unit, with_students: false, task_count: 0, stream_count: 0) + + task_def = FactoryBot.create(:task_definition, unit: unit, upload_requirements: [ + { + 'key' => 'file0', + 'name' => 'My document', + 'type' => 'document', + 'tii_check' => 'true', + 'tii_pct' => '10' + } + ]) + + # Setup users + convenor = unit.main_convenor_user + tutor = FactoryBot.create(:user, :tutor) + student = FactoryBot.create(:user, :student) + + # Add users to unit + tutor_unit_role = unit.employ_staff(tutor, Role.tutor) + project = unit.enrol_student(student, Campus.first) + + # Create tutorial and enrol + tutorial = FactoryBot.create(:tutorial, unit: unit, campus: Campus.first, unit_role: tutor_unit_role) + + project.enrol_in tutorial + + task = project.task_for_task_definition(task_def) + + # Create a submission + sub1 = TiiSubmission.create( + task: task, + idx: 0, + filename: 'test.doc', + status: :created, + submitted_by_user: student + ) + + action = TiiActionUploadSubmission.find_or_create_by(entity: sub1) + + # Test fail as not EULA accepted + action.perform + + assert_not action.retry + assert_not action.complete + assert_equal TiiActionUploadSubmission::NO_USER_ACCEPTED_EULA_ERROR, action.custom_error_message + + # Get the job + job = TiiCheckProgressJob.new + + # Performing the job does not chaange the action - no eula change + job.perform + + action.reload + assert_not action.retry + assert_not action.complete + + # Now have convenor accept EULA + convenor.tii_eula_date = DateTime.now + convenor.tii_eula_version = TurnItIn.eula_version + convenor.save + + # Perform progress check job + job.perform + + # Will trigger retry of action, but wont perform as it is not old + action.reload + assert action.retry + assert_not action.complete + + unit.destroy + end + def test_waits_to_process_action setup_tii_eula @@ -76,7 +287,7 @@ def test_waits_to_process_action assert_requested accept_request, times: 2 assert action.reload.retry - refute action.complete + assert_not action.complete action.update(last_run: DateTime.now - 31.minutes) job.perform # attempt 3 - but rate limited @@ -92,7 +303,7 @@ def test_waits_to_process_action # Check it was all success assert action.reload.complete - refute action.retry + assert_not action.retry assert user.reload.accepted_tii_eula? assert user.tii_eula_version_confirmed