fixes #5078 duplicate discussions

Avoid Assignment#update_quiz_or_discussion_topic auto-creating a
topic when the topic's reference to the assignment hasn't been
saved yet.

Change-Id: I2e31f3a9007dec0c7622bc8d5172c832967e8818
Reviewed-on: https://gerrit.instructure.com/5017
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>

app/controllers/discussion_topics_controller.rb

@@ -193,6 +193,7 @@ class DiscussionTopicsController < ApplicationController
       params[:discussion_topic][:assignment] = nil
       if @topic && @topic.assignment
         @topic.update_attribute(:assignment_id, nil)
+        @topic.assignment.saved_by = :discussion_topic
         @topic.assignment.destroy
       end
       return
@@ -201,6 +202,7 @@ class DiscussionTopicsController < ApplicationController
     @assignment ||= @topic.restore_old_assignment if @topic
     @assignment ||= @context.assignments.build
     @assignment.submission_types = 'discussion_topic'
+    @context.assert_assignment_group
     @assignment.assignment_group_id = assignment[:assignment_group_id] || @assignment.assignment_group_id || @context.assignment_groups.first.id
     @assignment.title = params[:discussion_topic][:title]
     @assignment.points_possible = assignment[:points_possible] || @assignment.points_possible

app/models/discussion_topic.rb

@@ -113,9 +113,8 @@ class DiscussionTopic < ActiveRecord::Base
       self.context_module_tag.confirm_valid_module_requirements
     end
     if @old_assignment_id
-      Assignment.update_all({:workflow_state => 'deleted', :updated_at => Time.now}, {:id => [@old_assignment_id, self.last_assignment_id].compact, :context_id => self.context_id, :context_type => self.context_type, :submission_types => 'discussion_topic'})
+      Assignment.update_all({:workflow_state => 'deleted', :updated_at => Time.now}, {:id => @old_assignment_id, :context_id => self.context_id, :context_type => self.context_type, :submission_types => 'discussion_topic'})
       ContentTag.delete_for(Assignment.find(@old_assignment_id)) if @old_assignment_id
-      ContentTag.delete_for(Assignment.find(self.last_assignment_id)) if self.last_assignment_id
     elsif self.assignment && @saved_by != :assignment
       self.assignment.title = self.title
       self.assignment.description = self.message
@@ -129,7 +128,9 @@ class DiscussionTopic < ActiveRecord::Base
   
   def restore_old_assignment
     return nil unless self.old_assignment && self.old_assignment.deleted?
-    self.old_assignment.update_attribute(:workflow_state, 'available')
+    self.old_assignment.workflow_state = 'available'
+    self.old_assignment.saved_by = :discussion_topic
+    self.old_assignment.save(false)
     self.old_assignment
   end
 

spec/controllers/discussion_topics_controller_spec.rb

@@ -207,6 +207,36 @@ describe DiscussionTopicsController do
       assigns[:topic].editor.should eql(@teacher)
       assigns[:topic].user.should eql(@student)
     end
+
+    it "should not duplicate when adding or removing an assignment" do
+      course_with_teacher_logged_in(:active_all => true)
+      course_topic
+
+      put 'update', :course_id => @course.id, :id => @topic.id, :discussion_topic => {:assignment => { :set_assignment => '1' }}
+      @topic.reload
+      @topic.assignment_id.should_not be_nil
+      @topic.old_assignment_id.should_not be_nil
+      old_assignment_id = @topic.old_assignment_id
+      DiscussionTopic.find_all_by_old_assignment_id(old_assignment_id).should == [ @topic ]
+
+      put 'update', :course_id => @course.id, :id => @topic.id, :discussion_topic => {:assignment => { :set_assignment => '0' }}
+      @topic.reload
+      @topic.assignment_id.should be_nil
+      @topic.old_assignment_id.should == old_assignment_id
+      DiscussionTopic.find_all_by_old_assignment_id(old_assignment_id).should == [ @topic ]
+
+      put 'update', :course_id => @course.id, :id => @topic.id, :discussion_topic => {:assignment => { :set_assignment => '1' }}
+      @topic.reload
+      @topic.assignment_id.should == old_assignment_id
+      @topic.old_assignment_id.should == old_assignment_id
+      DiscussionTopic.find_all_by_old_assignment_id(old_assignment_id).should == [ @topic ]
+
+      put 'update', :course_id => @course.id, :id => @topic.id, :discussion_topic => {:assignment => { :set_assignment => '0' }}
+      @topic.reload
+      @topic.assignment_id.should be_nil
+      @topic.old_assignment_id.should == old_assignment_id
+      DiscussionTopic.find_all_by_old_assignment_id(old_assignment_id).should == [ @topic ]
+    end
   end
   
   describe "DELETE 'destroy'" do