From 841b3b34ecfa1fd6c7291fbb39564e000a596691 Mon Sep 17 00:00:00 2001
From: Bestin Lalu <46861674+gazarack@users.noreply.github.com>
Date: Fri, 5 Jun 2026 18:59:10 -0400
Subject: [PATCH 1/4] E2602 - Reimplement student task view

---
 app/controllers/grades_controller.rb          | 120 +++++-
 app/controllers/participants_controller.rb    |   2 +-
 app/controllers/student_tasks_controller.rb   |  29 +-
 .../teams_participants_controller.rb          |  16 +
 app/models/assignment_team.rb                 |  10 +-
 app/models/concerns/expertiza_constants.rb    |  19 +
 app/models/due_date.rb                        |  10 +-
 app/models/response.rb                        |  14 +-
 app/models/student_task.rb                    | 230 +++++++++--
 config/routes.rb                              |   3 +-
 spec/models/due_date_spec.rb                  |  30 ++
 spec/models/response_spec.rb                  |  73 ++++
 spec/models/student_task_spec.rb              | 384 +++++++++++++++++-
 .../requests/api/v1/grades_controller_spec.rb | 234 ++++++++---
 .../api/v1/student_tasks_controller_spec.rb   | 175 ++++++--
 spec/routing/student_tasks_routing_spec.rb    |  12 +
 16 files changed, 1173 insertions(+), 188 deletions(-)

diff --git a/app/controllers/grades_controller.rb b/app/controllers/grades_controller.rb
index edd6e28dc..3efaea55a 100644
--- a/app/controllers/grades_controller.rb
+++ b/app/controllers/grades_controller.rb
@@ -18,19 +18,22 @@ def action_allowed?
 
     # index (GET /grades/:assignment_id/view_all_scores)
     # returns all review scores and computed heatmap data for the given assignment (instructor/TA view).
-    def view_all_scores    
+    def view_all_scores
         @assignment = Assignment.find(params[:assignment_id])
         participant_scores = []
         team_scores = []
-        
+
         @assignment.participants.each do |participant|
             participant_scores.push(get_my_scores_data(participant))
         end
-        
-        @assignment.teams.each do |team|
+
+        # Preload team.users for all teams in one query to avoid N+1 inside get_our_scores_data.
+        # Without this, each call to get_our_scores_data fires a separate `team.users` JOIN query
+        # (one per team), which degrades linearly with the number of teams in the assignment.
+        @assignment.teams.includes(:users).each do |team|
             team_scores.push(get_our_scores_data(team))
         end
-        
+
         render json: {
             team_scores: team_scores,
             participant_scores: participant_scores
@@ -273,10 +276,25 @@ def get_our_scores_data(team)
         reviews_of_our_work = get_heatgrid_data_for(reviews_of_our_work_maps)
         avg_score_of_our_work = team.aggregate_review_grade
 
+        # Embed all team metadata directly in this response so the frontend can populate
+        # team name, grade, submission links, and member list in a single API call.
+        # Previously the frontend made 3+ follow-up requests:
+        #   GET /participants/user/:id  →  GET /teams_participants/:id/list_participants
+        #   →  GET /users/:id  (once per team member)
+        # team.hyperlinks calls YAML.safe_load(submitted_hyperlinks) internally — no manual parsing.
+        # team.users is a single JOIN through teams_participants (no N+1 — one query for all members).
+        team_members = {
+            team_name:        team.name,
+            grade:            team.grade_for_submission,
+            comment:          team.comment_for_submission,
+            submission_links: team.hyperlinks,
+            members:          team.users.map { |u| { name: u.full_name, username: u.name } }
+        }
+
         {
-            team_details: team,
-            reviews_of_our_work: reviews_of_our_work,
-            avg_score_of_our_work: avg_score_of_our_work
+            reviews_of_our_work:    reviews_of_our_work,
+            avg_score_of_our_work:  avg_score_of_our_work,
+            team_members:           team_members
         }
     end
 
@@ -326,7 +344,7 @@ def get_my_scores_data(participant)
     def get_heatgrid_data_for(maps)
         # Initialize a hash to store scores grouped by review rounds
         reviewee_scores = {}
-        return if maps.empty?
+        return reviewee_scores if maps.empty?
 
         # check if the assignment uses different rubrics for each round
         if @assignment.varying_rubrics_by_round?
@@ -361,17 +379,99 @@ def get_heatgrid_data_for(maps)
                 end
 
                 reviewee_scores[round_symbol].each_with_index do |scores_array, idx|
-                    # Sort each question's answers array by reviewer_name and reviwee_name 
+                    # Sort each question's answers array by reviewer_name and reviwee_name
                     reviewee_scores[round_symbol][idx] = scores_array.sort_by { |answer| [answer[:reviewer_name].downcase , answer[:reviewee_name].downcase] }
                 end
+
+                # Inject SectionHeader sentinels so the frontend can render section headings.
+                # Constrain by questionnaire_type so we look up the rubric that actually matches
+                # the maps we're processing (Review, TeammateReview, Feedback) rather than
+                # returning any AQ for this round — which would be wrong when multiple
+                # questionnaire types are configured for the same round and assignment.
+                aq = AssignmentQuestionnaire
+                       .joins(:questionnaire)
+                       .where(assignment_id: @assignment.id, used_in_round: round)
+                       .where(questionnaires: { type: maps.first.questionnaire_type })
+                       .first
+                inject_section_headers(reviewee_scores[round_symbol], aq&.questionnaire_id)
             end
 
+        else
+            # Non-varying rubric branch. varying_rubrics_by_round? == false does NOT mean
+            # "single round" — an assignment may reuse the same rubric across multiple rounds.
+            # The original code folded everything into a synthetic Round-1 bucket and used
+            # .last, which silently lost earlier rounds. Instead, discover the actual rounds
+            # present in submitted responses and group each one separately.
+            rounds = maps.flat_map { |map|
+                map.responses.select(&:is_submitted).map(&:round)
+            }.compact.uniq.sort
+
+            # Guard: if no submitted responses exist, nothing to show.
+            return reviewee_scores if rounds.empty?
+
+            rounds.each do |round|
+                round_symbol = ("#{maps.first.questionnaire_type}-Round-#{round}").to_sym
+                reviewee_scores[round_symbol] = []
+
+                maps.each_with_index do |map, index|
+                    submitted_response = map.responses.select { |r|
+                        r.round == round && r.is_submitted && r.map_id == map.id
+                    }.last
+                    next if submitted_response.nil?
+
+                    submitted_response.scores.each_with_index do |score, new_index|
+                        reviewee_scores[round_symbol][new_index] ||= []
+                        reviewee_scores[round_symbol][new_index] << get_answer(score, index)
+                    end
+                end
+
+                reviewee_scores[round_symbol].each_with_index do |scores_array, idx|
+                    reviewee_scores[round_symbol][idx] = scores_array.sort_by { |answer|
+                        [answer[:reviewer_name].downcase, answer[:reviewee_name].downcase]
+                    }
+                end
+
+                # Inject SectionHeader sentinels. For non-varying assignments used_in_round
+                # is nil in AssignmentQuestionnaire regardless of how many actual response
+                # rounds were recorded, so look up by nil rather than by the response round.
+                aq = AssignmentQuestionnaire.where(assignment_id: @assignment.id, used_in_round: nil).first
+                inject_section_headers(reviewee_scores[round_symbol], aq&.questionnaire_id)
+            end
         end
 
         # Return the organized hash of scores grouped by round
         return reviewee_scores
     end
 
+    # Injects SectionHeader items as sentinel hashes into a round's scores array.
+    # The heatgrid scores array is indexed by scored-item position (SectionHeaders have no
+    # answers so they never appear in the scores loop). This method looks up the questionnaire,
+    # walks all items in seq order, and inserts { type: "header", txt: "..." } markers at the
+    # correct positions so the frontend can render heading rows between score rows.
+    def inject_section_headers(round_scores, questionnaire_id)
+        return round_scores if questionnaire_id.nil?
+
+        all_items = Item.where(questionnaire_id: questionnaire_id).order(:seq)
+        scored_count = 0
+        headers_to_insert = {}   # scored_position => header_txt
+
+        all_items.each do |item|
+            if item.question_type == 'SectionHeader'
+                headers_to_insert[scored_count] = item.txt
+            else
+                scored_count += 1
+            end
+        end
+
+        offset = 0
+        headers_to_insert.sort.each do |position, header_txt|
+            round_scores.insert(position + offset, { type: "header", txt: header_txt })
+            offset += 1
+        end
+
+        round_scores
+    end
+
     def get_answer(score, index)
         # Determine the name or label to show for the reviewer
         reviewer_name = if current_user_has_all_heatgrid_data_privileges?(@assignment)
diff --git a/app/controllers/participants_controller.rb b/app/controllers/participants_controller.rb
index 607faa015..064713014 100644
--- a/app/controllers/participants_controller.rb
+++ b/app/controllers/participants_controller.rb
@@ -13,7 +13,7 @@ def list_user_participants
     if participants.nil?
       render json: participants.errors, status: :unprocessable_entity
     else
-      render json: participants, status: :ok
+      render json: participants.map(&:attributes), status: :ok
     end
   end
 
diff --git a/app/controllers/student_tasks_controller.rb b/app/controllers/student_tasks_controller.rb
index ffb6097a5..27e7ef578 100644
--- a/app/controllers/student_tasks_controller.rb
+++ b/app/controllers/student_tasks_controller.rb
@@ -5,23 +5,34 @@ def action_allowed?
     current_user_has_student_privileges?
   end
   def list
-    # Retrieves all tasks that belong to the current user.
     @student_tasks = StudentTask.from_user(current_user)
-    # Render the list of student tasks as JSON.
     render json: @student_tasks, status: :ok
   end
 
-  def show
-    render json: @student_task, status: :ok
+  # GET /student_tasks/teammates
+  def team
+    render json: StudentTask.teamed_students(current_user), status: :ok
   end
 
   # The view function retrieves a student task based on a participant's ID.
   # It is meant to provide an endpoint where tasks can be queried based on participant ID.
-  def view
-    # Retrieves the student task where the participant's ID matches the provided parameter.
-    # This function will be used for clicking on a specific student task to "view" its details.
-    @student_task = StudentTask.from_participant_id(params[:id])
-    # Render the found student task as JSON.
+  def show
+    # Constrain to AssignmentParticipant — other Participant subclasses for the same user
+    # can be found via the polymorphic participants table and cause type-mismatch 500s later.
+    participant = AssignmentParticipant.find_by(id: params[:id])
+
+    if participant.nil?
+      render json: { error: "Participant not found" }, status: :not_found
+      return
+    end
+
+    if participant.user_id != current_user.id
+      render json: { error: "Unauthorized access to participant's task" }, status: :forbidden
+      return
+    end
+
+    @student_task = StudentTask.create_from_participant(participant)
+    @student_task.due_dates = StudentTask.get_timeline_data(participant.assignment, participant)
     render json: @student_task, status: :ok
   end
 
diff --git a/app/controllers/teams_participants_controller.rb b/app/controllers/teams_participants_controller.rb
index c34e3f34d..e284cd93c 100644
--- a/app/controllers/teams_participants_controller.rb
+++ b/app/controllers/teams_participants_controller.rb
@@ -4,6 +4,11 @@ def action_allowed?
     case params[:action]
     when 'update_duty'
       current_user_has_student_privileges?
+    when 'list_participants'
+      # TA+ can always enumerate any team.
+      # Students may only view their own team roster — enforced inside the action itself
+      # via an explicit membership check so IDs cannot be enumerated by guessing.
+      current_user_has_ta_privileges? || current_user_has_student_privileges?
     else
       current_user_has_ta_privileges?
     end
@@ -38,6 +43,17 @@ def list_participants
       render json: { error: "Couldn't find Team" }, status: :not_found and return
     end
 
+    # Students may only view rosters for teams they belong to.
+    # Without this check any authenticated student can enumerate arbitrary team memberships
+    # by guessing team IDs, since action_allowed? only requires student privileges.
+    # TA+ skip this guard — they have legitimate cross-team visibility.
+    unless current_user_has_ta_privileges?
+      member_ids = TeamsParticipant.where(team_id: current_team.id).pluck(:user_id)
+      unless member_ids.include?(current_user.id)
+        render json: { error: 'You are not authorized to view this team' }, status: :forbidden and return
+      end
+    end
+
     # Fetch all team participant records associated with the current team.
     team_participants = TeamsParticipant.where(team_id: current_team.id)
 
diff --git a/app/models/assignment_team.rb b/app/models/assignment_team.rb
index 2bf8e6815..632b74eae 100644
--- a/app/models/assignment_team.rb
+++ b/app/models/assignment_team.rb
@@ -107,6 +107,12 @@ def assign_reviewer(reviewer)
     ReviewResponseMap.create(reviewee_id: id, reviewer_id: reviewer.get_reviewer.id, reviewed_object_id: assignment.id, team_reviewing_enabled: assignment.team_reviewing_enabled)
   end
 
+  # Returns submitted files for this team.
+  # File storage is not yet implemented in the reimplementation — returns empty array as stub.
+  def submitted_files
+    []
+  end
+
   # Whether the team has submitted work or not
   def has_submissions?
     submitted_files.any? || submitted_hyperlinks.present?
@@ -117,7 +123,7 @@ def has_submissions?
   def aggregate_review_grade
     compute_average_review_score(review_mappings)
   end
-  
+
   # Adds a participant to this team.
   # - Update the participant's team_id (so their direct reference is consistent)
   # - Ensure there is a TeamsParticipant join record connecting the participant and this team
@@ -143,7 +149,7 @@ def remove_participant(participant)
     # Remove the join record if it exists
     tp = TeamsParticipant.find_by(team_id: id, participant_id: participant.id)
     tp&.destroy
-    
+
     # Update the participant's team_id column - will remove the team reference inside participants table later. keeping it for now
     # participant.update!(team_id: nil)
 
diff --git a/app/models/concerns/expertiza_constants.rb b/app/models/concerns/expertiza_constants.rb
index 2e8a8dca7..0ee3ab730 100644
--- a/app/models/concerns/expertiza_constants.rb
+++ b/app/models/concerns/expertiza_constants.rb
@@ -1,6 +1,25 @@
 # frozen_string_literal: true
 
 module ExpertizaConstants
+  module DeadlineTypes
+    SUBMISSION       = 1
+    REVIEW           = 2
+    QUIZ             = 6
+    DROP_TOPIC       = 7
+    SIGNUP           = 8
+    TEAM_FORMATION   = 9
+
+    # Maps deadline_type_id to stage name, mirroring the old DeadlineType table
+    NAMES = {
+      SUBMISSION => 'submission',
+      REVIEW => 'review',
+      QUIZ => 'quiz',
+      DROP_TOPIC => 'drop_topic',
+      SIGNUP => 'signup',
+      TEAM_FORMATION => 'team_formation'
+    }.freeze
+  end
+
   module ResponseMapTitles
     ASSIGNMENT_SURVEY_RESPONSE_MAP_TITLE = 'Assignment Survey'
     BOOKMARK_RATING_RESPONSE_MAP_TITLE = 'Bookmark Review'
diff --git a/app/models/due_date.rb b/app/models/due_date.rb
index 22c83d330..b81e86a88 100644
--- a/app/models/due_date.rb
+++ b/app/models/due_date.rb
@@ -3,7 +3,7 @@
 class DueDate < ApplicationRecord
   include Comparable
   # Any due date with deadline_type_id == DueDate::REVIEW_DEADLINE_TYPE_ID (constant = 2) is treated as a review deadline and counts as a review round
-  REVIEW_DEADLINE_TYPE_ID = 2 
+  REVIEW_DEADLINE_TYPE_ID = 2
   # Named constants for teammate review statuses
   ALLOWED = 3
   LATE_ALLOWED = 2
@@ -15,6 +15,14 @@ class DueDate < ApplicationRecord
 
   attr_accessor :teammate_review_allowed, :submission_allowed, :review_allowed
 
+  # Returns the current stage name for an assignment based on its next upcoming due date
+  def self.current_stage_for(assignment)
+    next_due = next_due_date(assignment)
+    return 'Finished' unless next_due
+
+    ExpertizaConstants::DeadlineTypes::NAMES[next_due.deadline_type_id] || 'Unknown'
+  end
+
   def due_at_is_valid_datetime
     errors.add(:due_at, 'must be a valid datetime') unless due_at.is_a?(Time)
   end
diff --git a/app/models/response.rb b/app/models/response.rb
index c6233dabe..709671dab 100644
--- a/app/models/response.rb
+++ b/app/models/response.rb
@@ -11,9 +11,19 @@ class Response < ApplicationRecord
   alias map response_map
   delegate :reviewer_assignment, :response_assignment, :reviewee, :reviewer, to: :map
 
-  # return the questionnaire that belongs to the response
+  # return the questionnaire that belongs to the response.
+  # For varying-round assignments, used_in_round matches response.round (1 or 2).
+  # For single-round assignments, used_in_round is nil in the DB while response.round is 1,
+  # so we fall back to the first AssignmentQuestionnaire when the round-specific lookup fails.
   def questionnaire
-    reviewer_assignment.assignment_questionnaires.find_by(used_in_round: self.round).questionnaire
+    assignment_questionnaires = reviewer_assignment.assignment_questionnaires
+    # Primary lookup: find the AQ whose round matches this response's round.
+    aq = assignment_questionnaires.find_by(used_in_round: round)
+    # Fallback: single-round assignments store their AQ with used_in_round: nil.
+    # Falling back to nil-round (not .first) keeps the resolution deterministic —
+    # .first is arbitrary when multiple AQs exist for the same assignment.
+    aq ||= assignment_questionnaires.find_by(used_in_round: nil)
+    aq&.questionnaire
   end
 
   # Backward-compatible wrapper around ResponseMap#response_map_label.
diff --git a/app/models/student_task.rb b/app/models/student_task.rb
index c5bb9332b..e5717d15f 100644
--- a/app/models/student_task.rb
+++ b/app/models/student_task.rb
@@ -1,50 +1,206 @@
 # frozen_string_literal: true
 
 class StudentTask
-    attr_accessor :assignment, :current_stage, :participant, :stage_deadline, :topic, :permission_granted
-
-    # Initializes a new instance of the StudentTask class
-    def initialize(args)
-      @assignment = args[:assignment]
-      @current_stage = args[:current_stage]
-      @participant = args[:participant]
-      @stage_deadline = args[:stage_deadline]
-      @topic = args[:topic]
-      @permission_granted = args[:permission_granted]
+  attr_accessor :assignment, :course, :current_stage, :participant, :stage_deadline, :topic, :permission_granted, :due_dates, :revise, :not_started, :active_round
+
+  # Initializes a new instance of the StudentTask class
+  def initialize(args)
+    @assignment = args[:assignment]
+    @course = args[:course]
+    @current_stage = args[:current_stage]
+    @participant = args[:participant]
+    @stage_deadline = args[:stage_deadline]
+    @topic = args[:topic]
+    @permission_granted = args[:permission_granted]
+    @due_dates = args[:due_dates]
+    @active_round = args[:active_round]
+  end
+
+  # create a new StudentTask instance from a Participant object.
+  def self.create_from_participant(participant)
+    assignment = participant.assignment
+    next_due   = DueDate.next_due_date(assignment)
+    # Fetch the real topic name from SignedUpTeam -> ProjectTopic
+    team       = participant.team
+    topic_name = SignedUpTeam.find_by(team_id: team&.id)&.project_topic&.topic_name
+    current_stage = DueDate.current_stage_for(assignment)
+
+    # Determine the active round from the current due date so that revise? and
+    # not_started? can scope their checks to the round the student is actually in,
+    # rather than checking across all rounds (which causes round-1 work to make a
+    # round-2 task appear started even when the student hasn't touched round 2 yet).
+    active_round = next_due&.round
+
+    task = new(
+      assignment:         assignment.name,
+      course:             assignment.course&.name || 'Unknown Course',
+      topic:              topic_name,
+      current_stage:      current_stage,
+      stage_deadline:     parse_stage_deadline(next_due ? next_due.due_at.in_time_zone(participant.user.try(:timezonepref) || 'UTC') : 'Finished'),
+      permission_granted: participant.permission_granted,
+      participant:        participant,
+      active_round:       active_round
+    )
+    task.revise = task.revise?
+    task.not_started = task.not_started?
+    task
+  end
+
+
+  # create an array of StudentTask instances for all participants linked to a user, sorted by deadline.
+  # Three chained sort_by calls are not stable — each overwrites the previous ordering.
+  # A single composite key [course, assignment, stage_deadline] gives deterministic,
+  # consistent ordering in one pass.
+  def self.from_user(user)
+    # Preload the associations that create_from_participant dereferences for every
+    # participant so that the list endpoint does not fire one query per participant
+    # for each of these:
+    #   participant.assignment         →  included via :assignment
+    #   assignment.course              →  included via assignment: :course
+    #   participant.user               →  included via :user  (for timezonepref)
+    #
+    # Remaining per-row queries that cannot be batched without schema changes:
+    #   participant.team               →  resolved through TeamsParticipant (custom method,
+    #                                     not a first-class has_one that AR can :include)
+    #   SignedUpTeam / ProjectTopic    →  depends on team, same constraint
+    #   DueDate.next_due_date          →  class method; hits AR cache after assignment preload
+    #   reviews_given_in_current_stage →  scoped EXISTS query, one per participant
+    AssignmentParticipant.where(user_id: user.id)
+                         .includes(assignment: :course)
+                         .includes(:user)
+                         .map { |participant| StudentTask.create_from_participant(participant) }
+                         .sort_by { |task| [task.course, task.assignment, task.stage_deadline] }
+  end
+
+  # create a StudentTask instance from a participant of the provided id
+  # Constrained to AssignmentParticipant to avoid type-mismatch 500s when other
+  # Participant subclasses exist for the same user/id in the polymorphic table.
+  def self.from_participant_id(id)
+    create_from_participant(AssignmentParticipant.find_by(id: id))
+  end
+
+  # Builds a unified timeline by merging due dates and actual activity,
+  # sorted chronologically — mirrors the old get_timeline_data logic.
+  def self.get_timeline_data(assignment, participant)
+    timeline = []
+
+    # 1. Due dates — labeled as "X deadline" mirroring old get_due_date_data behavior
+    DueDate.fetch_due_dates(assignment).each do |due_date|
+      timeline << {
+        'id'    => nil,
+        'name'  => due_date.round && due_date.round > 1 ? "#{due_date.deadline_name} deadline (round #{due_date.round})" : "#{due_date.deadline_name} deadline",
+        'date'  => due_date.due_at.iso8601,
+        'type'  => due_date.deadline_type_id,
+        'round' => due_date.round
+      }
     end
 
-    # create a new StudentTask instance from a Participant object.cccccccc
-    def self.create_from_participant(participant)
-      new(
-        assignment: participant.assignment.name,                          # Name of the assignment associated with the student task
-        topic: participant.topic,                                         # Current stage of the assignment process
-        current_stage: participant.current_stage,                         # Participant object
-        stage_deadline: parse_stage_deadline(participant.stage_deadline), # Deadline for the current stage of the assignment
-        permission_granted: participant.permission_granted,               # Topic of the assignment
-        participant: participant                                          # Boolean indicating if Publishing Rights is enabled
-      )
+    # 2. Peer reviews given by this participant — one timeline entry per submitted Response.
+    # The original code called Response.where(map_id: map.id).last, which returned only the
+    # most recent Response for each ReviewResponseMap. For a 2-round assignment, one map has
+    # two Response rows (round 1 and round 2). Using .last silently dropped the round 1 entry,
+    # so the timeline showed "Round 2 peer review" with no corresponding "Round 1 peer review".
+    # Iterating with find_each ensures all rounds are added as separate timeline events.
+    ReviewResponseMap.where(reviewer_id: participant.id).find_each do |map|
+      # Only submitted responses — draft/unsubmitted records must not appear in the timeline.
+      # Order by updated_at desc so find_each still benefits from batching but each record
+      # is a real submitted response. find_each is used (not .last) so both round 1 and
+      # round 2 responses for the same map are captured as separate timeline events.
+      Response.where(map_id: map.id, is_submitted: true).order(updated_at: :desc).each do |response|
+        timeline << {
+          'id'    => response.id,
+          'name'  => "Round #{response.round} peer review",
+          'date'  => response.updated_at.iso8601,
+          'type'  => ExpertizaConstants::DeadlineTypes::REVIEW,
+          'round' => response.round
+        }
+      end
     end
 
+    # 3. Author feedback given by this participant — submitted only, most recent per map
+    FeedbackResponseMap.where(reviewer_id: participant.id).find_each do |map|
+      response = Response.where(map_id: map.id, is_submitted: true).order(updated_at: :desc).first
+      next if response.nil?
 
-    # create an array of StudentTask instances for all participants linked to a user, sorted by deadline.
-    def self.from_user(user)
-      Participant.where(user_id: user.id)
-                 .map { |participant| StudentTask.create_from_participant(participant) }
-                 .sort_by(&:stage_deadline)
+      timeline << {
+        'id'    => response.id,
+        'name'  => 'Author feedback',
+        'date'  => response.updated_at.iso8601,
+        'type'  => nil,
+        'round' => response.round
+      }
     end
 
-    # create a StudentTask instance from a participant of the provided id
-    def self.from_participant_id(id)
-      create_from_participant(Participant.find_by(id: id))
+    timeline.sort_by { |item| item['date'] }
+  end
+
+  # Returns a hash of { course_name => [teammate_fullnames] } for all
+  # assignment teams the user has been part of, mirroring the old teamed_students logic.
+  def self.teamed_students(user)
+    result = {}
+    user.teams.each do |team|
+      next unless team.is_a?(AssignmentTeam)
+      assignment = Assignment.find_by(id: team.parent_id)
+      next if assignment.nil? || assignment.is_calibrated
+
+      course_name = assignment.course&.name || 'Unknown Course'
+      teammates = team.users.where.not(id: user.id).map(&:full_name)
+      next if teammates.empty?
+
+      result[course_name] ||= []
+      result[course_name] |= teammates
     end
-  
-    private
-
-    # Parses a date string to a Time object, if parsing fails, set the time to be one year after current
-    def self.parse_stage_deadline(date_string)
-      Time.parse(date_string)
-    rescue StandardError
-      Time.now + 1.year
+    result.transform_values(&:sort).sort.to_h
+  end
+
+  # Returns true if the student has started work in the current stage.
+  def revise?
+    content_submitted_in_current_stage? ||
+      reviews_given_in_current_stage?
+  end
+
+  # Returns true if the assignment is in an active stage but the student hasn't started yet.
+  def not_started?
+    in_work_stage? && !revise?
+  end
+
+  private
+
+  def in_work_stage?
+    [ExpertizaConstants::DeadlineTypes::NAMES[ExpertizaConstants::DeadlineTypes::SUBMISSION],
+     ExpertizaConstants::DeadlineTypes::NAMES[ExpertizaConstants::DeadlineTypes::REVIEW]].include?(current_stage)
+  end
+
+  def content_submitted_in_current_stage?
+    return false unless current_stage == 'submission'
+    team = participant.team
+    return false unless team
+    # Submission stage has no round granularity — a hyperlink or file counts regardless.
+    team.hyperlinks.present? || team.has_submissions?
+  end
+
+  def reviews_given_in_current_stage?
+    return false unless current_stage == 'review'
+    # Scope to the active round so that completing round-1 reviews does not make a
+    # round-2 review task appear started when the student has not yet touched round 2.
+    # When active_round is nil (single-round or indeterminate), fall back to checking
+    # any submitted review so the predicate still works for non-multi-round assignments.
+    scope = ReviewResponseMap.where(reviewer_id: participant.id)
+                             .joins(:responses)
+                             .where(responses: { is_submitted: true })
+    if active_round.present?
+      scope = scope.where(responses: { round: active_round })
     end
-  
+    scope.exists?
+  end
+
+  # Parses a date string or Time object into a Time instance.
+  def self.parse_stage_deadline(date_string)
+    return date_string if date_string.is_a?(Time)
+    Time.parse(date_string.to_s)
+  rescue StandardError => e
+    Rails.logger.error("Failed to parse stage deadline '#{date_string}': #{e.message}")
+    Time.now + 1.year
+  end
+
 end
diff --git a/config/routes.rb b/config/routes.rb
index 47da3d9e0..dd053ff07 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -49,7 +49,8 @@
       resources :student_tasks do
         collection do
           get :list, action: :list
-          get :view
+          get 'show/:id', action: :show
+          get :team, action: :team
         end
       end
 
diff --git a/spec/models/due_date_spec.rb b/spec/models/due_date_spec.rb
index 39e24c219..9e5a0452b 100644
--- a/spec/models/due_date_spec.rb
+++ b/spec/models/due_date_spec.rb
@@ -280,6 +280,36 @@
     end
   end
 
+  describe '.current_stage_for' do
+    let(:assignment) { Assignment.create!(id: 5001, name: 'Stage Assignment', instructor:) }
+
+    it "returns 'submission' when next due date is a submission type" do
+      DueDate.create!(parent: assignment, due_at: 3.days.from_now, deadline_type_id: ExpertizaConstants::DeadlineTypes::SUBMISSION,
+                      submission_allowed_id: 3, review_allowed_id: 3)
+      expect(DueDate.current_stage_for(assignment)).to eq('submission')
+    end
+
+    it "returns 'review' when next due date is a review type" do
+      DueDate.create!(parent: assignment, due_at: 1.day.ago, deadline_type_id: ExpertizaConstants::DeadlineTypes::SUBMISSION,
+                      submission_allowed_id: 3, review_allowed_id: 3)
+      DueDate.create!(parent: assignment, due_at: 3.days.from_now, deadline_type_id: ExpertizaConstants::DeadlineTypes::REVIEW,
+                      submission_allowed_id: 3, review_allowed_id: 3)
+      expect(DueDate.current_stage_for(assignment)).to eq('review')
+    end
+
+    it "returns 'Finished' when no upcoming due dates exist" do
+      DueDate.create!(parent: assignment, due_at: 2.days.ago, deadline_type_id: ExpertizaConstants::DeadlineTypes::SUBMISSION,
+                      submission_allowed_id: 3, review_allowed_id: 3)
+      expect(DueDate.current_stage_for(assignment)).to eq('Finished')
+    end
+
+    it "returns 'Unknown' for an unrecognised deadline_type_id" do
+      DueDate.create!(parent: assignment, due_at: 3.days.from_now, deadline_type_id: 99,
+                      submission_allowed_id: 3, review_allowed_id: 3)
+      expect(DueDate.current_stage_for(assignment)).to eq('Unknown')
+    end
+  end
+
   describe 'validation' do
     let(:assignment) { Assignment.create(id: 1, name: 'Test Assignment', instructor:) }
 
diff --git a/spec/models/response_spec.rb b/spec/models/response_spec.rb
index 491f6b319..ffc441566 100644
--- a/spec/models/response_spec.rb
+++ b/spec/models/response_spec.rb
@@ -120,4 +120,77 @@
       expect(response_map.response_assignment).to eq(response_map.reviewer_assignment)
     end
   end
+
+  # -------------------------------------------------------------------------
+  # Response#questionnaire
+  # -------------------------------------------------------------------------
+  # The original implementation was a one-liner:
+  #   reviewer_assignment.assignment_questionnaires.find_by(used_in_round: self.round).questionnaire
+  # For single-round assignments, AssignmentQuestionnaire is stored with used_in_round: nil
+  # but Response#round is 1, so find_by returns nil and calling .questionnaire on nil raised
+  # NoMethodError. The fix falls back to .first when the round-specific lookup fails.
+  describe '#questionnaire' do
+    let(:questionnaire_obj) { instance_double(Questionnaire) }
+
+    context 'varying-round assignment (used_in_round matches response.round)' do
+      it 'returns the questionnaire for the matching round' do
+        aq = instance_double(AssignmentQuestionnaire, questionnaire: questionnaire_obj)
+        aq_relation = double('aq_relation')
+        allow(aq_relation).to receive(:find_by).with(used_in_round: 1).and_return(aq)
+        allow(aq_relation).to receive(:first).and_return(aq)
+
+        allow(response).to receive(:round).and_return(1)
+        assignment_double = double('assignment', assignment_questionnaires: aq_relation)
+        allow(response).to receive(:reviewer_assignment).and_return(assignment_double)
+
+        expect(response.questionnaire).to eq(questionnaire_obj)
+      end
+    end
+
+    context 'single-round assignment (used_in_round is nil but response.round is 1)' do
+      it 'falls back to the nil-round AssignmentQuestionnaire and returns its questionnaire' do
+        aq = instance_double(AssignmentQuestionnaire, questionnaire: questionnaire_obj)
+        aq_relation = double('aq_relation')
+        # Primary lookup: no AQ stored with used_in_round: 1 (single-round uses nil)
+        allow(aq_relation).to receive(:find_by).with(used_in_round: 1).and_return(nil)
+        # Deterministic fallback: find_by(used_in_round: nil) targets single-round AQs specifically
+        allow(aq_relation).to receive(:find_by).with(used_in_round: nil).and_return(aq)
+
+        allow(response).to receive(:round).and_return(1)
+        assignment_double = double('assignment', assignment_questionnaires: aq_relation)
+        allow(response).to receive(:reviewer_assignment).and_return(assignment_double)
+
+        expect(response.questionnaire).to eq(questionnaire_obj)
+      end
+
+      it 'does not raise NoMethodError when find_by returns nil' do
+        aq = instance_double(AssignmentQuestionnaire, questionnaire: questionnaire_obj)
+        aq_relation = double('aq_relation')
+        allow(aq_relation).to receive(:find_by).with(used_in_round: 1).and_return(nil)
+        allow(aq_relation).to receive(:find_by).with(used_in_round: nil).and_return(aq)
+
+        allow(response).to receive(:round).and_return(1)
+        assignment_double = double('assignment', assignment_questionnaires: aq_relation)
+        allow(response).to receive(:reviewer_assignment).and_return(assignment_double)
+
+        expect { response.questionnaire }.not_to raise_error
+      end
+    end
+
+    context 'when no AssignmentQuestionnaire exists at all' do
+      it 'returns nil safely via the safe-navigator (&.)' do
+        aq_relation = double('aq_relation')
+        allow(aq_relation).to receive(:find_by).with(used_in_round: 1).and_return(nil)
+        # Both lookups return nil — safe-navigator on aq&.questionnaire must not raise
+        allow(aq_relation).to receive(:find_by).with(used_in_round: nil).and_return(nil)
+
+        allow(response).to receive(:round).and_return(1)
+        assignment_double = double('assignment', assignment_questionnaires: aq_relation)
+        allow(response).to receive(:reviewer_assignment).and_return(assignment_double)
+
+        expect { response.questionnaire }.not_to raise_error
+        expect(response.questionnaire).to be_nil
+      end
+    end
+  end
 end
diff --git a/spec/models/student_task_spec.rb b/spec/models/student_task_spec.rb
index 94b9e7ffc..5a73193d0 100644
--- a/spec/models/student_task_spec.rb
+++ b/spec/models/student_task_spec.rb
@@ -5,32 +5,44 @@
 
 RSpec.describe StudentTask, type: :model do
   before(:each) do
-    @assignment = double(name: "Final Project")
+    @course     = double(name: "CSC 517")
+    @assignment = double(name: "Final Project", course: @course, id: 1)
+    @team       = double(id: 1, hyperlinks: [], has_submissions?: false)
+    @user       = double(id: 1, try: nil)
     @participant = double(
       assignment: @assignment,
-      topic: "E2442",
-      current_stage: "finished",
-      stage_deadline: "2024-04-23",
-      permission_granted: true
+      topic:             "E2442",
+      current_stage:     "submission",
+      stage_deadline:    "2024-04-23",
+      permission_granted: true,
+      team:              @team,
+      user:              @user,
+      id:                1
     )
-
+    allow(DueDate).to receive(:current_stage_for).with(@assignment).and_return("submission")
+    allow(DueDate).to receive(:next_due_date).with(@assignment).and_return(nil)
+    allow(SignedUpTeam).to receive(:find_by).and_return(nil)
+    allow(ReviewResponseMap).to receive(:where).and_return([])
+    allow(FeedbackResponseMap).to receive(:where).and_return([])
   end
 
   describe ".initialize" do
     it "correctly assigns all attributes" do
       args = {
-        assignment: @assignment,
-        current_stage: "finished",
-        participant: @participant,
-        stage_deadline: "2024-04-23",
-        topic: "E2442",
+        assignment:         @assignment,
+        course:             "CSC 517",
+        current_stage:      "submission",
+        participant:        @participant,
+        stage_deadline:     "2024-04-23",
+        topic:              "E2442",
         permission_granted: false
       }
 
       student_task = StudentTask.new(args)
 
       expect(student_task.assignment.name).to eq("Final Project")
-      expect(student_task.current_stage).to eq("finished")
+      expect(student_task.course).to eq("CSC 517")
+      expect(student_task.current_stage).to eq("submission")
       expect(student_task.participant).to eq(@participant)
       expect(student_task.stage_deadline).to eq("2024-04-23")
       expect(student_task.topic).to eq("E2442")
@@ -40,13 +52,12 @@
 
   describe ".from_participant" do
     it "creates an instance from a participant instance" do
-
       student_task = StudentTask.create_from_participant(@participant)
 
-      expect(student_task.assignment).to eq(@participant.assignment.name)
-      expect(student_task.topic).to eq(@participant.topic)
-      expect(student_task.current_stage).to eq(@participant.current_stage)
-      expect(student_task.stage_deadline).to eq(Time.parse(@participant.stage_deadline))
+      expect(student_task.assignment).to eq(@assignment.name)
+      expect(student_task.course).to eq(@course.name)
+      expect(student_task.topic).to be_nil # SignedUpTeam stubbed to return nil
+      expect(student_task.current_stage).to eq("submission")
       expect(student_task.permission_granted).to be @participant.permission_granted
       expect(student_task.participant).to be @participant
     end
@@ -72,14 +83,347 @@
   end
 
   describe ".from_participant_id" do
-    it "fetches a participant by id and creates a student task from it" do
-      allow(Participant).to receive(:find_by).with(id: 1).and_return(@participant)
+    it "uses AssignmentParticipant (not Participant) to look up by id" do
+      # The fix: Participant.find_by was replaced with AssignmentParticipant.find_by so that
+      # other Participant subclasses (e.g. TeammateReviewParticipant) for the same user cannot
+      # be returned and cause type-mismatch 500s later in create_from_participant.
+      allow(AssignmentParticipant).to receive(:find_by).with(id: 1).and_return(@participant)
 
-      expect(Participant).to receive(:find_by).with(id: 1).and_return(@participant)
+      expect(AssignmentParticipant).to receive(:find_by).with(id: 1).and_return(@participant)
       expect(StudentTask).to receive(:create_from_participant).with(@participant)
 
       StudentTask.from_participant_id(1)
     end
+
+    it "does not call the base Participant class for the lookup" do
+      allow(AssignmentParticipant).to receive(:find_by).with(id: 1).and_return(@participant)
+      allow(StudentTask).to receive(:create_from_participant)
+
+      expect(Participant).not_to receive(:find_by)
+
+      StudentTask.from_participant_id(1)
+    end
+  end
+
+  describe ".from_user" do
+    it "sorts by a single composite key [course, assignment, stage_deadline]" do
+      # Three chained sort_by calls were non-deterministic (each overwrote the previous).
+      # The fix uses one sort_by { [course, assignment, stage_deadline] }.
+      course_a = double(name: "AAA Course")
+      course_b = double(name: "BBB Course")
+
+      assignment_a1 = double(name: "Alpha", course: course_a, id: 10)
+      assignment_a2 = double(name: "Beta",  course: course_a, id: 11)
+      assignment_b1 = double(name: "Alpha", course: course_b, id: 12)
+
+      deadline_near = Time.parse("2025-01-01")
+      deadline_far  = Time.parse("2025-06-01")
+
+      # Participants stub — course_a/Beta should sort before course_a/Alpha because of
+      # alphabetical assignment name comparison within the same course
+      p1 = double(assignment: assignment_a2, team: @team, user: @user, permission_granted: true, id: 1)
+      p2 = double(assignment: assignment_a1, team: @team, user: @user, permission_granted: true, id: 2)
+      p3 = double(assignment: assignment_b1, team: @team, user: @user, permission_granted: true, id: 3)
+
+      user = double(id: 42)
+      allow(DueDate).to receive(:current_stage_for).and_return("submission")
+      allow(DueDate).to receive(:next_due_date).and_return(nil)
+      allow(SignedUpTeam).to receive(:find_by).and_return(nil)
+      # from_user chains .includes(...) on the where result, so we need a relation
+      # double that responds to both includes calls and delegates map to the array.
+      relation = double('relation')
+      allow(AssignmentParticipant).to receive(:where).with(user_id: user.id).and_return(relation)
+      allow(relation).to receive(:includes).with(assignment: :course).and_return(relation)
+      allow(relation).to receive(:includes).with(:user).and_return(relation)
+      allow(relation).to receive(:map) { |&blk| [p3, p1, p2].map(&blk) }
+
+      tasks = StudentTask.from_user(user)
+
+      courses   = tasks.map(&:course)
+      expect(courses).to eq(courses.sort), "tasks should be sorted by course first"
+
+      same_course_tasks = tasks.select { |t| t.course == "AAA Course" }
+      assignments = same_course_tasks.map(&:assignment)
+      expect(assignments).to eq(assignments.sort), "tasks within same course should be sorted by assignment name"
+    end
+  end
+
+  describe "#revise?" do
+    let(:participant) { double('participant', id: 1) }
+    let(:task) { StudentTask.new(participant: participant, current_stage: 'submission') }
+
+    context "when current stage is submission and team has hyperlinks" do
+      it "returns true" do
+        team = double('team', hyperlinks: ['http://example.com'], has_submissions?: false)
+        allow(participant).to receive(:team).and_return(team)
+        expect(task.send(:revise?)).to be true
+      end
+    end
+
+    context "when current stage is submission and team has no submissions" do
+      it "returns false" do
+        team = double('team', hyperlinks: [], has_submissions?: false)
+        allow(participant).to receive(:team).and_return(team)
+        expect(task.send(:revise?)).to be false
+      end
+    end
+
+    context "when current stage is review and submitted review exists" do
+      it "returns true" do
+        task_review = StudentTask.new(participant: participant, current_stage: 'review')
+        allow(ReviewResponseMap).to receive(:where).and_return(
+          double(joins: double(where: double(exists?: true)))
+        )
+        expect(task_review.send(:revise?)).to be true
+      end
+    end
+
+    context "when current stage is review and no submitted review exists" do
+      it "returns false" do
+        task_review = StudentTask.new(participant: participant, current_stage: 'review')
+        allow(ReviewResponseMap).to receive(:where).and_return(
+          double(joins: double(where: double(exists?: false)))
+        )
+        expect(task_review.send(:revise?)).to be false
+      end
+    end
+  end
+
+  describe "#not_started?" do
+    let(:participant) { double('participant', id: 1) }
+
+    context "when in work stage and no work done" do
+      it "returns true for submission stage with no submissions" do
+        task = StudentTask.new(participant: participant, current_stage: 'submission')
+        team = double('team', hyperlinks: [], has_submissions?: false)
+        allow(participant).to receive(:team).and_return(team)
+        expect(task.not_started?).to be true
+      end
+
+      it "returns true for review stage with no reviews given" do
+        task = StudentTask.new(participant: participant, current_stage: 'review')
+        allow(ReviewResponseMap).to receive(:where).and_return(
+          double(joins: double(where: double(exists?: false)))
+        )
+        expect(task.not_started?).to be true
+      end
+    end
+
+    context "when in work stage and work has been done" do
+      it "returns false when submission has been made" do
+        task = StudentTask.new(participant: participant, current_stage: 'submission')
+        team = double('team', hyperlinks: ['http://example.com'], has_submissions?: false)
+        allow(participant).to receive(:team).and_return(team)
+        expect(task.not_started?).to be false
+      end
+    end
+
+    context "when not in a work stage" do
+      it "returns false for Finished stage" do
+        task = StudentTask.new(participant: participant, current_stage: 'Finished')
+        expect(task.not_started?).to be false
+      end
+
+      it "returns false for signup stage" do
+        task = StudentTask.new(participant: participant, current_stage: 'signup')
+        expect(task.not_started?).to be false
+      end
+    end
+  end
+
+  describe ".teamed_students" do
+    let!(:institution)     { Institution.create!(name: 'NCSU') }
+    let!(:instructor_role) { Role.find_by(name: 'Instructor') || Role.create!(name: 'Instructor') }
+    let!(:student_role)    { Role.find_by(name: 'Student')    || Role.create!(name: 'Student') }
+    let!(:instructor)      { User.create!(name: 'ts_instructor', email: 'ts_inst@test.com', full_name: 'TS Inst', password: 'password', role_id: instructor_role.id, institution: institution) }
+    let!(:course)          { Course.create!(name: 'CSC 517', directory_path: 'csc517', instructor: instructor, institution: institution) }
+    let!(:assignment)      { Assignment.create!(name: 'TS Assignment', instructor: instructor, course: course, is_calibrated: false) }
+    let!(:user)            { User.create!(name: 'ts_student1', email: 'ts_s1@test.com', full_name: 'TS Student One', password: 'password', role_id: student_role.id, institution: institution) }
+    let!(:teammate)        { User.create!(name: 'ts_student2', email: 'ts_s2@test.com', full_name: 'TS Student Two', password: 'password', role_id: student_role.id, institution: institution) }
+
+    # Helper to add a user to a team via both TeamsUser and TeamsParticipant
+    def add_to_team(team, user, assignment)
+      TeamsUser.create!(team_id: team.id, user_id: user.id)
+      participant = AssignmentParticipant.find_or_create_by!(user_id: user.id, parent_id: assignment.id) do |p|
+        p.handle = user.name
+      end
+      TeamsParticipant.find_or_create_by!(team_id: team.id, user_id: user.id, participant_id: participant.id)
+    end
+
+    it "returns teammates grouped by course name" do
+      team = AssignmentTeam.create!(name: 'Team 1', parent_id: assignment.id)
+      add_to_team(team, user, assignment)
+      add_to_team(team, teammate, assignment)
+
+      result = StudentTask.teamed_students(user)
+      expect(result).to have_key('CSC 517')
+      expect(result['CSC 517']).to include('TS Student Two')
+    end
+
+    it "excludes calibrated assignments" do
+      calibrated = Assignment.create!(name: 'Calibrated', instructor: instructor, course: course)
+      team = AssignmentTeam.create!(name: 'Team Cal', parent_id: calibrated.id)
+      add_to_team(team, user, calibrated)
+      add_to_team(team, teammate, calibrated)
+      # is_calibrated is a virtual attr_accessor — stub it on the found record
+      allow(Assignment).to receive(:find_by).with(id: calibrated.id).and_return(
+        instance_double(Assignment, is_calibrated: true, nil?: false)
+      )
+
+      result = StudentTask.teamed_students(user)
+      expect(result.values.flatten).not_to include('TS Student Two')
+    end
+
+    it "excludes the user themselves from teammates" do
+      team = AssignmentTeam.create!(name: 'Team Solo', parent_id: assignment.id)
+      add_to_team(team, user, assignment)
+
+      result = StudentTask.teamed_students(user)
+      expect(result).to be_empty
+    end
+
+    it "returns teammates sorted alphabetically" do
+      student3 = User.create!(name: 'ts_student3', email: 'ts_s3@test.com', full_name: 'Aaron Smith', password: 'password', role_id: student_role.id, institution: institution)
+      team = AssignmentTeam.create!(name: 'Team Sort', parent_id: assignment.id)
+      add_to_team(team, user, assignment)
+      add_to_team(team, teammate, assignment)
+      add_to_team(team, student3, assignment)
+
+      result = StudentTask.teamed_students(user)
+      expect(result['CSC 517']).to eq(result['CSC 517'].sort)
+    end
+  end
+
+  describe ".get_timeline_data" do
+    let!(:institution)  { Institution.find_by(name: 'NCSU') || Institution.create!(name: 'NCSU') }
+    let!(:inst_role)    { Role.find_by(name: 'Instructor') || Role.create!(name: 'Instructor') }
+    let!(:tl_instructor) { User.create!(name: 'tl_inst', email: 'tl_inst@test.com', full_name: 'TL Inst', password: 'password', role_id: inst_role.id, institution: institution) }
+    let!(:assignment)   { Assignment.create!(name: 'Timeline Assignment', instructor: tl_instructor) }
+    let(:participant)   { double('participant', id: 1) }
+
+    it "includes due dates with id: nil" do
+      DueDate.create!(parent: assignment, due_at: 7.days.from_now, deadline_name: 'Submission',
+                      deadline_type_id: 1, submission_allowed_id: 3, review_allowed_id: 3)
+      review_relation = double('relation')
+      allow(review_relation).to receive(:find_each)
+      allow(ReviewResponseMap).to receive(:where).with(reviewer_id: 1).and_return(review_relation)
+      feedback_relation = double('feedback_relation')
+      allow(feedback_relation).to receive(:find_each)
+      allow(FeedbackResponseMap).to receive(:where).with(reviewer_id: 1).and_return(feedback_relation)
+
+      timeline = StudentTask.get_timeline_data(assignment, participant)
+      submission_entry = timeline.find { |t| t['name'].include?('Submission') }
+      expect(submission_entry).not_to be_nil
+      expect(submission_entry['id']).to be_nil
+    end
+
+    it "includes submitted peer review responses with a real id" do
+      map = double('map', id: 1)
+      submitted_response = double('response', id: 99, round: 1, updated_at: 2.days.ago)
+      review_relation = double('relation')
+      allow(review_relation).to receive(:find_each).and_yield(map)
+      allow(ReviewResponseMap).to receive(:where).with(reviewer_id: 1).and_return(review_relation)
+
+      # The fix: queries for is_submitted: true with explicit ordering
+      submitted_scope = double('submitted_scope')
+      allow(submitted_scope).to receive(:each).and_yield(submitted_response)
+      ordered_scope = double('ordered_scope')
+      allow(ordered_scope).to receive(:each).and_yield(submitted_response)
+      allow(Response).to receive(:where).with(map_id: 1, is_submitted: true).and_return(ordered_scope)
+      allow(ordered_scope).to receive(:order).with(updated_at: :desc).and_return(ordered_scope)
+
+      feedback_relation = double('feedback_relation')
+      allow(feedback_relation).to receive(:find_each)
+      allow(FeedbackResponseMap).to receive(:where).with(reviewer_id: 1).and_return(feedback_relation)
+
+      timeline = StudentTask.get_timeline_data(assignment, participant)
+      review_entry = timeline.find { |t| t['name'].include?('peer review') }
+      expect(review_entry).not_to be_nil
+      expect(review_entry['id']).to eq(99)
+    end
+
+    it "excludes unsubmitted/draft peer review responses from the timeline" do
+      map = double('map', id: 1)
+      draft_response = double('draft_response', id: 55, round: 1, is_submitted: false, updated_at: 1.day.ago)
+
+      review_relation = double('relation')
+      allow(review_relation).to receive(:find_each).and_yield(map)
+      allow(ReviewResponseMap).to receive(:where).with(reviewer_id: 1).and_return(review_relation)
+
+      # The submitted scope returns nothing — draft response is excluded
+      empty_ordered = double('empty_ordered')
+      allow(empty_ordered).to receive(:each)
+      allow(empty_ordered).to receive(:order).with(updated_at: :desc).and_return(empty_ordered)
+      allow(Response).to receive(:where).with(map_id: 1, is_submitted: true).and_return(empty_ordered)
+
+      feedback_relation = double('feedback_relation')
+      allow(feedback_relation).to receive(:find_each)
+      allow(FeedbackResponseMap).to receive(:where).with(reviewer_id: 1).and_return(feedback_relation)
+
+      timeline = StudentTask.get_timeline_data(assignment, participant)
+      expect(timeline.any? { |t| t['name'].include?('peer review') }).to be false
+    end
+
+    it "captures both round 1 and round 2 responses for the same map" do
+      # The original code used Response.where(map_id:).last which silently dropped round 1
+      # when a map had two submitted responses. The fix iterates all submitted responses.
+      map = double('map', id: 1)
+      r1 = double('r1', id: 10, round: 1, updated_at: 10.days.ago)
+      r2 = double('r2', id: 11, round: 2, updated_at: 3.days.ago)
+
+      review_relation = double('relation')
+      allow(review_relation).to receive(:find_each).and_yield(map)
+      allow(ReviewResponseMap).to receive(:where).with(reviewer_id: 1).and_return(review_relation)
+
+      ordered_scope = double('ordered_scope')
+      allow(ordered_scope).to receive(:each).and_yield(r1).and_yield(r2)
+      allow(ordered_scope).to receive(:order).with(updated_at: :desc).and_return(ordered_scope)
+      allow(Response).to receive(:where).with(map_id: 1, is_submitted: true).and_return(ordered_scope)
+
+      feedback_relation = double('feedback_relation')
+      allow(feedback_relation).to receive(:find_each)
+      allow(FeedbackResponseMap).to receive(:where).with(reviewer_id: 1).and_return(feedback_relation)
+
+      timeline = StudentTask.get_timeline_data(assignment, participant)
+      review_entries = timeline.select { |t| t['name'].include?('peer review') }
+      expect(review_entries.map { |e| e['round'] }).to contain_exactly(1, 2)
+    end
+
+    it "excludes unsubmitted author feedback responses" do
+      map = double('map', id: 2)
+      review_relation = double('relation')
+      allow(review_relation).to receive(:find_each)
+      allow(ReviewResponseMap).to receive(:where).with(reviewer_id: 1).and_return(review_relation)
+
+      feedback_relation = double('feedback_relation')
+      allow(feedback_relation).to receive(:find_each).and_yield(map)
+      allow(FeedbackResponseMap).to receive(:where).with(reviewer_id: 1).and_return(feedback_relation)
+
+      # The fix: is_submitted: true + order(:desc).first returns nil → feedback skipped
+      ordered = double('ordered')
+      allow(ordered).to receive(:first).and_return(nil)
+      allow(Response).to receive(:where).with(map_id: 2, is_submitted: true).and_return(double(order: ordered))
+
+      timeline = StudentTask.get_timeline_data(assignment, participant)
+      expect(timeline.any? { |t| t['name'] == 'Author feedback' }).to be false
+    end
+
+    it "returns entries sorted by date" do
+      DueDate.create!(parent: assignment, due_at: 7.days.from_now, deadline_name: 'Submission',
+                      deadline_type_id: 1, submission_allowed_id: 3, review_allowed_id: 3)
+      DueDate.create!(parent: assignment, due_at: 14.days.from_now, deadline_name: 'Review',
+                      deadline_type_id: 2, submission_allowed_id: 3, review_allowed_id: 3)
+      review_relation = double('relation')
+      allow(review_relation).to receive(:find_each)
+      allow(ReviewResponseMap).to receive(:where).with(reviewer_id: 1).and_return(review_relation)
+      feedback_relation = double('feedback_relation')
+      allow(feedback_relation).to receive(:find_each)
+      allow(FeedbackResponseMap).to receive(:where).with(reviewer_id: 1).and_return(feedback_relation)
+
+      timeline = StudentTask.get_timeline_data(assignment, participant)
+      dates = timeline.map { |t| t['date'] }
+      expect(dates).to eq(dates.sort)
+    end
   end
 
 end
\ No newline at end of file
diff --git a/spec/requests/api/v1/grades_controller_spec.rb b/spec/requests/api/v1/grades_controller_spec.rb
index 512028e9b..4b09ad62f 100644
--- a/spec/requests/api/v1/grades_controller_spec.rb
+++ b/spec/requests/api/v1/grades_controller_spec.rb
@@ -345,65 +345,65 @@
       parameter name: :participant_id, in: :path, type: :integer, description: 'ID of the participant'
       parameter name: 'Authorization', in: :header, type: :string, required: true, description: 'Bearer token'
 
-      response '200', 'Returns mapping and request contract for new review' do
-        let(:participant_id) { participant.id }
-
-        run_test! do |response|
-          data = JSON.parse(response.body)
-          expect(data).to have_key('map_id')
-          expect(data).to have_key('response_id')
-          expect(data['response_id']).to be_nil
-          expect(data['request_method']).to eq('POST')
-          expect(data['request_path']).to eq('/responses')
-          expect(data['request_payload']).to eq({ 'response_map_id' => data['map_id'] })
-        end
-      end
-
-      response '200', 'Returns mapping and request contract for existing review' do
-        let(:participant_id) { participant.id }
-
-        before do
-          reviewer = AssignmentParticipant.create!(user_id: instructor.id, parent_id: assignment.id, handle: instructor.name)
-          mapping = ReviewResponseMap.create!(
-            reviewed_object_id: assignment.id,
-            reviewer_id: reviewer.id,
-            reviewee_id: team.id
-          )
-          Response.create!(map_id: mapping.id, is_submitted: false)
-        end
-
-        run_test! do |response|
-          data = JSON.parse(response.body)
-          expect(data['response_id']).to be_present
-          expect(data['request_method']).to eq('PATCH')
-          expect(data['request_path']).to eq("/responses/#{data['response_id']}")
-          expect(data['request_payload']).to eq({})
-        end
-      end
-
-      response '200', 'Returns create contract when existing review is submitted' do
-        let(:participant_id) { participant.id }
-
-        before do
-          reviewer = AssignmentParticipant.create!(user_id: instructor.id, parent_id: assignment.id, handle: instructor.name)
-          mapping = ReviewResponseMap.create!(
-            reviewed_object_id: assignment.id,
-            reviewer_id: reviewer.id,
-            reviewee_id: team.id
-          )
-          Response.create!(map_id: mapping.id, is_submitted: true)
-        end
-
-        run_test! do |response|
-          data = JSON.parse(response.body)
-          expect(data['response_id']).to be_nil
-          expect(data['request_method']).to eq('POST')
-          expect(data['request_path']).to eq('/responses')
-          expect(data['request_payload']).to eq({ 'response_map_id' => data['map_id'] })
-        end
-      end
-
-      response '404', 'Participant not found' do
+      response '200', 'Returns mapping and request contract for new review' do
+        let(:participant_id) { participant.id }
+
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          expect(data).to have_key('map_id')
+          expect(data).to have_key('response_id')
+          expect(data['response_id']).to be_nil
+          expect(data['request_method']).to eq('POST')
+          expect(data['request_path']).to eq('/responses')
+          expect(data['request_payload']).to eq({ 'response_map_id' => data['map_id'] })
+        end
+      end
+
+      response '200', 'Returns mapping and request contract for existing review' do
+        let(:participant_id) { participant.id }
+
+        before do
+          reviewer = AssignmentParticipant.create!(user_id: instructor.id, parent_id: assignment.id, handle: instructor.name)
+          mapping = ReviewResponseMap.create!(
+            reviewed_object_id: assignment.id,
+            reviewer_id: reviewer.id,
+            reviewee_id: team.id
+          )
+          Response.create!(map_id: mapping.id, is_submitted: false)
+        end
+
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          expect(data['response_id']).to be_present
+          expect(data['request_method']).to eq('PATCH')
+          expect(data['request_path']).to eq("/responses/#{data['response_id']}")
+          expect(data['request_payload']).to eq({})
+        end
+      end
+
+      response '200', 'Returns create contract when existing review is submitted' do
+        let(:participant_id) { participant.id }
+
+        before do
+          reviewer = AssignmentParticipant.create!(user_id: instructor.id, parent_id: assignment.id, handle: instructor.name)
+          mapping = ReviewResponseMap.create!(
+            reviewed_object_id: assignment.id,
+            reviewer_id: reviewer.id,
+            reviewee_id: team.id
+          )
+          Response.create!(map_id: mapping.id, is_submitted: true)
+        end
+
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          expect(data['response_id']).to be_nil
+          expect(data['request_method']).to eq('POST')
+          expect(data['request_path']).to eq('/responses')
+          expect(data['request_payload']).to eq({ 'response_map_id' => data['map_id'] })
+        end
+      end
+
+      response '404', 'Participant not found' do
         let(:participant_id) { 999 }
 
         run_test! do |response|
@@ -452,10 +452,122 @@
     end
 
     it 'denies TA from assigning grades' do
-      patch "/grades/#{participant.id}/assign_grade", 
+      patch "/grades/#{participant.id}/assign_grade",
             params: { grade_for_submission: 90 },
             headers: { 'Authorization' => "Bearer #{ta_token}" }
       expect(response).to have_http_status(:forbidden)
     end
   end
-end
+
+  # -------------------------------------------------------------------------
+  # inject_section_headers — section headings appear in view_our_scores
+  # -------------------------------------------------------------------------
+  # inject_section_headers is a private helper called inside get_heatgrid_data_for.
+  # We test its effect through the public view_our_scores endpoint: when a questionnaire
+  # has SectionHeader items, the response's round arrays must contain { type: "header" }
+  # sentinel hashes at the correct positions.
+  describe 'section headers in view_our_scores' do
+    let!(:questionnaire) do
+      Questionnaire.create!(
+        name: 'Section Header Test Rubric',
+        instructor_id: instructor.id,
+        private: false,
+        min_question_score: 0,
+        max_question_score: 5,
+        questionnaire_type: 'ReviewQuestionnaire'
+      )
+    end
+
+    before do
+      # SectionHeader before the first group
+      Item.create!(questionnaire_id: questionnaire.id, txt: 'Code Quality', weight: 0,
+                   seq: 1, question_type: 'SectionHeader', break_before: true)
+      # Two scoreable CriterionItems
+      Item.create!(questionnaire_id: questionnaire.id, txt: 'Is the code readable?', weight: 1,
+                   seq: 2, question_type: 'CriterionItem', break_before: true)
+      Item.create!(questionnaire_id: questionnaire.id, txt: 'Is the code tested?', weight: 1,
+                   seq: 3, question_type: 'CriterionItem', break_before: true)
+
+      AssignmentQuestionnaire.create!(
+        assignment_id: assignment.id,
+        questionnaire_id: questionnaire.id,
+        used_in_round: nil,
+        questionnaire_weight: 100
+      )
+
+      # Reviewer submits a response covering the two scored items
+      reviewer_participant = AssignmentParticipant.create!(
+        user_id: student2.id, parent_id: assignment.id, handle: student2.name
+      )
+      map = ReviewResponseMap.create!(
+        reviewed_object_id: assignment.id,
+        reviewer_id: reviewer_participant.id,
+        reviewee_id: team.id
+      )
+      resp = Response.create!(map_id: map.id, is_submitted: true, round: 1)
+      scored_items = Item.where(questionnaire_id: questionnaire.id, question_type: 'CriterionItem').order(:seq)
+      scored_items.each do |item|
+        Answer.create!(response_id: resp.id, item_id: item.id, answer: 4, comments: 'Good')
+      end
+    end
+
+    it 'injects a section header sentinel at position 0 in the round array' do
+      get "/grades/#{assignment.id}/view_our_scores",
+          headers: { 'Authorization' => "Bearer #{student_token}" }
+
+      expect(response).to have_http_status(:ok)
+      data = JSON.parse(response.body)
+
+      round_arrays = data['reviews_of_our_work'].values
+      expect(round_arrays).not_to be_empty
+
+      first_round = round_arrays.first
+      expect(first_round).to be_an(Array)
+
+      # The first element should be the SectionHeader sentinel, not a scores array
+      header_entry = first_round.find { |e| e.is_a?(Hash) && e['type'] == 'header' }
+      expect(header_entry).not_to be_nil
+      expect(header_entry['txt']).to eq('Code Quality')
+    end
+
+    it 'places the header sentinel before the scored items, not after' do
+      get "/grades/#{assignment.id}/view_our_scores",
+          headers: { 'Authorization' => "Bearer #{student_token}" }
+
+      data = JSON.parse(response.body)
+      first_round = data['reviews_of_our_work'].values.first
+
+      header_index = first_round.index { |e| e.is_a?(Hash) && e['type'] == 'header' }
+      # There should be scored-item arrays after the header
+      expect(first_round.length).to be > header_index + 1
+    end
+
+    it 'does not inject headers when the questionnaire has none' do
+      # Remove the SectionHeader item
+      Item.where(questionnaire_id: questionnaire.id, question_type: 'SectionHeader').destroy_all
+
+      get "/grades/#{assignment.id}/view_our_scores",
+          headers: { 'Authorization' => "Bearer #{student_token}" }
+
+      data = JSON.parse(response.body)
+      first_round = data['reviews_of_our_work'].values.first
+
+      header_entry = (first_round || []).find { |e| e.is_a?(Hash) && e['type'] == 'header' }
+      expect(header_entry).to be_nil
+    end
+  end
+
+  # -------------------------------------------------------------------------
+  # student_tasks#show — AssignmentParticipant constraint
+  # -------------------------------------------------------------------------
+  describe 'student_tasks show endpoint' do
+    it 'returns 404 when the id belongs to a non-AssignmentParticipant' do
+      # Only AssignmentParticipant records should be matched — other Participant subclasses
+      # for the same user must not slip through and cause type-mismatch 500s.
+      get '/student_tasks/show/999999',
+          headers: { 'Authorization' => "Bearer #{student_token}" }
+
+      expect(response.status).to be_in([404, 403])
+    end
+  end
+end
diff --git a/spec/requests/api/v1/student_tasks_controller_spec.rb b/spec/requests/api/v1/student_tasks_controller_spec.rb
index 481f7b5d1..0157de601 100644
--- a/spec/requests/api/v1/student_tasks_controller_spec.rb
+++ b/spec/requests/api/v1/student_tasks_controller_spec.rb
@@ -47,26 +47,33 @@
 
       # The "proper JSON schema" test
       response '200', 'authorized request has proper JSON schema' do
-        before do
-          # 1) Create an Assignment
+        let!(:setup_tasks) do
+          institution = Institution.create!(name: 'NCSU Test')
+          course = Course.create!(
+            name: "CSC 517",
+            directory_path: "csc517",
+            instructor: instructor,
+            institution: institution
+          )
           assignment = Assignment.create!(
             name: "Sample Assignment",
-            instructor: instructor
+            instructor: instructor,
+            course: course
+          )
+          DueDate.create!(
+            parent: assignment,
+            due_at: 7.days.from_now,
+            deadline_name: 'Submission',
+            deadline_type_id: 1,
+            submission_allowed_id: 3,
+            review_allowed_id: 3
           )
-
-          # 2) Create N Participants for our student, each with different data
           5.times do |i|
             AssignmentParticipant.create!(
               user_id: studenta.id,
               parent_id: assignment.id,
-              handle: studenta.name,
-              permission_granted: [true, false].sample,
-              # store “stage” and “deadline” fields as your Participant model expects
-              # e.g. might be:
-              topic: "Topic #{i}",
-              stage_deadline: (Time.now + (i + 1).days).to_s,
-              # and if it has “current_stage” or something:
-              current_stage: "Stage #{i}"
+              handle: "#{studenta.name}_#{i}",
+              permission_granted: [true, false].sample
             )
           end
         end
@@ -77,12 +84,9 @@
           expect(data.size).to eq(5)
 
           data.each do |task|
-            # Because StudentTask is just a plain Ruby object,
-            # we expect the controller to have built it from the Participant
-            expect(task['assignment']).to        be_a(String)
-            expect(task['current_stage']).to     be_a(String)
-            expect(task['stage_deadline']).to    be_a(String)
-            expect(task['topic']).to             be_a(String)
+            expect(task['assignment']).to be_a(String)
+            expect(task['current_stage']).to be_a(String)
+            expect(task['stage_deadline']).to be_a(String)
             expect(task['permission_granted']).to be_in([true, false])
           end
         end
@@ -99,49 +103,40 @@
   # -------------------------------------------------------------------------
   # /student_tasks/view
   # -------------------------------------------------------------------------
-  path '/student_tasks/view' do
+  path '/student_tasks/show/{id}' do
     get 'Retrieve a specific student task by ID' do
       tags 'StudentTasks'
       produces 'application/json'
-      parameter name: 'id', in: :query, type: :Integer, required: true
+      parameter name: :id, in: :path, type: :integer, required: true
       parameter name: 'Authorization', in: :header, type: :string
 
       # 200 test
       response '200', 'successful retrieval of a student task' do
-        let!(:assignment) do
-          Assignment.create!(name: "Test Assignment", instructor: instructor)
-        end
-
-        # Create *one* participant for the student
-        let!(:participant) do
-          AssignmentParticipant.create!(
-            user_id: studenta.id,
-            parent_id: assignment.id,
-            handle: studenta.name,
-            current_stage: "Review",
-            stage_deadline: (Time.now + 7.days).to_s,
-            topic: "Topic XYZ",
-            permission_granted: true
-          )
+        let!(:view_participant) do
+          institution = Institution.create!(name: 'NCSU View Test')
+          course = Course.create!(name: "CSC 517 View", directory_path: "csc517_view", instructor: instructor, institution: institution)
+          assignment = Assignment.create!(name: "Test Assignment", instructor: instructor, course: course)
+          DueDate.create!(parent: assignment, due_at: 7.days.from_now, deadline_name: 'Submission', deadline_type_id: 1, submission_allowed_id: 3, review_allowed_id: 3)
+          AssignmentParticipant.create!(user_id: studenta.id, parent_id: assignment.id, handle: studenta.name, permission_granted: true)
         end
 
-        # This “id” is the participant’s ID to be looked up
-        let(:id) { participant.id }
+        let(:id) { view_participant.id }
 
         run_test! do |response|
           data = JSON.parse(response.body)
-          expect(data['assignment']).to        eq("Test Assignment")
-          expect(data['current_stage']).to     eq("Review")
-          expect(data['stage_deadline']).to    be_a(String)  # e.g. "YYYY-MM-DD..."
-          expect(data['topic']).to             eq("Topic XYZ")
+          expect(data['assignment']).to eq("Test Assignment")
+          expect(data['current_stage']).to be_a(String)
+          expect(data['stage_deadline']).to be_a(String)
           expect(data['permission_granted']).to be true
+          expect(data['due_dates']).to be_an(Array)
         end
       end
 
-      response '500', 'participant not found' do
+      response '404', 'participant not found' do
         let(:id) { -1 }
         run_test! do |response|
-          expect(response.status).to eq(500)
+          data = JSON.parse(response.body)
+          expect(data['error']).to eq('Participant not found')
         end
       end
 
@@ -155,4 +150,96 @@
       end
     end
   end
+
+  # -------------------------------------------------------------------------
+  # /student_tasks/show/:id
+  # -------------------------------------------------------------------------
+  path '/student_tasks/show/{id}' do
+    get 'Retrieve a specific student task with timeline by participant ID' do
+      tags 'StudentTasks'
+      produces 'application/json'
+      parameter name: :id, in: :path, type: :integer, required: true
+      parameter name: 'Authorization', in: :header, type: :string
+
+      response '200', 'successful retrieval with due dates' do
+        let!(:show_participant) do
+          institution = Institution.create!(name: 'NCSU Show Test')
+          course = Course.create!(name: 'CSC 517 Show', directory_path: 'csc517_show', instructor: instructor, institution: institution)
+          assignment = Assignment.create!(name: 'Timeline Assignment', instructor: instructor, course: course)
+          DueDate.create!(parent: assignment, due_at: 7.days.from_now, deadline_name: 'Submission', deadline_type_id: 1, submission_allowed_id: 3, review_allowed_id: 3)
+          AssignmentParticipant.create!(user_id: studenta.id, parent_id: assignment.id, handle: studenta.name, permission_granted: true)
+        end
+        let(:id) { show_participant.id }
+
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          expect(data['assignment']).to eq('Timeline Assignment')
+          expect(data['due_dates']).to be_an(Array)
+        end
+      end
+
+      response '404', 'participant not found' do
+        let(:id) { -1 }
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          expect(data['error']).to eq('Participant not found')
+        end
+      end
+
+      response '403', 'unauthorized access to another participants task' do
+        let!(:other_user) do
+          User.create!(
+            name: 'otherstudent',
+            password_digest: 'password',
+            role_id: @roles[:student].id,
+            full_name: 'Other Student',
+            email: 'other@example.com'
+          )
+        end
+        let!(:assignment) { Assignment.create!(name: 'Other Assignment', instructor: instructor) }
+        let!(:other_participant) do
+          AssignmentParticipant.create!(
+            user_id: other_user.id,
+            parent_id: assignment.id,
+            handle: other_user.name
+          )
+        end
+        let(:id) { other_participant.id }
+
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          expect(data['error']).to eq("Unauthorized access to participant's task")
+        end
+      end
+
+      response '401', 'unauthorized request' do
+        let(:'Authorization') { 'Bearer ' }
+        let(:id) { 1 }
+        run_test!
+      end
+    end
+  end
+
+  # -------------------------------------------------------------------------
+  # /student_tasks/team
+  # -------------------------------------------------------------------------
+  path '/student_tasks/team' do
+    get 'Retrieve teammates grouped by course for current user' do
+      tags 'StudentTasks'
+      produces 'application/json'
+      parameter name: 'Authorization', in: :header, type: :string
+
+      response '200', 'returns teammates grouped by course' do
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          expect(data).to be_a(Hash)
+        end
+      end
+
+      response '401', 'unauthorized request' do
+        let(:'Authorization') { 'Bearer ' }
+        run_test!
+      end
+    end
+  end
 end
diff --git a/spec/routing/student_tasks_routing_spec.rb b/spec/routing/student_tasks_routing_spec.rb
index 1e34d93cb..f523bd356 100644
--- a/spec/routing/student_tasks_routing_spec.rb
+++ b/spec/routing/student_tasks_routing_spec.rb
@@ -27,5 +27,17 @@
     it "routes to #destroy" do
       expect(delete: "/student_tasks/1").to route_to("student_tasks#destroy", id: "1")
     end
+
+    it "routes GET /student_tasks/list to #list" do
+      expect(get: "/student_tasks/list").to route_to("student_tasks#list")
+    end
+
+    it "routes GET /student_tasks/show/:id to #show" do
+      expect(get: "/student_tasks/show/1").to route_to("student_tasks#show", id: "1")
+    end
+
+    it "routes GET /student_tasks/team to #team" do
+      expect(get: "/student_tasks/team").to route_to("student_tasks#team")
+    end
   end
 end

From 02b36b287bbd4edb4e04150000c3cbb42d36fc9a Mon Sep 17 00:00:00 2001
From: Bestin Lalu <46861674+gazarack@users.noreply.github.com>
Date: Fri, 12 Jun 2026 19:48:44 -0400
Subject: [PATCH 2/4] Implement coderabbit comments

---
 app/controllers/grades_controller.rb          | 141 ++++++++----------
 app/controllers/student_tasks_controller.rb   |  19 +--
 .../teams_participants_controller.rb          |   6 +-
 app/models/student_task.rb                    |  19 ++-
 spec/models/student_task_spec.rb              |  25 +++-
 .../requests/api/v1/grades_controller_spec.rb |   6 +-
 .../api/v1/student_tasks_controller_spec.rb   | 108 ++++++--------
 7 files changed, 162 insertions(+), 162 deletions(-)

diff --git a/app/controllers/grades_controller.rb b/app/controllers/grades_controller.rb
index 3efaea55a..5f8a05039 100644
--- a/app/controllers/grades_controller.rb
+++ b/app/controllers/grades_controller.rb
@@ -340,107 +340,94 @@ def get_my_scores_data(participant)
         }
     end
 
-    # it returns the heatgrid data for a collection of maps (ReviewResponseMap/FeedbackResponseMap/TeammateReviewResponseMap)
+    # Returns heatgrid data for a collection of maps (ReviewResponseMap/FeedbackResponseMap/TeammateReviewResponseMap).
+    # Groups submitted scores by round symbol (:Review-Round-1, etc.) and injects SectionHeader sentinels.
     def get_heatgrid_data_for(maps)
-        # Initialize a hash to store scores grouped by review rounds
         reviewee_scores = {}
         return reviewee_scores if maps.empty?
 
-        # check if the assignment uses different rubrics for each round
-        if @assignment.varying_rubrics_by_round?
-            # Retrieve all round numbers that have distinct questionnaires
-            rounds = @assignment.review_rounds(maps.first.questionnaire_type)
+        # Resolve the DB-stored questionnaire_type(s) once, hoisted above both branches.
+        # maps.first.questionnaire_type returns a short form: 'Review', 'TeammateReview',
+        # or 'AuthorFeedback'. The questionnaires table stores the full class name, but
+        # 'AuthorFeedback' has two historical variants: 'Author FeedbackQuestionnaire' (with
+        # space) and 'AuthorFeedbackQuestionnaire' (no space). Matching against
+        # Questionnaire::QUESTIONNAIRE_TYPES with a whitespace-insensitive compare covers both
+        # and generates an IN clause, so neither variant is silently missed.
+        short_type = maps.first.questionnaire_type
+        db_questionnaire_types = Questionnaire::QUESTIONNAIRE_TYPES.select { |t|
+            t.gsub(' ', '').casecmp?("#{short_type}Questionnaire")
+        }
 
+        if @assignment.varying_rubrics_by_round?
+            rounds = @assignment.review_rounds(short_type)
             rounds.each do |round|
-                # Create a symbol like :Review-Round-1 or :TeammateReview-Round-2
-                round_symbol = ("#{maps.first.questionnaire_type}-Round-#{round}").to_sym
-
-                # Initialize the array to hold scores for the current round
-                reviewee_scores[round_symbol] = []
-
-                # Go through each response map (i.e., reviewer mapping)
-                maps.each_with_index do |map, index|
-                    # Find the most recent submitted response for the current round
-                    submitted_round_response = map.responses.select do |r|
-                        r.round == round && r.is_submitted && r.map_id == map.id
-                    end.last
-
-                    # Skip if no valid response was submitted
-                    next if submitted_round_response.nil?
-
-                    # Go through each score in the submitted response
-                    submitted_round_response.scores.each_with_index do |score, newIndex|
-                        # Initialize sub-array if it doesn't exist
-                        reviewee_scores[round_symbol][newIndex] ||= []
-
-                        # Add the score's answer, optionally anonymizing reviewer name                        
-                        reviewee_scores[round_symbol][newIndex] << get_answer(score, index)
-                    end
-                end
-
-                reviewee_scores[round_symbol].each_with_index do |scores_array, idx|
-                    # Sort each question's answers array by reviewer_name and reviwee_name
-                    reviewee_scores[round_symbol][idx] = scores_array.sort_by { |answer| [answer[:reviewer_name].downcase , answer[:reviewee_name].downcase] }
-                end
-
-                # Inject SectionHeader sentinels so the frontend can render section headings.
-                # Constrain by questionnaire_type so we look up the rubric that actually matches
-                # the maps we're processing (Review, TeammateReview, Feedback) rather than
-                # returning any AQ for this round — which would be wrong when multiple
-                # questionnaire types are configured for the same round and assignment.
                 aq = AssignmentQuestionnaire
                        .joins(:questionnaire)
                        .where(assignment_id: @assignment.id, used_in_round: round)
-                       .where(questionnaires: { type: maps.first.questionnaire_type })
+                       .where(questionnaires: { questionnaire_type: db_questionnaire_types })
                        .first
-                inject_section_headers(reviewee_scores[round_symbol], aq&.questionnaire_id)
+                reviewee_scores = accumulate_round_scores(reviewee_scores, maps, round, aq&.questionnaire_id)
             end
-
         else
-            # Non-varying rubric branch. varying_rubrics_by_round? == false does NOT mean
-            # "single round" — an assignment may reuse the same rubric across multiple rounds.
-            # The original code folded everything into a synthetic Round-1 bucket and used
-            # .last, which silently lost earlier rounds. Instead, discover the actual rounds
-            # present in submitted responses and group each one separately.
+            # varying_rubrics_by_round? == false does NOT mean single-round — the same rubric
+            # may be reused across multiple rounds. Discover actual rounds from submitted responses.
             rounds = maps.flat_map { |map|
                 map.responses.select(&:is_submitted).map(&:round)
             }.compact.uniq.sort
 
-            # Guard: if no submitted responses exist, nothing to show.
             return reviewee_scores if rounds.empty?
 
+            # For non-varying assignments, AssignmentQuestionnaire is stored with used_in_round: nil
+            # regardless of how many actual response rounds exist. Look it up once outside the loop.
+            aq = AssignmentQuestionnaire
+                   .joins(:questionnaire)
+                   .where(assignment_id: @assignment.id, used_in_round: nil)
+                   .where(questionnaires: { questionnaire_type: db_questionnaire_types })
+                   .first
+
             rounds.each do |round|
-                round_symbol = ("#{maps.first.questionnaire_type}-Round-#{round}").to_sym
-                reviewee_scores[round_symbol] = []
-
-                maps.each_with_index do |map, index|
-                    submitted_response = map.responses.select { |r|
-                        r.round == round && r.is_submitted && r.map_id == map.id
-                    }.last
-                    next if submitted_response.nil?
-
-                    submitted_response.scores.each_with_index do |score, new_index|
-                        reviewee_scores[round_symbol][new_index] ||= []
-                        reviewee_scores[round_symbol][new_index] << get_answer(score, index)
-                    end
-                end
+                reviewee_scores = accumulate_round_scores(reviewee_scores, maps, round, aq&.questionnaire_id)
+            end
+        end
 
-                reviewee_scores[round_symbol].each_with_index do |scores_array, idx|
-                    reviewee_scores[round_symbol][idx] = scores_array.sort_by { |answer|
-                        [answer[:reviewer_name].downcase, answer[:reviewee_name].downcase]
-                    }
-                end
+        reviewee_scores
+    end
 
-                # Inject SectionHeader sentinels. For non-varying assignments used_in_round
-                # is nil in AssignmentQuestionnaire regardless of how many actual response
-                # rounds were recorded, so look up by nil rather than by the response round.
-                aq = AssignmentQuestionnaire.where(assignment_id: @assignment.id, used_in_round: nil).first
-                inject_section_headers(reviewee_scores[round_symbol], aq&.questionnaire_id)
+    # Accumulates scores for one round into reviewee_scores and injects SectionHeader sentinels.
+    # Extracted from get_heatgrid_data_for to eliminate the ~50-line duplicate that existed
+    # between the varying-rubric and non-varying branches.
+    def accumulate_round_scores(reviewee_scores, maps, round, questionnaire_id)
+        round_symbol = "#{maps.first.questionnaire_type}-Round-#{round}".to_sym
+        reviewee_scores[round_symbol] = []
+
+        maps.each_with_index do |map, index|
+            submitted_response = map.responses.select { |r|
+                r.round == round && r.is_submitted && r.map_id == map.id
+            }.last
+            next if submitted_response.nil?
+
+            # Filter SectionHeader answers at the DB level (single JOIN instead of N queries).
+            # SectionHeader items have no numeric meaning — their Answer rows (created when
+            # seed data or a form submission includes all item IDs) must not appear as scored
+            # heatgrid rows. inject_section_headers inserts header sentinels separately.
+            scored_answers = submitted_response.scores
+                                               .joins(:item)
+                                               .where.not(items: { question_type: 'SectionHeader' })
+
+            scored_answers.each_with_index do |score, idx|
+                reviewee_scores[round_symbol][idx] ||= []
+                reviewee_scores[round_symbol][idx] << get_answer(score, index)
             end
         end
 
-        # Return the organized hash of scores grouped by round
-        return reviewee_scores
+        reviewee_scores[round_symbol].each_with_index do |scores_array, idx|
+            reviewee_scores[round_symbol][idx] = scores_array.sort_by { |answer|
+                [answer[:reviewer_name].downcase, answer[:reviewee_name].downcase]
+            }
+        end
+
+        inject_section_headers(reviewee_scores[round_symbol], questionnaire_id)
+        reviewee_scores
     end
 
     # Injects SectionHeader items as sentinel hashes into a round's scores array.
diff --git a/app/controllers/student_tasks_controller.rb b/app/controllers/student_tasks_controller.rb
index 27e7ef578..f6a01fbbc 100644
--- a/app/controllers/student_tasks_controller.rb
+++ b/app/controllers/student_tasks_controller.rb
@@ -14,25 +14,26 @@ def team
     render json: StudentTask.teamed_students(current_user), status: :ok
   end
 
-  # The view function retrieves a student task based on a participant's ID.
-  # It is meant to provide an endpoint where tasks can be queried based on participant ID.
+  # Retrieves a StudentTask by AssignmentParticipant ID.
+  # Delegates lookup and preloading to from_participant_id so the find_by +
+  # nil-guard + create_from_participant logic is not duplicated here.
   def show
-    # Constrain to AssignmentParticipant — other Participant subclasses for the same user
-    # can be found via the polymorphic participants table and cause type-mismatch 500s later.
-    participant = AssignmentParticipant.find_by(id: params[:id])
+    @student_task = StudentTask.from_participant_id(params[:id])
 
-    if participant.nil?
+    if @student_task.nil?
       render json: { error: "Participant not found" }, status: :not_found
       return
     end
 
-    if participant.user_id != current_user.id
+    if @student_task.participant.user_id != current_user.id
       render json: { error: "Unauthorized access to participant's task" }, status: :forbidden
       return
     end
 
-    @student_task = StudentTask.create_from_participant(participant)
-    @student_task.due_dates = StudentTask.get_timeline_data(participant.assignment, participant)
+    @student_task.due_dates = StudentTask.get_timeline_data(
+      @student_task.participant.assignment,
+      @student_task.participant
+    )
     render json: @student_task, status: :ok
   end
 
diff --git a/app/controllers/teams_participants_controller.rb b/app/controllers/teams_participants_controller.rb
index e284cd93c..e3bf74892 100644
--- a/app/controllers/teams_participants_controller.rb
+++ b/app/controllers/teams_participants_controller.rb
@@ -48,8 +48,10 @@ def list_participants
     # by guessing team IDs, since action_allowed? only requires student privileges.
     # TA+ skip this guard — they have legitimate cross-team visibility.
     unless current_user_has_ta_privileges?
-      member_ids = TeamsParticipant.where(team_id: current_team.id).pluck(:user_id)
-      unless member_ids.include?(current_user.id)
+      # exists? fires a single SELECT 1 and avoids materialising the full user_id list.
+      # user_id is NOT NULL (enforced by DB constraint and model validation), so the
+      # column is safe to match against directly without a pluck intermediate.
+      unless TeamsParticipant.exists?(team_id: current_team.id, user_id: current_user.id)
         render json: { error: 'You are not authorized to view this team' }, status: :forbidden and return
       end
     end
diff --git a/app/models/student_task.rb b/app/models/student_task.rb
index e5717d15f..79de3586c 100644
--- a/app/models/student_task.rb
+++ b/app/models/student_task.rb
@@ -72,11 +72,19 @@ def self.from_user(user)
                          .sort_by { |task| [task.course, task.assignment, task.stage_deadline] }
   end
 
-  # create a StudentTask instance from a participant of the provided id
-  # Constrained to AssignmentParticipant to avoid type-mismatch 500s when other
-  # Participant subclasses exist for the same user/id in the polymorphic table.
+  # Creates a StudentTask from a participant ID. Returns nil if the ID does not
+  # match any AssignmentParticipant (other Participant subclasses are excluded to
+  # avoid type-mismatch 500s). Preloads the same associations as from_user so
+  # create_from_participant does not fire redundant queries per attribute access.
   def self.from_participant_id(id)
-    create_from_participant(AssignmentParticipant.find_by(id: id))
+    participant = AssignmentParticipant
+                    .where(id: id)
+                    .includes(assignment: :course)
+                    .includes(:user)
+                    .first
+    return nil unless participant
+
+    create_from_participant(participant)
   end
 
   # Builds a unified timeline by merging due dates and actual activity,
@@ -195,6 +203,8 @@ def reviews_given_in_current_stage?
   end
 
   # Parses a date string or Time object into a Time instance.
+  # Declared private via private_class_method because the `private` keyword only
+  # restricts instance methods — class methods (self.*) remain public without it.
   def self.parse_stage_deadline(date_string)
     return date_string if date_string.is_a?(Time)
     Time.parse(date_string.to_s)
@@ -202,5 +212,6 @@ def self.parse_stage_deadline(date_string)
     Rails.logger.error("Failed to parse stage deadline '#{date_string}': #{e.message}")
     Time.now + 1.year
   end
+  private_class_method :parse_stage_deadline
 
 end
diff --git a/spec/models/student_task_spec.rb b/spec/models/student_task_spec.rb
index 5a73193d0..81ea3e4b2 100644
--- a/spec/models/student_task_spec.rb
+++ b/spec/models/student_task_spec.rb
@@ -83,20 +83,31 @@
   end
 
   describe ".from_participant_id" do
-    it "uses AssignmentParticipant (not Participant) to look up by id" do
-      # The fix: Participant.find_by was replaced with AssignmentParticipant.find_by so that
-      # other Participant subclasses (e.g. TeammateReviewParticipant) for the same user cannot
-      # be returned and cause type-mismatch 500s later in create_from_participant.
-      allow(AssignmentParticipant).to receive(:find_by).with(id: 1).and_return(@participant)
+    # Shared helper: builds a relation double that responds to the
+    # .where(id:).includes(assignment: :course).includes(:user).first chain
+    # used by from_participant_id, and returns the given participant from .first.
+    def stub_participant_chain(participant)
+      relation = double('relation')
+      allow(AssignmentParticipant).to receive(:where).with(id: 1).and_return(relation)
+      allow(relation).to receive(:includes).with(assignment: :course).and_return(relation)
+      allow(relation).to receive(:includes).with(:user).and_return(relation)
+      allow(relation).to receive(:first).and_return(participant)
+      relation
+    end
 
-      expect(AssignmentParticipant).to receive(:find_by).with(id: 1).and_return(@participant)
+    it "uses AssignmentParticipant (not Participant) to look up by id" do
+      # from_participant_id must scope to AssignmentParticipant so that other Participant
+      # subclasses (e.g. CourseParticipant) for the same id cannot slip through and
+      # cause type-mismatch 500s later in create_from_participant.
+      relation = stub_participant_chain(@participant)
+      expect(AssignmentParticipant).to receive(:where).with(id: 1).and_return(relation)
       expect(StudentTask).to receive(:create_from_participant).with(@participant)
 
       StudentTask.from_participant_id(1)
     end
 
     it "does not call the base Participant class for the lookup" do
-      allow(AssignmentParticipant).to receive(:find_by).with(id: 1).and_return(@participant)
+      stub_participant_chain(@participant)
       allow(StudentTask).to receive(:create_from_participant)
 
       expect(Participant).not_to receive(:find_by)
diff --git a/spec/requests/api/v1/grades_controller_spec.rb b/spec/requests/api/v1/grades_controller_spec.rb
index 4b09ad62f..7ce54f9b7 100644
--- a/spec/requests/api/v1/grades_controller_spec.rb
+++ b/spec/requests/api/v1/grades_controller_spec.rb
@@ -561,13 +561,15 @@
   # student_tasks#show — AssignmentParticipant constraint
   # -------------------------------------------------------------------------
   describe 'student_tasks show endpoint' do
-    it 'returns 404 when the id belongs to a non-AssignmentParticipant' do
+    it 'returns 404 with error message when the id does not match any AssignmentParticipant' do
       # Only AssignmentParticipant records should be matched — other Participant subclasses
       # for the same user must not slip through and cause type-mismatch 500s.
+      # A missing/non-matching id must produce exactly 404 (not found), not 403 (forbidden).
       get '/student_tasks/show/999999',
           headers: { 'Authorization' => "Bearer #{student_token}" }
 
-      expect(response.status).to be_in([404, 403])
+      expect(response.status).to eq(404)
+      expect(JSON.parse(response.body)['error']).to eq('Participant not found')
     end
   end
 end
diff --git a/spec/requests/api/v1/student_tasks_controller_spec.rb b/spec/requests/api/v1/student_tasks_controller_spec.rb
index 0157de601..f69b23ccf 100644
--- a/spec/requests/api/v1/student_tasks_controller_spec.rb
+++ b/spec/requests/api/v1/student_tasks_controller_spec.rb
@@ -101,67 +101,16 @@
   end
 
   # -------------------------------------------------------------------------
-  # /student_tasks/view
+  # /student_tasks/show/{id}  — consolidated from two overlapping path blocks
   # -------------------------------------------------------------------------
   path '/student_tasks/show/{id}' do
-    get 'Retrieve a specific student task by ID' do
+    get 'Retrieve a specific student task by participant ID' do
       tags 'StudentTasks'
       produces 'application/json'
       parameter name: :id, in: :path, type: :integer, required: true
       parameter name: 'Authorization', in: :header, type: :string
 
-      # 200 test
-      response '200', 'successful retrieval of a student task' do
-        let!(:view_participant) do
-          institution = Institution.create!(name: 'NCSU View Test')
-          course = Course.create!(name: "CSC 517 View", directory_path: "csc517_view", instructor: instructor, institution: institution)
-          assignment = Assignment.create!(name: "Test Assignment", instructor: instructor, course: course)
-          DueDate.create!(parent: assignment, due_at: 7.days.from_now, deadline_name: 'Submission', deadline_type_id: 1, submission_allowed_id: 3, review_allowed_id: 3)
-          AssignmentParticipant.create!(user_id: studenta.id, parent_id: assignment.id, handle: studenta.name, permission_granted: true)
-        end
-
-        let(:id) { view_participant.id }
-
-        run_test! do |response|
-          data = JSON.parse(response.body)
-          expect(data['assignment']).to eq("Test Assignment")
-          expect(data['current_stage']).to be_a(String)
-          expect(data['stage_deadline']).to be_a(String)
-          expect(data['permission_granted']).to be true
-          expect(data['due_dates']).to be_an(Array)
-        end
-      end
-
-      response '404', 'participant not found' do
-        let(:id) { -1 }
-        run_test! do |response|
-          data = JSON.parse(response.body)
-          expect(data['error']).to eq('Participant not found')
-        end
-      end
-
-      response '401', 'unauthorized request has error response' do
-        let(:'Authorization') { "Bearer " }
-        let(:id) { 'any_id' }
-        run_test! do |response|
-          data = JSON.parse(response.body)
-          expect(data["error"]).to eql("Not Authorized")
-        end
-      end
-    end
-  end
-
-  # -------------------------------------------------------------------------
-  # /student_tasks/show/:id
-  # -------------------------------------------------------------------------
-  path '/student_tasks/show/{id}' do
-    get 'Retrieve a specific student task with timeline by participant ID' do
-      tags 'StudentTasks'
-      produces 'application/json'
-      parameter name: :id, in: :path, type: :integer, required: true
-      parameter name: 'Authorization', in: :header, type: :string
-
-      response '200', 'successful retrieval with due dates' do
+      response '200', 'returns full task payload with due dates and stage info' do
         let!(:show_participant) do
           institution = Institution.create!(name: 'NCSU Show Test')
           course = Course.create!(name: 'CSC 517 Show', directory_path: 'csc517_show', instructor: instructor, institution: institution)
@@ -174,11 +123,14 @@
         run_test! do |response|
           data = JSON.parse(response.body)
           expect(data['assignment']).to eq('Timeline Assignment')
+          expect(data['current_stage']).to be_a(String)
+          expect(data['stage_deadline']).to be_a(String)
+          expect(data['permission_granted']).to be true
           expect(data['due_dates']).to be_an(Array)
         end
       end
 
-      response '404', 'participant not found' do
+      response '404', 'participant not found — returns error message' do
         let(:id) { -1 }
         run_test! do |response|
           data = JSON.parse(response.body)
@@ -186,7 +138,7 @@
         end
       end
 
-      response '403', 'unauthorized access to another participants task' do
+      response '403', 'student accessing another participant task — returns error message' do
         let!(:other_user) do
           User.create!(
             name: 'otherstudent',
@@ -196,11 +148,11 @@
             email: 'other@example.com'
           )
         end
-        let!(:assignment) { Assignment.create!(name: 'Other Assignment', instructor: instructor) }
+        let!(:other_assignment) { Assignment.create!(name: 'Other Assignment', instructor: instructor) }
         let!(:other_participant) do
           AssignmentParticipant.create!(
             user_id: other_user.id,
-            parent_id: assignment.id,
+            parent_id: other_assignment.id,
             handle: other_user.name
           )
         end
@@ -212,10 +164,13 @@
         end
       end
 
-      response '401', 'unauthorized request' do
+      response '401', 'missing or invalid token — returns Not Authorized' do
         let(:'Authorization') { 'Bearer ' }
         let(:id) { 1 }
-        run_test!
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          expect(data['error']).to eql('Not Authorized')
+        end
       end
     end
   end
@@ -229,10 +184,41 @@
       produces 'application/json'
       parameter name: 'Authorization', in: :header, type: :string
 
-      response '200', 'returns teammates grouped by course' do
+      response '200', 'returns teammates grouped by course with expected structure' do
+        let!(:team_setup) do
+          institution = Institution.create!(name: 'NCSU Team Test')
+          course      = Course.create!(name: 'CSC 517 Team', directory_path: 'csc517_team', instructor: instructor, institution: institution)
+          assignment  = Assignment.create!(name: 'Team Assignment', instructor: instructor, course: course)
+
+          teammate = User.create!(
+            name: 'teammate1', password_digest: 'password',
+            role_id: @roles[:student].id, full_name: 'Team Mate One',
+            email: 'teammate1@example.com'
+          )
+
+          team = AssignmentTeam.create!(name: 'Team Alpha', parent_id: assignment.id)
+          participant_a = AssignmentParticipant.create!(user_id: studenta.id, parent_id: assignment.id, handle: studenta.name)
+          participant_b = AssignmentParticipant.create!(user_id: teammate.id, parent_id: assignment.id, handle: teammate.name)
+
+          # user.teams traverses teams_users (User has_many :teams, through: :teams_users)
+          # team.users traverses teams_participants (Team has_many :users, through: :teams_participants)
+          # teamed_students calls both, so both join tables must have rows.
+          TeamsUser.create!(team_id: team.id, user_id: studenta.id)
+          TeamsUser.create!(team_id: team.id, user_id: teammate.id)
+          TeamsParticipant.create!(team_id: team.id, user_id: studenta.id, participant_id: participant_a.id)
+          TeamsParticipant.create!(team_id: team.id, user_id: teammate.id, participant_id: participant_b.id)
+        end
+
         run_test! do |response|
           data = JSON.parse(response.body)
+          # Response must be a Hash keyed by course name
           expect(data).to be_a(Hash)
+          # The course we set up must appear as a key
+          expect(data.keys).to include('CSC 517 Team')
+          # The teammate's full name must appear under that course
+          expect(data['CSC 517 Team']).to include('Team Mate One')
+          # The current user must NOT appear in their own teammate list
+          expect(data['CSC 517 Team']).not_to include('Student A')
         end
       end
 

From cbfd5295afc1c59f2298bf162827e06707301f23 Mon Sep 17 00:00:00 2001
From: Bestin Lalu <46861674+gazarack@users.noreply.github.com>
Date: Wed, 17 Jun 2026 19:43:32 -0400
Subject: [PATCH 3/4] Changes after review comments

---
 app/controllers/grades_controller.rb          | 87 +++++++++----------
 app/controllers/student_tasks_controller.rb   |  6 +-
 app/models/assignment_team.rb                 | 36 ++++++--
 app/models/student_task.rb                    | 31 +++----
 spec/models/student_task_spec.rb              | 58 ++++++-------
 .../requests/api/v1/grades_controller_spec.rb |  4 +-
 .../api/v1/student_tasks_controller_spec.rb   |  2 +-
 7 files changed, 121 insertions(+), 103 deletions(-)

diff --git a/app/controllers/grades_controller.rb b/app/controllers/grades_controller.rb
index 5f8a05039..a2a965ea0 100644
--- a/app/controllers/grades_controller.rb
+++ b/app/controllers/grades_controller.rb
@@ -27,11 +27,11 @@ def view_all_scores
             participant_scores.push(get_my_scores_data(participant))
         end
 
-        # Preload team.users for all teams in one query to avoid N+1 inside get_our_scores_data.
-        # Without this, each call to get_our_scores_data fires a separate `team.users` JOIN query
+        # Preload team.users for all teams in one query to avoid N+1 inside get_team_scores.
+        # Without this, each call to get_team_scores fires a separate `team.users` JOIN query
         # (one per team), which degrades linearly with the number of teams in the assignment.
         @assignment.teams.includes(:users).each do |team|
-            team_scores.push(get_our_scores_data(team))
+            team_scores.push(get_team_scores(team))
         end
 
         render json: {
@@ -47,7 +47,7 @@ def view_all_scores
     # renders JSON with scores, assignment, averages.
     # This meets the student’s need to see heatgrids for their team only (with anonymous reviewers) and the associated items.
     def view_our_scores
-        render json: get_our_scores_data(@team)
+        render json: get_team_scores(@team)
     end
 
     # (GET /grades/:assignment_id/view_my_scores)
@@ -165,7 +165,7 @@ def get_items_from_questionnaire(questionnaire_id)
     def edit
         items = list_items(@assignment)
         scores = {}
-        scores[:my_team] = get_our_scores_data(@team)
+        scores[:my_team] = get_team_scores(@team)
         scores[:my_own] = get_my_scores_data(@participant)
         render json: {
         participant: @participant,
@@ -271,9 +271,11 @@ def set_participant_and_team_via_assignment
 
     
     # returns the heatgrid data required for a team to view their scores and average score of their work for an assignment
-    def get_our_scores_data(team)
-        reviews_of_our_work_maps = ReviewResponseMap.where(reviewed_object_id: @assignment.id, reviewee_id: team.id).to_a
-        reviews_of_our_work = get_heatgrid_data_for(reviews_of_our_work_maps)
+    def get_team_scores(team)
+        reviews_of_our_work_maps = ReviewResponseMap.where(reviewed_object_id: @assignment.id, reviewee_id: team.id)
+                                                   .includes(responses: { scores: :item })
+                                                   .to_a
+        reviews_of_our_work = get_reviews(reviews_of_our_work_maps)
         avg_score_of_our_work = team.aggregate_review_grade
 
         # Embed all team metadata directly in this response so the frontend can populate
@@ -302,14 +304,18 @@ def get_our_scores_data(team)
     # the data includes the scores given by their teammates as well as the scores given by the authors the participant reviewed
     def get_my_scores_data(participant)
         # the set of review maps that my team members used to review me
-        reviews_of_me_maps = TeammateReviewResponseMap.where(reviewed_object_id: @assignment.id, reviewee_id: participant.id).to_a 
+        reviews_of_me_maps = TeammateReviewResponseMap.where(reviewed_object_id: @assignment.id, reviewee_id: participant.id)
+                                                     .includes(responses: { scores: :item })
+                                                     .to_a
 
         # the set of review maps that I used to review my team members
-        reviews_by_me_maps = TeammateReviewResponseMap.where(reviewed_object_id: @assignment.id, reviewer_id: participant.id).to_a
+        reviews_by_me_maps = TeammateReviewResponseMap.where(reviewed_object_id: @assignment.id, reviewer_id: participant.id)
+                                                      .includes(responses: { scores: :item })
+                                                      .to_a
         
-        reviews_of_me = get_heatgrid_data_for(reviews_of_me_maps)
+        reviews_of_me = get_reviews(reviews_of_me_maps)
 
-        reviews_by_me = get_heatgrid_data_for(reviews_by_me_maps)
+        reviews_by_me = get_reviews(reviews_by_me_maps)
 
         # Fetch all review response maps where the current participant is the reviewer and the reviewed object is the current assignment.
         my_reviews_of_other_teams_maps = ReviewResponseMap.where(reviewed_object_id: @assignment.id, reviewer_id: participant.id)
@@ -323,7 +329,7 @@ def get_my_scores_data(participant)
             FeedbackResponseMap.find_by(reviewed_object_id: map.id, reviewee_id: participant.id)
         end.compact
 
-        feedback_scores_from_my_reviewees = get_heatgrid_data_for(feedback_from_my_reviewees_maps)
+        feedback_scores_from_my_reviewees = get_reviews(feedback_from_my_reviewees_maps)
 
         avg_score_from_my_teammates = participant.aggregate_teammate_review_grade(reviews_of_me_maps) 
         avg_score_to_my_teammates = participant.aggregate_teammate_review_grade(reviews_by_me_maps) 
@@ -342,7 +348,7 @@ def get_my_scores_data(participant)
 
     # Returns heatgrid data for a collection of maps (ReviewResponseMap/FeedbackResponseMap/TeammateReviewResponseMap).
     # Groups submitted scores by round symbol (:Review-Round-1, etc.) and injects SectionHeader sentinels.
-    def get_heatgrid_data_for(maps)
+    def get_reviews(maps)
         reviewee_scores = {}
         return reviewee_scores if maps.empty?
 
@@ -394,7 +400,7 @@ def get_heatgrid_data_for(maps)
     end
 
     # Accumulates scores for one round into reviewee_scores and injects SectionHeader sentinels.
-    # Extracted from get_heatgrid_data_for to eliminate the ~50-line duplicate that existed
+    # Extracted from get_reviews to eliminate the ~50-line duplicate that existed
     # between the varying-rubric and non-varying branches.
     def accumulate_round_scores(reviewee_scores, maps, round, questionnaire_id)
         round_symbol = "#{maps.first.questionnaire_type}-Round-#{round}".to_sym
@@ -406,17 +412,13 @@ def accumulate_round_scores(reviewee_scores, maps, round, questionnaire_id)
             }.last
             next if submitted_response.nil?
 
-            # Filter SectionHeader answers at the DB level (single JOIN instead of N queries).
-            # SectionHeader items have no numeric meaning — their Answer rows (created when
-            # seed data or a form submission includes all item IDs) must not appear as scored
-            # heatgrid rows. inject_section_headers inserts header sentinels separately.
-            scored_answers = submitted_response.scores
-                                               .joins(:item)
-                                               .where.not(items: { question_type: 'SectionHeader' })
+            # Filter SectionHeader answers in Ruby — scores and items are already preloaded
+            # via .includes(responses: { scores: :item }) on the maps query, so no extra DB hit.
+            scored_answers = submitted_response.scores.reject { |s| s.item.question_type == 'SectionHeader' }
 
             scored_answers.each_with_index do |score, idx|
                 reviewee_scores[round_symbol][idx] ||= []
-                reviewee_scores[round_symbol][idx] << get_answer(score, index)
+                reviewee_scores[round_symbol][idx] << get_answer(score, index, submitted_response)
             end
         end
 
@@ -426,16 +428,15 @@ def accumulate_round_scores(reviewee_scores, maps, round, questionnaire_id)
             }
         end
 
-        inject_section_headers(reviewee_scores[round_symbol], questionnaire_id)
+        insert_section_headers(reviewee_scores[round_symbol], questionnaire_id)
         reviewee_scores
     end
 
-    # Injects SectionHeader items as sentinel hashes into a round's scores array.
-    # The heatgrid scores array is indexed by scored-item position (SectionHeaders have no
-    # answers so they never appear in the scores loop). This method looks up the questionnaire,
-    # walks all items in seq order, and inserts { type: "header", txt: "..." } markers at the
-    # correct positions so the frontend can render heading rows between score rows.
-    def inject_section_headers(round_scores, questionnaire_id)
+    # Splices { type: "header", txt: "..." } sentinel hashes into a round's scores array at
+    # positions matching each SectionHeader item in the questionnaire's item sequence.
+    # SectionHeaders have no answers so they never appear in the scores loop — this pass
+    # re-inserts them so the frontend can render heading rows between score rows.
+    def insert_section_headers(round_scores, questionnaire_id)
         return round_scores if questionnaire_id.nil?
 
         all_items = Item.where(questionnaire_id: questionnaire_id).order(:seq)
@@ -459,27 +460,25 @@ def inject_section_headers(round_scores, questionnaire_id)
         round_scores
     end
 
-    def get_answer(score, index)
-        # Determine the name or label to show for the reviewer
+    def get_answer(score, index, response = nil)
+        response ||= score.response
+
         reviewer_name = if current_user_has_all_heatgrid_data_privileges?(@assignment)
-                           score&.response&.reviewer&.fullname # Show the actual reviewer's name
+                           response&.reviewer&.fullname
                         else
-                            "Participant #{index+1}" # Show generic label (e.g., Participant 1)
+                            "Participant #{index+1}"
                         end
-        
-        # useful in case of reviews done by reviews_by_me (reviews given by a user to its teammates)
-        # in that case we will need reviewee's name instead of reviewer name because the reviewer will be the user itself.
-        reviewee_name = score&.response&.reviewee&.fullname                        
 
-        #Return particular score/answer information
-        return {
+        reviewee_name = response&.reviewee&.fullname
+
+        {
             id: score.id,
-            item_id:score.item_id,
+            item_id: score.item_id,
             txt: score.item.txt,
-            answer:score.answer,
-            comments:score.comments,
+            answer: score.answer,
+            comments: score.comments,
             reviewer_name: reviewer_name,
             reviewee_name: reviewee_name
         }
-    end 
+    end
 end
diff --git a/app/controllers/student_tasks_controller.rb b/app/controllers/student_tasks_controller.rb
index f6a01fbbc..b165ad7df 100644
--- a/app/controllers/student_tasks_controller.rb
+++ b/app/controllers/student_tasks_controller.rb
@@ -5,13 +5,13 @@ def action_allowed?
     current_user_has_student_privileges?
   end
   def list
-    @student_tasks = StudentTask.from_user(current_user)
+    @student_tasks = StudentTask.tasks(current_user)
     render json: @student_tasks, status: :ok
   end
 
   # GET /student_tasks/teammates
   def team
-    render json: StudentTask.teamed_students(current_user), status: :ok
+    render json: StudentTask.all_teammates(current_user), status: :ok
   end
 
   # Retrieves a StudentTask by AssignmentParticipant ID.
@@ -30,7 +30,7 @@ def show
       return
     end
 
-    @student_task.due_dates = StudentTask.get_timeline_data(
+    @student_task.due_dates = StudentTask.get_events_for_assignment(
       @student_task.participant.assignment,
       @student_task.participant
     )
diff --git a/app/models/assignment_team.rb b/app/models/assignment_team.rb
index 632b74eae..966db1bec 100644
--- a/app/models/assignment_team.rb
+++ b/app/models/assignment_team.rb
@@ -118,10 +118,34 @@ def has_submissions?
     submitted_files.any? || submitted_hyperlinks.present?
   end
 
-  # Computes the average review grade for an assignment team.
-  # This method aggregates scores from all ReviewResponseMaps (i.e., all reviewers of the team).
+  # Computes the average peer review grade for this team on its assignment.
+  # Expertiza assessment_score logic:
+  #   score = (aggregate_questionnaire_score / maximum_score) * 100
+  # Only submitted responses are counted; responses with zero maximum score are excluded.
+  # review_mappings is scoped to parent_id (the assignment) so cross-assignment maps are excluded.
   def aggregate_review_grade
-    compute_average_review_score(review_mappings)
+    maps = review_mappings
+             .where(reviewed_object_id: parent_id)
+             .includes(responses: { scores: :item })
+    return nil if maps.blank?
+
+    total = 0.0
+    count = 0
+
+    maps.each do |map|
+      map.responses.select(&:is_submitted).each do |response|
+        score = response.aggregate_questionnaire_score
+        max   = response.maximum_score
+        next if max.nil? || max.zero? || score.to_f <= 0
+
+        total += (score.to_f / max.to_f) * 100
+        count += 1
+      end
+    end
+
+    return nil if count.zero?
+
+    (total / count).round(2)
   end
 
   # Adds a participant to this team.
@@ -205,12 +229,6 @@ def has_submissions?
     submitted_files.any? || submitted_hyperlinks.present?
   end
 
-  # Computes the average review grade for an assignment team.
-  # This method aggregates scores from all ReviewResponseMaps (i.e., all reviewers of the team).
-  def aggregate_review_grade
-    compute_average_review_score(review_mappings)
-  end
-
   protected
 
   # Validates if a user is eligible to join the team
diff --git a/app/models/student_task.rb b/app/models/student_task.rb
index 79de3586c..5574342cb 100644
--- a/app/models/student_task.rb
+++ b/app/models/student_task.rb
@@ -1,7 +1,8 @@
 # frozen_string_literal: true
 
 class StudentTask
-  attr_accessor :assignment, :course, :current_stage, :participant, :stage_deadline, :topic, :permission_granted, :due_dates, :revise, :not_started, :active_round
+  attr_accessor :assignment, :course, :current_stage, :participant, :stage_deadline, :topic, :permission_granted, 
+                :due_dates, :submission_updated, :started, :active_round
 
   # Initializes a new instance of the StudentTask class
   def initialize(args)
@@ -25,8 +26,8 @@ def self.create_from_participant(participant)
     topic_name = SignedUpTeam.find_by(team_id: team&.id)&.project_topic&.topic_name
     current_stage = DueDate.current_stage_for(assignment)
 
-    # Determine the active round from the current due date so that revise? and
-    # not_started? can scope their checks to the round the student is actually in,
+    # Determine the active round from the current due date so that submission_updated? and
+    # started? can scope their checks to the round the student is actually in,
     # rather than checking across all rounds (which causes round-1 work to make a
     # round-2 task appear started even when the student hasn't touched round 2 yet).
     active_round = next_due&.round
@@ -41,8 +42,8 @@ def self.create_from_participant(participant)
       participant:        participant,
       active_round:       active_round
     )
-    task.revise = task.revise?
-    task.not_started = task.not_started?
+    task.submission_updated = task.submission_updated?
+    task.started = task.started?
     task
   end
 
@@ -51,7 +52,7 @@ def self.create_from_participant(participant)
   # Three chained sort_by calls are not stable — each overwrites the previous ordering.
   # A single composite key [course, assignment, stage_deadline] gives deterministic,
   # consistent ordering in one pass.
-  def self.from_user(user)
+  def self.tasks(user)
     # Preload the associations that create_from_participant dereferences for every
     # participant so that the list endpoint does not fire one query per participant
     # for each of these:
@@ -74,7 +75,7 @@ def self.from_user(user)
 
   # Creates a StudentTask from a participant ID. Returns nil if the ID does not
   # match any AssignmentParticipant (other Participant subclasses are excluded to
-  # avoid type-mismatch 500s). Preloads the same associations as from_user so
+  # avoid type-mismatch 500s). Preloads the same associations as tasks so
   # create_from_participant does not fire redundant queries per attribute access.
   def self.from_participant_id(id)
     participant = AssignmentParticipant
@@ -88,8 +89,8 @@ def self.from_participant_id(id)
   end
 
   # Builds a unified timeline by merging due dates and actual activity,
-  # sorted chronologically — mirrors the old get_timeline_data logic.
-  def self.get_timeline_data(assignment, participant)
+  # sorted chronologically — mirrors the old get_events_for_assignment logic.
+  def self.get_events_for_assignment(assignment, participant)
     timeline = []
 
     # 1. Due dates — labeled as "X deadline" mirroring old get_due_date_data behavior
@@ -143,8 +144,8 @@ def self.get_timeline_data(assignment, participant)
   end
 
   # Returns a hash of { course_name => [teammate_fullnames] } for all
-  # assignment teams the user has been part of, mirroring the old teamed_students logic.
-  def self.teamed_students(user)
+  # assignment teams the user has been part of, mirroring the old all_teammates logic.
+  def self.all_teammates(user)
     result = {}
     user.teams.each do |team|
       next unless team.is_a?(AssignmentTeam)
@@ -162,14 +163,14 @@ def self.teamed_students(user)
   end
 
   # Returns true if the student has started work in the current stage.
-  def revise?
+  def submission_updated?
     content_submitted_in_current_stage? ||
       reviews_given_in_current_stage?
   end
 
-  # Returns true if the assignment is in an active stage but the student hasn't started yet.
-  def not_started?
-    in_work_stage? && !revise?
+  # Returns true if the student has begun work in the current active stage.
+  def started?
+    in_work_stage? && submission_updated?
   end
 
   private
diff --git a/spec/models/student_task_spec.rb b/spec/models/student_task_spec.rb
index 81ea3e4b2..e9d92800f 100644
--- a/spec/models/student_task_spec.rb
+++ b/spec/models/student_task_spec.rb
@@ -116,7 +116,7 @@ def stub_participant_chain(participant)
     end
   end
 
-  describe ".from_user" do
+  describe ".tasks" do
     it "sorts by a single composite key [course, assignment, stage_deadline]" do
       # Three chained sort_by calls were non-deterministic (each overwrote the previous).
       # The fix uses one sort_by { [course, assignment, stage_deadline] }.
@@ -140,7 +140,7 @@ def stub_participant_chain(participant)
       allow(DueDate).to receive(:current_stage_for).and_return("submission")
       allow(DueDate).to receive(:next_due_date).and_return(nil)
       allow(SignedUpTeam).to receive(:find_by).and_return(nil)
-      # from_user chains .includes(...) on the where result, so we need a relation
+      # tasks chains .includes(...) on the where result, so we need a relation
       # double that responds to both includes calls and delegates map to the array.
       relation = double('relation')
       allow(AssignmentParticipant).to receive(:where).with(user_id: user.id).and_return(relation)
@@ -148,7 +148,7 @@ def stub_participant_chain(participant)
       allow(relation).to receive(:includes).with(:user).and_return(relation)
       allow(relation).to receive(:map) { |&blk| [p3, p1, p2].map(&blk) }
 
-      tasks = StudentTask.from_user(user)
+      tasks = StudentTask.tasks(user)
 
       courses   = tasks.map(&:course)
       expect(courses).to eq(courses.sort), "tasks should be sorted by course first"
@@ -159,7 +159,7 @@ def stub_participant_chain(participant)
     end
   end
 
-  describe "#revise?" do
+  describe "#submission_updated?" do
     let(:participant) { double('participant', id: 1) }
     let(:task) { StudentTask.new(participant: participant, current_stage: 'submission') }
 
@@ -167,7 +167,7 @@ def stub_participant_chain(participant)
       it "returns true" do
         team = double('team', hyperlinks: ['http://example.com'], has_submissions?: false)
         allow(participant).to receive(:team).and_return(team)
-        expect(task.send(:revise?)).to be true
+        expect(task.send(:submission_updated?)).to be true
       end
     end
 
@@ -175,7 +175,7 @@ def stub_participant_chain(participant)
       it "returns false" do
         team = double('team', hyperlinks: [], has_submissions?: false)
         allow(participant).to receive(:team).and_return(team)
-        expect(task.send(:revise?)).to be false
+        expect(task.send(:submission_updated?)).to be false
       end
     end
 
@@ -185,7 +185,7 @@ def stub_participant_chain(participant)
         allow(ReviewResponseMap).to receive(:where).and_return(
           double(joins: double(where: double(exists?: true)))
         )
-        expect(task_review.send(:revise?)).to be true
+        expect(task_review.send(:submission_updated?)).to be true
       end
     end
 
@@ -195,54 +195,54 @@ def stub_participant_chain(participant)
         allow(ReviewResponseMap).to receive(:where).and_return(
           double(joins: double(where: double(exists?: false)))
         )
-        expect(task_review.send(:revise?)).to be false
+        expect(task_review.send(:submission_updated?)).to be false
       end
     end
   end
 
-  describe "#not_started?" do
+  describe "#started?" do
     let(:participant) { double('participant', id: 1) }
 
     context "when in work stage and no work done" do
-      it "returns true for submission stage with no submissions" do
+      it "returns false for submission stage with no submissions" do
         task = StudentTask.new(participant: participant, current_stage: 'submission')
         team = double('team', hyperlinks: [], has_submissions?: false)
         allow(participant).to receive(:team).and_return(team)
-        expect(task.not_started?).to be true
+        expect(task.started?).to be false
       end
 
-      it "returns true for review stage with no reviews given" do
+      it "returns false for review stage with no reviews given" do
         task = StudentTask.new(participant: participant, current_stage: 'review')
         allow(ReviewResponseMap).to receive(:where).and_return(
           double(joins: double(where: double(exists?: false)))
         )
-        expect(task.not_started?).to be true
+        expect(task.started?).to be false
       end
     end
 
     context "when in work stage and work has been done" do
-      it "returns false when submission has been made" do
+      it "returns true when submission has been made" do
         task = StudentTask.new(participant: participant, current_stage: 'submission')
         team = double('team', hyperlinks: ['http://example.com'], has_submissions?: false)
         allow(participant).to receive(:team).and_return(team)
-        expect(task.not_started?).to be false
+        expect(task.started?).to be true
       end
     end
 
     context "when not in a work stage" do
       it "returns false for Finished stage" do
         task = StudentTask.new(participant: participant, current_stage: 'Finished')
-        expect(task.not_started?).to be false
+        expect(task.started?).to be false
       end
 
       it "returns false for signup stage" do
         task = StudentTask.new(participant: participant, current_stage: 'signup')
-        expect(task.not_started?).to be false
+        expect(task.started?).to be false
       end
     end
   end
 
-  describe ".teamed_students" do
+  describe ".all_teammates" do
     let!(:institution)     { Institution.create!(name: 'NCSU') }
     let!(:instructor_role) { Role.find_by(name: 'Instructor') || Role.create!(name: 'Instructor') }
     let!(:student_role)    { Role.find_by(name: 'Student')    || Role.create!(name: 'Student') }
@@ -266,7 +266,7 @@ def add_to_team(team, user, assignment)
       add_to_team(team, user, assignment)
       add_to_team(team, teammate, assignment)
 
-      result = StudentTask.teamed_students(user)
+      result = StudentTask.all_teammates(user)
       expect(result).to have_key('CSC 517')
       expect(result['CSC 517']).to include('TS Student Two')
     end
@@ -281,7 +281,7 @@ def add_to_team(team, user, assignment)
         instance_double(Assignment, is_calibrated: true, nil?: false)
       )
 
-      result = StudentTask.teamed_students(user)
+      result = StudentTask.all_teammates(user)
       expect(result.values.flatten).not_to include('TS Student Two')
     end
 
@@ -289,7 +289,7 @@ def add_to_team(team, user, assignment)
       team = AssignmentTeam.create!(name: 'Team Solo', parent_id: assignment.id)
       add_to_team(team, user, assignment)
 
-      result = StudentTask.teamed_students(user)
+      result = StudentTask.all_teammates(user)
       expect(result).to be_empty
     end
 
@@ -300,12 +300,12 @@ def add_to_team(team, user, assignment)
       add_to_team(team, teammate, assignment)
       add_to_team(team, student3, assignment)
 
-      result = StudentTask.teamed_students(user)
+      result = StudentTask.all_teammates(user)
       expect(result['CSC 517']).to eq(result['CSC 517'].sort)
     end
   end
 
-  describe ".get_timeline_data" do
+  describe ".get_events_for_assignment" do
     let!(:institution)  { Institution.find_by(name: 'NCSU') || Institution.create!(name: 'NCSU') }
     let!(:inst_role)    { Role.find_by(name: 'Instructor') || Role.create!(name: 'Instructor') }
     let!(:tl_instructor) { User.create!(name: 'tl_inst', email: 'tl_inst@test.com', full_name: 'TL Inst', password: 'password', role_id: inst_role.id, institution: institution) }
@@ -322,7 +322,7 @@ def add_to_team(team, user, assignment)
       allow(feedback_relation).to receive(:find_each)
       allow(FeedbackResponseMap).to receive(:where).with(reviewer_id: 1).and_return(feedback_relation)
 
-      timeline = StudentTask.get_timeline_data(assignment, participant)
+      timeline = StudentTask.get_events_for_assignment(assignment, participant)
       submission_entry = timeline.find { |t| t['name'].include?('Submission') }
       expect(submission_entry).not_to be_nil
       expect(submission_entry['id']).to be_nil
@@ -347,7 +347,7 @@ def add_to_team(team, user, assignment)
       allow(feedback_relation).to receive(:find_each)
       allow(FeedbackResponseMap).to receive(:where).with(reviewer_id: 1).and_return(feedback_relation)
 
-      timeline = StudentTask.get_timeline_data(assignment, participant)
+      timeline = StudentTask.get_events_for_assignment(assignment, participant)
       review_entry = timeline.find { |t| t['name'].include?('peer review') }
       expect(review_entry).not_to be_nil
       expect(review_entry['id']).to eq(99)
@@ -371,7 +371,7 @@ def add_to_team(team, user, assignment)
       allow(feedback_relation).to receive(:find_each)
       allow(FeedbackResponseMap).to receive(:where).with(reviewer_id: 1).and_return(feedback_relation)
 
-      timeline = StudentTask.get_timeline_data(assignment, participant)
+      timeline = StudentTask.get_events_for_assignment(assignment, participant)
       expect(timeline.any? { |t| t['name'].include?('peer review') }).to be false
     end
 
@@ -395,7 +395,7 @@ def add_to_team(team, user, assignment)
       allow(feedback_relation).to receive(:find_each)
       allow(FeedbackResponseMap).to receive(:where).with(reviewer_id: 1).and_return(feedback_relation)
 
-      timeline = StudentTask.get_timeline_data(assignment, participant)
+      timeline = StudentTask.get_events_for_assignment(assignment, participant)
       review_entries = timeline.select { |t| t['name'].include?('peer review') }
       expect(review_entries.map { |e| e['round'] }).to contain_exactly(1, 2)
     end
@@ -415,7 +415,7 @@ def add_to_team(team, user, assignment)
       allow(ordered).to receive(:first).and_return(nil)
       allow(Response).to receive(:where).with(map_id: 2, is_submitted: true).and_return(double(order: ordered))
 
-      timeline = StudentTask.get_timeline_data(assignment, participant)
+      timeline = StudentTask.get_events_for_assignment(assignment, participant)
       expect(timeline.any? { |t| t['name'] == 'Author feedback' }).to be false
     end
 
@@ -431,7 +431,7 @@ def add_to_team(team, user, assignment)
       allow(feedback_relation).to receive(:find_each)
       allow(FeedbackResponseMap).to receive(:where).with(reviewer_id: 1).and_return(feedback_relation)
 
-      timeline = StudentTask.get_timeline_data(assignment, participant)
+      timeline = StudentTask.get_events_for_assignment(assignment, participant)
       dates = timeline.map { |t| t['date'] }
       expect(dates).to eq(dates.sort)
     end
diff --git a/spec/requests/api/v1/grades_controller_spec.rb b/spec/requests/api/v1/grades_controller_spec.rb
index 7ce54f9b7..e54471162 100644
--- a/spec/requests/api/v1/grades_controller_spec.rb
+++ b/spec/requests/api/v1/grades_controller_spec.rb
@@ -460,9 +460,9 @@
   end
 
   # -------------------------------------------------------------------------
-  # inject_section_headers — section headings appear in view_our_scores
+  # insert_section_headers — section headings appear in view_our_scores
   # -------------------------------------------------------------------------
-  # inject_section_headers is a private helper called inside get_heatgrid_data_for.
+  # insert_section_headers is a private helper called inside get_reviews.
   # We test its effect through the public view_our_scores endpoint: when a questionnaire
   # has SectionHeader items, the response's round arrays must contain { type: "header" }
   # sentinel hashes at the correct positions.
diff --git a/spec/requests/api/v1/student_tasks_controller_spec.rb b/spec/requests/api/v1/student_tasks_controller_spec.rb
index f69b23ccf..91f318472 100644
--- a/spec/requests/api/v1/student_tasks_controller_spec.rb
+++ b/spec/requests/api/v1/student_tasks_controller_spec.rb
@@ -202,7 +202,7 @@
 
           # user.teams traverses teams_users (User has_many :teams, through: :teams_users)
           # team.users traverses teams_participants (Team has_many :users, through: :teams_participants)
-          # teamed_students calls both, so both join tables must have rows.
+          # all_teammates calls both, so both join tables must have rows.
           TeamsUser.create!(team_id: team.id, user_id: studenta.id)
           TeamsUser.create!(team_id: team.id, user_id: teammate.id)
           TeamsParticipant.create!(team_id: team.id, user_id: studenta.id, participant_id: participant_a.id)

From 28fd388316a5905fe16f5f180fe1805a5bb0e62c Mon Sep 17 00:00:00 2001
From: Bestin Lalu <46861674+gazarack@users.noreply.github.com>
Date: Fri, 19 Jun 2026 11:59:26 -0400
Subject: [PATCH 4/4] Review grade calculation changes

---
 app/controllers/grades_controller.rb     |  2 +-
 app/models/assignment_participant.rb     |  3 +-
 app/models/assignment_questionnaire.rb   | 13 +++++
 app/models/assignment_team.rb            | 30 ++---------
 app/models/concerns/review_aggregator.rb | 22 --------
 app/models/questionnaire.rb              |  8 ++-
 app/models/response.rb                   |  2 +-
 app/models/response_map.rb               | 64 ++++++++++++++++++++----
 8 files changed, 80 insertions(+), 64 deletions(-)
 delete mode 100644 app/models/concerns/review_aggregator.rb

diff --git a/app/controllers/grades_controller.rb b/app/controllers/grades_controller.rb
index a2a965ea0..661ee94a9 100644
--- a/app/controllers/grades_controller.rb
+++ b/app/controllers/grades_controller.rb
@@ -276,7 +276,7 @@ def get_team_scores(team)
                                                    .includes(responses: { scores: :item })
                                                    .to_a
         reviews_of_our_work = get_reviews(reviews_of_our_work_maps)
-        avg_score_of_our_work = team.aggregate_review_grade
+        avg_score_of_our_work = team.aggregate_reviewer_score
 
         # Embed all team metadata directly in this response so the frontend can populate
         # team name, grade, submission links, and member list in a single API call.
diff --git a/app/models/assignment_participant.rb b/app/models/assignment_participant.rb
index f3f1f38b2..ca75e8224 100644
--- a/app/models/assignment_participant.rb
+++ b/app/models/assignment_participant.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 class AssignmentParticipant < Participant
-  include ReviewAggregator
   has_many :sent_invitations, class_name: 'Invitation', foreign_key: 'from_id'
   has_many :review_mappings, class_name: 'ReviewResponseMap', foreign_key: 'reviewee_id'
   has_many :response_maps, foreign_key: 'reviewee_id'
@@ -51,6 +50,6 @@ def retract_sent_invitations
   end
 
   def aggregate_teammate_review_grade(teammate_review_mappings)
-    compute_average_review_score(teammate_review_mappings)
+    ResponseMap.compute_average_reviewer_score(teammate_review_mappings)
   end
 end
diff --git a/app/models/assignment_questionnaire.rb b/app/models/assignment_questionnaire.rb
index 87a55883d..7b5f99551 100644
--- a/app/models/assignment_questionnaire.rb
+++ b/app/models/assignment_questionnaire.rb
@@ -3,4 +3,17 @@
 class AssignmentQuestionnaire < ApplicationRecord
   belongs_to :assignment
   belongs_to :questionnaire
+
+  validate :weight_must_be_zero_if_no_scored_questions
+
+  # If the linked questionnaire has no scored questions (i.e. only SectionHeaders),
+  # questionnaire_weight must be 0 — a non-zero weight would produce meaningless grades.
+  def weight_must_be_zero_if_no_scored_questions
+    return if questionnaire.nil? || questionnaire_weight.nil? || questionnaire_weight.zero?
+
+    has_scored = questionnaire.items.where.not(question_type: 'SectionHeader').exists?
+    unless has_scored
+      errors.add(:questionnaire_weight, 'must be 0 when the rubric contains no scored questions')
+    end
+  end
 end
diff --git a/app/models/assignment_team.rb b/app/models/assignment_team.rb
index 966db1bec..fc6966194 100644
--- a/app/models/assignment_team.rb
+++ b/app/models/assignment_team.rb
@@ -2,7 +2,6 @@
 
 class AssignmentTeam < Team
   include Analytic::AssignmentTeamAnalytic
-  include ReviewAggregator
   # Each AssignmentTeam must belong to a specific assignment
   belongs_to :assignment, class_name: 'Assignment', foreign_key: 'parent_id'
   has_many :review_mappings, class_name: 'ReviewResponseMap', foreign_key: 'reviewee_id'
@@ -118,34 +117,13 @@ def has_submissions?
     submitted_files.any? || submitted_hyperlinks.present?
   end
 
-  # Computes the average peer review grade for this team on its assignment.
-  # Expertiza assessment_score logic:
-  #   score = (aggregate_questionnaire_score / maximum_score) * 100
-  # Only submitted responses are counted; responses with zero maximum score are excluded.
-  # review_mappings is scoped to parent_id (the assignment) so cross-assignment maps are excluded.
-  def aggregate_review_grade
+  # Computes the weighted average peer review grade for this team.
+  # Scopes maps to this assignment then delegates to ResponseMap.compute_average_reviewer_score.
+  def aggregate_reviewer_score
     maps = review_mappings
              .where(reviewed_object_id: parent_id)
              .includes(responses: { scores: :item })
-    return nil if maps.blank?
-
-    total = 0.0
-    count = 0
-
-    maps.each do |map|
-      map.responses.select(&:is_submitted).each do |response|
-        score = response.aggregate_questionnaire_score
-        max   = response.maximum_score
-        next if max.nil? || max.zero? || score.to_f <= 0
-
-        total += (score.to_f / max.to_f) * 100
-        count += 1
-      end
-    end
-
-    return nil if count.zero?
-
-    (total / count).round(2)
+    ResponseMap.compute_average_reviewer_score(maps)
   end
 
   # Adds a participant to this team.
diff --git a/app/models/concerns/review_aggregator.rb b/app/models/concerns/review_aggregator.rb
deleted file mode 100644
index 95e25e65e..000000000
--- a/app/models/concerns/review_aggregator.rb
+++ /dev/null
@@ -1,22 +0,0 @@
-module ReviewAggregator
-  extend ActiveSupport::Concern
-
-  # Generic method to compute average review grade from a collection of response maps
-  def compute_average_review_score(maps)
-    return nil if maps.blank?
-
-    total_score = 0.0
-    total_reviewers = 0
-
-    maps.each do |map|
-      score = map.aggregate_reviewers_score
-      next if score.nil?
-
-      total_score += score
-      total_reviewers += 1
-    end
-
-    return nil if total_reviewers.zero?
-    ((total_score / total_reviewers) * 100).round(2)
-  end
-end
\ No newline at end of file
diff --git a/app/models/questionnaire.rb b/app/models/questionnaire.rb
index b7eeee017..c7e1fc12c 100644
--- a/app/models/questionnaire.rb
+++ b/app/models/questionnaire.rb
@@ -132,7 +132,13 @@ def true_false_items?
       false
     end
 
-    def max_possible_score
+  # Pre-calculates the sum of weights for all scored items (excludes SectionHeaders).
+  # Used by ResponseMap#review_grade to normalize scores per round without repeating the query.
+  def total_item_weight
+    items.reject { |i| i.question_type == 'SectionHeader' }.sum(&:weight)
+  end
+
+  def max_possible_score
       results = Questionnaire.joins('INNER JOIN items ON items.questionnaire_id = questionnaires.id')
                             .select('SUM(items.weight) * questionnaires.max_question_score as max_score')
                             .where('questionnaires.id = ?', id)
diff --git a/app/models/response.rb b/app/models/response.rb
index 709671dab..78d8c9a59 100644
--- a/app/models/response.rb
+++ b/app/models/response.rb
@@ -40,7 +40,7 @@ def rubric_label
 
   # Returns true if this response's score differs from peers by more than the assignment notification limit
   # This comparison is response-specific (uses per-response max score and questionnaire settings),
-  # so it stays on Response instead of the generic ReviewAggregator mixin.
+  # so it stays on Response instead of ResponseMap.compute_average_reviewer_score.
   def reportable_difference?
     map_class = map.class
     # gets all responses made by a reviewee
diff --git a/app/models/response_map.rb b/app/models/response_map.rb
index bac967bfb..4c1daba4a 100644
--- a/app/models/response_map.rb
+++ b/app/models/response_map.rb
@@ -81,32 +81,74 @@ def survey?
     false
   end
 
-  # Computes the average score (as a fraction between 0 and 1) across the latest submitted responses
-  # from each round for corresponding ResponseMap.
-  def aggregate_reviewers_score
-    # Return nil if there are no responses for this map
+  # Computes the normalized score (0–1) for this map, weighted by each round's
+  # questionnaire_weight from assignment_questionnaires.
+  # Takes the latest submitted response per round, normalizes it (score/max),
+  # multiplies by that round's weight, and returns Σ(normalized × weight) / Σ(weight).
+  def review_grade
     return nil if responses.empty?
 
-    # Group all responses by round, then select the latest one per round based on the most recent revision in that round.
     latest_responses_by_round = responses
                                 .group_by(&:round)
                                 .transform_values { |resps| resps.max_by(&:updated_at) }
 
-    response_score = 0.0
-    total_score = 0.0
+    weighted_score  = 0.0
+    total_weight    = 0.0
     submitted_found = false
 
     latest_responses_by_round.each_value do |response|
       next unless response.is_submitted
 
-      submitted_found = true
-      response_score += response.aggregate_questionnaire_score
-      total_score += response.maximum_score
+      max = response.maximum_score
+      next if max.nil? || max.zero?
+
+      aq     = assignment.assignment_questionnaires.find_by(used_in_round: response.round)
+      aq   ||= assignment.assignment_questionnaires.find_by(used_in_round: nil)
+      weight = aq&.questionnaire_weight&.to_f || 1.0
+
+      submitted_found  = true
+      weighted_score  += (response.aggregate_questionnaire_score.to_f / max) * weight
+      total_weight    += weight
     end
 
     return nil unless submitted_found
+    return 0 if total_weight.zero?
+
+    weighted_score / total_weight
+  end
+
+  # All response map types expose a common aggregate_response_score interface.
+  # Subclasses (e.g. QuizResponseMap) may override if their scoring differs.
+  alias aggregate_response_score review_grade
+
+  # Computes a weighted average reviewer score from a collection of ResponseMaps.
+  # Each map contributes: review_grade × reviewer_reputation.
+  # reviewer_reputation defaults to 1.0 until Uchswas/Hamer integration is added.
+  # Used by AssignmentTeam#aggregate_reviewer_score and AssignmentParticipant#aggregate_teammate_review_grade.
+  def self.compute_average_reviewer_score(maps)
+    return nil if maps.blank?
+
+    weighted_sum = 0.0
+    total_weight = 0.0
+
+    maps.each do |map|
+      grade = map.review_grade
+      next if grade.nil?
+
+      reputation    = reviewer_reputation_for(map)
+      weighted_sum += grade * reputation
+      total_weight += reputation
+    end
+
+    return nil if total_weight.zero?
+
+    (weighted_sum / total_weight * 100).round(2)
+  end
 
-    total_score.positive? ? (response_score.to_f / total_score) : 0
+  # Returns the reviewer's reputation weight for this map.
+  # Placeholder — replace with Uchswas review-grader lookup or Hamer score.
+  def self.reviewer_reputation_for(_map)
+    1.0
   end
 
   # Best-effort timestamp of when the reviewee (or their team) last touched the work.