update files API for uploading via url

fixes COMMS-2257

This adds a flag to the Files API when uploading a file via a url to be
able to avoid automatically submitting the assignment associated with
the file. The flag is defaulted to true to avoid any issues that could
arrise from other services depending on the current API.

Test Plan:
* passes jenkins

Change-Id: I40e569a5126a0d9e5607f1a799269dc7d20204ed
Reviewed-on: https://gerrit.instructure.com/203105
Tested-by: Jenkins
Reviewed-by: Mysti Lilla <mysti@instructure.com>
QA-Review: Steven Burnett <sburnett@instructure.com>
Product-Review: Steven Burnett <sburnett@instructure.com>

app/controllers/files_controller.rb

@@ -897,9 +897,10 @@ class FilesController < ApplicationController
 
     if params[:progress_id]
       progress = Progress.find(params[:progress_id])
+      submit_assignment = params.key?(:submit_assignment) ? value_to_boolean(params[:submit_assignment]) : true
 
-      # If the attachment is for an Assignment's upload_via_url, submit it
-      if progress.tag == 'upload_via_url' && progress.context.is_a?(Assignment)
+      # If the attachment is for an Assignment's upload_via_url and the submit_assignment flag is set, submit it
+      if progress.tag == 'upload_via_url' && progress.context.is_a?(Assignment) && submit_assignment
         homework_service = Services::SubmitHomeworkService.new(@attachment, progress)
 
         begin

app/controllers/submissions_api_controller.rb

@@ -606,6 +606,7 @@ class SubmissionsApiController < ApplicationController
     @assignment = @context.assignments.active.find(params[:assignment_id])
     @user = get_user_considering_section(params[:user_id])
     permission = @assignment.submission_types.include?("online_upload") ? :submit : :nothing
+    submit_assignment = params.key?(:submit_assignment) ? value_to_boolean(params[:submit_assignment]) : true
     # rationale for allowing other user ids at all: eventually, you'll be able
     # to use this api for uploading an attachment to a submission comment.
     # teachers will be able to do that for any submission they can grade, so
@@ -616,7 +617,8 @@ class SubmissionsApiController < ApplicationController
         @user, request,
         check_quota: false, # we don't check quota when uploading a file for assignment submission
         folder: @user.submissions_folder(@context), # organize attachment into the course submissions folder
-        assignment: @assignment
+        assignment: @assignment,
+        submit_assignment: submit_assignment
       )
     end
   end

doc/api/file_uploads.md

@@ -163,11 +163,13 @@ public HTTP or HTTPS URL from which to retrieve the file.
 ### Step 1a: Posting the file URL to Canvas
 
 The first step is the same as with the "Uploading via POST" flow above,
-with the addition of one new parameter:
+with the addition of a few new parameters:
 
 <dl>
   <dt>url</dt>
   <dd>The full URL to the file to be uploaded. This URL must be publicly accessible.</dd>
+  <dt>submit_assignment</dt>
+  <dd>A boolean to indicate whether or not to automatically submit the assignment the file is associated with if it is associated with an assignment. Defaults to true.</dd>
 </dl>
 
 Example Request:

lib/api/v1/attachment.rb

@@ -250,7 +250,8 @@ module Api::V1::Attachment
 
         if progress_context.is_a? Assignment
           additional_capture_params = {
-            eula_agreement_timestamp: params[:eula_agreement_timestamp]
+            eula_agreement_timestamp: params[:eula_agreement_timestamp],
+            submit_assignment: opts[:submit_assignment]
           }
         end
 
@@ -298,7 +299,7 @@ module Api::V1::Attachment
         )
 
         Services::SubmitHomeworkService.submit_job(
-          @attachment, progress, params[:eula_agreement_timestamp], executor
+          @attachment, progress, params[:eula_agreement_timestamp], executor, opts[:submit_assignment]
         )
 
         json = { progress: progress_json(progress, @current_user, session) }

lib/services/submit_homework_service.rb

@@ -112,8 +112,8 @@ module Services
         CloneUrlExecutor.new(url, duplicate_handling, check_quota, opts)
       end
 
-      def submit_job(attachment, progress, eula_agreement_timestamp, executor)
-        if progress.context.is_a? Assignment
+      def submit_job(attachment, progress, eula_agreement_timestamp, executor, submit_assignment)
+        if progress.context.is_a?(Assignment) && submit_assignment
           SubmitWorker.
             new(attachment.id, progress.id, eula_agreement_timestamp, executor).
             tap { |worker| enqueue_attachment_job(worker) }

spec/apis/v1/submissions_api_spec.rb

@@ -4029,6 +4029,13 @@ describe 'Submissions API', type: :request do
           job = Delayed::Job.order(:id).last
           expect(job.handler).to include Services::SubmitHomeworkService::SubmitWorker.name
         end
+
+        it 'should enqueue the copy job when the submit_assignment parameter is false' do
+          preflight(url: 'http://example.com/test', filename: 'test.txt', submit_assignment: false)
+          JSON.parse(response.body)
+          job = Delayed::Job.order(:id).last
+          expect(job.handler).to include Services::SubmitHomeworkService::CopyWorker.name
+        end
       end
     end
 

spec/controllers/files_controller_spec.rb

@@ -1339,6 +1339,7 @@ describe FilesController do
               tap(&:start).
               tap(&:save!)
           end
+
           let(:progress_params) do
             assignment_params.merge(
               progress_id: progress.id,
@@ -1352,11 +1353,21 @@ describe FilesController do
             allow(homework_service).to receive(:queue_email)
           end
 
-          it 'should submit the attachment' do
+          it 'should submit the attachment if the submit_assignment flag is not provided' do
             expect(homework_service).to receive(:submit).with(eula_agreement_timestamp)
             request
           end
 
+          it 'should submit the attachment if the submit_assignment param is set to true' do
+            expect(homework_service).to receive(:submit).with(eula_agreement_timestamp)
+            post "api_capture", params: progress_params.merge(submit_assignment: true)
+          end
+
+          it 'should not submit the attachment if the submit_assignment param is set to false' do
+            expect(homework_service).not_to receive(:submit)
+            post "api_capture", params: progress_params.merge(submit_assignment: false)
+          end
+
           it 'should save the eula_agreement_timestamp' do
             request
             submission = Submission.where(assignment_id: assignment.id)

spec/lib/services/submit_homework_service_spec.rb

@@ -33,6 +33,7 @@ module Services
         filename: 'Some File'
       )
     end
+    let(:submit_assignment) { true }
     let(:failure_email) do
       OpenStruct.new(
         from_name: 'notifications@instructure.com',
@@ -74,7 +75,7 @@ module Services
     describe '.submit_job' do
       let(:service) { described_class.new(attachment, progress) }
       let(:worker) do
-        described_class.submit_job(attachment, progress, eula_agreement_timestamp, executor)
+        described_class.submit_job(attachment, progress, eula_agreement_timestamp, executor, submit_assignment)
       end
 
       before do
@@ -82,7 +83,7 @@ module Services
         allow(worker).to receive(:attachment).and_return(attachment)
       end
 
-      it 'should clone and submit the url' do
+      it 'should clone and submit the url when submit_assignment is true' do
         expect(attachment).to receive(:clone_url).with(url, dup_handling, check_quota, opts)
         expect(service).to receive(:submit).with(eula_agreement_timestamp)
         worker.perform
@@ -90,6 +91,17 @@ module Services
         expect(progress.reload.workflow_state).to eq 'completed'
       end
 
+      it 'should clone and not submit the url when submit_assignment is false' do
+        worker = described_class.submit_job(attachment, progress, eula_agreement_timestamp, executor, false)
+        allow(worker).to receive(:homework_service).and_return(service)
+        allow(worker).to receive(:attachment).and_return(attachment)
+        expect(attachment).to receive(:clone_url).with(url, dup_handling, check_quota, opts)
+        expect(service).not_to receive(:submit)
+        worker.perform
+
+        expect(progress.reload.workflow_state).to eq 'completed'
+      end
+
       context 'on an error' do
         before { worker.on_permanent_failure("error") }