diff --git a/app/controllers/assignments_controller.rb b/app/controllers/assignments_controller.rb index 84d99f7c0..dba0ef3db 100644 --- a/app/controllers/assignments_controller.rb +++ b/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 diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 45ac849fb..58f88f3c7 100644 --- a/app/models/assignment.rb +++ b/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? diff --git a/app/models/due_date.rb b/app/models/due_date.rb index ed310bef5..7a2a5a883 100644 --- a/app/models/due_date.rb +++ b/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 diff --git a/db/migrate/20251125012619_add_vary_by_round_to_assignments.rb b/db/migrate/20251125012619_add_vary_by_round_to_assignments.rb new file mode 100644 index 000000000..280161bc8 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index d3a15fcfa..3c067678e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_10_29_071649) do +ActiveRecord::Schema[8.0].define(version: 2025_11_25_012619) do create_table "account_requests", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "username" t.string "full_name" @@ -103,6 +103,7 @@ t.boolean "enable_pair_programming", default: false t.boolean "has_teams", default: false t.boolean "has_topics", default: false + t.boolean "vary_by_round", default: false, null: false t.index ["course_id"], name: "index_assignments_on_course_id" t.index ["instructor_id"], name: "index_assignments_on_instructor_id" end @@ -125,11 +126,6 @@ t.datetime "updated_at", null: false end - create_table "cakes", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - end - create_table "courses", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "name" t.string "directory_path" @@ -176,16 +172,14 @@ create_table "invitations", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.integer "assignment_id" + t.integer "from_id" + t.integer "to_id" t.string "reply_status", limit: 1 t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.bigint "from_id", null: false - t.bigint "to_id", null: false - t.bigint "participant_id", null: false t.index ["assignment_id"], name: "fk_invitation_assignments" - t.index ["from_id"], name: "index_invitations_on_from_id" - t.index ["participant_id"], name: "index_invitations_on_participant_id" - t.index ["to_id"], name: "index_invitations_on_to_id" + t.index ["from_id"], name: "fk_invitationfrom_users" + t.index ["to_id"], name: "fk_invitationto_users" end create_table "items", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| @@ -211,7 +205,7 @@ t.integer "participant_id" t.integer "team_id" t.text "comments" - t.string "reply_status" + t.string "status" end create_table "nodes", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| @@ -295,7 +289,6 @@ t.integer "reviewee_id", default: 0, null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.string "type" t.index ["reviewer_id"], name: "fk_response_map_reviewer" end @@ -342,8 +335,6 @@ t.integer "preference_priority_number" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.text "comments_for_advertisement" - t.boolean "advertise_for_partner" t.index ["sign_up_topic_id"], name: "index_signed_up_teams_on_sign_up_topic_id" t.index ["team_id"], name: "index_signed_up_teams_on_team_id" end @@ -360,12 +351,17 @@ create_table "teams", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "name", null: false + t.integer "parent_id" t.string "type", null: false + t.boolean "advertise_for_partner", default: false, null: false + t.text "submitted_hyperlinks" + t.integer "directory_num" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.integer "parent_id", null: false t.integer "grade_for_submission" t.string "comment_for_submission" + t.index ["parent_id"], name: "index_teams_on_parent_id" + t.index ["type"], name: "index_teams_on_type" end create_table "teams_participants", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| @@ -385,6 +381,7 @@ t.bigint "user_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.index ["team_id", "user_id"], name: "index_teams_users_on_team_id_and_user_id", unique: true t.index ["team_id"], name: "index_teams_users_on_team_id" t.index ["user_id"], name: "index_teams_users_on_user_id" end @@ -422,8 +419,6 @@ add_foreign_key "assignments", "users", column: "instructor_id" add_foreign_key "courses", "institutions" add_foreign_key "courses", "users", column: "instructor_id" - add_foreign_key "invitations", "participants", column: "from_id" - add_foreign_key "invitations", "participants", column: "to_id" add_foreign_key "items", "questionnaires" add_foreign_key "participants", "join_team_requests" add_foreign_key "participants", "teams" diff --git a/spec/models/assignment_spec.rb b/spec/models/assignment_spec.rb index 58fd6a45f..1defe623f 100644 --- a/spec/models/assignment_spec.rb +++ b/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: "instructor@example.com", 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 diff --git a/spec/requests/api/v1/assignment_controller_spec.rb b/spec/requests/api/v1/assignment_controller_spec.rb index 6b64f4270..dbd3f2682 100644 --- a/spec/requests/api/v1/assignment_controller_spec.rb +++ b/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: "testuser@example.com", - 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} # -------------------------------------------------------------------------