allow more fine-grained locking for blueprint assignments

extract :points, :due_dates, and :availability_dates
from :settings

also allow {:all => true} shorthand in restrictions

test plan: specs (fine-grained locking is not supported in the
user-interface yet)

closes MC-99

Change-Id: I885192e6828b6628e73fcf4a532dda19143dd20a
Reviewed-on: https://gerrit.instructure.com/104163
Tested-by: Jenkins
Reviewed-by: James Williams  <jamesw@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
Jeremy Stanley 2017-03-06 15:42:44 -07:00
parent c9fb9a4036
commit fe535f92c0
17 changed files with 60 additions and 35 deletions

View File

@ -45,14 +45,7 @@ class Assignment < ActiveRecord::Base
include MasterCourses::Restrictor
restrict_columns :content, [:title, :description]
RESTRICTED_SETTINGS = [:points_possible, :assignment_group_id,
:grading_type, :omit_from_final_grade, :submission_types,
:group_category, :group_category_id,
:grade_group_students_individually, :peer_reviews,
:moderated_grading,
:due_at, :lock_at, :unlock_at, :peer_reviews_due_at].freeze
restrict_columns :settings, RESTRICTED_SETTINGS
restrict_assignment_columns
has_many :submissions, :dependent => :destroy
has_many :provisional_grades, :through => :submissions

View File

@ -37,7 +37,7 @@ class DiscussionTopic < ActiveRecord::Base
restrict_columns :content, [:title, :message]
restrict_columns :settings, [:delayed_post_at, :require_initial_post, :discussion_type,
:lock_at, :pinned, :locked, :allow_rating, :only_graders_can_rate, :sort_by_rating]
restrict_columns :settings, Assignment::RESTRICTED_SETTINGS
restrict_assignment_columns
attr_accessor :user_has_posted, :saved_by

View File

@ -11,5 +11,5 @@ module MasterCourses
MIGRATION_ID_PREFIX = "mastercourse_".freeze
LOCK_TYPES = [:content, :settings].freeze
LOCK_TYPES = [:content, :settings, :points, :due_dates, :availability_dates].freeze
end

View File

@ -19,7 +19,7 @@ class MasterCourses::MasterContentTag < ActiveRecord::Base
def require_valid_restrictions
# this may be changed in the future
if self.restrictions_changed? && (self.restrictions.keys - MasterCourses::LOCK_TYPES).any?
if self.restrictions_changed? && (self.restrictions.keys != [:all]) && (self.restrictions.keys - MasterCourses::LOCK_TYPES).any?
self.errors.add(:restrictions, "Invalid settings")
end
end

View File

@ -28,6 +28,15 @@ module MasterCourses::Restrictor
current = self.restricted_column_settings[edit_type] || []
self.restricted_column_settings[edit_type] = (current + columns).uniq
end
def restrict_assignment_columns
restrict_columns :settings, [:assignment_group_id, :grading_type, :omit_from_final_grade, :submission_types,
:group_category, :group_category_id, :grade_group_students_individually,
:peer_reviews, :moderated_grading, :peer_reviews_due_at]
restrict_columns :due_dates, [:due_at]
restrict_columns :availability_dates, [:lock_at, :unlock_at]
restrict_columns :points, [:points_possible]
end
end
def mark_as_importing!(cm)
@ -51,11 +60,12 @@ module MasterCourses::Restrictor
end
end
def editing_restricted?(edit_type=:all) # edit_type can be :all, :any, or a specific type: :content, :settings
def editing_restricted?(edit_type=:all) # edit_type can be :all, :any, or a specific type: :content, :settings, :due_dates, :availability_dates, :points
return false unless is_child_content?
restrictions = self.master_course_restrictions
return false unless restrictions.present?
return true if restrictions[:all]
case edit_type
when :all

View File

@ -92,7 +92,7 @@ class Quizzes::Quiz < ActiveRecord::Base
:ip_filter, :require_lockdown_browser, :require_lockdown_browser_for_results,
:lock_at, :unlock_at
]
restrict_columns :settings, Assignment::RESTRICTED_SETTINGS
restrict_assignment_columns
# override has_one relationship provided by simply_versioned
def current_version_unidirectional

View File

@ -37,7 +37,7 @@ class WikiPage < ActiveRecord::Base
include MasterCourses::Restrictor
restrict_columns :content, [:body, :title]
restrict_columns :settings, [:editing_roles]
restrict_columns :settings, Assignment::RESTRICTED_SETTINGS
restrict_assignment_columns
after_update :post_to_pandapub_when_revised

View File

@ -52,7 +52,7 @@ describe MasterCourses::MasterContentTag do
topic = @copy_from.discussion_topics.create!
topic_master_tag = @template.create_content_tag_for!(topic)
assmt = @copy_from.assignments.create!
restrictions = {:content => true, :settings => true}
restrictions = {:all => true}
assmt_master_tag = @template.create_content_tag_for!(assmt, {:restrictions => restrictions})
@copy_to = course_factory

View File

@ -535,7 +535,7 @@ describe MasterCourses::MasterMigration do
[quiz, topic].each do |c|
mtag = @template.content_tag_for(c)
Timecop.freeze(2.seconds.from_now) do
mtag.update_attribute(:restrictions, {:settings => true}) # lock the quiz/topic master tags
mtag.update_attribute(:restrictions, {:due_dates => true}) # lock the quiz/topic master tags
end
end
@ -566,7 +566,7 @@ describe MasterCourses::MasterMigration do
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})
att_tag = @template.create_content_tag_for!(att, :restrictions => {:all => true})
run_master_migration

View File

@ -51,20 +51,42 @@ describe MasterCourses::Restrictor do
it "should return false by default" do
expect(@page_copy.editing_restricted?(:any)).to be_falsey
expect(@page_copy.editing_restricted?(:content)).to be_falsey
expect(@page_copy.editing_restricted?(:settings)).to be_falsey
expect(@page_copy.editing_restricted?(:availability_dates)).to be_falsey
expect(@page_copy.editing_restricted?(:due_dates)).to be_falsey
expect(@page_copy.editing_restricted?(:points)).to be_falsey
expect(@page_copy.editing_restricted?(:all)).to be_falsey
end
it "should return what you would expect" do
@tag.update_attribute(:restrictions, {:content => true})
expect(@page_copy.editing_restricted?(:content)).to be_truthy
expect(@page_copy.editing_restricted?(:settings)).to be_falsey
expect(@page_copy.editing_restricted?(:availability_dates)).to be_falsey
expect(@page_copy.editing_restricted?(:due_dates)).to be_falsey
expect(@page_copy.editing_restricted?(:points)).to be_falsey
expect(@page_copy.editing_restricted?(:any)).to be_truthy
expect(@page_copy.editing_restricted?(:all)).to be_falsey
end
it "should return true if fully locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
it "should return true if fully/individually locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true, :points => true, :availability_dates => true, :due_dates => true})
expect(@page_copy.editing_restricted?(:content)).to be_truthy
expect(@page_copy.editing_restricted?(:settings)).to be_truthy
expect(@page_copy.editing_restricted?(:points)).to be_truthy
expect(@page_copy.editing_restricted?(:availability_dates)).to be_truthy
expect(@page_copy.editing_restricted?(:due_dates)).to be_truthy
expect(@page_copy.editing_restricted?(:any)).to be_truthy
expect(@page_copy.editing_restricted?(:all)).to be_truthy
end
it "should return true if locked via :all" do
@tag.update_attribute(:restrictions, {:all => true})
expect(@page_copy.editing_restricted?(:content)).to be_truthy
expect(@page_copy.editing_restricted?(:settings)).to be_truthy
expect(@page_copy.editing_restricted?(:points)).to be_truthy
expect(@page_copy.editing_restricted?(:availability_dates)).to be_truthy
expect(@page_copy.editing_restricted?(:due_dates)).to be_truthy
expect(@page_copy.editing_restricted?(:any)).to be_truthy
expect(@page_copy.editing_restricted?(:all)).to be_truthy
end
@ -95,7 +117,7 @@ describe MasterCourses::Restrictor do
# should also work for associated assignments (since they share a master content tag)
quiz = @copy_from.quizzes.create!(:title => "quiz", :quiz_type => "assignment")
assignment = quiz.assignment
tag3 = @template.create_content_tag_for!(quiz, {:restrictions => {:content => true, :settings => true}})
tag3 = @template.create_content_tag_for!(quiz, {:restrictions => {:all => true}})
MasterCourses::Restrictor.preload_default_template_restrictions([@original_page, page2, assignment], @copy_from)

View File

@ -24,7 +24,7 @@ describe "master courses - child courses - assignment locking" do
end
it "should not show the cog-menu options on the index when locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
@tag.update_attribute(:restrictions, {:all => true})
get "/courses/#{@copy_to.id}/assignments"
@ -42,7 +42,7 @@ describe "master courses - child courses - assignment locking" do
end
it "should not show the edit/delete options on the show page when locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
@tag.update_attribute(:restrictions, {:all => true})
get "/courses/#{@copy_to.id}/assignments/#{@assmt_copy.id}"

View File

@ -24,7 +24,7 @@ describe "master courses - child courses - discussion locking" do
end
it "should not show the cog-menu options on the index when locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
@tag.update_attribute(:restrictions, {:all => true})
get "/courses/#{@copy_to.id}/discussion_topics"
@ -42,7 +42,7 @@ describe "master courses - child courses - discussion locking" do
end
it "should not show the edit/delete options on the show page when locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
@tag.update_attribute(:restrictions, {:all => true})
get "/courses/#{@copy_to.id}/discussion_topics/#{@topic_copy.id}"
@ -66,7 +66,7 @@ describe "master courses - child courses - discussion locking" do
end
it "should not show the cog-menu options on the index when locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
@tag.update_attribute(:restrictions, {:all => true})
get "/courses/#{@copy_to.id}/announcements"
@ -84,7 +84,7 @@ describe "master courses - child courses - discussion locking" do
end
it "should not show the edit/delete options on the show page when locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
@tag.update_attribute(:restrictions, {:all => true})
get "/courses/#{@copy_to.id}/discussion_topics/#{@topic_copy.id}"

View File

@ -26,7 +26,7 @@ describe "master courses - child courses - external tool locking" do
end
it "should not show the cog-menu options on the index when locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
@tag.update_attribute(:restrictions, {:all => true})
get "/courses/#{@copy_to.id}/settings#tab-tools"

View File

@ -25,7 +25,7 @@ describe "master courses - child courses - file locking" do
end
it "should not show the manageable cog-menu options when a file is locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
@tag.update_attribute(:restrictions, {:all => true})
get "/courses/#{@copy_to.id}/files"
@ -42,7 +42,7 @@ describe "master courses - child courses - file locking" 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})
@tag.update_attribute(:restrictions, {:all => true})
get "/courses/#{@copy_to.id}/files"

View File

@ -10,7 +10,7 @@ describe "master courses - child courses - module item locking" do
@copy_from = course_factory(:active_all => true)
@template = MasterCourses::MasterTemplate.set_as_master_course(@copy_from)
@original_page = @copy_from.wiki.wiki_pages.create!(:title => "blah", :body => "bloo")
@page_mc_tag = @template.create_content_tag_for!(@original_page, :restrictions => {:content => true, :settings => true})
@page_mc_tag = @template.create_content_tag_for!(@original_page, :restrictions => {:all => true})
@original_topic = @copy_from.discussion_topics.create!(:title => "blah", :message => "bloo")
@topic_mc_tag = @template.create_content_tag_for!(@original_topic)
@ -61,7 +61,7 @@ describe "master courses - child courses - module item locking" do
it "loads new restriction info as needed when adding an item" do
title = "new quiz"
original_quiz = @copy_from.quizzes.create!(:title => title)
quiz_mc_tag = @template.create_content_tag_for!(original_quiz, :restrictions => {:content => true, :settings => true})
quiz_mc_tag = @template.create_content_tag_for!(original_quiz, :restrictions => {:all => true})
quiz_copy = @copy_to.quizzes.create!(:title => title, :migration_id => quiz_mc_tag.migration_id)
@sub.create_content_tag_for!(quiz_copy)

View File

@ -24,7 +24,7 @@ describe "master courses - child courses - wiki page locking" do
end
it "should not show the edit/delete cog-menu options on the index when locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
@tag.update_attribute(:restrictions, {:all => true})
get "/courses/#{@copy_to.id}/pages"
@ -46,7 +46,7 @@ describe "master courses - child courses - wiki page locking" do
end
it "should not show the edit/delete options on the show page when locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
@tag.update_attribute(:restrictions, {:all => true})
get "/courses/#{@copy_to.id}/pages/#{@page_copy.url}"

View File

@ -24,7 +24,7 @@ describe "master courses - child courses - quiz locking" do
end
it "should not show the cog-menu options on the index when locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
@tag.update_attribute(:restrictions, {:all => true})
get "/courses/#{@copy_to.id}/quizzes"
@ -42,7 +42,7 @@ describe "master courses - child courses - quiz locking" do
end
it "should not show the edit/delete options on the show page when locked" do
@tag.update_attribute(:restrictions, {:content => true, :settings => true})
@tag.update_attribute(:restrictions, {:all => true})
get "/courses/#{@copy_to.id}/quizzes/#{@quiz_copy.id}"