don't pass extra parameters to attachment_fu

when s3 storage is enabled, attachment_fu's url generation is...
finicky. it'll blow up if parameters it's not expecting are present.

test-plan:
- have s3 storage enabled in your environment
- have a file in your user's file area (new or old, doesn't matter, as
  long as it's on s3)
- select the file in the file UI and click the "download" button
- should download the file instead of giving an error

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

app/models/attachment.rb

@@ -749,7 +749,9 @@ def authenticated_url(*thumbnail, **options)
     if InstFS.enabled? && self.instfs_uuid
       InstFS.authenticated_url(self, options)
     else
-      disposition = options[:download] ? "attachment" : "inline"
+      # attachment_fu doesn't like the extra option when building s3 urls
+      should_download = options.delete(:download)
+      disposition = should_download ? "attachment" : "inline"
       options[:response_content_disposition] = "#{disposition}; #{disposition_filename}"
       self.authenticated_s3_url(*thumbnail, **options)
     end

spec/models/attachment_spec.rb

@@ -988,6 +988,13 @@ def configure_canvadocs(opts = {})
       course_model
     end
 
+    it "should work with s3 storage" do
+      s3_storage!
+      attachment = attachment_with_context(@course, :display_name => 'foo')
+      expect(attachment.download_url).to match(/response-content-disposition=attachment/)
+      expect(attachment.inline_url).to match(/response-content-disposition=inline/)
+    end
+
     it 'should allow custom ttl for download_url' do
       attachment = attachment_with_context(@course, :display_name => 'foo')
       allow(attachment).to receive(:authenticated_url) # allow other calls due to, e.g., save