E2567. Review rubrics varying by round

app/controllers/assignments_controller.rb

@@ -9,8 +9,16 @@ def index
 
   # GET /assignments/:id
   def show
-    assignment = Assignment.find(params[:id])
-    render json: assignment
+    assignment = Assignment.includes(assignment_questionnaires: :questionnaire, due_dates: {}).find(params[:id])
+    render json: assignment.as_json(
+      methods: [:num_review_rounds, :varying_rubrics_by_round], # for review rubrics vary by round functionality
+      include: {
+        assignment_questionnaires: {
+          include: :questionnaire
+        },
+        due_dates: {}
+      }
+    )
   end
 
   # POST /assignments
@@ -213,7 +221,8 @@ def varying_rubrics_by_round?
   private
   # Only allow a list of trusted parameters through.
   def assignment_params
-    params.require(:assignment).permit(:title, :description)
+    params.require(:assignment).permit(:title, :description, :name, :directory_path, :spec_location, :vary_by_round, :rounds_of_reviews,
+                                       assignment_questionnaires_attributes: [:id, :questionnaire_id, :used_in_round, :questionnaire_weight, :notification_limit, :_destroy])
   end
 
   # Helper method to determine staggered_and_no_topic for the assignment

app/models/assignment.rb

@@ -15,6 +15,7 @@ class Assignment < ApplicationRecord
   has_many :due_dates,as: :parent, class_name: 'DueDate',  dependent: :destroy
   belongs_to :course, optional: true
   belongs_to :instructor, class_name: 'User', inverse_of: :assignments
+  accepts_nested_attributes_for :assignment_questionnaires, allow_destroy: true
 
   #This method return the value of the has_badge field for the given assignment object.
   attr_accessor :title, :description, :has_badge, :enable_pair_programming, :is_calibrated, :staggered_deadline
@@ -26,8 +27,12 @@ def review_questionnaire_id
   def teams?
     @has_teams ||= teams.any?
   end
+
+  # Returns review round count: prefer the number of review due dates; 
+  # if none exist, fall back to the persisted rounds_of_reviews column (or 0).
   def num_review_rounds
-    rounds_of_reviews
+    review_rounds = due_dates.where(deadline_type_id: DueDate::REVIEW_DEADLINE_TYPE_ID).count
+    review_rounds.positive? ? review_rounds : (rounds_of_reviews || 0)
   end
 
   # Add a participant to the assignment based on the provided user_id.
@@ -188,15 +193,20 @@ def valid_num_review(review_type)
   end
 
 
-  #This method check if for the given assignment,different type of rubrics are used in different round.
-  # Checks if for the given assignment any questionnaire is present with used_in_round field not nil.
-  # Returns a boolean value whether such questionnaire is present.
+  # Returns true only if per-round rubrics are enabled on the assignment and at least one attached questionnaire specifies a round via used_in_round
   def varying_rubrics_by_round?
+    return false unless vary_by_round
+
     rubric_with_round = AssignmentQuestionnaire.where(assignment_id: id).where.not(used_in_round: nil).first
     # Check if any rubric has a specified round
     rubric_with_round.present?
   end
 
+  # Alias without question mark for JSON rendering convenience
+  def varying_rubrics_by_round
+    varying_rubrics_by_round?
+  end
+
   def review_rounds(questionnaireType)
     review_rounds = []
     if varying_rubrics_by_round?

app/models/due_date.rb

@@ -2,6 +2,8 @@
 
 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 
   # Named constants for teammate review statuses
   ALLOWED = 3
   LATE_ALLOWED = 2

db/migrate/20251125012619_add_vary_by_round_to_assignments.rb

@@ -0,0 +1,5 @@
+class AddVaryByRoundToAssignments < ActiveRecord::Migration[8.0]
+  def change
+    add_column :assignments, :vary_by_round, :boolean, default: false, null: false
+  end
+end

spec/models/assignment_spec.rb

@@ -10,6 +10,56 @@
   let(:review_response_map) { ReviewResponseMap.new(assignment: assignment, reviewee: team) }
   let(:answer) { Answer.new(answer: 1, comments: 'Answer text', item_id: 1) }
   let(:answer2) { Answer.new(answer: 1, comments: 'Answer text', item_id: 1) }
+
+  include RolesHelper
+  before(:all) { @roles = create_roles_hierarchy } # Create the full roles hierarchy once for creating the instructor role later
+  let(:institution) { Institution.create!(name: "NC State") } # All users belong to the same institution to satisfy foreign key constraints.
+  let(:instructor) { User.create!(name: "instructor", full_name: "Instructor User", email: "[email protected]", password_digest: "password", role_id: @roles[:instructor].id, institution_id: institution.id) }
+
+  describe '#num_review_rounds' do
+    it 'counts review due dates to determine the number of rounds' do
+      assignment = Assignment.create!(name: 'Round Count', instructor: instructor, vary_by_round: true)
+      AssignmentDueDate.create!(parent: assignment, due_at: 1.day.from_now,
+                                deadline_type_id: DueDate::REVIEW_DEADLINE_TYPE_ID,
+                                submission_allowed_id: 3, review_allowed_id: 3)
+      AssignmentDueDate.create!(parent: assignment, due_at: 2.days.from_now,
+                                deadline_type_id: DueDate::REVIEW_DEADLINE_TYPE_ID,
+                                submission_allowed_id: 3, review_allowed_id: 3)
+
+      expect(assignment.num_review_rounds).to eq(2)
+    end
+
+    it 'ignores non-review deadlines when counting rounds' do
+      assignment = Assignment.create!(name: 'Mixed Deadlines', instructor: instructor, vary_by_round: true)
+      AssignmentDueDate.create!(parent: assignment, due_at: 1.day.from_now,
+                                deadline_type_id: 99,
+                                submission_allowed_id: 3, review_allowed_id: 3)
+      AssignmentDueDate.create!(parent: assignment, due_at: 2.days.from_now,
+                                deadline_type_id: DueDate::REVIEW_DEADLINE_TYPE_ID,
+                                submission_allowed_id: 3, review_allowed_id: 3)
+
+      expect(assignment.num_review_rounds).to eq(1)
+    end
+  end
+
+  describe '#varying_rubrics_by_round?' do
+    let(:questionnaire) { Questionnaire.create!(name: 'Review Q', instructor_id: instructor.id, questionnaire_type: 'ReviewQuestionnaire',
+                                                display_type: 'Review', min_question_score: 0, max_question_score: 5) }
+
+    it 'returns false when vary_by_round is disabled even if rounds exist' do
+      assignment = Assignment.create!(name: 'No Vary', instructor: instructor, vary_by_round: false)
+      AssignmentQuestionnaire.create!(assignment: assignment, questionnaire: questionnaire, used_in_round: 1)
+
+      expect(assignment.varying_rubrics_by_round?).to be false
+    end
+
+    it 'returns true when vary_by_round is enabled and a round-specific rubric exists' do
+      assignment = Assignment.create!(name: 'Vary', instructor: instructor, vary_by_round: true)
+      AssignmentQuestionnaire.create!(assignment: assignment, questionnaire: questionnaire, used_in_round: 1)
+
+      expect(assignment.varying_rubrics_by_round?).to be true
+    end
+  end
 
   describe '.get_all_review_comments' do
     it 'returns concatenated review comments and # of reviews in each round' do

spec/requests/api/v1/assignment_controller_spec.rb

@@ -33,13 +33,14 @@
   end
 
   let!(:prof) do
-    User.create!(
+    Instructor.create!(
       name: "profa",
       password_digest: "password",
       role_id: @roles[:instructor].id,
       full_name: "Prof A",
       email: "[email protected]",
-      mru_directory_path: "/home/testuser"
+      mru_directory_path: "/home/testuser",
+      institution: institution
     )
   end
 
@@ -62,6 +63,17 @@
   let(:token) { JsonWebToken.encode({ id: prof.id }) }
   let(:Authorization) { "Bearer #{token}" }
 
+  let!(:questionnaire) do
+    Questionnaire.create!(
+      name: "Review Rubric",
+      instructor: prof,
+      private: false,
+      min_question_score: 0,
+      max_question_score: 10,
+      questionnaire_type: "ReviewQuestionnaire"
+    )
+  end
+
   # -------------------------------------------------------------------------
   # GET /assignments  (Get assignments)
   # -------------------------------------------------------------------------
@@ -82,6 +94,57 @@
     end
   end
 
+  # -------------------------------------------------------------------------
+  # GET /assignments/{id} (Show assignment)
+  # -------------------------------------------------------------------------
+  path '/assignments/{id}' do
+    parameter name: 'id', in: :path, type: :integer, description: 'Assignment ID'
+
+    get 'Show assignment details with rubrics and due dates' do
+      tags 'Assignments'
+      produces 'application/json'
+      parameter name: 'Content-Type', in: :header, type: :string
+      let('Content-Type') { 'application/json' }
+
+      response '200', 'assignment found' do
+        let(:id) { assignment.id }
+
+        before do
+          assignment.update!(vary_by_round: true)
+          AssignmentQuestionnaire.create!(assignment: assignment, questionnaire: questionnaire, used_in_round: 1)
+          AssignmentDueDate.create!(
+            parent: assignment,
+            due_at: Time.zone.now + 1.day,
+            deadline_type_id: DueDate::REVIEW_DEADLINE_TYPE_ID,
+            submission_allowed_id: 1,
+            review_allowed_id: 1,
+            round: 1
+          )
+        end
+
+        run_test! do
+          data = JSON.parse(response.body)
+
+          expect(data['id']).to eq(assignment.id)
+          expect(data['assignment_questionnaires'].first['questionnaire']['id']).to eq(questionnaire.id)
+          expect(data['due_dates'].length).to eq(1)
+          expect(data['num_review_rounds']).to eq(1)
+          expect(data['varying_rubrics_by_round']).to eq(true)
+        end
+      end
+
+      response '404', 'assignment not found' do
+        let(:id) { 999 }
+
+        run_test! do
+          data = JSON.parse(response.body)
+          expect(response).to have_http_status(:not_found)
+          expect(data['error']).to eq('Assignment not found')
+        end
+      end
+    end
+  end
+
   # -------------------------------------------------------------------------
   # POST /assignments/{assignment_id}/add_participant/{user_id}
   # -------------------------------------------------------------------------