ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

module progressions for files not working 

If a teacher said students must view a file before
continuing, then the system needs to be sure to
mark the file as viewed whether it's actually downloaded
or just viewed inline.  This wasn't happening
correctly.

Also DRYed up a few methods in the files controller.

fixes #4035

Change-Id: I5ed6fa6e556ed78d85319f99fe84ddee0b4d2033
Reviewed-on: https://gerrit.instructure.com/2640
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>
This commit is contained in:
Brian Whitmer 2011-03-14 13:51:44 -06:00 committed by Zach Wily
parent 39954fd3ac
commit d91eebfca4
3 changed files with 131 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

125
app/controllers/files_controller.rb
View File

@ -180,53 +180,61 @@ class FilesController < ApplicationController
end
return
end
if (params[:download] && params[:verifier] && params[:verifier] == @attachment.uuid) || authorized_action(@attachment, @current_user, :read)
if params[:download]
if (params[:verifier] && params[:verifier] == @attachment.uuid) || (@attachment.grants_right?(@current_user, session, :download))
disable_page_views if params[:preview]
@attachment.context_module_action(@current_user, :read) unless params[:preview]
log_asset_access(@attachment, "files", "files") unless params[:preview]
begin
send_s3_file(@attachment)
send_attachment(@attachment)
rescue => e
@headers = false if params[:ts] && params[:verifier]
@not_found_message = "It looks like something went wrong when this file was uploaded, and we can't find the actual file. You may want to notify the owner of the file and have them re-upload it."
logger.error "Error downloading a file: #{e} - #{e.backtrace}"
render :template => 'shared/errors/404_message', :status => :bad_request
end
return
elsif authorized_action(@attachment, @current_user, :read)
respond_to do |format|
if params[:preview] && @attachment.mime_class == 'image'
format.html { redirect_to '/images/lock.png' }
else
format.html { render :action => 'show' }
end
@attachment.scribd_doc = nil unless @attachment.grants_right?(@current_user, session, :download)
format.json { render :json => @attachment.to_json(:permissions => {:user => @current_user}) }
end
render_attachment(@attachment)
end
# This action is a callback used in our system to help record when
# a user views an inline preview of a file instead of downloading
# it, since this should also count as an access.
elsif params[:inline]
generate_new_page_view
@attachment.context_module_action(@current_user, :read) if @current_user
log_asset_access(@attachment, 'files', 'files')
render :json => {:ok => true}.to_json
else
respond_to do |format|
if params[:preview] && @attachment.mime_class == 'image'
format.html { redirect_to '/images/lock.png' }
else
if @files_domain
@headers = false
@show_left_side = false
end
format.html # show.rhtml
end
@attachment.scribd_doc = nil unless @attachment.grants_right?(@current_user, session, :download)
format.json { render :json => @attachment.to_json(:permissions => {:user => @current_user}) }
end
render_attachment(@attachment)
end
end
end
def render_attachment(attachment)
respond_to do |format|
if params[:preview] && attachment.mime_class == 'image'
format.html { redirect_to '/images/lock.png' }
else
if @files_domain
@headers = false
@show_left_side = false
end
format.html { render :action => 'show' }
end
if request.format == :json
attachment.context_module_action(@current_user, :read) if @current_user && attachment.scribd_doc
log_asset_access(@attachment, "files", "files")
end
format.json {
attachment.scribd_doc = nil unless attachment.grants_right?(@current_user, session, :download)
# Right now we assume if they ask for json data on the attachment
# which includes the scribd doc data, then that means they have
# viewed or are about to view the file in some form.
render :json => attachment.to_json(:permissions => {:user => @current_user})
}
end
end
protected :render_attachment
def show_relative
path = params[:file_path]
@ -289,55 +297,44 @@ class FilesController < ApplicationController
end
end
def send_s3_file(attachment)
def send_attachment(attachment)
if params[:inline] && attachment.content_type && (attachment.content_type.match(/\Atext/) || attachment.mime_class == 'text' || attachment.mime_class == 'html' || attachment.mime_class == 'code')
if safer_domain_available?
redirect_to safe_domain_file_url(attachment, @safer_domain_host)
elsif Attachment.local_storage?
@headers = false if @files_domain
send_file(attachment.full_filename, :type => attachment.content_type, :disposition => 'inline')
else
require 'aws/s3'
send_file_headers!( :length=>AWS::S3::S3Object.about(attachment.full_filename, attachment.bucket_name)["content-length"], :filename=>attachment.filename, :disposition => 'inline', :type => attachment.content_type)
render :status => 200, :text => Proc.new { |response, output|
AWS::S3::S3Object.stream(attachment.full_filename, attachment.bucket_name) do |chunk|
output.write chunk
end
}
end
send_stored_file(attachment)
elsif attachment.inline_content? && !@context.is_a?(AssessmentQuestion)
if params[:file_path] || !params[:wrap]
if safer_domain_available?
redirect_to safe_domain_file_url(attachment, @safer_domain_host)
elsif Attachment.local_storage?
@headers = false if @files_domain
send_file(attachment.full_filename, :type => attachment.content_type, :disposition => 'inline')
else
require 'aws/s3'
send_file_headers!( :length=>AWS::S3::S3Object.about(attachment.full_filename, attachment.bucket_name)["content-length"], :filename=>attachment.filename, :disposition => 'inline', :type => attachment.content_type)
render :status => 200, :text => Proc.new { |response, output|
AWS::S3::S3Object.stream(attachment.full_filename, attachment.bucket_name) do |chunk|
output.write chunk
end
}
end
send_stored_file(attachment)
else
# If the file is inlineable then redirect to the 'show' action
# so we can wrap it in all the Canvas header/footer stuff
redirect_to(named_context_url(@context, :context_file_url, attachment.id))
end
else
if safer_domain_available?
redirect_to safe_domain_file_url(attachment, @safer_domain_host)
elsif Attachment.local_storage?
@headers = false if @files_domain
send_file(attachment.full_filename, :type => attachment.content_type, :disposition => 'attachment')
else
redirect_to attachment.cacheable_s3_url
end
send_stored_file(attachment, false, true)
end
end
protected :send_s3_file
protected :send_attachment
def send_stored_file(attachment, inline=true, redirect_to_s3=false)
attachment.context_module_action(@current_user, :read) if @current_user && !params[:preview]
log_asset_access(@attachment, "files", "files") unless params[:preview]
if safer_domain_available?
redirect_to safe_domain_file_url(attachment, @safer_domain_host)
elsif Attachment.local_storage?
@headers = false if @files_domain
send_file(attachment.full_filename, :type => attachment.content_type, :disposition => (inline ? 'inline' : 'attachment'))
elsif redirect_to_s3
redirect_to attachment.cacheable_s3_url
else
require 'aws/s3'
send_file_headers!( :length=>AWS::S3::S3Object.about(attachment.full_filename, attachment.bucket_name)["content-length"], :filename=>attachment.filename, :disposition => 'inline', :type => attachment.content_type)
render :status => 200, :text => Proc.new { |response, output|
AWS::S3::S3Object.stream(attachment.full_filename, attachment.bucket_name) do |chunk|
output.write chunk
end
}
end
end
protected :send_stored_file
# GET /files/new
def new

8
app/views/files/show.html.erb
View File

@ -1,5 +1,6 @@
<% content_for :page_title do %><%= @attachment.display_name %>: <%= @context.name %><% end %>
<% add_crumb @attachment.display_name, context_url(@context, :context_file_url, @attachment) %>
<% download_url = context_url(@context, :context_file_download_url, @attachment.id) %>
<% if !can_do(@context, @current_user, :manage_files) && (locked = @attachment.locked_for?(@current_user)) %>
<div style="margin: 10px 50px;">
<% if @attachment.folder && @attachment.folder.locked? %>
@ -29,18 +30,20 @@
<iframe id="file_content" src="<%= safe_domain_file_url(@attachment) %>" style="width: 100%; height: 400px;"></iframe>
<% elsif @attachment.content_type && @attachment.content_type.match(/\Aimage\//) %>
<h2><%= @attachment.display_name %></h2>
<%= link_to(image_tag(@attachment.cacheable_s3_url, :alt => @attachment.display_name), @attachment.cached_s3_url) %>
<%= link_to(image_tag(download_url, :alt => @attachment.display_name), download_url) %>
<% else %>
<h2><%= @attachment.display_name %></h2>
<span style="font-size: 1.2em;">
<%= link_to "Download #{@attachment.filename}", @attachment.cacheable_s3_url %>
<%= link_to "Download #{@attachment.filename}", download_url %>
</span> (<%= @attachment.readable_size %>)
<% if @attachment.scribdable? && @attachment.scribd_doc %>
<div id="scribd_preview" data-doc-id="<%= @attachment.scribd_doc.doc_id %>" data-access-key="<%= @attachment.scribd_doc.access_key %>"></div>
<a href="<%= context_url(@context, :context_file_inline_view_url, @attachment.id) %>" style="display: none;" class="inline_view_attachment_url">&nbsp;</a>
<% js_block do %>
<script>
$(function() {
$('#scribd_preview').ifExists(function($preview){
var url = $(".inline_view_attachment_url").attr('href');
var sd = scribd.Document.getDoc( $preview.data('doc-id'), $preview.data('access-key') );
$.each({
'jsapi_version': 1,
@ -50,6 +53,7 @@
sd.addParam(key, value);
});
sd.write('scribd_preview');
$.ajaxJSON(url, 'POST', {}, function() { }, function() { });
});
});
</script>

64
spec/controllers/files_controller_spec.rb
View File

@ -38,6 +38,19 @@ describe FilesController do
@file
end
def file_in_a_module
course_with_student_logged_in(:active_all => true)
@file = factory_with_protected_attributes(@course.attachments, :uploaded_data => io)
@module = @course.context_modules.create!(:name => "module")
@tag = @module.add_item({:type => 'attachment', :id => @file.id})
@module.reload
hash = {}
hash[@tag.id.to_s] = {:type => 'must_view'}
@module.completion_requirements = hash
@module.save!
@module.evaluate_for(@user, true, true).state.should eql(:unlocked)
end
describe "GET 'quota'" do
it "should require authorization" do
course_with_teacher(:active_all => true)
@ -154,6 +167,57 @@ describe FilesController do
e.to_s.should eql("Not Found")
end
end
it "should mark files as viewed for module progressions if the file is downloaded" do
file_in_a_module
get 'show', :course_id => @course.id, :id => @file.id, :download => 1
@module.reload
@module.evaluate_for(@user, true, true).state.should eql(:completed)
end
it "should not mark a file as viewed for module progressions if the file is locked" do
file_in_a_module
@file.locked = true
@file.save!
get 'show', :course_id => @course.id, :id => @file.id, :download => 1
@module.reload
@module.evaluate_for(@user, true, true).state.should eql(:unlocked)
end
it "should not mark a file as viewed for module progressions just because the files#show view is rendered" do
file_in_a_module
@file.locked = true
@file.save!
get 'show', :course_id => @course.id, :id => @file.id
@module.reload
@module.evaluate_for(@user, true, true).state.should eql(:unlocked)
end
it "should mark files as viewed for module progressions if the file is previewed inline" do
file_in_a_module
get 'show', :course_id => @course.id, :id => @file.id, :inline => 1
response.body.should eql({:ok => true}.to_json)
@module.reload
@module.evaluate_for(@user, true, true).state.should eql(:completed)
end
it "should mark files as viewed for module progressions if the file data is requested and it includes the scribd_doc data" do
file_in_a_module
@file.scribd_doc = Scribd::Document.new
@file.save!
get 'show', :course_id => @course.id, :id => @file.id, :format => :json
@module.reload
@module.evaluate_for(@user, true, true).state.should eql(:completed)
end
it "should not mark files as viewed for module progressions if the file data is requested and it doesn't include the scribd_doc data" do
file_in_a_module
@file.scribd_doc = nil
@file.save!
get 'show', :course_id => @course.id, :id => @file.id, :format => :json
@module.reload
@module.evaluate_for(@user, true, true).state.should eql(:unlocked)
end
end
describe "GET 'new'" do