read from Location on 201 response from upload

fixes CNVS-39065

vs. success_url for legacy path or body of S3 redirect. for parity with
those old responses, we need to have the Location returned from the
capture include the `enhanced_preview_url` when appropriate.

not sure why, but when S3 gives you a redirect after successful upload,
the XHR follows the redirect happily. when inst-fs does, Canvas gives a
CORS error (at least during local development with docker). rather than
try and tell the difference and fix, it, we really shouldn't be
redirecting anyways. use the appropriate REST status and let the client
know that it wants to fetch from the Location explicitly. this explicit
request is happy without any CORS mangling.

test-plan:
 - have your local canvas connected to local inst-fs
 - go to <canvas>/files
 - upload a new file
 - upload should succeed, including adding the new file to the UI
 - repeat for any other place you can think of to upload a file in
   canvas, including:
   - avatars
   - gradebook
   - submissions
   - conversations
   - etc.

Change-Id: I312aaa17fd000843131c89023e827410cc5e13c6
Reviewed-on: https://gerrit.instructure.com/126012
Tested-by: Jenkins
Reviewed-by: Andrew Huff <[email protected]>
QA-Review: Collin Parrish <[email protected]>
Product-Review: Jacob Fugal <[email protected]>

app/coffeescripts/models/File.coffee

@@ -69,14 +69,20 @@ define [
       @set @uploadParams
       el.name = data.file_param
       @url = -> data.upload_url
+      onUpload = (data) =>
+        dfrd.resolve(data)
+        options.success?(data)
+      onError = (error) =>
+        dfrd.reject(error)
+        options.error?(error)
       FilesystemObject::save.call this, null,
         multipart: true
         success: (data) =>
-          dfrd.resolve(data)
-          options.success?(data)
-        error: (error) =>
-          dfrd.reject(error)
-          options.error?(error)
+          if data.location?
+            $.ajaxJSON(data.location, 'GET', {}, onUpload, onError)
+          else
+            onUpload(data)
+        error: onError
 
     toJSON: ->
       return super unless @get('file')

app/coffeescripts/react_files/modules/FileUploader.coffee

@@ -30,7 +30,11 @@ define [
         @deferred.reject(event.target.status)
         return
 
-      url = @uploadData.upload_params.success_url
+      url = if event.target.status == 201
+        $.parseJSON(event.target.response).location
+      else
+        @uploadData.upload_params.success_url
+
       if url
         $.getJSON(url).then (results) =>
           f = @addFileToCollection(results)

app/coffeescripts/react_files/modules/ZipUploader.coffee

@@ -58,7 +58,6 @@ define [
         $.getJSON(url).then (results) =>
           @getContentMigration()
       else
-        results = $.parseJSON(uploadResults.target.response)
         @getContentMigration()
 
     # get the content migration when ready and use progress api to pull migration progress

app/coffeescripts/views/profiles/AvatarDialogView.coffee

@@ -171,11 +171,16 @@ define [
       })
 
     onPostAvatar: (preflightResponse, postAvatarResponse) =>
-      if preflightResponse.success_url
-        @s3Success(preflightResponse, postAvatarResponse).then(@onS3Success)
+      if postAvatarResponse.location
+        @getCanvasJSON(postAvatarResponse.location).then(@onServiceSuccess)
+      else if preflightResponse.success_url
+        @s3Success(preflightResponse, postAvatarResponse).then(@onServiceSuccess)
       else
         @waitAndSaveUserAvatar(postAvatarResponse.avatar.token, postAvatarResponse.avatar.url)
 
+    getCanvasJSON: (location) =>
+      $.getJSON("#{location}?include=avatar")
+
     s3Success: (preflightResponse, s3Response) =>
       $s3 = $(s3Response)
       $.getJSON(preflightResponse.success_url, {
@@ -184,7 +189,7 @@ define [
         etag:   $s3.find('ETag').text()
       })
 
-    onS3Success: (response) =>
+    onServiceSuccess: (response) =>
       @waitAndSaveUserAvatar(response.avatar.token, response.avatar.url)
 
     # need to wait for the avatar to get processed by background jobs before

app/controllers/files_controller.rb

@@ -809,11 +809,13 @@ def api_capture
     @attachment.handle_duplicates(params[:on_duplicate])
 
     # trigger upload success callbacks
-    if @attachment.context.respond_to?(:file_upload_success_callback)
-      @attachment.context.file_upload_success_callback(@attachment)
+    if @context.respond_to?(:file_upload_success_callback)
+      @context.file_upload_success_callback(@attachment)
     end
 
-    render json: {}, status: :created, location: api_v1_attachment_url(@attachment)
+    url_params = {}
+    url_params[:include] = 'enhanced_preview_url' if @context.is_a?(User) || @context.is_a?(Course)
+    render json: {}, status: :created, location: api_v1_attachment_url(@attachment, url_params)
   end
 
   def api_create_success

doc/api/file_uploads.md

@@ -115,13 +115,19 @@ current directory.
 
 ### Step 3: Confirm the upload's success
 
-If Step 2 is successful, the response will be a 3XX redirect with a
-Location header set as normal. The application needs to perform a POST to
-this location in order to complete the upload, otherwise the new file may
-not be marked as available. This request is back against Canvas again,
-and needs to be authenticated using the normal API access token
+If Step 2 is successful, the response will be either a 3XX redirect or
+201 Created with a Location header set as normal.
+
+In the case of a 3XX redirect, the application needs to perform a POST
+to this location in order to complete the upload, otherwise the new file
+may not be marked as available. This request is back against Canvas
+again, and needs to be authenticated using the normal API access token
 authentication.
 
+In the case of a 201 Created, the upload has been complete and the
+Canvas JSON representation of the file can be retrieved with a GET from
+the provided Location.
+
 Example Request:
 
 ```bash

lib/api/v1/attachment.rb

@@ -125,6 +125,9 @@ def attachment_json(attachment, user, url_options = {}, options = {})
     if includes.include? "context_asset_string"
       hash['context_asset_string'] = attachment.context.try(:asset_string)
     end
+    if includes.include? 'avatar' && respond_to?(:avatar_json)
+      hash['avatar'] = avatar_json(user, attachment, type: 'attachment')
+    end
 
     if options[:master_course_status]
       hash.merge!(attachment.master_course_api_restriction_data(options[:master_course_status]))

public/javascripts/jquery.instructure_forms.js

@@ -299,20 +299,28 @@ import './vendor/jquery.scrollTo'
     var uploadFile = function(parameters, file) {
       $.ajaxJSON(options.uploadDataUrl || "/files/pending", 'POST', parameters, function(data) {
         try {
+        var old_name = $(file).attr('name');
+        var onError = function(error) {
+          $(file).attr('name', old_name);
+          (options.upload_error || options.error).call($this, error);
+        };
+        var onUpload = function(data) {
+          $(file).attr('name', old_name);
+          attachments.push(data);
+          next.call($this);
+        };
         if(data && data.upload_url) {
           var post_params = data.upload_params;
-          var old_name = $(file).attr('name');
           $(file).attr('name', data.file_param);
           $.ajaxJSONFiles(data.upload_url, 'POST', post_params, $(file), function(data) {
-            attachments.push(data);
-            $(file).attr('name', old_name);
-            next.call($this);
-          }, function(data) {
-            $(file).attr('name', old_name);
-            (options.upload_error || options.error).call($this, data);
-          }, {onlyGivenParameters: true });
+            if (data.location) {
+              $.ajaxJSON(data.location, 'GET', {}, onUpload, onError);
+            } else {
+              onUpload(data)
+            }
+          }, onError, {onlyGivenParameters: true });
         } else {
-          (options.upload_error || options.error).call($this, data);
+          onError(data);
         }
         } finally {}