fix downloading content exports with local storage + safefiles

Change-Id: I867fbd6a2c99e44be7ada992618f8c508aa43590
Reviewed-on: https://gerrit.instructure.com/4346
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>

app/controllers/application_controller.rb

@@ -252,8 +252,6 @@ class ApplicationController < ActionController::Base
       elsif request.path.match(/\A\/profile/) || request.path == '/' || request.path.match(/\A\/dashboard\/files/) || request.path.match(/\A\/calendar/) || request.path.match(/\A\/assignments/) || request.path.match(/\A\/files/)
         @context = @current_user
         @context_membership = @context
-      elsif params[:content_export_id]
-        @context = ContentExport.find(params[:content_export_id])
       end
       if @context.try_rescue(:only_wiki_is_public) && params[:controller].match(/wiki/) && !@current_user && (!@context.is_a?(Course) || session[:enrollment_uuid_course_id] != @context.id)
         @show_left_side = false

app/controllers/content_exports_controller.rb

@@ -87,10 +87,12 @@ class ContentExportsController < ApplicationController
       render :template => 'shared/errors/404_message', :status => :bad_request
     end
   end
-  
+
   private
-  
+
   def render_export(export)
-    render :json => export.to_json(:only => [:id, :progress, :workflow_state],:methods => [:download_url, :error_message])
+    json = export.as_json(:only => [:id, :progress, :workflow_state],:methods => [:error_message])
+    json['content_export']['download_url'] = verified_file_download_url(export.attachment) if export.attachment
+    render :json => json
   end
-end
+end

app/models/content_export.rb

@@ -68,10 +68,6 @@ class ContentExport < ActiveRecord::Base
   end
   handle_asynchronously :export_course, :priority => Delayed::LOW_PRIORITY, :max_attempts => 1
   
-  def download_url
-    self.attachment ? self.attachment.authenticated_s3_url : nil
-  end
-  
   def error_message
     self.settings[:last_error]
   end

app/views/content_exports/index.html.erb

@@ -19,7 +19,7 @@
           <p>
             <%= t 'links.download', "Course Export from %{date}: *Click here to download*",
                     :date => "<span class=\"created_at time_ago_date\">#{datetime_string export.created_at}</span>".html_safe,
-                    :wrapper => "<a href=\"#{export.download_url}\">\\1</a>" %>
+                    :wrapper => link_to('\1', export.attachment ? verified_file_download_url(export.attachment) : nil) %>
           </p>
         <% end %>
       <% end %>

config/routes.rb

@@ -636,11 +636,6 @@ ActionController::Routing::Routes.draw do |map|
   
   # Routes for course exports
   map.connect 'xsd/:version.xsd', :controller => 'content_exports', :action => 'xml_schema'
-  map.resources :content_exports do |ce|
-    ce.resources :files do |file|
-      file.download 'download', :controller => 'files', :action => 'show', :download => '1'
-    end
-  end
 
   map.resources :jobs, :only => %w(index), :collection => %w[batch_update]
 

spec/integration/files_spec.rb

@@ -35,7 +35,30 @@ describe FilesController do
       response['Cache-Control'].should_not match(/no-cache/)
     end
   end
-  
+
+  it "should use relative urls for safefiles in course context" do
+    course_with_teacher_logged_in(:active_all => true)
+    a1 = attachment_model(:uploaded_data => stub_png_data, :content_type => 'image/png', :context => @course)
+    HostUrl.stub!(:file_host).and_return('files-test.host')
+    get "http://test.host/courses/#{@course.id}/files/#{a1.id}/download", :inline => '1'
+    response.should be_redirect
+    uri = URI.parse response['Location']
+    qs = Rack::Utils.parse_nested_query(uri.query)
+    uri.host.should == 'files-test.host'
+    uri.path.should == "/courses/#{@course.id}/files/#{a1.id}/course%20files/test.png"
+    @user.valid_access_verifier?(qs['ts'], qs['sf_verifier']).should be_true
+    qs['verifier'].should be_nil
+
+    get response['Location']
+    response.should be_success
+    response.content_type.should == 'image/png'
+  end
+
+  it "shouldn't use relative urls for safefiles in other contexts" do
+    course_with_teacher_logged_in(:active_all => true)
+    a1 = attachment_model(:uploaded_data => stub_png_data, :content_type => 'image/png', :context => @course)
+  end
+
   context "should support ContextMessage as a context" do
     before(:each) do
       course_with_teacher_logged_in(:active_all => true)

spec/selenium/courses_sel.rb

@@ -18,6 +18,18 @@ shared_examples_for "course selenium tests" do
     wizard_box.displayed?.should be_false
   end
 
+  it "should allow content export downloads" do
+    course_with_teacher_logged_in
+    get "/courses/#{@course.id}/content_exports"
+    driver.find_element(:css, "button.submit_button").click
+    job = Delayed::Job.last(:conditions => { :tag => 'ContentExport#export_course_without_send_later' })
+    export = keep_trying_until { ContentExport.last }
+    export.export_course_without_send_later
+    new_download_link = keep_trying_until { driver.find_element(:css, "div#exports a") }
+    url = new_download_link.attribute 'href'
+    url.should match(%r{/files/\d+/download\?verifier=})
+  end
+
   it "should allow moving a student to a different section" do
     c = course :active_course => true
     users = {:plain => {}, :sis => {}}