Skip to content

Commit add7e1e

Browse files
codekitchencodekitchen
committed
display inline files in user context
fixes #6034 Change-Id: I6e48d0979b3082019528b6a3deadf9565d28027e Reviewed-on: https://gerrit.instructure.com/6345 Reviewed-by: Brian Palmer <brianp@instructure.com> Tested-by: Hudson <hudson@instructure.com>
1 parent 51dc49b commit add7e1e

3 files changed

Lines changed: 87 additions & 6 deletions

File tree

‎app/controllers/files_controller.rb‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@ def full_index
103103
return
104104
end
105105
return unless tab_enabled?(@context.class::TAB_FILES)
106-
@context = UserProfile.new(@context) if @context == @current_user
107106
log_asset_access("files:#{@context.asset_string}", "files", 'other') if @context
108107
respond_to do |format|
109108
if @contexts.empty?
@@ -162,15 +161,18 @@ def show
162161
original_params = params.dup
163162
params[:id] ||= params[:file_id]
164163
get_context
164+
# note that the /files/XXX URL implicitly uses the current user as the
165+
# context, even though it doesn't search for the file using
166+
# @current_user.attachments.find , since it might not actually be a user
167+
# attachment.
168+
# this implicit context magic happens in ApplicationController#get_context
165169
if @context && !@context.is_a?(User)
166170
@attachment = @context.attachments.find(params[:id])
167171
else
168172
@attachment = Attachment.find(params[:id])
169-
@context = nil
170-
@skip_crumb = true
173+
@skip_crumb = true unless @context
171174
end
172175
params[:download] ||= params[:preview]
173-
@context = UserProfile.new(@context) if (@context == @current_user) && @current_user
174176
add_crumb(t('#crumbs.files', "Files"), named_context_url(@context, :context_files_url)) unless @skip_crumb
175177
if @attachment.deleted?
176178
flash[:notice] = t 'notices.deleted', "The file %{display_name} has been deleted", :display_name => @attachment.display_name

‎spec/factories/attachment_factory.rb‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,15 @@ def valid_attachment_attributes(opts={})
3737
}
3838
end
3939

40-
def stub_png_data(filename = 'test my file? hai!&.png', data = nil)
40+
def stub_file_data(filename, data, content_type)
4141
$stub_file_counter ||= 0
4242
data ||= "ohai#{$stub_file_counter += 1}"
4343
sio = StringIO.new(data)
4444
sio.stub!(:original_filename).and_return(filename)
45-
sio.stub!(:content_type).and_return('image/png')
45+
sio.stub!(:content_type).and_return(content_type)
4646
sio
4747
end
48+
49+
def stub_png_data(filename = 'test my file? hai!&.png', data = nil)
50+
stub_file_data(filename, data, 'image/png')
51+
end

‎spec/integration/files_spec.rb‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,81 @@
4141
end
4242
end
4343

44+
context "should support User as a context" do
45+
before(:each) do
46+
user_with_pseudonym
47+
login_as
48+
@me = @user
49+
@att = @me.attachments.create(:uploaded_data => stub_png_data('my-pic.png'))
50+
end
51+
52+
it "with safefiles" do
53+
HostUrl.stub!(:file_host).and_return('files-test.host')
54+
get "http://test.host/users/#{@me.id}/files/#{@att.id}/download"
55+
response.should be_redirect
56+
uri = URI.parse response['Location']
57+
qs = Rack::Utils.parse_nested_query(uri.query)
58+
uri.host.should == 'files-test.host'
59+
# redirects to a relative url, since relative files are available in user context
60+
uri.path.should == "/users/#{@me.id}/files/#{@att.id}/my%20files/unfiled/my-pic.png"
61+
@me.valid_access_verifier?(qs['ts'], qs['sf_verifier']).should be_true
62+
location = response['Location']
63+
reset!
64+
65+
get location
66+
response.should be_success
67+
response.content_type.should == 'image/png'
68+
# ensure that the user wasn't logged in by the normal means
69+
controller.instance_variable_get(:@current_user).should be_nil
70+
end
71+
72+
it "without safefiles" do
73+
HostUrl.stub!(:file_host).and_return('test.host')
74+
get "http://test.host/users/#{@me.id}/files/#{@att.id}/download"
75+
response.should be_success
76+
response.content_type.should == 'image/png'
77+
response['Pragma'].should be_nil
78+
response['Cache-Control'].should_not match(/no-cache/)
79+
end
80+
81+
context "with inlineable html files" do
82+
before do
83+
@att = @me.attachments.create(:uploaded_data => stub_file_data("ohai.html", "<html><body>ohai</body></html>", "text/html"))
84+
end
85+
86+
it "with safefiles" do
87+
HostUrl.stub!(:file_host).and_return('files-test.host')
88+
get "http://test.host/users/#{@me.id}/files/#{@att.id}/download", :wrap => '1'
89+
response.should be_redirect
90+
uri = URI.parse response['Location']
91+
qs = Rack::Utils.parse_nested_query(uri.query)
92+
uri.host.should == 'test.host'
93+
uri.path.should == "/users/#{@me.id}/files/#{@att.id}"
94+
location = response['Location']
95+
96+
get location
97+
# the response will be on the main domain, with an iframe pointing to the files domain and the actual uploaded html file
98+
response.should be_success
99+
response.content_type.should == 'text/html'
100+
doc = Nokogiri::HTML::DocumentFragment.parse(response.body)
101+
doc.at_css('iframe#file_content')['src'].should =~ %r{^http://files-test.host/users/#{@me.id}/files/#{@att.id}/my%20files/unfiled/ohai.html}
102+
end
103+
104+
it "without safefiles" do
105+
HostUrl.stub!(:file_host).and_return('test.host')
106+
get "http://test.host/users/#{@me.id}/files/#{@att.id}/download", :wrap => '1'
107+
response.should be_redirect
108+
location = response['Location']
109+
URI.parse(location).path.should == "/users/#{@me.id}/files/#{@att.id}"
110+
get location
111+
response.content_type.should == 'text/html'
112+
doc = Nokogiri::HTML::DocumentFragment.parse(response.body)
113+
doc.at_css('iframe#file_content')['src'].should =~ %r{^http://test.host/users/#{@me.id}/files/#{@att.id}/my%20files/unfiled/ohai.html}
114+
end
115+
116+
end
117+
end
118+
44119
it "should use relative urls for safefiles in course context" do
45120
course_with_teacher(:active_all => true, :user => user_with_pseudonym)
46121
login_as

0 commit comments

Comments
 (0)