fix stream item hiding with submission comments from students

test plan:
* create an assignment
* as a student,
 submit to the assignment and add a submission comment
* mute the assignment
* dashboard notifications for instructors should still be shown

* adding another submission comment while still muted should also
 show another dashboard notification

closes #CNVS-30670

Change-Id: I734ca0a53699bb6b0ba7bd6a76dc919b9ccf56d9
Reviewed-on: https://gerrit.instructure.com/86724
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Heath Hales <hhales@instructure.com>
Product-Review: James Williams  <jamesw@instructure.com>

app/models/stream_item.rb

@@ -249,10 +249,6 @@ class StreamItem < ActiveRecord::Base
     res = StreamItem.generate_or_update(object)
     prepare_object_for_unread(object)
 
-    # set the hidden flag if an assignment and muted
-    hidden = object.is_a?(Submission) && object.assignment.muted? ? true : false
-
-
     l_context_type = res.context_type
     Shard.partition_by_shard(user_ids) do |user_ids_subset|
       #these need to be determined per shard
@@ -267,12 +263,18 @@ class StreamItem < ActiveRecord::Base
         {
           :stream_item_id => stream_item_id,
           :user_id => user_id,
-          :hidden => hidden,
+          :hidden => false,
           :workflow_state => object_unread_for_user(object, user_id),
           :context_type => l_context_type,
           :context_id => l_context_id,
         }
       end
+      if object.is_a?(Submission) && object.assignment.muted?
+        # set the hidden flag if an assignment and muted (for the owner of the submission)
+        if owner_insert = inserts.detect{|i| i[:user_id] == object.user_id}
+          owner_insert[:hidden] = true
+        end
+      end
 
       StreamItemInstance.unique_constraint_retry do
         StreamItemInstance.where(:stream_item_id => stream_item_id, :user_id => user_ids_subset).delete_all

lib/mutable.rb

@@ -50,9 +50,11 @@ module Mutable
           where(:asset_type => 'Submission', :asset_id => submissions).
           preload(:context).to_a
       stream_item_contexts = stream_items.map { |si| [si.context_type, si.context_id] }
-      associated_shards = stream_items.inject([]) { |result, si| result | si.associated_shards }
-      Shard.with_each_shard(associated_shards) do
-        StreamItemInstance.where(:stream_item_id => stream_items).
+      user_ids = submissions.map(&:user_id).uniq # hide stream items for submission owners, not instructors
+      # note: unfortunately this will hide items for an instructor if instructor (somehow) has a submission too
+
+      Shard.partition_by_shard(user_ids) do |user_ids_subset|
+        StreamItemInstance.where(:stream_item_id => stream_items, :user_id => user_ids_subset).
             update_all_with_invalidation(stream_item_contexts, :hidden => true)
       end
     end

spec/models/submission_spec.rb

@@ -353,6 +353,28 @@ describe Submission do
         expect(@user.stream_item_instances.last).to be_hidden
       end
 
+      it "should not create hidden stream_item_instances for instructors when muted, graded, and published" do
+        submission_spec_model
+        @cc = @teacher.communication_channels.create :path => "somewhere"
+        @assignment.mute!
+        expect {
+          @submission.add_comment(:author => @student, :comment => "some comment")
+        }.to change StreamItemInstance, :count
+        expect(@teacher.stream_item_instances.last).to_not be_hidden
+      end
+
+      it "should not hide any existing stream_item_instances for instructors when muted" do
+        submission_spec_model
+        @cc = @teacher.communication_channels.create :path => "somewhere"
+        expect {
+          @submission.add_comment(:author => @student, :comment => "some comment")
+        }.to change StreamItemInstance, :count
+        expect(@teacher.stream_item_instances.last).to_not be_hidden
+        @assignment.mute!
+        @teacher.reload
+        expect(@teacher.stream_item_instances.last).to_not be_hidden
+      end
+
       it "should not create a message for admins and teachers with quiz submissions" do
         course_with_teacher(:active_all => true)
         assignment = @course.assignments.create!(