Skip to content

Commit

Permalink
master courses: don't delete non-empty assignment groups
Browse files Browse the repository at this point in the history
test plan:
* have a blueprint course with an assignment group
 and an assignment
* sync to an associated course
* add an assignment to the group in the associated course
* delete the group in the blueprint course
* re-sync
* it should not delete the group in the associated course

closes #ADMIN-827

Change-Id: Ibeab4fe7209ca4d726476935776227457534dc2c
Reviewed-on: https://gerrit.instructure.com/142556
Tested-by: Jenkins
Reviewed-by: Mysti Sadler <[email protected]>
QA-Review: Deepeeca Soundarrajan <[email protected]>
Product-Review: James Williams  <[email protected]>
  • Loading branch information
maneframe committed Mar 6, 2018
1 parent ab7e4dc commit de135ca
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 1 deletion.
7 changes: 6 additions & 1 deletion app/models/content_migration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,12 @@ def process_master_deletions(deletions)
end
item_scope.each do |content|
child_tag = master_course_subscription.content_tag_for(content)
if child_tag.downstream_changes.any? && !content.editing_restricted?(:any)
skip_item = child_tag.downstream_changes.any? && !content.editing_restricted?(:any)
if content.is_a?(AssignmentGroup) && !skip_item && content.assignments.active.exists?
skip_item = true # don't delete an assignment group if an assignment is left (either they added one or changed one so it was skipped)
end

if skip_item
Rails.logger.debug("skipping deletion sync for #{content.asset_string} due to downstream changes #{child_tag.downstream_changes}")
add_skipped_item(child_tag)
else
Expand Down
37 changes: 37 additions & 0 deletions spec/models/master_courses/master_migration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,43 @@ def run_master_migration(opts={})
expect(quiz_to.reload.quiz_questions.where(:migration_id => mig_id(@new_qq)).first).to_not be_nil
end

it "shouldn't delete an assignment group if it's not empty downstream" do
@copy_to = course_factory
sub = @template.add_child_course!(@copy_to)

ag1 = @copy_from.assignment_groups.create!(:name => "group1")
a1 = @copy_from.assignments.create!(:title => "assmt1", :assignment_group => ag1)
ag2 = @copy_from.assignment_groups.create!(:name => "group2")
a2 = @copy_from.assignments.create!(:title => "assmt2", :assignment_group => ag2)
ag3 = @copy_from.assignment_groups.create!(:name => "group3")
a3 = @copy_from.assignments.create!(:title => "assmt3", :assignment_group => ag3)

run_master_migration

ag1_to = @copy_to.assignment_groups.where(:migration_id => mig_id(ag1)).first
a1_to = ag1_to.assignments.first
ag2_to = @copy_to.assignment_groups.where(:migration_id => mig_id(ag2)).first
a2_to = ag2_to.assignments.first
ag3_to = @copy_to.assignment_groups.where(:migration_id => mig_id(ag3)).first
a3_to = ag3_to.assignments.first

Timecop.freeze(30.seconds.from_now) do
[ag1, ag2, ag3].each(&:destroy!)
a2_to.update_attribute(:name, "some other downstream name")
@new_assmt = @copy_to.assignments.create!(:title => "a new assignment created downstream", :assignment_group => ag3_to)
end

run_master_migration

expect(ag1_to.reload).to be_deleted # should still delete
expect(a1_to.reload).to be_deleted
expect(ag2_to.reload).to_not be_deleted # should skip deletion because a2's deletion was skipped
expect(a2_to.reload).to_not be_deleted
expect(ag3_to.reload).to_not be_deleted # should skip deletion because of @new_assmt
expect(a3_to.reload).to be_deleted # but should have still deleted the assigment
expect(@new_assmt.reload).to_not be_deleted
end

it "should sync unpublished quiz points possible" do
@copy_to = course_factory
sub = @template.add_child_course!(@copy_to)
Expand Down

0 comments on commit de135ca

Please sign in to comment.