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

master courses: add restriction icons to files and folders 

should not be able to delete a folder if it contains a
 locked file (even nested within subfolders)

also should not get a prompt to overwrite a locked
file when uploading one with the same name

closes #MC-67

Change-Id: I7cc731377e35d27bb4e97550db8c1deb0b527cac
Reviewed-on: https://gerrit.instructure.com/100494
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Product-Review: James Williams  <jamesw@instructure.com>
QA-Review: James Williams  <jamesw@instructure.com>
This commit is contained in:
James Williams 2017-01-25 14:10:46 -07:00
parent 1511fa6c6a
commit 435f4684e9
17 changed files with 274 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
app/coffeescripts/react_files/modules/FileOptionsCollection.coffee
View File

@ -33,7 +33,7 @@ define [
toFilesOptionArray: (fList) ->
[].slice.call(fList, 0).map((file) -> {file})
fileNameExists: (name) ->
findMatchingFile: (name) ->
_.find @folder.files.models, (f) -> f.get('display_name') is name
isZipFile: (file) ->
@ -43,14 +43,18 @@ define [
segregateOptionBuckets: (selectedFiles) ->
[collisions, resolved, zips] = [[], [], []]
for file in selectedFiles
nameToTest = file.name || file.file.name
if (@isZipFile(file.file) and typeof file.expandZip is 'undefined')
zips.push file
# only mark as collision if it is a collision that hasn't been resolved, or is is a zip that will be expanded
else if @fileNameExists(nameToTest) && (file.dup != 'overwrite' && (!file.expandZip? || file.expandZip is false))
collisions.push file
else
resolved.push file
nameToTest = file.name || file.file.name
matchingFile = @findMatchingFile(nameToTest)
if matchingFile && (file.dup != 'overwrite' && (!file.expandZip? || file.expandZip is false))
if matchingFile.get('restricted_by_master_course')
file.cannotOverwrite = true
collisions.push file
else
resolved.push file
{collisions, resolved, zips}

1
app/controllers/files_controller.rb
View File

@ -1081,6 +1081,7 @@ class FilesController < ApplicationController
def destroy
@attachment = Attachment.find(params[:id])
if can_do(@attachment, @current_user, :delete)
return render_unauthorized_action if master_courses? && editing_restricted?(@attachment)
@attachment.destroy
respond_to do |format|
format.html {

12
app/controllers/folders_controller.rb
View File

@ -135,6 +135,12 @@ class FoldersController < ApplicationController
folder = Folder.find(params[:id])
if authorized_action(folder, @current_user, :read_contents)
can_view_hidden_files = can_view_hidden_files?(folder.context, @current_user, session)
opts = {:can_view_hidden_files => can_view_hidden_files}
if can_view_hidden_files && folder.context.is_a?(Course) &&
master_courses? && MasterCourses::ChildSubscription.is_child_course?(folder.context)
opts[:master_course_restricted_folder_ids] = MasterCourses::FolderLockingHelper.locked_folder_ids_for_course(folder.context)
end
scope = folder.active_sub_folders
unless can_view_hidden_files
@ -146,7 +152,7 @@ class FoldersController < ApplicationController
scope = scope.by_name
end
@folders = Api.paginate(scope, self, api_v1_list_folders_url(folder))
render :json => folders_json(@folders, @current_user, session, :can_view_hidden_files => can_view_hidden_files)
render :json => folders_json(@folders, @current_user, session, opts)
end
end
@ -538,6 +544,10 @@ class FoldersController < ApplicationController
if authorized_action(@folder, @current_user, :delete)
if @folder.root_folder?
render :json => {:message => t('no_deleting_root', "Can't delete the root folder")}, :status => 400
elsif @folder.context.is_a?(Course) && master_courses? &&
MasterCourses::ChildSubscription.is_child_course?(@folder.context) &&
MasterCourses::FolderLockingHelper.locked_folder_ids_for_course(@folder.context).include?(@folder.id)
render :json => {:message => "Can't delete folder containing files locked by Blueprint Course"}, :status => 400
elsif @folder.has_contents? && params[:force] != 'true'
render :json => {:message => t('no_deleting_folders_with_content', "Can't delete a folder with content")}, :status => 400
else

30
app/jsx/files/FileRenameForm.jsx
View File

@ -11,7 +11,7 @@ define([
FileRenameForm.buildContent = function () {
var nameToUse = this.state.fileOptions.name || this.state.fileOptions.file.name;
var buildContentToRender;
if (!this.state.isEditing) {
if (!this.state.isEditing && !this.state.fileOptions.cannotOverwrite) {
buildContentToRender = (
<div ref='bodyContent'>
<p id='renameFileMessage'>
@ -20,10 +20,17 @@ define([
</div>
);
} else {
var renameMessage;
if (this.state.fileOptions.cannotOverwrite) {
renameMessage = I18n.t('A locked item named "%{name}" already exists in this location. Please enter a new name.', {name: nameToUse});
} else {
renameMessage = I18n.t('Change "%{name}" to', {name: nameToUse});
}
buildContentToRender = (
<div ref='bodyContent'>
<p>
{I18n.t('Change "%{name}" to', {name: nameToUse})}
{renameMessage}
</p>
<form onSubmit={this.handleFormSubmit}>
<label className='file-rename-form__form-label'>
@ -46,7 +53,20 @@ define([
FileRenameForm.buildButtons = function () {
var buildButtonsToRender;
if (!this.state.isEditing) {
if (this.state.fileOptions.cannotOverwrite) {
buildButtonsToRender = (
[
<button
key='commitChangeBtn'
ref='commitChangeBtn'
className='btn btn-primary'
onClick={this.handleChangeClick}
>
{I18n.t('Change')}
</button>
]
);
} else if (!this.state.isEditing) {
buildButtonsToRender = (
[
<button
@ -108,7 +128,7 @@ define([
style={{
overlay : {
backgroundColor: 'rgba(0,0,0,0.5)'
},
},
content : {
position: 'static',
top: '0',
@ -134,4 +154,4 @@ define([
return React.createClass(FileRenameForm);
});
});

18
app/jsx/files/FolderChild.jsx
View File

@ -43,6 +43,23 @@ define([
);
}
}
FolderChild.renderMasterCourseIcon = function (canManage) {
if (canManage && this.props.model.get('is_master_course_content')) {
if (this.props.model.get('restricted_by_master_course')) {
return (
<span className="master-course-cell">
<i className="icon-lock"></i>
</span>
);
} else {
return (
<span className="master-course-cell">
<i className="icon-unlock icon-Line"></i>
</span>
);
}
}
}
FolderChild.renderEditingState = function () {
if(this.state.editing) {
@ -188,6 +205,7 @@ define([
{ this.renderUsageRightsIndicator() }
<div className= 'ef-links-col' role= 'gridcell'>
{ this.renderMasterCourseIcon(canManage) }
{ this.renderPublishCloud(canManage && this.props.userCanRestrictFilesForContext) }
{ this.renderItemCog(canManage) }
</div>

2
app/jsx/files/ItemCog.jsx
View File

@ -115,7 +115,7 @@ define([
menuItems.push(<li key='download' role='presentation'><a onClick={wrap(this.downloadFile)} href={this.props.model.get('url')} ref='download' role='menuitem' tabIndex='-1'>{I18n.t('Download')}</a></li>);
}
if (this.props.userCanManageFilesForContext) {
if (this.props.userCanManageFilesForContext && !this.props.model.get('restricted_by_master_course')) {
// Rename Link
menuItems.push(<li key='rename' role='presentation'><a href='#' onClick={preventDefault(this.props.startEditingName)} ref='editName' role='menuitem' tabIndex='-1'>{I18n.t('Rename')}</a></li>);
// Move Link

5
app/jsx/files/Toolbar.jsx
View File

@ -185,7 +185,10 @@ define([
submissionsFolderSelected = submissionsFolderSelected || this.props.selectedItems.some(function(item) {
return item.get('for_submissions');
});
var canManage = this.props.userCanManageFilesForContext && !submissionsFolderSelected;
var restrictedByMasterCourse = this.props.selectedItems.some(function(item) {
return item.get('restricted_by_master_course');
});
var canManage = this.props.userCanManageFilesForContext && !submissionsFolderSelected && !restrictedByMasterCourse;
this.showingButtons = this.props.selectedItems.length

1
app/models/content_migration.rb
View File

@ -495,6 +495,7 @@ class ContentMigration < ActiveRecord::Base
# copy the attachments
source_export = ContentExport.find(self.migration_settings[:master_course_export_id])
self.context.copy_attachments_from_course(source_export.context, :content_export => source_export, :content_migration => self)
MasterCourses::FolderLockingHelper.recalculate_locked_folders(self.context)
data = JSON.parse(self.exported_attachment.open, :max_nesting => 50)
data = prepare_data(data)

45
app/models/master_courses/folder_locking_helper.rb Normal file
View File

@ -0,0 +1,45 @@
class MasterCourses::FolderLockingHelper
# couldn't think of an ideal place to put this
def self.cache_key(child_course)
["locked_folder_ids_for_master_courses", Shard.global_id_for(child_course)].cache_key
end
def self.recalculate_locked_folders(child_course)
Rails.cache.delete(cache_key(child_course))
locked_folder_ids_for_course(child_course) # preload the cache
end
def self.locked_folder_ids_for_course(child_course)
child_course.shard.activate do
Rails.cache.fetch(cache_key(child_course)) do
folder_id_restriction_pairs = child_course.attachments.not_deleted.
where("#{Attachment.table_name}.migration_id IS NOT NULL AND
#{Attachment.table_name}.migration_id LIKE ?", "#{MasterCourses::MIGRATION_ID_PREFIX}%").
joins("INNER JOIN #{MasterCourses::MasterContentTag.quoted_table_name} ON
#{Attachment.table_name}.migration_id=#{MasterCourses::MasterContentTag.table_name}.migration_id").
distinct.pluck(:folder_id, :restrictions)
locked_folder_ids = Set.new
folder_id_restriction_pairs.each do |folder_id, restrictions|
locked_folder_ids << folder_id if restrictions.present? # treat folder as locked if any part is locked
end
if locked_folder_ids.any?
# now find all parents for locked folders
all_ids = Folder.connection.select_values(<<-SQL)
WITH RECURSIVE t AS (
SELECT id, parent_folder_id FROM #{Folder.quoted_table_name} WHERE id IN (#{locked_folder_ids.to_a.sort.join(",")})
UNION
SELECT folders.id, folders.parent_folder_id FROM #{Folder.quoted_table_name} INNER JOIN t ON folders.id=t.parent_folder_id
)
SELECT DISTINCT id FROM t
SQL
all_ids.map(&:to_i)
else
[]
end
end
end
end
end

2
app/models/master_courses/master_template.rb
View File

@ -111,8 +111,6 @@ class MasterCourses::MasterTemplate < ActiveRecord::Base
end
def ensure_attachment_tags_on_export
return unless self.default_restrictions.present?
# because attachments don't get "added" to the export
self.course.attachments.where("file_state <> 'deleted'").each do |att|
ensure_tag_on_export(att)

5
app/stylesheets/pages/react_files/_react_files.scss
View File

@ -412,3 +412,8 @@ body:not(.with-left-side) & .ic-app-nav-toggle-and-crumbs--files {
height: auto;
}
}
.master-course-cell {
margin-top: 6px;
margin-right: 6px;
}

8
lib/api/v1/attachment.rb
View File

@ -27,6 +27,10 @@ module Api::V1::Attachment
end
def attachments_json(files, user, url_options = {}, options = {})
if options[:can_view_hidden_files] && master_courses?
options[:include_master_course_restrictions] = true
MasterCourses::Restrictor.preload_restrictions(files)
end
files.map do |f|
attachment_json(f, user, url_options, options)
end
@ -118,6 +122,10 @@ module Api::V1::Attachment
hash['context_asset_string'] = attachment.context.try(:asset_string)
end
if options[:include_master_course_restrictions]
hash.merge!(attachment.master_course_api_restriction_data)
end
hash
end

4
lib/api/v1/folders.rb
View File

@ -30,6 +30,10 @@ module Api::V1::Folders
json = api_json(folder, user, session,
:only => %w(id name full_name position parent_folder_id context_type context_id unlock_at lock_at created_at updated_at))
if folder
if opts[:master_course_restricted_folder_ids]&.include?(folder.id)
json['is_master_course_content'] = true
json['restricted_by_master_course'] = true
end
json['locked'] = !!folder.locked
json['folders_url'] = api_v1_list_folders_url(folder)
json['files_url'] = api_v1_list_files_url(folder)

8
spec/coffeescripts/react_files/modules/FileOptionsCollectionSpec.coffee
View File

@ -33,13 +33,13 @@ define [
teardown: ->
FileOptionsCollection.resetState()
test 'fileNameExists correctly finds existing files by display_name', ->
test 'findMatchingFile correctly finds existing files by display_name', ->
setupFolderWith(['foo', 'bar', 'baz'])
ok FileOptionsCollection.fileNameExists('foo')
ok FileOptionsCollection.findMatchingFile('foo')
test 'fileNameExists returns falsy value when no matching file exists', ->
test 'findMatchingFile returns falsy value when no matching file exists', ->
setupFolderWith(['foo', 'bar', 'baz'])
equal FileOptionsCollection.fileNameExists('xyz')?, false
equal FileOptionsCollection.findMatchingFile('xyz')?, false
test 'segregateOptionBuckets divides files into collsion and resolved buckets', ->
setupFolderWith(['foo', 'bar', 'baz'])

32
spec/models/master_courses/folder_locking_helper_spec.rb Normal file
View File

@ -0,0 +1,32 @@
require 'spec_helper'
describe MasterCourses::FolderLockingHelper do
it "should be able to fetch a list of folder ids with restricted files (even recursively via sub-folders)" do
@copy_from = course_factory
@template = MasterCourses::MasterTemplate.set_as_master_course(@copy_from)
@copy_to = course_factory
# master course
master_root = Folder.root_folders(@copy_from).first
locked_master_att = attachment_model(context: @copy_from, folder: master_root, filename: 'lockedfile.txt')
locked_master_tag = @template.create_content_tag_for!(locked_master_att, :restrictions => {:content => true})
unlocked_master_att = attachment_model(context: @copy_from, folder: master_root, filename: 'unlockedfile.txt')
unlocked_master_tag = @template.create_content_tag_for!(unlocked_master_att)
# child course
child_root = Folder.root_folders(@copy_to).first
locked_parent_folder = child_root.sub_folders.create!(:name => "locked parent", :context => @copy_to)
locked_child_folder = locked_parent_folder.sub_folders.create!(:name => "locked child", :context => @copy_to)
unlocked_parent_folder = child_root.sub_folders.create!(:name => "unlocked parent", :context => @copy_to)
unlocked_child_folder = unlocked_parent_folder.sub_folders.create!(:name => "unlocked child", :context => @copy_to)
locked_att = attachment_model(context: @copy_to, folder: locked_child_folder,
filename: 'lockedfile.txt', migration_id: locked_master_tag.migration_id)
unlocked_att = attachment_model(context: @copy_to, folder: unlocked_child_folder,
filename: 'unlockedfile.txt', migration_id: unlocked_master_tag.migration_id)
expect(MasterCourses::FolderLockingHelper.locked_folder_ids_for_course(@copy_to)).
to match_array([child_root, locked_parent_folder, locked_child_folder].map(&:id))
end
end

23
spec/models/master_courses/master_migration_spec.rb
View File

@ -554,6 +554,29 @@ describe MasterCourses::MasterMigration do
expect(@copy_to.reload.is_public).to_not be_truthy
end
it "should trigger folder locking data cache invalidation" do
@copy_to = course_factory
@sub = @template.add_child_course!(@copy_to)
enable_cache do
expect(MasterCourses::FolderLockingHelper.locked_folder_ids_for_course(@copy_to)).to be_empty
master_parent_folder = Folder.root_folders(@copy_from).first.sub_folders.create!(:name => "parent", :context => @copy_from)
master_sub_folder = master_parent_folder.sub_folders.create!(:name => "child", :context => @copy_from)
att = Attachment.create!(:filename => 'file.txt', :uploaded_data => StringIO.new('1'), :folder => master_sub_folder, :context => @copy_from)
att_tag = @template.create_content_tag_for!(att, :restrictions => {:content => true, :settings => true})
run_master_migration
copied_att = @copy_to.attachments.where(:migration_id => att_tag.migration_id).first
child_sub_folder = copied_att.folder
child_parent_folder = child_sub_folder.parent_folder
expected_ids = [child_sub_folder, child_parent_folder, Folder.root_folders(@copy_to).first].map(&:id)
Folder.connection.expects(:select_values).never # should have already been cached in migration
expect(MasterCourses::FolderLockingHelper.locked_folder_ids_for_course(@copy_to)).to match_array(expected_ids)
end
end
context "master courses + external migrations" do
class TestExternalContentService
cattr_reader :course, :imported_content

83
spec/selenium/master_courses/child_course_files_spec.rb Normal file
View File

@ -0,0 +1,83 @@
require_relative '../common'
describe "master courses - child courses - file locking" do
include_context "in-process server selenium tests"
before :once do
Account.default.enable_feature!(:master_courses)
@copy_from = course_factory(:active_all => true)
@template = MasterCourses::MasterTemplate.set_as_master_course(@copy_from)
@filename = 'file.txt'
@original_file = Attachment.create!(:filename => @filename, :uploaded_data => StringIO.new('1'),
:folder => Folder.root_folders(@copy_from).first, :context => @copy_from)
@tag = @template.create_content_tag_for!(@original_file)
course_with_teacher(:active_all => true)
@copy_to = @course
@template.add_child_course!(@copy_to)
@file_copy = Attachment.create!(:filename => @filename, :uploaded_data => StringIO.new('1'),
:folder => Folder.root_folders(@copy_to).first, :context => @copy_to, :migration_id => @tag.migration_id)
end
before :each do
user_session(@teacher)
end
it "should not show the manageable cog-menu options when a file is locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
get "/courses/#{@copy_to.id}/files"
expect(f('.ef-item-row .master-course-cell')).to contain_css('.icon-lock')
f('.ef-item-row .ef-date-created-col').click # select the file
expect(f('.ef-header')).to_not contain_css('.btn-delete')
f('.ef-item-row .al-trigger').click
expect(f('.al-options').text).to_not include("Delete")
end
it "should not show the manageable cog-menu options when a folder contains a locked file" do
subfolder = Folder.root_folders(@copy_to).first.sub_folders.create!(:name => "subfolder", :context => @copy_to)
@file_copy.folder = subfolder
@file_copy.save!
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
get "/courses/#{@copy_to.id}/files"
expect(f('.ef-item-row .ef-name-col').text).to include("subfolder") # we're looking at the folder right?
expect(f('.ef-item-row .master-course-cell')).to contain_css('.icon-lock')
f('.ef-item-row .ef-date-created-col').click # select the file
expect(f('.ef-header')).to_not contain_css('.btn-delete')
f('.ef-item-row .al-trigger').click
expect(f('.al-options').text).to_not include("Delete")
end
it "should show the manageable cog-menu options when a file is unlocked" do
get "/courses/#{@copy_to.id}/files"
expect(f('.ef-item-row .master-course-cell')).to contain_css('.icon-unlock')
f('.ef-item-row .ef-date-created-col').click # select the file
expect(f('.ef-header')).to contain_css('.btn-delete')
f('.ef-item-row .al-trigger').click
expect(f('.al-options').text).to include("Delete")
end
it "should show the manageable cog-menu options when a folder contains an unlocked file" do
subfolder = Folder.root_folders(@copy_to).first.sub_folders.create!(:name => "subfolder", :context => @copy_to)
@file_copy.folder = subfolder
@file_copy.save!
get "/courses/#{@copy_to.id}/files"
f('.ef-item-row .ef-date-created-col').click # select the file
expect(f('.ef-header')).to contain_css('.btn-delete')
f('.ef-item-row .al-trigger').click
expect(f('.al-options').text).to include("Delete")
end
end