Handle concurrent destroy / import of outcome groups

Fixes OUT-2336

Test Plan:
- Create or import a group with lots of outcomes and groups.
- Attempt to import that group into another context.
- Immediately open another tab and navigate to the parent group of the
  import.
- Verify that the imported group does not appear currently.
- Reload / wait for the import to finish and group to appear.

Change-Id: Ic97f86201932ff7751914e4cc2f95290b6e1c8b8
Reviewed-on: https://gerrit.instructure.com/157757
Tested-by: Jenkins
Reviewed-by: Michael Brewer-Davis <mbd@instructure.com>
Reviewed-by: Augusto Callejas <acallejas@instructure.com>
QA-Review: Matt Berns <mberns@instructure.com>
Product-Review: Sidharth Oberoi <soberoi@instructure.com>

app/models/learning_outcome_group.rb

@@ -79,27 +79,29 @@ class LearningOutcomeGroup < ActiveRecord::Base
   # commit!
   def add_outcome_group(original, opts={})
     # copy group into this group
-    copy = child_outcome_groups.build
+    transaction do
-    copy.title = original.title
+      copy = child_outcome_groups.build
-    copy.description = original.description
+      copy.title = original.title
-    copy.vendor_guid = original.vendor_guid
+      copy.description = original.description
-    copy.context = self.context
+      copy.vendor_guid = original.vendor_guid
-    copy.save!
+      copy.context = self.context
+      copy.save!
 
-    # copy the group contents
+      # copy the group contents
-    original.child_outcome_groups.active.each do |group|
+      original.child_outcome_groups.active.each do |group|
-      next if opts[:only] && opts[:only][group.asset_string] != "1"
+        next if opts[:only] && opts[:only][group.asset_string] != "1"
-      copy.add_outcome_group(group, opts)
+        copy.add_outcome_group(group, opts)
+      end
+
+      original.child_outcome_links.active.each do |link|
+        next if opts[:only] && opts[:only][link.asset_string] != "1"
+        copy.add_outcome(link.content)
+      end
+
+      touch_parent_group
+      # done
+      copy
     end
-
-    original.child_outcome_links.active.each do |link|
-      next if opts[:only] && opts[:only][link.asset_string] != "1"
-      copy.add_outcome(link.content)
-    end
-
-    touch_parent_group
-    # done
-    copy
   end
 
   # moves an existing outcome link from the same context to be under this

spec/models/learning_outcome_group_spec.rb

@@ -138,19 +138,21 @@ describe LearningOutcomeGroup do
   end
 
   describe '#add_outcome_group' do
+    before :each do
+      @group1 = @course.learning_outcome_groups.create!(:title => 'group1')
+      @group2 = @course.learning_outcome_groups.create!(:title => 'group2')
+      @outcome1 = @course.created_learning_outcomes.create!(:title => 'o1')
+      @group2.add_outcome(@outcome1)
+    end
+
     it 'adds a child outcome group and copies all contents' do
-      group1 = @course.learning_outcome_groups.create!(:title => 'group1')
+      expect(@group1.child_outcome_groups).to be_empty
-      group2 = @course.learning_outcome_groups.create!(:title => 'group2')
-      outcome1 = @course.created_learning_outcomes.create!(:title => 'o1')
-      group2.add_outcome(outcome1)
 
-      expect(group1.child_outcome_groups).to be_empty
+      child_outcome_group = @group1.add_outcome_group(@group2)
 
-      child_outcome_group = group1.add_outcome_group(group2)
+      expect(child_outcome_group.title).to eq(@group2.title)
-
-      expect(child_outcome_group.title).to eq(group2.title)
       expect(child_outcome_group.child_outcome_links.map(&:content_id)).to eq(
-        group2.child_outcome_links.map(&:content_id)
+        @group2.child_outcome_links.map(&:content_id)
       )
     end
   end