uncheck group comment by default, fixes #7206

also added checkbox (and api support, etc.) to assignment submission
forms and gradebook2

test plan:
1.  create a group assignment
2.  as a student, go to submit the assignment
3.  ensure the group comment checkbox is present and unchecked on all
    submission forms (file upload, text entry, etc.)
4.  ensure that checking it sends a group comment and leaving it
    unchecked does not
5.  go to the submission details
6.  ensure that the group comment checkbox is unchecked by default
7.  as a teacher, go to the speedgrader and ensure the checkbox is
    unchecked
8.  go to the gradebook and ensure the checkbox is unchecked
9.  go to gradebook2 and ensure it is unchecked and functional (wasn't
    present before)
10. delete all comments and go to the dashboard. ensure that when you go
    to add the first comment, you have a working group comment checkbox
    (if there are existing comments on the stream item, you won't have a
    real comment form, just a link that looks like a text area)

Change-Id: I009b797c2b391c20f41f33c7c0b8afdd4b57dbd3
Reviewed-on: https://gerrit.instructure.com/8978
Reviewed-by: Jon Jensen <jon@instructure.com>
Tested-by: Hudson <hudson@instructure.com>

app/controllers/assignments_api_controller.rb

@@ -42,6 +42,7 @@ class AssignmentsApiController < ApplicationController
   # @response_field rubric [Rubric]
   #   A list of rows and ratings for each row. TODO: need more discussion of the
   #   rubric data format and usage for grading.
+  # @response_field group_category_id [Integer] The unique identifier of the assignment's group set (if this is a group assignment)
   #
   # @example_response
   #   [
@@ -57,7 +58,7 @@ class AssignmentsApiController < ApplicationController
   #         "online_text_entry",
   #         "online_url",
   #         "media_recording"
-  #        ]
+  #        ],
   #       "use_rubric_for_grading": true,
   #       "rubric": [
   #         {
@@ -99,7 +100,8 @@ class AssignmentsApiController < ApplicationController
   #           "id": "crit2",
   #           "description": "Crit2"
   #         }
-  #       ]
+  #       ],
+  #       "group_category_id: 1
   #     }
   #   ]
   def index

app/controllers/submissions_api_controller.rb

@@ -164,6 +164,8 @@ class SubmissionsApiController < ApplicationController
   #
   # @argument comment[text_comment] Add a textual comment to the submission.
   #
+  # @argument comment[group_comment] [Boolean] Whether or not this comment should be sent to the entire group (defaults to false). Ignored if this is not a group assignment or if no text_comment is provided.
+  #
   # @argument submission[posted_grade] Assign a score to the submission,
   #   updating both the "score" and "grade" fields on the submission record.
   #   This parameter can be passed in a few different formats:
@@ -254,8 +256,9 @@ class SubmissionsApiController < ApplicationController
           # but we need to implement a way to abstract it away from kaltura and
           # make it generic. This will probably involve a proxy outside of
           # rails.
-          comment.slice(:media_comment_id, :media_comment_type))
-          @submission.add_comment(comment)
+          comment.slice(:media_comment_id, :media_comment_type, :group_comment)
+        ).with_indifferent_access
+        @assignment.update_submission(@submission.user, comment)
       end
       # We need to reload because some of this stuff is getting set on the
       # submission without going through the model instance -- it'd be nice to

app/models/assignment.rb

@@ -956,7 +956,7 @@ class Assignment < ActiveRecord::Base
     end
 
     students.each do |student|
-      if (opts['comment'] && opts['group_comment'] == "1") || student == original_student
+      if (opts['comment'] && Canvas::Plugin.value_to_boolean(opts['group_comment'])) || student == original_student
         s = self.find_or_create_submission(student)
         s.assignment_id = self.id
         s.user_id = student.id
@@ -974,11 +974,12 @@ class Assignment < ActiveRecord::Base
     # Only allow a few fields to be submitted.  Cannot submit the grade of a
     # homework assignment, for instance.
     opts.keys.each { |k|
-      opts.delete(k) unless [:body, :url, :attachments, :submission_type, :comment, :media_comment_id, :media_comment_type].include?(k.to_sym)
+      opts.delete(k) unless [:body, :url, :attachments, :submission_type, :comment, :media_comment_id, :media_comment_type, :group_comment].include?(k.to_sym)
     }
     raise "Student Required" unless original_student
     raise "User must be enrolled in the course as a student to submit homework" unless context.students.include?(original_student)
-    comment = opts.delete :comment
+    comment = opts.delete(:comment)
+    group_comment = opts.delete(:group_comment)
     group, students = group_students(original_student)
     homeworks = []
     primary_homework = nil
@@ -1018,7 +1019,7 @@ class Assignment < ActiveRecord::Base
     primary_homework.broadcast_group_submission if group
     homeworks.each do |homework|
       context_module_action(homework.student, :submitted)
-      homework.add_comment({:comment => comment, :author => original_student, :unique_key => ts}) if comment
+      homework.add_comment({:comment => comment, :author => original_student, :unique_key => ts}) if comment && (group_comment || homework == primary_homework)
     end
     touch_context
     return primary_homework

app/models/stream_item.rb

@@ -152,7 +152,7 @@ class StreamItem < ActiveRecord::Base
     when Submission
       res = object.attributes
       res.delete 'body' # this can be pretty large, and we don't display it
-      res['assignment'] = object.assignment.attributes.slice('id', 'title', 'due_at', 'points_possible', 'submission_types')
+      res['assignment'] = object.assignment.attributes.slice('id', 'title', 'due_at', 'points_possible', 'submission_types', 'group_category_id')
       res[:submission_comments] = object.submission_comments.map do |comment|
         hash = comment.attributes
         hash['formatted_body'] = comment.formatted_body(250)

app/stylesheets/submissionDetailsDialog.sass

@@ -68,6 +68,11 @@
   #add_a_comment
     width: 99%
     display: block
+  #group-comment-container
+    float: left
+    label
+      display: inline
+      font-weight: normal
   .button
     float: right
     

app/views/assignments/_group_comment.html.erb

@@ -0,0 +1,10 @@
+<% if @assignment.has_group_category? && can_do(@assignment.submissions.build(:user => @current_user), @current_user, :make_group_comment) %>
+<tr>
+  <td colspan="2">
+    <label>
+      <input type="checkbox" name="submission[group_comment]" />
+      <%= t('labels.group_comment', 'Send Comment to the Whole Group') %>
+    </label>
+  </td>
+</tr>
+<% end %>

app/views/assignments/_submit_assignment.html.erb

@@ -65,6 +65,7 @@
           <%= text_area :submission, :comment, :style => "width: 100%; height: 75px;" %>
         </td>
       </tr>
+      <%= render :partial => "group_comment" %>
       <% if @assignment.turnitin_enabled? %>
         <%= render :partial => "turnitin" %>
       <% end %>
@@ -104,6 +105,7 @@
           <%= text_area :submission, :comment, :style => "width: 100%; height: 75px;" %>
         </td>
       </tr>
+      <%= render :partial => "group_comment" %>
       <% if @assignment.turnitin_enabled? %>
         <%= render :partial => "turnitin" %>
       <% end %>
@@ -139,7 +141,9 @@
           </div>
           <%= text_area :submission, :comment, :style => "width: 100%; height: 75px;" %>
         </td>
-      </tr><tr>
+      </tr>
+      <%= render :partial => "group_comment" %>
+      <tr>
         <td colspan="2" class='button-container'>
           <button type="submit" class="button"><%= t 'buttons.submit_assignment', "Submit Assignment" %></button>
           <button type="button" class='cancel_button button-secondary'><%= t '#buttons.cancel', "Cancel" %></button>
@@ -222,6 +226,7 @@
               <%= text_area :submission, :comment, :style => "width: 100%; height: 75px;" %>
             </td>
           </tr>
+          <%= render :partial => "group_comment" %>
           <% if @assignment.turnitin_enabled? %>
             <%= render :partial => "turnitin" %>
           <% end %>
@@ -288,7 +293,9 @@
               </div>
               <%= text_area :submission, :comment, :style => "width: 100%; height: 75px;" %>
             </td>
-          </tr><tr>
+          </tr>
+          <%= render :partial => "group_comment" %>
+          <tr>
             <td colspan="2" class='button-container'>
               <button type="submit" class="button"><%= t 'buttons.submit_assignment', "Submit Assignment" %></button>
               <button type="button" class='cancel_button button-secondary'><%= t '#buttons.cancel', "Cancel" %></button>

app/views/context/_dashboard_submission_comment.html.erb

@@ -25,8 +25,12 @@
     <% if !comment %>
      <% form_for :submission, :url => "#{context_prefix(context_code)}/assignments/#{submission ? submission.assignment_id : '{{ assignment_id }}'}/submissions/#{submission ? submission.user_id : '{{ user_id }}'}", :html => {:method => 'put', :class => "add_sub_message_form submission_comment_form"} do |f| %>
         <input type="hidden" name="from_dashboard" value="1"/>
-        <%= f.hidden_field :group_comment, :value => '1'%>
         <%= f.text_area :comment, :style => "height: 50px;" %>
+        <% if submission && submission.assignment.group_category_id %>
+          <br/>
+          <input type="checkbox" name="submission[group_comment]" id="submission_group_comment"/>
+          <label for="submission_group_comment"><%= t('labels.group_comment', 'Send Comment to the Whole Group') %></label>
+        <% end %>
         <div style="float: right;">
           <a href="<%= context_prefix(context_code) %>/assignments/<%= submission ? submission.assignment_id : "{{ assignment_id }}" %>/submissions/<%= submission ? submission.user_id : "{{ user_id }}" %>#comment" style="padding-right: 10px;" class="more_options_reply_link"><%= t('#links.more_options', %{more options}) %></a>
         </div>

app/views/jst/SubmissionDetailsDialog.handlebars

@@ -61,6 +61,12 @@
   <form class="submission_details_add_comment_form clearfix">
     <label for="add_a_comment">{{#t "add_a_comment"}}Add a comment{{/t}}</label>
     <textarea id="add_a_comment" name="comment[text_comment]"></textarea>
+    {{#if assignment.group_category_id}}
+    <span id="group-comment-container">
+      <input type="checkbox" id="group_comment" name="comment[group_comment]" />
+      <label for="group_comment">{{#t "labels.group_comment"}}Send Comment to the Whole Group{{/t}}</label>
+    </span>
+    {{/if}}
     <button class="button" type="submit">{{#t "post_comment"}}Post Comment{{/t}}</button>
   </form>
 </div>

app/views/submissions/show.html.erb

@@ -211,7 +211,7 @@
             </div>
             <% if @assignment.has_group_category? && can_do(@submission, @current_user, :make_group_comment) %>
               <br/>
-              <input type="checkbox" name="group_comment" id="submission_group_comment" checked/>
+              <input type="checkbox" name="group_comment" id="submission_group_comment"/>
               <label for="submission_group_comment"><%= t('sent_to_whole_group', 'Send Comment to the Whole Group') %></label>
             <% end %>
             <div class="button-container">

lib/api/v1/assignment.rb

@@ -22,7 +22,7 @@ module Api::V1::Assignment
 
   def assignment_json(assignment, user, session, includes = [], show_admin_fields = false)
     # no includes supported right now
-    hash = api_json(assignment, user, session, :only => %w(id grading_type points_possible position due_at description assignment_group_id))
+    hash = api_json(assignment, user, session, :only => %w(id grading_type points_possible position due_at description assignment_group_id group_category_id))
 
     hash['course_id'] = assignment.context_id
     hash['name'] = assignment.title

public/javascripts/gradebooks.js

@@ -1680,7 +1680,7 @@ require([
       .find(".submission_comments").empty().end()
       .find(".comment_attachments").empty().end()
       .find(".save_buttons,.add_comment").showIf(!readOnlyGradebook).end()
-      .find(".group_comment").showIf(assignment && assignment.group_category).find(":checkbox").attr('checked', true).end().end();
+      .find(".group_comment").showIf(assignment && assignment.group_category).find(":checkbox").attr('checked', false).end().end();
 
     if(readOnlyGradebook) {
       $submission_information.find(".grade_entry").text(grade || "-");

spec/apis/v1/assignment_groups_api_spec.rb

@@ -125,6 +125,7 @@ describe AssignmentGroupsController, :type => :integration do
                 ],
               },
             ],
+            'group_category_id' => nil
           },
           {
             'id' => a4.id,
@@ -141,6 +142,7 @@ describe AssignmentGroupsController, :type => :integration do
               "none",
             ],
             'grading_type' => 'points',
+            'group_category_id' => nil
           },
         ],
       },
@@ -166,6 +168,7 @@ describe AssignmentGroupsController, :type => :integration do
               "none",
             ],
             'grading_type' => 'points',
+            'group_category_id' => nil
           },
           {
             'id' => a2.id,
@@ -182,6 +185,7 @@ describe AssignmentGroupsController, :type => :integration do
               "none",
             ],
             'grading_type' => 'points',
+            'group_category_id' => nil
           },
         ],
       },

spec/apis/v1/assignments_api_spec.rb

@@ -105,6 +105,7 @@ describe AssignmentsApiController, :type => :integration do
               ],
             },
           ],
+          "group_category_id" => nil
         },
       ]
     end
@@ -160,6 +161,7 @@ describe AssignmentsApiController, :type => :integration do
       'submission_types' => [
         'none',
       ],
+      'group_category_id' => nil
     }
 
     Assignment.count.should == 1
@@ -201,6 +203,7 @@ describe AssignmentsApiController, :type => :integration do
       'submission_types' => [
         'none',
       ],
+      'group_category_id' => nil
     }
 
     Assignment.count.should == 1

spec/apis/v1/submissions_api_spec.rb

@@ -1155,6 +1155,35 @@ describe 'Submissions API', :type => :integration do
     json['submission_comments'].first['comment'].should == 'ohai!'
   end
 
+  it "should allow posting a group comment on a submission" do
+    student1 = user(:active_all => true)
+    student2 = user(:active_all => true)
+    course_with_teacher(:active_all => true)
+    @course.enroll_student(student1).accept!
+    @course.enroll_student(student2).accept!
+    group_category = @course.group_categories.create(:name => "Category")
+    @group = @course.groups.create(:name => "Group", :group_category => group_category, :context => @course)
+    @group.users = [student1, student2]
+    @assignment = @course.assignments.create!(:title => 'assignment1', :grading_type => 'points', :points_possible => 12, :group_category => group_category)
+    submit_homework(@assignment, student1)
+
+    json = api_call(:put,
+          "/api/v1/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{student1.id}.json",
+          { :controller => 'submissions_api', :action => 'update',
+            :format => 'json', :course_id => @course.id.to_s,
+            :assignment_id => @assignment.id.to_s, :id => student1.id.to_s },
+          { :comment =>
+            { :text_comment => "ohai!", :group_comment => "1" } })
+    json['submission_comments'].size.should == 1
+    json['submission_comments'].first['comment'].should == 'ohai!'
+
+    Submission.count.should == 2
+    Submission.all.each do |submission|
+      submission.submission_comments.size.should eql 1
+      submission.submission_comments.first.comment.should eql 'ohai!'
+    end
+  end
+
   it "should allow posting a media comment on a submission, given a kaltura id" do
     student = user(:active_all => true)
     course_with_teacher(:active_all => true)

spec/controllers/submissions_controller_spec.rb

@@ -100,6 +100,33 @@ describe SubmissionsController do
       assigns[:submission].should_not be_nil
       assigns[:submission].url.should eql("http://www.google.com")
     end
+
+    context "group comments" do
+      before do
+        course_with_student_logged_in(:active_all => true)
+        @u1 = @user
+        student_in_course(:course => @course)
+        @u2 = @user
+        @assignment = @course.assignments.create!(:title => "some assignment", :submission_types => "online_text_entry", :group_category => GroupCategory.create!(:name => "groups", :context => @course), :grade_group_students_individually => true)
+        @group = @assignment.group_category.groups.create!(:name => 'g1')
+        @group.users << @u1
+        @group.users << @user
+      end
+
+      it "should not send a comment to the entire group by default" do
+        post 'create', :course_id => @course.id, :assignment_id => @assignment.id, :submission => {:submission_type => 'online_text_entry', :body => 'blah', :comment => "some comment"}
+        subs = @assignment.submissions
+        subs.size.should == 2
+        subs.all.sum{ |s| s.submission_comments.size }.should eql 1
+      end
+
+      it "should send a comment to the entire group if requested" do
+        post 'create', :course_id => @course.id, :assignment_id => @assignment.id, :submission => {:submission_type => 'online_text_entry', :body => 'blah', :comment => "some comment", :group_comment => '1'}
+        subs = @assignment.submissions
+        subs.size.should == 2
+        subs.all.sum{ |s| s.submission_comments.size }.should eql 2
+      end
+    end
   end
   
   describe "PUT update" do

spec/models/assignment_spec.rb

@@ -1268,6 +1268,15 @@ describe Assignment do
       res.map{|s| s.user}.should be_include(@u1)
       res.map{|s| s.user}.should be_include(@u2)
     end
+    it "should create an initial submission comment for only the submitter by default" do
+      setup_assignment_with_group
+      sub = @a.submit_homework(@u1, :submission_type => "online_text_entry", :body => "Some text for you", :comment => "hey teacher, i hate my group. i did this entire project by myself :(")
+      sub.user_id.should eql(@u1.id)
+      sub.submission_comments.size.should eql 1
+      @a.reload
+      other_sub = (@a.submissions - [sub])[0]
+      other_sub.submission_comments.size.should eql 0
+    end
     it "should add a submission comment for only the specified user by default" do
       setup_assignment_with_group
       res = @a.grade_student(@u1, :comment => "woot")
@@ -1286,6 +1295,15 @@ describe Assignment do
       res.length.should eql(1)
       res[0].user.should eql(@u1)
     end
+    it "should create an initial submission comment for all group members if specified" do
+      setup_assignment_with_group
+      sub = @a.submit_homework(@u1, :submission_type => "online_text_entry", :body => "Some text for you", :comment => "ohai teacher, we had so much fun working together", :group_comment => "1")
+      sub.user_id.should eql(@u1.id)
+      sub.submission_comments.size.should eql 1
+      @a.reload
+      other_sub = (@a.submissions - [sub])[0]
+      other_sub.submission_comments.size.should eql 1
+    end
     it "should add a submission comment for all group members if specified" do
       setup_assignment_with_group
       res = @a.grade_student(@u1, :comment => "woot", :group_comment => "1")

spec/selenium/assignments_spec.rb

@@ -255,5 +255,19 @@ describe "assignments" do
         Submission.count.should == 0
       end
     end
+
+    it "should have group comment checkboxes for group assignments" do
+      @u1 = @user
+      student_in_course(:course => @course)
+      @u2 = @user
+      @assignment = @course.assignments.create!(:title => "some assignment", :submission_types => "online_url,online_upload,online_text_entry", :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
+
+      get "/courses/#{@course.id}/assignments/#{@assignment.id}"
+
+      find_all_with_jquery('table.formtable input[name="submission[group_comment]"]').size.should eql 3
+    end
   end
 end

spec/selenium/gradebook2_spec.rb

@@ -65,9 +65,9 @@ describe "gradebook2" do
       element_to_click.click if element_to_click != nil
     end
 
-    def open_comment_dialog
+    def open_comment_dialog(x=0, y=0)
       #move_to occasionally breaks in the hudson build
-      cell = driver.execute_script "return $('#gradebook_grid .slick-row:first .slick-cell:first').addClass('hover')[0]"
+      cell = driver.execute_script "return $('#gradebook_grid .slick-row:nth-child(#{y+1}) .slick-cell:nth-child(#{x+1})').addClass('hover')[0]"
       cell.find_element(:css, '.gradebook-cell-comment').click
       # the dialog fetches the comments async after it displays and then innerHTMLs the whole
       # thing again once it has fetched them from the server, completely replacing it
@@ -505,6 +505,37 @@ describe "gradebook2" do
       comment.should include_text(comment_text)
     end
 
+    it "should let you post a group comment to a group assignment" do
+      group_assignment = assignment_model({
+                                               :course => @course,
+                                               :name => 'group assignment',
+                                               :due_at => (Time.now + 1.week),
+                                               :points_possible => ASSIGNMENT_3_POINTS,
+                                               :submission_types => 'online_text_entry',
+                                               :assignment_group => @group,
+                                               :group_category => GroupCategory.create!(:name => "groups", :context => @course),
+                                               :grade_group_students_individually => true
+                                          })
+      project_group = group_assignment.group_category.groups.create!(:name => 'g1', :context => @course)
+      project_group.users << @student_1
+      project_group.users << @student_2
+
+      comment_text = "This is a new group comment!"
+
+      get "/courses/#{@course.id}/gradebook2"
+      wait_for_ajaximations
+
+      dialog = open_comment_dialog(3)
+      set_value(dialog.find_element(:id, "add_a_comment"), comment_text)
+      dialog.find_element(:id, "group_comment").click
+      driver.find_element(:css, "form.submission_details_add_comment_form.clearfix > button.button").click
+      wait_for_ajaximations
+
+      #make sure it's on the other student's submission
+      comment = open_comment_dialog(3, 1).find_element(:css, '.comment')
+      comment.should include_text(comment_text)
+    end
+
     it "should validate assignment details" do
       submissions_count = @second_assignment.submissions.count.to_s + ' submissions'