discussions: prevent creation of new group graded discussions

test plan:
  * create a graded discussion
  * using the Assignments API, try to set group_category_id
      on the discussion's linked assignment
  * verify that you get an error message
  * using the Discussion Topics API, set a group_category_id
      on the discussion
  * verify that the discussion becomes a group discussion

Change-Id: Ida1bae1f370a5d5f13a5002b9cdeb40c24dba0b2
Reviewed-on: https://gerrit.instructure.com/35084
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jon Willesen <jonw@instructure.com>
QA-Review: Steven Shepherd <sshepherd@instructure.com>
Product-Review: Braden Anderson <braden@instructure.com>

app/controllers/assignments_api_controller.rb

@@ -303,7 +303,7 @@
 #           "type": "datetime"
 #         },
 #         "group_category_id": {
-#           "description": "the ID of the assignment’s group set (if this is a group assignment)",
+#           "description": "The ID of the assignment’s group set, if this is a group assignment. For group discussions, set group_category_id on the discussion topic, not the linked assignment.",
 #           "example": 1,
 #           "type": "integer"
 #         },

app/models/assignment.rb

@@ -76,6 +76,7 @@ class Assignment < ActiveRecord::Base
   has_one :external_tool_tag, :class_name => 'ContentTag', :as => :context, :dependent => :destroy
   validates_associated :external_tool_tag, :if => :external_tool?
   validate :group_category_changes_ok?
+  validate :discussion_group_ok?
   validate :positive_points_possible?
 
   accepts_nested_attributes_for :external_tool_tag, :update_only => true, :reject_if => proc { |attrs|
@@ -119,6 +120,13 @@ class Assignment < ActiveRecord::Base
     end
   end
 
+  def discussion_group_ok?
+    return unless new_record? || group_category_id_changed?
+    return unless group_category_id && submission_types == 'discussion_topic'
+    errors.add :group_category_id, I18n.t("discussion_group_category_locked",
+      "Group categories cannot be set directly on a discussion assignment, but should be set on the discussion instead")
+  end
+
   API_NEEDED_FIELDS = %w(
     id
     title

lib/api/v1/submission.rb

@@ -139,7 +139,7 @@ module Api::V1::Submission
       # group assignments will have a child topic for each group.
       # it's also possible the student posted in the main topic, as well as the
       # individual group one. so we search far and wide for all student entries.
-      if assignment.has_group_category?
+      if assignment.discussion_topic.has_group_category?
         entries = assignment.discussion_topic.child_topics.map {|t| t.discussion_entries.active.for_user(attempt.user_id) }.flatten.sort_by{|e| e.created_at}
       else
         entries = assignment.discussion_topic.discussion_entries.active.for_user(attempt.user_id)

spec/apis/v1/assignments_api_spec.rb

@@ -665,6 +665,29 @@ describe AssignmentsApiController, type: :request do
         should == "must be a positive number"
     end
 
+    context "discussion topic assignments" do
+      it "should prevent creating assignments with group category IDs and discussions" do
+        course_with_teacher(:active_all => true)
+        group_category = @course.group_categories.create!(name: "foo")
+        raw_api_call(:post, "/api/v1/courses/#{@course.id}/assignments",
+          { :controller => 'assignments_api',
+            :action => 'create',
+            :format => 'json',
+            :course_id => @course.id.to_s },
+          { :assignment => {
+              'name' => 'some assignment',
+              'group_category_id' => group_category.id,
+              'submission_types' => [
+                 'discussion_topic'
+              ],
+              'discussion_topic' => {
+                'title' => 'some assignment'
+              }
+            }
+          })
+        response.code.should eql '400'
+      end
+    end
   end
 
   describe "PUT /courses/:course_id/assignments/:id (#update)" do
@@ -1274,6 +1297,26 @@ describe AssignmentsApiController, type: :request do
         @assignment.grading_standard.should == nil
       end
     end
+
+    context "discussion topic assignments" do
+      it "should prevent setting group category ID on assignments with discussions" do
+        course_with_teacher(:active_all => true)
+        group_category = @course.group_categories.create!(name: "foo")
+        @assignment = factory_with_protected_attributes(
+          @course.assignments,
+          {
+            :title => 'assignment1',
+          })
+        @topic = @course.discussion_topics.build(assignment: @assignment, title: 'asdf')
+        @topic.save
+        raw_api_update_assignment(@course, @assignment, {
+          :group_category_id => group_category.id
+        })
+        @assignment.reload
+        @assignment.group_category.should be_nil
+        response.code.should eql '400'
+      end
+    end
   end
 
   describe "DELETE /courses/:course_id/assignments/:id (#delete)" do

spec/apis/v1/submissions_api_spec.rb

@@ -364,7 +364,7 @@ describe 'Submissions API', type: :request do
     json['discussion_entries'].should be_nil
   end
 
-  it "should return student discussion entries from child topics for discussion_topic group assignments" do
+  it "should return student discussion entries from child topics for group discussion_topic assignments" do
     @student = user(:active_all => true)
     course_with_teacher(:active_all => true)
     @course.enroll_student(@student).accept!
@@ -372,7 +372,7 @@ describe 'Submissions API', type: :request do
     @group = @course.groups.create(:name => "Group", :group_category => group_category)
     @group.add_user(@student)
     @context = @course
-    @assignment = factory_with_protected_attributes(@course.assignments, {:title => 'assignment1', :submission_types => 'discussion_topic', :discussion_topic => discussion_topic_model, :group_category => @group.group_category})
+    @assignment = factory_with_protected_attributes(@course.assignments, {:title => 'assignment1', :submission_types => 'discussion_topic', :discussion_topic => discussion_topic_model(:group_category => @group.group_category)})
     @topic.refresh_subtopics # since the DJ won't happen in time
     @child_topic = @group.discussion_topics.find_by_root_topic_id(@topic.id)
 

spec/controllers/submissions_controller_spec.rb

@@ -267,7 +267,7 @@ describe SubmissionsController do
       @u1 = @user
       student_in_course(:course => @course)
       @u2 = @user
-      @assignment = @course.assignments.create!(:title => "some assignment", :submission_types => "discussion_topic", :group_category => GroupCategory.create!(:name => "groups", :context => @course), :grade_group_students_individually => true)
+      @assignment = @course.assignments.create!(:title => "some assignment", :submission_types => "online_url,online_upload", :group_category => GroupCategory.create!(:name => "groups", :context => @course), :grade_group_students_individually => true)
       @group = @assignment.group_category.groups.create!(:name => 'g1', :context => @course)
       @group.users << @u1
       @group.users << @user

spec/factories/discussion_topic_factory.rb

@@ -30,9 +30,14 @@ end
 
 def group_assignment_discussion(opts = {})
   course = opts[:course] || course_model(:reusable => true)
-  assignment_model(:course => course, :group_category => 'Project Group', :submission_types => 'discussion_topic', :title => 'Group Assignment Discussion')
-  group_model(:name => 'Project Group 1', :group_category => @group_category, :context => course)
+  assignment_model(:course => course, :submission_types => 'discussion_topic', :title => 'Group Assignment Discussion')
+
   @root_topic = DiscussionTopic.find_by_assignment_id(@assignment.id)
+  @group_category = course.group_categories.create(:name => 'Project Group')
+  group_model(:name => 'Project Group 1', :group_category => @group_category, :context => course)
+  @root_topic.group_category = @group_category
+  @root_topic.save!
+
   @root_topic.refresh_subtopics
   @topic = @group.discussion_topics.find_by_root_topic_id(@root_topic.id)
 end

spec/migrations/populate_group_category_on_discussion_topics_spec.rb

@@ -25,9 +25,10 @@ describe DataFixup::PopulateGroupCategoryOnDiscussionTopics do
     group_category = @course.group_categories.create(:name => "category")
     @group = @course.groups.create(:name => "group", :group_category => group_category)
 
-    assignment1 = course.assignments.build(:submission_types => 'discussion_topic', :title => 'a1')
-    assignment1.group_category = group_category
-    assignment1.save!
+    assignment1 = course.assignments.create!(:submission_types => 'discussion_topic', :title => 'a1')
+    # bypass validation
+    Assignment.where(id: assignment1).update_all(group_category_id: group_category)
+    assignment1.reload
     topic1 = @course.discussion_topics.create!(:title => "topic 1")
     topic1.assignment = assignment1
     topic1.save!