ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

creating submission comments no longer goes to author's inbox 

To support instructors of MOOCs, making a submission comment
via gradebook or speedgrader should no longer create a
conversation message in the instructor's inbox. The
conversation will only exist in the sent folder.

If a private conversation between the teacher and student
already exists, the message will be added to the private
conversation and its state will not be modified for the teacher.

In general, the behavior of submission comments was changed so
that creating a submission comment does not create a message in
the author's inbox. Instead the message should appear in the
author's sent folder. This applies to all users (teachers and
students).

fixes #CNVS-1162

test plan:
 - As a teacher, create a submission comment from gradebook.
 Make sure the comment doesn't show up in the teacher's inbox.
 - As a teacher, create a submission comment from speedgrader.
 Make sure the comment doesn't show up in the teacher's inbox.

 - In general, create submission comments as a teacher and a
 student and run through the following combinations of states:

 - Existing Private Conversations:
   - If a private conversation between the student and the
   teacher already exists, the state of the message should not
   be updated for the submission comment author.
   - If a private conversation between the student and the
   teacher does not already exist, the message should only show
   up in the sent folder for the submission comment author.

 - Notification Preference: Mark new submission comments as read
   - Setting this should still prevent incoming submission
   comment messages from showing up in the instructor's inbox
   or changing the state of an existing private conversation.

 - Muted Assignments:
   - When an assignment is unmuted and only one user has made
   submission comments, that user should be treated as the
   message author.
   - When an assignment is unmuted and more than one user has
   commented on the submission, the message is treated as a new
   message for everyone.

Change-Id: Ic55cc64f181e8a0c560fe325bd31ea65082568d2
Reviewed-on: https://gerrit.instructure.com/16007
Reviewed-by: Zach Pendleton <zachp@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: Marc LeGendre <marc@instructure.com>
This commit is contained in:
Jon Willesen 2012-12-07 15:02:54 -07:00
parent 953a593f79
commit bb22a92259
3 changed files with 54 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/helpers/stream_items_helper.rb
View File

@ -43,7 +43,7 @@ module StreamItemsHelper
# this needs to be queried relative to the user
user.conversation_participant(item.asset_id)
end
next if participant.last_author?
next if participant.last_message.nil? || participant.last_author?
item.data.write_attribute(:last_message, participant.last_message)
item.data.write_attribute(:last_author, item.data.last_message.author)

6
app/models/submission.rb
View File

@ -907,12 +907,8 @@ class Submission < ActiveRecord::Base
case trigger
when :create
options[:update_participants] = true
options[:update_for_skips] = false
options[:skip_ids] = overrides[:skip_ids] || [conversation_message_data[:author_id]] # don't mark-as-unread for the author
if commenting_instructors.empty?
# until the first instructor comments, we don't want the submitter to see
# the message in existing conversations with anyone
options[:update_for_skips] = false
end
participating_instructors.each do |t|
# Check their settings and add to :skip_ids if set to suppress.
options[:skip_ids] << t.id if t.preferences[:no_submission_comments_inbox] == true

57
spec/models/submission_comment_spec.rb
View File

@ -251,6 +251,31 @@ This text has a http://www.google.com link in it...
tconvo.reload.should be_read
end
context "teacher makes first submission comment" do
it "should only show as sent for the teacher if private converstation does not already exist" do
@submission1.add_comment(:author => @teacher1, :comment => "test comment")
@teacher1.conversations.should be_empty
@teacher1.all_conversations.size.should eql 1
@teacher1.all_conversations.sent.size.should eql 1
end
it "should reuse an existing private conversation, but not change its state for teacher" do
convo = Conversation.initiate([@teacher1.id, @student1.id], true)
convo.add_message(@teacher1, 'direct message')
@teacher1.conversations.count.should == 1
convo = @teacher1.conversations.first
convo.workflow_state = 'archived'
convo.save!
@teacher1.reload.conversations.default.should be_empty
@submission1.add_comment(:author => @teacher1, :comment => "test comment")
@teacher1.reload
@teacher1.all_conversations.size.should eql 1
@teacher1.conversations.default.should be_empty
@teacher1.all_conversations.archived.size.should eql 1
end
end
context "with no_submission_comments_inbox" do
context "when teacher sets after conversation started" do
before :each do
@ -332,10 +357,11 @@ This text has a http://www.google.com link in it...
it "should update conversations when assignments are unmuted" do
@submission1.add_comment(:author => @teacher1, :comment => "!", :hidden => true)
@teacher1.conversations.size.should eql 0
@teacher1.all_conversations.sent.size.should eql 0
@student1.conversations.size.should eql 0
@assignment.unmute!
@teacher1.reload.conversations.size.should eql 1
@teacher1.conversations.first.should be_read
@teacher1.reload.conversations.size.should eql 0
@teacher1.all_conversations.sent.size.should eql 1
@student1.reload.conversations.size.should eql 1
@student1.conversations.first.should be_unread
end
@ -387,20 +413,41 @@ This text has a http://www.google.com link in it...
c3 = @submission1.add_comment(:author => @teacher2, :comment => "no", :hidden => true)
@student1.conversations.size.should eql 0
@teacher1.conversations.size.should eql 0
@teacher1.all_conversations.sent.size.should eql 0
t2convo = @teacher2.conversations.first
t2convo.workflow_state = :read
t2convo.save!
@assignment.unmute!
t1convo = @teacher1.reload.conversations.first
t1convo.should_not be_nil
t1convo.should be_read
# If there is more than one author in the set of submission comments,
# then it is treated as a new message for everyone.
@teacher1.reload.conversations.should be_empty
@teacher1.all_conversations.size.should eql 1
@teacher1.all_conversations.sent.size.should eql 0
t2convo.reload.should be_unread
@student1.reload.conversations.size.should eql 2
@student1.conversations.first.should be_unread
@student1.conversations.last.should be_unread
end
it "should reuse an existing private conversation, but not change its state for teacher on unmute" do
convo = Conversation.initiate([@teacher1.id, @student1.id], true)
convo.add_message(@teacher1, 'direct message')
@teacher1.conversations.count.should == 1
convo = @teacher1.conversations.first
convo.workflow_state = 'archived'
convo.save!
@submission1.add_comment(:author => @teacher1, :comment => "test comment")
@assignment.unmute!
@teacher1.reload.conversations.default.should be_empty
@teacher1.all_conversations.size.should eql 1
@teacher1.all_conversations.archived.size.should eql 1
@teacher1.all_conversations.sent.size.should eql 1
end
end
context "deletion" do