assignment freezing improvements and fixes

Adds an option to not allow already copied assignments from
being copied again

Assignment groups that contain a frozen assignment can not
be deleted

When an assignment is copied that didn't have the flag set
to be frozen, the copied flag isn't set. This allows the
assignment to be locked in the new course, but still edited

Test Plan:
 * In the plugin settings for assignment freezer check the option to not allow assignments to be copied twice
 * Create an assignment and flag it to be frozen
 * Copy the course, verify it is frozen
 * Try to copy the course again as a non-admin - The frozen assignments should not be copied
 * As a teacher try to delete an assignment group for a frozen assignment. The trash can should not appear
 * Copy an assignment that is flagged to be frozen
 * In the new course, you should be able to set the flag and still edit the assignment

closes #8736 #8549 #8537

Change-Id: Ia3f1be4d5c1b6112507338df7e5a49dd76a5ed7f
Reviewed-on: https://gerrit.instructure.com/10995
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Zach Pendleton <zachp@instructure.com>
This commit is contained in:
Bracken Mosbacker 2012-05-23 15:45:22 -06:00
parent 55fe0ed12f
commit b34ced0bcb
14 changed files with 154 additions and 12 deletions

View File

@ -175,6 +175,15 @@ class AssignmentGroupsController < ApplicationController
def destroy
@assignment_group = AssignmentGroup.find(params[:id])
if authorized_action(@assignment_group, @current_user, :delete)
if @assignment_group.has_frozen_assignments?(@current_user)
@assignment_group.errors.add('workflow_state', t('errors.cannot_delete_group', "You can not delete a group with a locked assignment.", :att_name => 'workflow_state'))
respond_to do |format|
format.html { redirect_to named_context_url(@context, :context_assignments_url) }
format.json { render :json => @assignment_group.errors.to_json, :status => :bad_request }
end
return
end
if params[:move_assignments_to]
@new_group = @context.assignment_groups.active.find(params[:move_assignments_to])
order = @new_group.assignments.active.map(&:id)

View File

@ -1526,8 +1526,12 @@ class Assignment < ActiveRecord::Base
description += hash[:instructions_in_html] == false ? ImportedHtmlConverter.convert_text(hash[:instructions] || "", context) : ImportedHtmlConverter.convert(hash[:instructions] || "", context)
description += Attachment.attachment_list_from_migration(context, hash[:attachment_ids])
item.description = description
item.copied = true
item.copying = true
if hash[:freeze_on_copy]
item.freeze_on_copy = true
item.copied = true
item.copying = true
end
if !hash[:submission_types].blank?
item.submission_types = hash[:submission_types]
elsif ['discussion_topic'].include?(hash[:submission_format])
@ -1608,7 +1612,7 @@ class Assignment < ActiveRecord::Base
[:all_day, :turnitin_enabled, :peer_reviews_assigned, :peer_reviews,
:automatic_peer_reviews, :anonymous_peer_reviews,
:grade_group_students_individually, :allowed_extensions, :min_score,
:max_score, :mastery_score, :position, :peer_review_count, :freeze_on_copy
:max_score, :mastery_score, :position, :peer_review_count
].each do |prop|
item.send("#{prop}=", hash[prop]) unless hash[prop].nil?
end
@ -1729,6 +1733,10 @@ class Assignment < ActiveRecord::Base
false
end
def can_copy?(user)
!att_frozen?("no_copying", user)
end
def frozen_atts_not_altered
return if self.copying
FREEZABLE_ATTRIBUTES.each do |att|

View File

@ -235,4 +235,15 @@ class AssignmentGroup < ActiveRecord::Base
group.save
end
def has_frozen_assignments?(user)
return false unless PluginSetting.settings_for_plugin(:assignment_freezer)
return false unless self.active_assignments.length > 0
self.active_assignments.each do |asmnt|
return true if asmnt.frozen_for_user?(user)
end
false
end
end

View File

@ -131,7 +131,7 @@ class ContentExport < ActiveRecord::Base
selected_content[asset_type][CC::CCHelper.create_key(obj)] = true
end
def add_error(user_message, exception_or_info)
def add_error(user_message, exception_or_info=nil)
self.settings[:errors] ||= []
if exception_or_info.is_a?(Exception)
er = ErrorReport.log_exception(:course_export, exception_or_info)

View File

@ -9,6 +9,12 @@ TEXT
%></p>
</td>
</tr>
<tr>
<td colspan="2">
<%= f.check_box "no_copying", {}, 'yes', 'no' %>
<%= f.label "no_copying", t('no_copying_frozen', "Don't allow frozen assignments to be copied.") %>
</td>
</tr>
<% Assignment::FREEZABLE_ATTRIBUTES.sort.each do |att| %>
<tr>
<td colspan=2><%= f.check_box att, {}, 'yes', 'no' %>

View File

@ -30,9 +30,11 @@
<a href="#" class="edit_group_link no-hover">
<%= image_tag 'edit.png', :alt => t('alts.edit_group_details', "Edit"), :title => t('links.edit_group_details', "Edit Group Details") %>
</a>
<a href="<%= context_url(@context, :controller => :assignment_groups, :action => :destroy, :id => (assignment_group ? assignment_group.id : "{{ id }}")) %>" class="delete_group_link no-hover">
<%= image_tag 'delete.png', :alt => t('alts.delete_assignment_group', "Delete"), :title => t('titles.delete_assignment_group', "Delete Assignment Group") %>
</a>
<% if !group || !group.has_frozen_assignments?(@current_user) %>
<a href="<%= context_url(@context, :controller => :assignment_groups, :action => :destroy, :id => (assignment_group ? assignment_group.id : "{{ id }}")) %>" class="delete_group_link no-hover">
<%= image_tag 'delete.png', :alt => t('alts.delete_assignment_group', "Delete"), :title => t('titles.delete_assignment_group', "Delete Assignment Group") %>
</a>
<% end %>
</div>
<div class="clear"></div>
<div class="more_info" style="min-height: 30px; display: none; padding-left: 30px; font-size: 0.8em;">

View File

@ -21,10 +21,17 @@ module CC
def add_assignments
@course.assignments.active.no_graded_quizzes_or_topics.each do |assignment|
next unless export_object?(assignment)
title = assignment.title rescue I18n.t('course_exports.unknown_titles.assignment', "Unknown assignment")
if !assignment.can_copy?(@user)
add_error(I18n.t('course_exports.errors.assignment_is_locked', "The assignment \"%{title}\" could not be copied because it is locked.", :title => title))
next
end
begin
add_assignment(assignment)
rescue
title = assignment.title rescue I18n.t('course_exports.unknown_titles.assignment', "Unknown assignment")
add_error(I18n.t('course_exports.errors.assignment', "The assignment \"%{title}\" failed to export", :title => title), $!)
end
end

View File

@ -20,7 +20,7 @@ module CC
include CCHelper
attr_accessor :exporter, :weblinks, :basic_ltis
delegate :add_error, :set_progress, :export_object?, :for_course_copy, :add_item_to_export, :to => :exporter
delegate :add_error, :set_progress, :export_object?, :for_course_copy, :add_item_to_export, :user, :to => :exporter
def initialize(exporter)
@exporter = exporter

View File

@ -24,6 +24,7 @@ module CC
def initialize(manifest, resources_node, html_exporter)
@manifest = manifest
@user = manifest.user
@resources_node = resources_node
@course = manifest.course
@export_dir = @manifest.export_dir
@ -55,10 +56,17 @@ module CC
@course.quizzes.active.each do |quiz|
next unless export_object?(quiz) || export_object?(quiz.assignment)
title = quiz.title rescue I18n.t('unknown_quiz', "Unknown quiz")
if quiz.assignment && !quiz.assignment.can_copy?(@user)
add_error(I18n.t('course_exports.errors.quiz_is_locked', "The quiz \"%{title}\" could not be copied because it is locked.", :title => title))
next
end
begin
generate_quiz(quiz)
rescue
title = quiz.title rescue I18n.t('unknown_quiz', "Unknown quiz")
add_error(I18n.t('course_exports.errors.quiz', "The quiz \"%{title}\" failed to export", :title => title), $!)
end
end

View File

@ -21,7 +21,7 @@ module QTI
include CC::CCHelper
attr_accessor :exporter
delegate :add_error, :set_progress, :export_object?, :qti_export?, :course, :to => :exporter
delegate :add_error, :set_progress, :export_object?, :qti_export?, :course, :user, :to => :exporter
delegate :referenced_files, :to => :@html_exporter
def initialize(exporter)

View File

@ -35,6 +35,7 @@ module CC
@manifest = manifest
@manifest_node = manifest_node
@course = @manifest.course
@user = @manifest.user
@export_dir = @manifest.export_dir
@resources = nil
@zip_file = manifest.zip_file

View File

@ -21,10 +21,16 @@ module CC
def add_topics
@course.discussion_topics.active.each do |topic|
next unless export_object?(topic) || export_object?(topic.assignment)
title = topic.title rescue I18n.t('course_exports.unknown_titles.topic', "Unknown topic")
if topic.assignment && !topic.assignment.can_copy?(@user)
add_error(I18n.t('course_exports.errors.topic_is_locked', "The topic \"%{title}\" could not be copied because it is locked.", :title => title))
next
end
begin
add_topic(topic)
rescue
title = topic.title rescue I18n.t('course_exports.unknown_titles.topic', "Unknown topic")
add_error(I18n.t('course_exports.errors.topic', "The discussion topic \"%{title}\" failed to export", :title => title), $!)
end
end

View File

@ -941,6 +941,83 @@ equation: <img class="equation_image" title="Log_216" src="/equation_images/Log_
aq.question_data[:answers][1][:left_html].should == data2[:answers][1][:left_html]
end
context "copying frozen assignments" do
append_before (:each) do
@setting = PluginSetting.create!(:name => "assignment_freezer", :settings => {"no_copying" => "yes"})
@asmnt = @copy_from.assignments.create!(:title => 'lock locky')
@asmnt.copied = true
@asmnt.freeze_on_copy = true
@asmnt.save!
@quiz = @copy_from.quizzes.create(:title => "quiz", :quiz_type => "assignment")
@quiz.workflow_state = 'available'
@quiz.save!
@quiz.assignment.copied = true
@quiz.assignment.freeze_on_copy = true
@quiz.save!
@topic = @copy_from.discussion_topics.build(:title => "topic")
assignment = @copy_from.assignments.build(:submission_types => 'discussion_topic', :title => @topic.title)
assignment.infer_due_at
assignment.saved_by = :discussion_topic
assignment.copied = true
assignment.freeze_on_copy = true
@topic.assignment = assignment
@topic.save
@admin = account_admin_user(opts={})
end
it "should copy for admin" do
@cm.user = @admin
@cm.save!
run_course_copy
@copy_to.assignments.count.should == (Qti.qti_enabled? ? 3 : 2)
@copy_to.quizzes.count.should == 1 if Qti.qti_enabled?
@copy_to.discussion_topics.count.should == 1
@cm.content_export.error_messages.should == []
end
it "should copy for teacher if flag not set" do
@setting.settings = {}
@setting.save!
run_course_copy
@copy_to.assignments.count.should == (Qti.qti_enabled? ? 3 : 2)
@copy_to.quizzes.count.should == 1 if Qti.qti_enabled?
@copy_to.discussion_topics.count.should == 1
@cm.content_export.error_messages.should == []
end
it "should not copy for teacher" do
run_course_copy
@copy_to.assignments.count.should == 0
@copy_to.quizzes.count.should == 0
@copy_to.discussion_topics.count.should == 0
@cm.content_export.error_messages.should == [
["The assignment \"lock locky\" could not be copied because it is locked.", nil],
["The topic \"topic\" could not be copied because it is locked.", nil],
["The quiz \"quiz\" could not be copied because it is locked.", nil]]
end
it "should not mark assignment as copied if not set to be frozen" do
@asmnt.freeze_on_copy = false
@asmnt.copied = false
@asmnt.save!
run_course_copy
asmnt_2 = @copy_to.assignments.find_by_migration_id(mig_id(@asmnt))
asmnt_2.freeze_on_copy.should be_nil
asmnt_2.copied.should be_nil
end
end
context "notifications" do
before(:each) do
Notification.create!(:name => 'Migration Export Ready', :category => 'Migration')

View File

@ -259,6 +259,13 @@ describe "assignments" do
f('#edit_assignment_form #assignment_description').should_not be_nil
end
end
it "should not allow assignment group to be deleted" do
get "/courses/#{@course.id}/assignments"
f("#group_#{@asmnt.assignment_group_id} .delete_group_link").should be_nil
f("#assignment_#{@asmnt.id} .delete_assignment_link").should be_nil
end
end
it "should show a \"more errors\" errorBox if any invalid fields are hidden" do