Skip to content

Commit 51eea02

Browse files
author
Mark Ericksen
committed
adjust discussion unread and total counts when deleting
fixes CNVS-5480 testing direction: ================== using both threaded and unthreaded discussions: * create a discussion using two different users * create discussion replies (entries) and verify the "unread" count correctly changes * delete an "unread" entry before the other user has seen it. verify the unread count is correctly updated. * after deleting entries, verify the "total" count is correct on the index and show pages. * verify that the discussion's list page counts match a specific discussion's show page counts Change-Id: Id29ee1d03a60c6a78565661ee214bf9031fae69c Reviewed-on: https://gerrit.instructure.com/20201 Product-Review: Marc LeGendre <marc@instructure.com> QA-Review: Marc LeGendre <marc@instructure.com> Reviewed-by: Mark Ericksen <marke@instructure.com> Tested-by: Jenkins <jenkins@instructure.com>
1 parent 240e087 commit 51eea02

6 files changed

Lines changed: 80 additions & 2 deletions

File tree

app/models/discussion_entry.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ def destroy
174174
self.deleted_at = Time.now
175175
save!
176176
update_topic_submission
177+
decrement_unread_counts_for_this_entry
177178
end
178179

179180
def update_discussion
@@ -207,6 +208,20 @@ def update_topic_submission
207208
end
208209
end
209210

211+
def decrement_unread_counts_for_this_entry
212+
# decrement unread_entry_count for every participant that has not read the
213+
transaction do
214+
# get a list of users who have not read the entry yet
215+
users = discussion_topic.discussion_topic_participants.
216+
where(['user_id NOT IN (?)', discussion_entry_participants.read.pluck(:user_id)]).pluck(:user_id)
217+
# decrement unread_entry_count for topic participants
218+
if users.present?
219+
DiscussionTopicParticipant.where(:discussion_topic_id => self.discussion_topic_id, :user_id => users).
220+
update_all('unread_entry_count = unread_entry_count - 1')
221+
end
222+
end
223+
end
224+
210225
scope :active, where("discussion_entries.workflow_state<>'deleted'")
211226
scope :deleted, where(:workflow_state => 'deleted')
212227

app/models/discussion_entry_participant.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,6 @@ def self.read_entry_ids(entry_ids, user)
3333
state :unread
3434
state :read
3535
end
36+
37+
scope :read, where(:workflow_state => 'read')
3638
end

app/models/discussion_topic.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ def change_all_read_state(new_state, current_user = nil)
280280
end
281281

282282
def default_unread_count
283-
self.discussion_entries.count
283+
self.discussion_entries.active.count
284284
end
285285

286286
def unread_count(current_user = nil)

app/views/discussion_topics/show.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@
148148

149149
<div class="discussion-header-right pull-right">
150150
<div class="discussion-pubdate"><%= friendly_datetime @topic.created_at %></div>
151-
<%= render :partial => 'new_and_total_badge', :locals => { :unread_count => @topic.unread_count(@current_user), :reply_count => @topic.discussion_entries.size } %>
151+
<%= render :partial => 'new_and_total_badge', :locals => { :unread_count => @topic.unread_count(@current_user), :reply_count => @topic.discussion_subentry_count } %>
152152
</div>
153153
<div class="discussion-title"><%= @topic.title %></div>
154154
<div class="discussion-subtitle">

spec/models/discussion_entry_spec.rb

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,48 @@
224224
end
225225
end
226226

227+
context "deleting entry" do
228+
before :each do
229+
course_with_student(:active_all => true)
230+
@author = @user
231+
@reader = user()
232+
@course.enroll_student(@author)
233+
@course.enroll_student(@reader)
234+
235+
@topic = @course.discussion_topics.create!(:title => "title", :message => "message")
236+
237+
# Create 4 entries, first 2 are 'read' by reader.
238+
@entry_1 = @topic.discussion_entries.create!(:message => "entry 1", :user => @author)
239+
@entry_1.change_read_state('read', @reader)
240+
@entry_2 = @topic.discussion_entries.create!(:message => "entry 2", :user => @author)
241+
@entry_2.change_read_state('read', @reader)
242+
@entry_3 = @topic.discussion_entries.create!(:message => "entry 3", :user => @author)
243+
@entry_4 = @topic.discussion_entries.create!(:message => "entry 4", :user => @author)
244+
end
245+
246+
describe "#destroy" do
247+
it "should call decrement_unread_counts_for_this_entry" do
248+
@entry_4.expects(:decrement_unread_counts_for_this_entry)
249+
@entry_4.destroy
250+
end
251+
end
252+
253+
it "should decrement unread topic counts" do
254+
@topic.unread_count(@reader).should == 2
255+
256+
# delete one read and one unread entry and check again
257+
@entry_1.destroy
258+
@entry_4.destroy
259+
@topic.unread_count(@reader).should == 1
260+
# delete remaining unread
261+
@entry_3.destroy
262+
@topic.unread_count(@reader).should == 0
263+
# delete final 'read' entry
264+
@entry_2.destroy
265+
@topic.unread_count(@reader).should == 0
266+
end
267+
end
268+
227269
it "should touch all parent discussion_topics through root_topic_id, on update" do
228270
course_with_student(:active_all => true)
229271
@topic = @course.discussion_topics.create!(:title => "title", :message => "message")

spec/selenium/discussions_spec.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,25 @@ def edit(title, message)
389389
f('.discussion-topic .icon-rss').should be_displayed
390390
DiscussionTopic.last.podcast_enabled.should be_true
391391
end
392+
393+
it "should exclude deleted entries from unread and total reply count" do
394+
course_with_teacher_logged_in
395+
discussion_topic_model(:user => @teacher)
396+
@student = student_in_course(:active_all => true).user
397+
398+
@assignment = assignment_model(:course => @course)
399+
@topic.assignment = @assignment
400+
@topic.save
401+
402+
# Add two replies, delete one
403+
@topic.reply_from(:user => @student, :text => "entry")
404+
entry = @topic.reply_from(:user => @student, :text => "another entry")
405+
entry.destroy
406+
407+
get "/courses/#{@course.id}/discussion_topics/#{@topic.id}"
408+
f('.new-items').text.should == '1'
409+
f('.total-items').text.should == '1'
410+
end
392411
end
393412

394413
context "editing" do

0 commit comments

Comments
 (0)