preserve answer ids in questions on course copy

test plan:
* create a quiz in a blueprint course
 with various question types with answers
* copy to an associated course
* in the associated course, take the quiz
 as a student
* edit the blueprint quiz description and re-sync
* the quiz statistics page for the student
 should not be broken

closes #QO-416 #QO-197

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

gems/plugins/qti_exporter/lib/qti/assessment_item_converter.rb

@@ -222,6 +222,10 @@ class AssessmentItemConverter
     end
   end
 
+  def get_or_generate_answer_id(response_identifier)
+    (@flavor == Qti::Flavors::CANVAS && response_identifier.to_s.sub(/response_/i, "").presence&.to_i) || unique_local_id
+  end
+
   def unique_local_id
     @@ids ||= {}
     id = rand(100_000)

gems/plugins/qti_exporter/lib/qti/associate_interaction.rb

@@ -130,7 +130,7 @@ class AssociateInteraction < AssessmentItemConverter
       @question[:answers] << answer
       answer_map[ci['responseIdentifier']] = answer
       extract_answer!(answer, ci.at_css('prompt'))
-      answer[:id] = unique_local_id
+      answer[:id] = get_or_generate_answer_id(ci['responseIdentifier'])
     end
 
     # connect to match

gems/plugins/qti_exporter/lib/qti/calculated_interaction.rb

@@ -83,7 +83,8 @@ class CalculatedInteraction < AssessmentItemConverter
 
   def get_answer_sets
     @doc.css('calculated var_sets var_set').each do |vs|
-      set = {:variables=>[], :id=>unique_local_id, :weight=>100}
+      set = {:variables=>[], :weight=>100}
+      set[:id] = vs['ident'].presence || unique_local_id
       @question[:answers] << set
       set[:answer] = vs.at_css('answer').text.to_f if vs.at_css('answer')
 

gems/plugins/qti_exporter/lib/qti/choice_interaction.rb

@@ -108,8 +108,8 @@ class ChoiceInteraction < AssessmentItemConverter
       ci.search('simpleChoice').each do |choice|
         answer = {}
         answer[:weight] = AssessmentItemConverter::DEFAULT_INCORRECT_WEIGHT
-        answer[:id] = unique_local_id
         answer[:migration_id] = choice['identifier']
+        answer[:id] = get_or_generate_answer_id(answer[:migration_id])
 
         if feedback = choice.at_css('feedbackInline')
           # weird Angel feedback

gems/plugins/qti_exporter/lib/qti/extended_text_interaction.rb

@@ -20,7 +20,7 @@ require 'nokogiri'
 module Qti
 class ExtendedTextInteraction < AssessmentItemConverter
   include Canvas::Migration::XMLHelper
-  
+
   def initialize(opts)
     super(opts)
   end
@@ -34,9 +34,9 @@ class ExtendedTextInteraction < AssessmentItemConverter
       # a short answer question with no answers is an essay question
       @question[:question_type] = "essay_question"
     end
-    
+
     get_feedback
-    
+
     @question
   end
 
@@ -55,7 +55,7 @@ class ExtendedTextInteraction < AssessmentItemConverter
         match_data = regex.match(match_data.post_match)
       end
       @question.delete :is_vista_fib
-    elsif @question[:question_type] == 'fill_in_multiple_blanks_question' 
+    elsif @question[:question_type] == 'fill_in_multiple_blanks_question'
       # the python tool "fixes" IDs that aren't quite legal QTI (e.g., "1a" becomes "RESPONSE_1a")
       # but does not update the question text, breaking fill-in-multiple-blanks questions.
       # fortunately it records what it does in an XML comment at the top of the doc, so we can undo it.
@@ -80,7 +80,7 @@ class ExtendedTextInteraction < AssessmentItemConverter
           answer = {}
         end
         answer[:text] ||= text
-        unless answer[:feedback_id] 
+        unless answer[:feedback_id]
           if f_id = get_feedback_id(cond)
             answer[:feedback_id] = f_id
           end
@@ -102,7 +102,8 @@ class ExtendedTextInteraction < AssessmentItemConverter
           @question[:answers] << answer
           answer[:weight] = 100
           answer[:comments] = ""
-          answer[:id] = unique_local_id
+          bv = match.at_css('baseValue')
+          answer[:id] = get_or_generate_answer_id(bv && bv['identifier'])
         end
       end
     end

gems/plugins/qti_exporter/lib/qti/fill_in_the_blank.rb

@@ -86,8 +86,8 @@ class FillInTheBlank < AssessmentItemConverter
       ci.search('simpleChoice').each do |choice|
         answer = {}
         answer[:weight] = @type == 'multiple_dropdowns_question' ? 0 : 100
-        answer[:id] = unique_local_id
         answer[:migration_id] = choice['identifier']
+        answer[:id] = get_or_generate_answer_id(answer[:migration_id])
         answer[:text] = choice.text.strip
         answer[:blank_id] = blank_id
         @question[:answers] << answer

gems/plugins/qti_exporter/lib/qti/numeric_interaction.rb

@@ -54,7 +54,9 @@ class NumericInteraction < AssessmentItemConverter
 
   def get_canvas_answers
     @doc.css('responseIf, responseElseIf').each do |r_if|
-      answer = {:weight=>100, :id=>unique_local_id, :text=>'answer_text'}
+      answer = {:weight=>100, :text=>'answer_text'}
+      bv = r_if.at_css('baseValue')
+      answer[:id] = get_or_generate_answer_id(bv && bv['identifier'])
       answer[:feedback_id] = get_feedback_id(r_if)
 
       if or_node = r_if.at_css('or')

lib/cc/qti/qti_items.rb

@@ -256,7 +256,7 @@ module CC
 
       def calculated_response_str(node, question)
         node.response_str(
-                :ident => "response1",
+                :ident => question["answers"].first["id"],
                 :rcardinality => 'Single'
         ) do |r_node|
           r_node.render_fib(:fibtype=>'Decimal') {|n| n.response_label(:ident=>'answer1')}
@@ -338,7 +338,7 @@ module CC
         node.respcondition(:continue=>'No') do |res_node|
           res_node.conditionvar do |c_node|
             question['answers'].each do |answer|
-              c_node.varequal answer['text'], :respident=>"response1"
+              c_node.varequal answer['text'], :respident => answer['id']
             end
           end #c_node
           res_node.setvar '100', :action => 'Set', :varname => 'SCORE'
@@ -354,13 +354,13 @@ module CC
                 # exact answer
                 c_node.or do |or_node|
                   exact = answer['exact'].to_f
-                  or_node.varequal exact, :respident=>"response1"
+                  or_node.varequal exact, :respident=>answer['id']
                   unless answer['margin'].blank?
                     or_node.and do |and_node|
                       exact = BigDecimal.new(answer['exact'].to_s)
                       margin = BigDecimal.new(answer['margin'].to_s)
-                      and_node.vargte((exact - margin).to_f, :respident=>"response1")
-                      and_node.varlte((exact + margin).to_f, :respident=>"response1")
+                      and_node.vargte((exact - margin).to_f, :respident=>answer['id'])
+                      and_node.varlte((exact + margin).to_f, :respident=>answer['id'])
                     end
                   end
                 end
@@ -368,7 +368,7 @@ module CC
                 # this might be one of the worst hacks i've ever done
                 c_node.or do |or_node|
                   approx = answer['approximate'].to_d
-                  or_node.varequal approx, :respident=>"response1"
+                  or_node.varequal approx, :respident=>answer['id']
 
                   precision = answer['precision'].to_i
                   if precision > 0
@@ -380,15 +380,15 @@ module CC
                     ceil = "#{prefix.to_d + range}E#{exp}".to_d # 1.3405E+01
 
                     or_node.and do |and_node|
-                      and_node.vargt(floor, :respident=>"response1")
-                      and_node.varlte(ceil, :respident=>"response1")
+                      and_node.vargt(floor, :respident=>answer['id'])
+                      and_node.varlte(ceil, :respident=>answer['id'])
                     end
                   end
                 end
               else
                 # answer in range
-                c_node.vargte(answer['start'], :respident=>"response1")
-                c_node.varlte(answer['end'], :respident=>"response1")
+                c_node.vargte(answer['start'], :respident=>answer['id'])
+                c_node.varlte(answer['end'], :respident=>answer['id'])
               end
             end #c_node
 

spec/models/master_courses/master_migration_spec.rb

@@ -387,6 +387,36 @@ describe MasterCourses::MasterMigration do
       expect(aq4_to.reload).to_not be_deleted # should have been left alone
     end
 
+    it "should preserve all answer ids on re-copy" do
+      @copy_to = course_factory
+      sub = @template.add_child_course!(@copy_to)
+
+      q = @copy_from.quizzes.create!(:title => "q")
+      datas = [
+        multiple_choice_question_data,
+        true_false_question_data,
+        short_answer_question_data,
+        calculated_question_data,
+        numerical_question_data,
+        multiple_answers_question_data,
+        multiple_dropdowns_question_data,
+        matching_question_data
+      ]
+      datas.each{|d| q.quiz_questions.create!(:question_data => d)}
+
+      run_master_migration
+
+      q_to = @copy_to.quizzes.where(:migration_id => mig_id(q)).first
+      copied_answers = Hash[q_to.quiz_questions.to_a.map{|qq| [qq.id, qq.question_data.to_hash["answers"]]}]
+
+      Quizzes::Quiz.where(:id => q).update_all(:updated_at => 1.minute.from_now) # recopy
+      run_master_migration
+
+      q_to.reload.quiz_questions.to_a.each do |qq_to|
+        expect(copied_answers[qq_to.id]).to eq qq_to.question_data.to_hash["answers"] # should be unchanged
+      end
+    end
+
     it "should sync quiz group attributes (unless changed downstream)" do
       @copy_to = course_factory
       sub = @template.add_child_course!(@copy_to)