From eaab8895a7f56b31b8d6bb38b5e65a3cb01b2ec8 Mon Sep 17 00:00:00 2001 From: Mysti Lilla Date: Wed, 24 Mar 2021 14:48:10 -0600 Subject: [PATCH] Prevent duplicate submissions from TurnItIn 1.1 tool fixes INTEROP-6426 flag=none Test plan - Set up the TurnItIn 1.1 LTI tool on an assignment - Upload a document to their tool as a student - Send a request to get the TurnItIn submission download process started (see below) - Try to get DocViewer annotations working and add some - Resend the request to get the TurnItIn submission - Ensure you don't lose the annotations EX: In IRB fill out the following: require 'json' require 'oauth' oauth_key = (from your tool consumer key) oauth_secret = (from your tool shared secret) paperid = (after you've uploaded a document via the LTI tool, click on it and use the o parameter from the URL) outcomes_tool_placement_url = (https://sandbox.turnitin.com/ api/lti/1p0/outcome_tool_data/#{paperid}?lang=en_us) sourcedid = (from the LTI launch the value in lis_result_sourcedid) tool_id = string tool id from the tool post_url = (from the LTI launch the value in ext_outcomes_tool_placement_url) json = {"outcomes_tool_placement_url"=>outcomes_tool_placement_url, "paperid"=>paperid, "lis_result_sourcedid"=>sourcedid, "tool_id"=>tool_id}.to_json consumer = OAuth::Consumer.new(oauth_key, oauth_secret) token = OAuth::AccessToken.new(consumer) response = token.post(post_url, json, 'Content-Type' => 'application/json') This starts a delayed job that checks with TurnItIn for the score. If done right, it should not take long. Change-Id: I80ac8bd666a25b8b52ace46befe66daaad32b2c4 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/261550 Tested-by: Service Cloud Jenkins Reviewed-by: Wagner Goncalves QA-Review: Wagner Goncalves Product-Review: Mysti Lilla --- lib/turnitin/outcome_response_processor.rb | 15 ++++++++------- .../outcome_response_processor_spec.rb | 18 ++++++++++++++++-- spec/lib/turnitin/turnitin_spec_helper.rb | 2 +- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/lib/turnitin/outcome_response_processor.rb b/lib/turnitin/outcome_response_processor.rb index 9330b0673da..8220fc14898 100644 --- a/lib/turnitin/outcome_response_processor.rb +++ b/lib/turnitin/outcome_response_processor.rb @@ -35,6 +35,11 @@ module Turnitin end def process + submission = @assignment.submissions.find_by(user: @user, submitted_at: (turnitin_client.uploaded_at || Time.zone.now)) + submission.nil? ? new_submission : submission.retrieve_lti_tii_score + end + + def new_submission # Create an attachment for the file submitted via the TII tool. # If the score is still pending, this will raise # `Errors::ScoreStillPendingError` @@ -58,10 +63,6 @@ module Turnitin # will terminate and retry up to # the max_attempts limit # - # TODO: This job does not use any kind of - # exponential backoff. We should add - # one to give TII more time to process - # attachments. stash_turnitin_client do delay(max_attempts: self.class.max_attempts).update_originality_data(submission, asset_string) end @@ -76,7 +77,7 @@ module Turnitin priority: Delayed::LOW_PRIORITY, attempts: attempt_number, run_at: Time.now.utc + (attempt_number ** 4) + 5). - process + new_submission end end rescue StandardError @@ -137,10 +138,10 @@ module Turnitin # serialize the object for job processing (a turnitin client # will be created in the job when necessary). def stash_turnitin_client - old_turnit_client = @_turnitin_client + old_turnitin_client = @_turnitin_client @_turnitin_client = nil result = yield - @_turnitin_client = old_turnit_client + @_turnitin_client = old_turnitin_client result end diff --git a/spec/lib/turnitin/outcome_response_processor_spec.rb b/spec/lib/turnitin/outcome_response_processor_spec.rb index e69a606440e..d925f30cfab 100644 --- a/spec/lib/turnitin/outcome_response_processor_spec.rb +++ b/spec/lib/turnitin/outcome_response_processor_spec.rb @@ -79,18 +79,26 @@ module Turnitin submission = lti_assignment.submissions.first expect(submission.attempt).to eq 1 end + + it 'does not create a new submission version if processed twice' do + subject.process + submission = lti_assignment.submissions.first + subject.process + expect(submission.versions.count).to eq 1 + end end describe "#process with request errors" do context 'when it is not the last attempt' do it 'does not create an error attachment' do allow_any_instance_of(subject.class).to receive(:attempt_number).and_return(subject.class.max_attempts-1) - expect_any_instance_of(TurnitinApi::OutcomesResponseTransformer).to receive(:original_submission).and_raise(Faraday::TimeoutError, 'Net::ReadTimeout') + expect_any_instance_of(TurnitinApi::OutcomesResponseTransformer).to receive(:response).and_raise(Faraday::TimeoutError, 'Net::ReadTimeout') expect { subject.process }.to raise_error(Faraday::TimeoutError) expect(lti_assignment.attachments.count).to eq 0 end it 'creates a new job' do + allow_any_instance_of(TurnitinApi::OutcomesResponseTransformer).to receive(:uploaded_at).and_return(tii_response['meta']['date_uploaded']) time = Time.now.utc attempt_number = subject.class.max_attempts-1 original_submission_response = double('original_submission_mock') @@ -103,7 +111,7 @@ module Turnitin priority: Delayed::LOW_PRIORITY, attempts: attempt_number, run_at: time + (attempt_number ** 4) + 5).and_return(mock) - expect(mock).to receive(:process) + expect(mock).to receive(:new_submission) Timecop.freeze(time) do subject.process end @@ -111,6 +119,12 @@ module Turnitin end context 'when it is the last attempt' do + before(:each) do + response_response = double('response_mock') + allow(response_response).to receive(:body).and_return(tii_response) + allow_any_instance_of(TurnitinApi::OutcomesResponseTransformer).to receive(:response).and_return(response_response) + end + it 'creates an attachment for "Errors::ScoreStillPendingError"' do allow(subject.class).to receive(:max_attempts).and_return(1) original_submission_response = double('original_submission_mock') diff --git a/spec/lib/turnitin/turnitin_spec_helper.rb b/spec/lib/turnitin/turnitin_spec_helper.rb index 2c1aae581a3..827159f2b07 100644 --- a/spec/lib/turnitin/turnitin_spec_helper.rb +++ b/spec/lib/turnitin/turnitin_spec_helper.rb @@ -17,7 +17,7 @@ # You should have received a copy of the GNU Affero General Public License along # with this program. If not, see . -require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb') +require 'spec_helper.rb' RSpec.shared_context "shared_tii_lti", :shared_context => :metadata do before do