fix account-level outcome rubric association copying

just copy the external identifier like the way we
 do for the rubrics themselves - when the account-level
 group is added the outcomes aren't copied down to
 the blueprint so we can't rely on the old way
 of copying them

test plan:
* have an account-level outcome group
* create an outcome in the group
* create a blueprint course
* add the account-level group (not the outcome alone)
 to the blueprint course
* sync to an associated course
* create a rubric on the blueprint and link
 to the account-level outcome
* sync to the associated course
* the rubric in the associated course should still
 be linked to the outcome

closes #ADMIN-1460

Change-Id: I7a0458200b467f8c9d1fbb056cebfcea4adfabb5
Reviewed-on: https://gerrit.instructure.com/165062
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: James Williams  <jamesw@instructure.com>

app/models/importers/rubric_importer.rb

@@ -72,14 +72,19 @@ module Importers
 
         item.data = hash[:data]
         item.data.each do |crit|
-          if crit[:learning_outcome_migration_id]
+          if crit[:learning_outcome_migration_id].present?
             if migration.respond_to?(:outcome_to_id_map) && id = migration.outcome_to_id_map[crit[:learning_outcome_migration_id]]
               crit[:learning_outcome_id] = id
             elsif lo = context.created_learning_outcomes.where(migration_clause(crit[:learning_outcome_migration_id])).first
               crit[:learning_outcome_id] = lo.id
             end
-            crit.delete :learning_outcome_migration_id
+          elsif crit[:learning_outcome_external_identifier].present? && !migration.cross_institution?
+            if lo = context.available_outcome(crit[:learning_outcome_external_identifier])
+              crit[:learning_outcome_id] = lo.id
+            end
           end
+          crit.delete(:learning_outcome_migration_id)
+          crit.delete(:learning_outcome_external_identifier)
         end
 
         item.skip_updating_points_possible = true

lib/cc/importer/canvas/rubrics_converter.rb

@@ -46,6 +46,7 @@ module CC::Importer::Canvas
           crit[:mastery_points] = get_float_val(c_node, 'mastery_points')
           crit[:ignore_for_scoring] = get_bool_val(c_node, 'ignore_for_scoring')
           crit[:learning_outcome_migration_id] = get_node_val(c_node, 'learning_outcome_identifierref')
+          crit[:learning_outcome_external_identifier] = get_node_val(c_node, 'learning_outcome_external_identifier')
           crit[:title] = get_node_val(c_node, 'description')
           crit[:criterion_use_range] = get_bool_val(c_node, 'criterion_use_range')
           crit[:ratings] = []

lib/cc/rubrics.rb

@@ -93,7 +93,11 @@ module CC
         c_node.criterion_use_range criterion[:criterion_use_range] if criterion[:criterion_use_range].present?
         if criterion[:learning_outcome_id].present?
           if lo = @course.available_outcome(criterion[:learning_outcome_id])
-            c_node.learning_outcome_identifierref create_key(lo)
+            if lo.context_type == "Course" && lo.context_id == @course.id
+              c_node.learning_outcome_identifierref create_key(lo)
+            else
+              c_node.learning_outcome_external_identifier lo.id
+            end
           end
         end
 

lib/cc/xsd/cccv1p0.xsd

@@ -545,6 +545,7 @@
                           <xs:element name="mastery_points" type="optional_float" minOccurs="0"/>
                           <xs:element name="ignore_for_scoring" type="xs:boolean" minOccurs="0"/>
                           <xs:element name="learning_outcome_identifierref" type="xs:string" minOccurs="0"/>
+                          <xs:element name="learning_outcome_external_identifier" type="xs:string" minOccurs="0"/>
                           <xs:element name="criterion_use_range" type="xs:boolean" minOccurs="0"/>
                           <xs:element name="ratings" type="ratingsType" minOccurs="0"/>
                         </xs:all>

spec/models/master_courses/master_migration_spec.rb

@@ -649,6 +649,40 @@ describe MasterCourses::MasterMigration do
       expect(rub_to.data.first["learning_outcome_id"]).to eq lo.id
     end
 
+    it "copies links to account outcomes in imported groups on rubrics" do
+      @copy_to = course_factory
+      @template.add_child_course!(@copy_to)
+
+      account = @copy_from.account
+      a_group = account.root_outcome_group
+      lo = account.created_learning_outcomes.create!({:title => 'new outcome'})
+      a_group.add_outcome(lo)
+
+      root = @copy_from.root_outcome_group
+      root.add_outcome_group(a_group) # add the group - not the outcome
+
+      run_master_migration
+
+      rub = Rubric.new(:context => @copy_from)
+      rub.data = [
+        {
+          :points => 3,
+          :description => "Outcome row",
+          :id => 1,
+          :ratings => [{:points => 3,:description => "Rockin'",:criterion_id => 1,:id => 2}],
+          :learning_outcome_id => lo.id
+        }
+      ]
+      rub.save!
+      rub.associate_with(@copy_from, @copy_from)
+      Rubric.where(:id => rub.id).update_all(:updated_at => 5.minute.from_now)
+
+      run_master_migration
+
+      rub_to = @copy_to.rubrics.first
+      expect(rub_to.data.first["learning_outcome_id"]).to eq lo.id
+    end
+
     it "doesn't restore deleted associated files unless relocked" do
       @copy_to = course_factory
       @template.add_child_course!(@copy_to)