Skip to content

Commit f80696b

Browse files
committed
fix file preview for hidden and nonexistent files
test plan: 1. enable "better file browsing" 2. go to the files page 3. click the cloud icon next to a file, select "Restricted Access", and then "Only available to students with link". 4. as a teacher, attempt to preview the file. you should see the file, not a not-found error. 5. attempt to preview a nonexistent file by going to the URL /courses/X/files/Y/file_preview (where X is a *valid* course ID and Y is an *invalid* file id). you should see an error message without Canvas chrome (no header or left-side menu) fixes CNVS-16394 Change-Id: I9c632d664e9fd8758dc63b21e6751a914f4a4ceb Reviewed-on: https://gerrit.instructure.com/43175 Reviewed-by: Ryan Shaw <ryan@instructure.com> Product-Review: Ryan Shaw <ryan@instructure.com> Tested-by: Jenkins <jenkins@instructure.com> QA-Review: Jahnavi Yetukuri <jyetukuri@instructure.com>
1 parent 2af63dd commit f80696b

2 files changed

Lines changed: 16 additions & 2 deletions

File tree

app/controllers/file_previews_controller.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,12 @@ class FilePreviewsController < ApplicationController
4949
# renders (or redirects to) appropriate content for the file, such as
5050
# canvadocs, crocodoc, inline image, etc.
5151
def show
52-
@file = @context.attachments.active.find(params[:file_id])
52+
@file = @context.attachments.not_deleted.find_by_id(params[:file_id])
53+
unless @file
54+
@headers = false
55+
@show_left_side = false
56+
return render template: 'shared/errors/404_message', status: :not_found
57+
end
5358
if authorized_action(@file, @current_user, :download)
5459
# mark item seen for module progression purposes
5560
@file.context_module_action(@current_user, :read) if @current_user

spec/controllers/file_previews_controller_spec.rb

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,14 @@
3535
expect(response.status).to eq 401
3636
end
3737

38-
it "should 404 if the file doesn't exist" do
38+
it "should 404 (w/o canvas chrome) if the file doesn't exist" do
3939
attachment_model
4040
file_id = @attachment.id
4141
@attachment.destroy!
4242
get :show, course_id: @course.id, file_id: file_id
4343
expect(response.status).to eq 404
44+
expect(assigns['headers']).to eq false
45+
expect(assigns['show_left_side']).to eq false
4446
end
4547

4648
it "should redirect to crododoc_url if available and params[:annotate] is given" do
@@ -104,4 +106,11 @@
104106
get :show, course_id: @course.id, file_id: @attachment.id
105107
expect(mod.evaluate_for(@user).workflow_state).to eq "completed"
106108
end
109+
110+
it "should work with hidden files" do
111+
attachment_model content_type: 'image/png'
112+
@attachment.update_attribute(:file_state, 'hidden')
113+
get :show, course_id: @course.id, file_id: @attachment.id
114+
expect(response).to be_success
115+
end
107116
end

0 commit comments

Comments
 (0)