Graded discussions with new replies show up in new activity API

fixes ADMIN-105

Test plan
- Create a graded discussion
- As user 1 (probably a teacher), add a comment to the discussion
- As user 2 (probably a student), go to
  /api/v1/planner/items?filter=new_activity
- Ensure the graded discussion shows up in the API
- Ensure it doesn't show up twice in /api/v1/planner/items (or
  anywhere else it shouldn't)

Change-Id: I21a88a4e5181e2a43ce5d03cf92230d3107d5093
Reviewed-on: https://gerrit.instructure.com/132096
Tested-by: Jenkins
Reviewed-by: Dan Minkevitch <dan@instructure.com>
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Product-Review: Pert Eilers <peilers@instructure.com>

app/controllers/planner_overrides_controller.rb

@@ -325,7 +325,7 @@ class PlannerOverridesController < ApplicationController
   def unread_discussion_topic_collection
     item_collection('unread_discussion_topics',
                     @current_user.discussion_topics_needing_viewing(scope_only: true, include_ignored: true,
-                      due_before: end_date, due_after: start_date).
+                      due_before: end_date, due_after: start_date, new_activity: true).
                       unread_for(@current_user),
                     DiscussionTopic, [:todo_date, :posted_at, :delayed_post_at, :last_reply_at, :created_at], :id)
   end

app/models/user.rb

@@ -1650,7 +1650,7 @@ class User < ActiveRecord::Base
 
       if opts[:scope_only]
         Shard.partition_by_shard(course_ids) do |shard_course_ids|
-          next unless Shard.current == original_shard # only provideo scope on current shard
+          next unless Shard.current == original_shard # only provide scope on current shard
           return yield(*arguments_for_needing_viewing(object_type, shard_course_ids, opts))
         end
         return object_type.constantize.none
@@ -1673,8 +1673,13 @@ class User < ActiveRecord::Base
   def arguments_for_needing_viewing(object_type, shard_course_ids, opts)
     scope = object_type.constantize.for_courses_and_groups(shard_course_ids, cached_current_group_memberships.map(&:group_id))
     scope = scope.not_ignored_by(self, 'viewing') unless opts[:include_ignored]
-    scope = scope.todo_date_between(opts[:due_after], opts[:due_before])
+    scope_todo = scope.todo_date_between(opts[:due_after], opts[:due_before])
-    [scope, opts.merge(:shard_course_ids => shard_course_ids)]
+    if object_type == 'DiscussionTopic' && opts[:new_activity]
+      scope_todo = scope_todo.or(scope.where(assignment_id:
+        Assignment.active.where(context_id: shard_course_ids, context_type: 'Course', submission_types: 'discussion_topic').
+        due_between_with_overrides(opts[:due_after], opts[:due_before]).pluck(:id)))
+    end
+    [scope_todo, opts.merge(:shard_course_ids => shard_course_ids)]
   end
 
   def discussion_topics_needing_viewing(opts={})

lib/api/v1/planner_item.rb

@@ -35,6 +35,7 @@ module Api::V1::PlannerItem
   }.freeze
 
   def planner_item_json(item, user, session, opts = {})
+    item = item.assignment if item.respond_to?(:assignment) && item.assignment
     context_data(item).merge({
       :plannable_id => item.id,
       :plannable_date => item.planner_date,

spec/controllers/planner_overrides_controller_spec.rb

@@ -507,6 +507,30 @@ describe PlannerOverridesController do
               get :items_index, params: {filter: "new_activity"}
               expect(json_parse(response.body).length).to eq 1
             end
+
+            it "should return graded discussions with unread replies" do
+              @topic.change_read_state('read', @student)
+              assign = assignment_model(course: @course, due_at: Time.zone.now)
+              topic = @course.discussion_topics.create!(course: @course, assignment: assign)
+              topic.change_read_state('read', @student)
+
+              get :items_index, params: {filter: "new_activity"}
+              expect(json_parse(response.body)).to be_empty
+
+              entry = topic.discussion_entries.create!(:message => "Hello!", :user => @student)
+              reply = entry.reply_from(:user => @teacher, :text => "ohai!")
+              topic.reload
+
+              get :items_index, params: {filter: "new_activity"}
+              response_json = json_parse(response.body)
+              expect(response_json.length).to eq 1
+              expect(response_json.first["plannable_id"]).to eq assign.id
+              expect(response_json.first["plannable"]["id"]).to eq topic.id
+
+              reply.change_read_state('read', @student)
+              get :items_index, params: {filter: "new_activity"}
+              expect(json_parse(response.body)).to be_empty
+            end
           end
         end
       end

spec/factories/discussion_topic_factory.rb

@@ -80,6 +80,7 @@ module Factories
     @assignment.saved_by = :discussion_topic
     @topic.assignment = @assignment
     @topic.save!
+    @assignment.reload
     @topic
   end
 

spec/lib/api/v1/planner_item_spec.rb

@@ -77,6 +77,13 @@ describe Api::V1::PlannerItem do
       expect(annc_hash[:plannable_date]).to eq annc_post_date
     end
 
+    it 'should show the assignment for the plannable_id if one exists' do
+      asg = assignment_model course: @couse, submission_types: 'online_text_entry'
+      dt = @course.discussion_topics.create!(assignment: asg, title: 'Assignment DT')
+      dt_hash = api.planner_item_json(dt, @student, session)
+      expect(dt_hash[:plannable_id]).to eq asg.id
+    end
+
     context 'with an existing planner override' do
       it 'should return the planner visibility state' do
         teacher_hash = api.planner_item_json(@assignment, @teacher, session)

spec/models/user_spec.rb

@@ -2342,8 +2342,8 @@ describe User do
         expect(@student.discussion_topics_needing_viewing(opts)).to eq []
       end
 
-      it 'should not show discussions that are graded' do
+      it 'should not show discussions that are graded unless new_activity is true' do
-        a = @course.assignments.create!(title: "some assignment", points_possible: 5)
+        a = @course.assignments.create!(title: "some assignment", points_possible: 5, due_at: 1.day.from_now)
         t = @course.discussion_topics.build(assignment: a, title: "some topic", message: "a little bit of content")
         t.save
         expect(t.assignment_id).to eql(a.id)
@@ -2351,6 +2351,15 @@ describe User do
         expect(@student.discussion_topics_needing_viewing(opts)).not_to include t
       end
 
+      it 'should show graded discussion if new activity is true' do
+        a = @course.assignments.create!(title: "some assignment", points_possible: 5, due_at: 1.day.from_now)
+        t = @course.discussion_topics.build(assignment: a, title: "some topic", message: "a little bit of content")
+        t.save
+        expect(t.assignment_id).to eql(a.id)
+        expect(t.assignment).to eql(a)
+        expect(@student.discussion_topics_needing_viewing(opts.merge({new_activity: true}))).to include t
+      end
+
       context "locked discussion topics" do
         it 'should show for ungraded discussion topics with unlock dates and todo dates within the opts date range' do
           @topic.unlock_at = 1.day.from_now