avoid several extra cases of saving learning outcome results

Refs #4189

No need to create results synchronously in RubricAssociation#assess
just for Assignments - RubricAssessment#track_outcomes already
does that automatically.  Just make it use send_later_if_production
instead of plain send_later so the spec still works.

Also, don't do the weird find_and_create in a loop to avoid a
minisculely possible race condition, to avoid creating two versions
off-the-bat.

Change-Id: If6948b9b4fae31782e47874ab1b1b6aa4dd8edb5
Reviewed-on: https://gerrit.instructure.com/3911
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: JT Olds <jt@instructure.com>

app/models/content_tag.rb

@@ -226,17 +226,12 @@ class ContentTag < ActiveRecord::Base
     association_type = association.class.to_s
     result = nil
     attempts = 0
-    begin
-      if association.is_a?(Quiz)
-        result = LearningOutcomeResult.find_by_learning_outcome_id_and_user_id_and_association_id_and_association_type_and_content_tag_id_and_associated_asset_type_and_associated_asset_id(self.learning_outcome_id, user.id, association.id, association_type, self.id, 'AssessmentQuestion', assessment_question.id)
-        result ||= LearningOutcomeResult.create(:learning_outcome => self.learning_outcome, :user => user, :association => association, :content_tag => self, :associated_asset => assessment_question)
-      else
-        result = LearningOutcomeResult.find_by_learning_outcome_id_and_user_id_and_association_id_and_association_type_and_content_tag_id(self.learning_outcome_id, user.id, association.id, association_type, self.id)
-        result ||= LearningOutcomeResult.create(:learning_outcome => self.learning_outcome, :user => user, :association => association, :content_tag => self)
-      end
-    rescue => e
-      attempts += 1
-      retry if attempts < 3
+    if association.is_a?(Quiz)
+      result = LearningOutcomeResult.find_by_learning_outcome_id_and_user_id_and_association_id_and_association_type_and_content_tag_id_and_associated_asset_type_and_associated_asset_id(self.learning_outcome_id, user.id, association.id, association_type, self.id, 'AssessmentQuestion', assessment_question.id)
+      result ||= LearningOutcomeResult.new(:learning_outcome => self.learning_outcome, :user => user, :association => association, :content_tag => self, :associated_asset => assessment_question)
+    else
+      result = LearningOutcomeResult.find_by_learning_outcome_id_and_user_id_and_association_id_and_association_type_and_content_tag_id(self.learning_outcome_id, user.id, association.id, association_type, self.id)
+      result ||= LearningOutcomeResult.new(:learning_outcome => self.learning_outcome, :user => user, :association => association, :content_tag => self)
     end
     result.context = self.context
     if artifact

app/models/learning_outcome_result.rb

@@ -56,8 +56,8 @@ class LearningOutcomeResult < ActiveRecord::Base
   end
   
   def save_to_version(attempt)
-    current_version = self.versions.current.model
-    if current_version.attempt && attempt < current_version.attempt
+    current_version = self.versions.current.try(:model)
+    if current_version.try(:attempt) && attempt < current_version.attempt
       versions = self.versions.sort_by(&:created_at).reverse.select{|v| v.model.attempt == attempt}
       if !versions.empty?
         versions.each do |version|

app/models/rubric_assessment.rb

@@ -41,7 +41,7 @@ class RubricAssessment < ActiveRecord::Base
   
   def track_outcomes
     outcome_ids = (self.data || []).map{|r| r[:learning_outcome_id] }.compact.uniq
-    send_later(:update_outcomes_for_assessment, outcome_ids) unless outcome_ids.empty?
+    send_later_if_production(:update_outcomes_for_assessment, outcome_ids) unless outcome_ids.empty?
   end
   
   def update_outcomes_for_assessment(outcome_ids=[])

app/models/rubric_association.rb

@@ -306,11 +306,6 @@ class RubricAssociation < ActiveRecord::Base
       assessment.comments = params[:comments] if params[:comments]
 
       assessment.save
-      if self.association.is_a?(Assignment)
-        self.association.learning_outcome_tags.each do |tag|
-          tag.create_outcome_result(opts[:user], self.association, assessment)
-        end
-      end
       assessment_to_return = assessment if assessment.artifact == opts[:artifact]
     end
     assessment_to_return

spec/models/learning_outcome_spec.rb

@@ -219,6 +219,7 @@ describe LearningOutcome do
       @result.original_score.should eql(2.0)
       @result.original_possible.should eql(3.0)
       @result.mastery.should eql(nil)
+      @result.versions.length.should eql(1)
       n = @result.version_number
       @assessment = @a.assess({
         :user => @user,
@@ -233,6 +234,7 @@ describe LearningOutcome do
         }
       })
       @result.reload
+      @result.versions.length.should eql(2)
       @result.version_number.should > n
       @result.score.should eql(3.0)
       @result.possible.should eql(3.0)