Merge pull request doubtfire-lms#473 from b0ink/fix/fetch-my-students-only-inbox

refactor: optional param to fetch tutor's student's tasks only

Author: b0ink

app/api/units_api.rb

@@ -281,14 +281,19 @@ class UnitsApi < Grape::API
   end
 
   desc 'Download the tasks that should be listed under the task inbox'
+  params do
+    optional :my_students_only, type: Boolean, desc: 'Show tasks from all tutorials or just the ones you teach'
+  end
   get '/units/:id/tasks/inbox' do
     unit = Unit.find(params[:id])
 
     unless authorise? current_user, unit, :get_students
       error!({ error: 'Not authorised to provide feedback for this unit' }, 403)
     end
 
-    tasks = unit.tasks_for_task_inbox(current_user)
+    my_students_only = params[:my_students_only] || false
+
+    tasks = unit.tasks_for_task_inbox(current_user, my_students_only)
     present unit.tasks_as_hash(tasks), with: Grape::Presenters::Presenter
   end
 

app/models/unit.rb

@@ -566,6 +566,10 @@ def employ_staff(user, role)
     end
   end
 
+  def unit_role_for(user)
+    unit_roles.find_by(user: user)
+  end
+
   # Adds a user to this project.
   def enrol_student(user, campus)
     # Validates that a student is not already assigned to the unit
@@ -1727,54 +1731,59 @@ def tasks_as_hash(data)
   def tutorial_enrolment_subquery
     tutorial_enrolments
       .joins(:tutorial)
-      .select('tutorials.tutorial_stream_id as tutorial_stream_id', 'tutorials.id as tutorial_id', 'project_id').to_sql
+      .select('tutorials.tutorial_stream_id as tutorial_stream_id', 'tutorials.id as tutorial_id', 'project_id', 'tutorials.unit_role_id as unit_role_id').to_sql
   end
 
   #
   # Return all tasks from the database for this unit and given user
   #
-  def get_all_tasks_for(user)
-    student_tasks.
-      joins(:task_status).
-      joins("LEFT OUTER JOIN (#{tutorial_enrolment_subquery}) as sq ON sq.project_id = projects.id AND (sq.tutorial_stream_id = task_definitions.tutorial_stream_id OR sq.tutorial_stream_id IS NULL)").
-      joins("LEFT JOIN task_comments ON task_comments.task_id = tasks.id AND (task_comments.type IS NULL OR task_comments.type <> 'TaskStatusComment')").
-      joins("LEFT JOIN comments_read_receipts crr ON crr.task_comment_id = task_comments.id AND crr.user_id = #{user.id}").
-      joins("LEFT JOIN task_pins ON task_pins.task_id = tasks.id AND task_pins.user_id = #{user.id}").
-      joins('LEFT OUTER JOIN task_similarities ON tasks.id = task_similarities.task_id').
-      select(
-        'sq.tutorial_id AS tutorial_id',
-        'sq.tutorial_stream_id AS tutorial_stream_id',
-        'tasks.id',
-        "SUM(case when crr.user_id is null AND NOT task_comments.id is null then 1 else 0 end) as number_unread",
-        'COUNT(distinct task_pins.task_id) != 0 as pinned',
-        "SUM(case when task_comments.date_extension_assessed IS NULL AND task_comments.type = 'ExtensionComment' AND NOT task_comments.id IS NULL THEN 1 ELSE 0 END) > 0 as has_extensions",
-        'project_id',
-        'tasks.id as task_id',
-        'task_definition_id',
-        'task_definitions.start_date as start_date',
-        'task_statuses.id as status_id',
-        'completion_date',
-        'times_assessed',
-        'submission_date',
-        'tasks.grade as grade',
-        'quality_pts',
-        'SUM(case when task_similarities.flagged then 1 else 0 end) as similar_to_count'
-      ).
-      group(
-        'sq.tutorial_id',
-        'sq.tutorial_stream_id',
-        'task_statuses.id',
-        'project_id',
-        'tasks.id',
-        'task_definition_id',
-        'task_definitions.start_date',
-        'status_id',
-        'completion_date',
-        'times_assessed',
-        'submission_date',
-        'grade',
-        'quality_pts'
-      )
+  def get_all_tasks_for(user, my_tutorials_only = false)
+    result =  student_tasks.
+              joins(:task_status).
+              joins("LEFT OUTER JOIN (#{tutorial_enrolment_subquery}) as sq ON sq.project_id = projects.id AND (sq.tutorial_stream_id = task_definitions.tutorial_stream_id OR sq.tutorial_stream_id IS NULL)").
+              joins("LEFT JOIN task_comments ON task_comments.task_id = tasks.id AND (task_comments.type IS NULL OR task_comments.type <> 'TaskStatusComment')").
+              joins("LEFT JOIN comments_read_receipts crr ON crr.task_comment_id = task_comments.id AND crr.user_id = #{user.id}").
+              joins("LEFT JOIN task_pins ON task_pins.task_id = tasks.id AND task_pins.user_id = #{user.id}").
+              joins('LEFT OUTER JOIN task_similarities ON tasks.id = task_similarities.task_id').
+              select(
+                'sq.tutorial_id AS tutorial_id',
+                'sq.tutorial_stream_id AS tutorial_stream_id',
+                'tasks.id',
+                "SUM(case when crr.user_id is null AND NOT task_comments.id is null then 1 else 0 end) as number_unread",
+                'COUNT(distinct task_pins.task_id) != 0 as pinned',
+                "SUM(case when task_comments.date_extension_assessed IS NULL AND task_comments.type = 'ExtensionComment' AND NOT task_comments.id IS NULL THEN 1 ELSE 0 END) > 0 as has_extensions",
+                'project_id',
+                'tasks.id as task_id',
+                'task_definition_id',
+                'task_definitions.start_date as start_date',
+                'task_statuses.id as status_id',
+                'completion_date',
+                'times_assessed',
+                'submission_date',
+                'tasks.grade as grade',
+                'quality_pts',
+                'SUM(case when task_similarities.flagged then 1 else 0 end) as similar_to_count'
+              ).
+              group(
+                'sq.tutorial_id',
+                'sq.tutorial_stream_id',
+                'task_statuses.id',
+                'project_id',
+                'tasks.id',
+                'task_definition_id',
+                'task_definitions.start_date',
+                'status_id',
+                'completion_date',
+                'times_assessed',
+                'submission_date',
+                'grade',
+                'quality_pts'
+              )
+    if my_tutorials_only
+      result = result.where('sq.unit_role_id = :unit_role_id', unit_role_id: unit_role_for(user).id)
+    end
+
+    result
   end
 
   #
@@ -1797,8 +1806,8 @@ def tasks_awaiting_feedback(user)
   # time a task has been "actioned", either the submission date or latest
   # student comment -- whichever is newer.
   #
-  def tasks_for_task_inbox(user)
-    get_all_tasks_for(user)
+  def tasks_for_task_inbox(user, my_students_only = false)
+    get_all_tasks_for(user, my_students_only)
       .having('task_statuses.id IN (:ids) OR COUNT(task_pins.task_id) > 0 OR SUM(case when crr.user_id is null AND NOT task_comments.id is null then 1 else 0 end) > 0', ids: [TaskStatus.ready_for_feedback, TaskStatus.need_help])
       .order('pinned DESC, submission_date ASC, MAX(task_comments.created_at) ASC, task_definition_id ASC')
   end

test/api/units/feedback_test.rb

@@ -33,26 +33,109 @@ def test_get_awaiting_feedback
   end
 
   def test_tasks_for_task_inbox
-    unit = FactoryBot.create(:unit, perform_submissions: true, unenrolled_student_count: 0, part_enrolled_student_count: 0, tutorials: 2, staff_count: 2)
-
-    expected_count = unit.tasks.where(task_status: [TaskStatus.ready_for_feedback, TaskStatus.need_help]).count
-
-    unit.teaching_staff.each do |user|
-      expected_response = unit.tasks_for_task_inbox(user)
-
+    unit = FactoryBot.create(:unit, perform_submissions: true, unenrolled_student_count: 0, part_enrolled_student_count: 0, tutorials: 2, staff_count: 2, student_count: 2, task_count: 0)
+
+    td1 = TaskDefinition.create!({
+                                   unit_id: unit.id,
+                                   tutorial_stream: unit.tutorial_streams.first,
+                                   name: 'Code task',
+                                   description: 'Code task',
+                                   weighting: 4,
+                                   target_grade: 0,
+                                   start_date: Time.zone.now - 2.weeks,
+                                   target_date: Time.zone.now + 1.week,
+                                   abbreviation: 'CodeTask',
+                                   restrict_status_updates: false,
+                                   upload_requirements: [{ "key" => 'file0', "name" => 'Shape Class', "type" => 'code' }],
+                                   plagiarism_warn_pct: 0.8,
+                                   is_graded: true,
+                                   max_quality_pts: 0
+                                 })
+
+    td2 = TaskDefinition.create!({
+                                   unit_id: unit.id,
+                                   tutorial_stream: unit.tutorial_streams.first,
+                                   name: 'Code task2',
+                                   description: 'Code task2',
+                                   weighting: 4,
+                                   target_grade: 0,
+                                   start_date: Time.zone.now - 2.weeks,
+                                   target_date: Time.zone.now + 1.week,
+                                   abbreviation: 'CodeTask2',
+                                   restrict_status_updates: false,
+                                   upload_requirements: [{ "key" => 'file0', "name" => 'Shape Class', "type" => 'code' }],
+                                   plagiarism_warn_pct: 0.8,
+                                   is_graded: true,
+                                   max_quality_pts: 0
+                                 })
+
+    student1 = unit.active_projects.first
+    student2 = unit.active_projects.second
+    tutor1 = unit.tutors.first
+    tutor2 = unit.tutors.second
+
+    tutorial1 = unit.tutorials.first
+    tutorial2 = unit.tutorials.second
+
+    assert_not_nil student1
+    assert_not_nil student2
+    assert_not_nil tutor1
+    assert_not_nil tutor2
+    assert_not_nil tutorial1
+    assert_not_nil tutorial2
+
+    unit.employ_staff(tutor1, Role.tutor)
+    unit.employ_staff(tutor2, Role.tutor)
+
+    tutorial1.assign_tutor(tutor1)
+    tutorial2.assign_tutor(tutor2)
+
+    student1.enrol_in(tutorial1)
+    student2.enrol_in(tutorial2)
+
+    # Submit tasks ready for feedback
+    student1_task1 = student1.task_for_task_definition(td1)
+    student2_task1 = student2.task_for_task_definition(td1)
+    DatabasePopulator.assess_task(student1, student1_task1, tutor1, TaskStatus.ready_for_feedback, Time.zone.now)
+    DatabasePopulator.assess_task(student2, student2_task1, tutor2, TaskStatus.ready_for_feedback, Time.zone.now)
+
+    # Add comments to unsubmitted task
+    student1_task2 = student1.task_for_task_definition(td2)
+    student2_task2 = student2.task_for_task_definition(td2)
+    comment1 = student1_task2.add_text_comment(student1.user, "Test 1")
+    comment2 = student2_task2.add_text_comment(student2.user, "Test 1")
+    assert_not_nil comment1
+    assert_not_nil comment2
+
+    [tutor1, tutor2].each do |user|
       # Add auth_token and username to header
       add_auth_header_for(user: user)
 
+      # Defaults inbox?my_students_only = false
       get "/api/units/#{unit.id}/tasks/inbox"
 
+      # 2 submissions ready for feedback + 2 tasks with unread comments = 4 total
       assert_equal 200, last_response.status, last_response_body
-      assert_equal expected_count, last_response_body.count, last_response_body
+      assert_equal 4, last_response_body.count, last_response_body
 
       # check each is the same
+      expected_response = unit.tasks_for_task_inbox(user)
       last_response_body.zip(expected_response).each do |response, expected|
         assert_json_matches_model expected, response, ['id']
       end
 
+      get "/api/units/#{unit.id}/tasks/inbox?my_students_only=true"
+
+      # 1 submission ready for feedback + 1 task with unread comments = 2 total
+      assert_equal 200, last_response.status, last_response_body
+      assert_equal 2, last_response_body.count, last_response_body
+
+      # check each is the same
+      expected_response2 = unit.tasks_for_task_inbox(user, true)
+      last_response_body.zip(expected_response2).each do |response, expected|
+        assert_json_matches_model expected, response, ['id']
+      end
+
       # Test task explorer
       get "/api/units/#{unit.id}/task_definitions/#{unit.task_definitions.first.id}/tasks"