diff --git a/app/controllers/assignments_controller.rb b/app/controllers/assignments_controller.rb index 8bc8b277f..396ac4071 100644 --- a/app/controllers/assignments_controller.rb +++ b/app/controllers/assignments_controller.rb @@ -9,7 +9,7 @@ def action_allowed? return current_user_has_ta_privileges? if action_name.in?(%w[index create]) assignment = Assignment.find_by(id: params[:id] || params[:assignment_id]) - return true unless assignment # let the action itself render 404 + return true unless assignment # let the action itself render 404 current_user_can_manage?(assignment) end @@ -28,22 +28,40 @@ def index else Assignment.none end - render json: assignments + render json: assignments.map { |a| + a.attributes.merge( + 'course_name' => a.course&.name, + 'created_at' => a.created_at, + 'updated_at' => a.updated_at + ) + } end # GET /assignments/:id def show assignment = Assignment.find(params[:id]) data = assignment.attributes + data['has_topics'] = assignment.has_topics data['assignment_questionnaires'] = assignment.assignment_questionnaires - .includes(:questionnaire) - .map { |aq| aq.attributes.merge('questionnaire' => aq.questionnaire&.attributes) } + .includes(:questionnaire) + .map { |aq| aq.attributes.merge('questionnaire' => aq.questionnaire&.attributes) } + data['due_dates'] = assignment.due_dates.map(&:attributes) render body: data.to_json, content_type: 'application/json' end # POST /assignments def create assignment = Assignment.new(assignment_params) + # Instructor for an assignment is taken from the course instructor if the assignment + # belongs to a course; otherwise, the instructor is the current user. + # (If the assignment doesn't belong to a course, there can't be a TA, so we don't + # have to worry about that case. If an admin creates an assignment and wants it to + # belong to an instructor, they can impersonate the instructor instead.) + assignment.instructor_id = if assignment.course_id.present? + Course.find_by(id: assignment.course_id)&.instructor_id || current_user.id + else + current_user.id + end if assignment.save render json: assignment, status: :created else @@ -55,14 +73,74 @@ def create def update assignment = Assignment.find(params[:id]) if assignment.update(assignment_params) - render body: assignment.attributes.to_json, content_type: 'application/json', status: :ok + data = assignment.attributes + data['due_dates'] = assignment.due_dates.map(&:attributes) + render body: data.to_json, content_type: 'application/json', status: :ok else render json: assignment.errors, status: :unprocessable_entity end end + # GET /assignments/:id/calibration_submissions + # Returns all teams for the assignment with their submitted content and the + # instructor's calibration review status. + def calibration_submissions + assignment = Assignment.find(params[:id]) + + instructor_participant = AssignmentParticipant.find_by( + parent_id: assignment.id, + user_id: assignment.instructor_id + ) + + teams = AssignmentTeam.where(parent_id: assignment.id) + + payload = teams.map do |team| + member_names = TeamsUser.where(team_id: team.id).filter_map do |tu| + user = User.find_by(id: tu.user_id) + next unless user + + "#{user.name} (#{user.full_name})" + end.join(', ') + + # Resolve calibration review map and its status + calibration_map = instructor_participant && ReviewResponseMap.find_by( + reviewed_object_id: assignment.id, + reviewer_id: instructor_participant.id, + reviewee_id: team.id, + for_calibration: true + ) + + review_status = if calibration_map.nil? + 'not_started' + elsif calibration_map.responses.exists?(is_submitted: true) + 'completed' + elsif calibration_map.responses.exists? + 'in_progress' + else + 'not_started' + end + + hyperlinks = begin + team.hyperlinks + rescue StandardError + [] + end + files = SubmissionRecord.where(assignment_id: assignment.id, team_id: team.id, record_type: 'file') + .pluck(:content) + + { + id: team.id, + participant_name: member_names, + review_status: review_status, + submitted_content: { hyperlinks: hyperlinks, files: files } + } + end + + render json: payload, status: :ok + end + def not_found - render json: { error: "Assignment not found" }, status: :not_found + render json: { error: 'Assignment not found' }, status: :not_found end # DELETE /assignments/:id @@ -70,15 +148,16 @@ def destroy assignment = Assignment.find_by(id: params[:id]) if assignment if assignment.destroy - render json: { message: "Assignment deleted successfully!" }, status: :ok + render json: { message: 'Assignment deleted successfully!' }, status: :ok else - render json: { error: "Failed to delete assignment", details: assignment.errors.full_messages }, status: :unprocessable_entity + render json: { error: 'Failed to delete assignment', details: assignment.errors.full_messages }, + status: :unprocessable_entity end else - render json: { error: "Assignment not found" }, status: :not_found + render json: { error: 'Assignment not found' }, status: :not_found end end - + # When a user wants to change the min/max score scale for a rubric, the code # needs to find how many previously assigned ReviewGrade scores would become # invalid (out of bounds) under the new scale, so the UI can warn the @@ -86,7 +165,7 @@ def destroy # GET /assignments/:id/review_grades_out_of_bounds?min=0&max=4 def review_grades_out_of_bounds assignment = Assignment.find_by(id: params[:id]) - return render json: { error: "Assignment not found" }, status: :not_found unless assignment + return render json: { error: 'Assignment not found' }, status: :not_found unless assignment new_min = params[:min].presence&.to_f new_max = params[:max].presence&.to_f @@ -101,11 +180,11 @@ def review_grades_out_of_bounds render json: { conflict_count: conflicts }, status: :ok end - #add participant to assignment + # add participant to assignment def add_participant assignment = Assignment.find_by(id: params[:assignment_id]) if assignment.nil? - render json: { error: "Assignment not found" }, status: :not_found + render json: { error: 'Assignment not found' }, status: :not_found else new_participant = assignment.add_participant(params[:user_id]) if new_participant.save @@ -116,41 +195,39 @@ def add_participant end end - #remove participant from assignment + # remove participant from assignment def remove_participant user = User.find_by(id: params[:user_id]) assignment = Assignment.find_by(id: params[:assignment_id]) if user && assignment assignment.remove_participant(user.id) if assignment.save - render json: { message: "Participant removed successfully!" }, status: :ok + render json: { message: 'Participant removed successfully!' }, status: :ok else render json: assignment.errors, status: :unprocessable_entity end else - not_found_message = user ? "Assignment not found" : "User not found" + not_found_message = user ? 'Assignment not found' : 'User not found' render json: { error: not_found_message }, status: :not_found end end - # make course_id of assignment null def remove_assignment_from_course assignment = Assignment.find(params[:assignment_id]) if assignment.nil? - render json: { error: "Assignment not found" }, status: :not_found + render json: { error: 'Assignment not found' }, status: :not_found else assignment = assignment.remove_assignment_from_course if assignment.save - render json: assignment , status: :ok + render json: assignment, status: :ok else render json: assignment.errors, status: :unprocessable_entity end end - end - #update course id of an assignment/ assign the assign to some together course + # update course id of an assignment/ assign the assign to some together course def assign_course assignment = Assignment.find(params[:assignment_id]) course = Course.find(params[:course_id]) @@ -162,16 +239,16 @@ def assign_course render json: assignment.errors, status: :unprocessable_entity end else - not_found_message = course ? "Assignment not found" : "Course not found" + not_found_message = course ? 'Assignment not found' : 'Course not found' render json: { error: not_found_message }, status: :not_found end end - #copy existing assignment + # copy existing assignment def copy_assignment assignment = Assignment.find_by(id: params[:assignment_id]) if assignment.nil? - render json: { error: "Assignment not found" }, status: :not_found + render json: { error: 'Assignment not found' }, status: :not_found else new_assignment = assignment.copy if new_assignment.save @@ -182,17 +259,16 @@ def copy_assignment end end - # Retrieves assignment details including `has_badge`, `pair_programming_enabled`, + # Retrieves assignment details including `pair_programming_enabled`, # `is_calibrated`, and `staggered_and_no_topic`. def show_assignment_details assignment = Assignment.find_by(id: params[:assignment_id]) if assignment.nil? - render json: { error: "Assignment not found" }, status: :not_found + render json: { error: 'Assignment not found' }, status: :not_found else render json: { id: assignment.id, name: assignment.name, - has_badge: assignment.has_badge?, pair_programming_enabled: assignment.pair_programming_enabled?, is_calibrated: assignment.is_calibrated?, staggered_and_no_topic: get_staggered_and_no_topic(assignment) @@ -201,22 +277,22 @@ def show_assignment_details end # check if assignment has topics - # has_topics is set to true if there is ProjectTopic corresponding to the input assignment id + # has_topics is set to true if there is ProjectTopic corresponding to the input assignment id def has_topics assignment = Assignment.find_by(id: params[:assignment_id]) if assignment.nil? - render json: { error: "Assignment not found" }, status: :not_found + render json: { error: 'Assignment not found' }, status: :not_found else render json: assignment.topics?, status: :ok end end - # check if assignment is a team assignment + # check if assignment is a team assignment # true if assignment's max team size is greater than 1 def team_assignment assignment = Assignment.find_by(id: params[:assignment_id]) if assignment.nil? - render json: { error: "Assignment not found" }, status: :not_found + render json: { error: 'Assignment not found' }, status: :not_found else render json: assignment.team_assignment?, status: :ok end @@ -228,7 +304,7 @@ def valid_num_review assignment = Assignment.find_by(id: params[:assignment_id]) review_type = params[:review_type] if assignment.nil? - render json: { error: "Assignment not found" }, status: :not_found + render json: { error: 'Assignment not found' }, status: :not_found else render json: assignment.valid_num_review(review_type), status: :ok end @@ -239,7 +315,7 @@ def valid_num_review def has_teams assignment = Assignment.find_by(id: params[:assignment_id]) if assignment.nil? - render json: { error: "Assignment not found" }, status: :not_found + render json: { error: 'Assignment not found' }, status: :not_found else render json: assignment.teams?, status: :ok end @@ -250,50 +326,72 @@ def has_teams def varying_rubrics_by_round? assignment = Assignment.find_by(id: params[:assignment_id]) if assignment.nil? - render json: { error: "Assignment not found" }, status: :not_found + render json: { error: 'Assignment not found' }, status: :not_found + elsif AssignmentQuestionnaire.exists?(assignment_id: assignment.id) + render json: assignment.varying_rubrics_by_round?, status: :ok else - if AssignmentQuestionnaire.exists?(assignment_id: assignment.id) - render json: assignment.varying_rubrics_by_round?, status: :ok - else - render json: { error: "No questionnaire/rubric exists for this assignment." }, status: :not_found - end + render json: { error: 'No questionnaire/rubric exists for this assignment.' }, status: :not_found end end - + private + # Only allow a list of trusted parameters through. def assignment_params params.require(:assignment).permit( - # Real DB columns - :name, - :directory_path, - :spec_location, + # Identity + :name, :title, :description, :directory_path, :spec_location, :course_id, + # Visibility :private, - :course_id, - :require_quiz, - :has_teams, - :max_team_size, + # Feature flags (DB columns or aliases defined in Assignment model) + :require_quiz, :staggered_deadline, :is_calibrated, + :has_teams, :max_team_size, + :show_teammate_review, # alias → show_teammate_reviews + :is_pair_programming, # alias → enable_pair_programming :has_topics, + :available_to_students, # alias → availability_flag + :allow_tag_prompts, # alias → is_answer_tagging_allowed + # Review configuration :review_topic_threshold, - :max_reviews_per_submission, - :days_between_submissions, - :late_policy_id, - :is_penalty_calculated, - :calculate_penalty, - :vary_by_round, - :rounds_of_reviews, - :instructor_grade_min_score, - :instructor_grade_max_score, - # Virtual attr_accessors defined on Assignment - :title, - :description, - # DB boolean columns - :has_badge, # legacy column from old Expertiza schema; not actively used but kept to avoid unknown-attribute errors on round-trips - :enable_pair_programming, - :is_calibrated, - :staggered_deadline, - # Nested assignment_questionnaires - assignment_questionnaires_attributes: [:id, :questionnaire_id, :used_in_round, :questionnaire_weight, :_destroy] + :maximum_number_of_reviews_per_submission, # alias → max_reviews_per_submission + :review_strategy, # alias → review_assignment_strategy + :review_rubric_varies_by_round, # alias → vary_by_round + :review_rubric_varies_by_topic, # alias → vary_by_topic + :review_rubric_varies_by_role, # alias → vary_by_role + :is_review_anonymous, # alias → is_anonymous + :is_review_done_by_teams, # alias → reviewing_is_done_by_teams + :allow_self_reviews, # alias → is_selfreview_enabled + :reviews_visible_to_other_reviewers, # alias → reviews_visible_to_all + :has_max_review_limit, # virtual (no DB column, UI toggle only) + :set_allowed_number_of_reviews_per_reviewer, # alias → num_reviews_allowed + :set_required_number_of_reviews_per_reviewer, # alias → num_reviews_required + :number_of_review_rounds, # alias → rounds_of_reviews + :is_role_based, # alias → team_members_have_duties + # Topics / bidding + :allow_topic_suggestion_from_students, # alias → allow_suggestions + :topics_assigned_by_bidding, + :enable_bidding_for_reviews, # alias → bidding_for_reviews_enabled + :can_review_same_topic, + :allow_reviewer_to_choose_topic_to_review, # alias → can_choose_topic_to_review + :staggered_deadline_assignment, # alias → staggered_deadline + # Penalties / late policy + :days_between_submissions, :late_policy_id, :is_penalty_calculated, + :calculate_penalty, :apply_late_policy, # apply_late_policy is virtual + # Mentors + :has_mentors, :auto_assign_mentors, # auto_assign_mentors → auto_assign_mentor + # UI-only virtual flag + :show_template_review, + # Grade scale + :instructor_grade_min_score, :instructor_grade_max_score, + # Rubric rows saved via nested attributes + assignment_questionnaires_attributes: %i[ + id questionnaire_id used_in_round questionnaire_weight notification_limit dropdown _destroy + ], + due_dates_attributes: %i[ + id deadline_type_id due_at round + submission_allowed_id review_allowed_id teammate_review_allowed_id + flag threshold _destroy + ] ) end @@ -301,10 +399,11 @@ def assignment_params def get_staggered_and_no_topic(assignment) topic_id = SignedUpTeam .joins(team: :teams_users) - .where(teams_users: { user_id: current_user.id, team_id: Team.where(parent_id: assignment.id).pluck(:id) }) + .where(teams_users: { user_id: current_user.id, + team_id: Team.where(parent_id: assignment.id).pluck(:id) }) .pluck(:project_topic_id) .first assignment.staggered_and_no_topic?(topic_id) end -end \ No newline at end of file +end diff --git a/app/models/assignment.rb b/app/models/assignment.rb index d05847892..bc9c9a7e8 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -7,27 +7,107 @@ class Assignment < ApplicationRecord has_many :teams, class_name: 'AssignmentTeam', foreign_key: 'parent_id', dependent: :destroy, inverse_of: :assignment has_many :invitations, class_name: 'Invitation', foreign_key: 'assignment_id', dependent: :destroy # , inverse_of: :assignment has_many :assignment_questionnaires, dependent: :destroy + accepts_nested_attributes_for :assignment_questionnaires, allow_destroy: true has_many :questionnaires, through: :assignment_questionnaires has_many :response_maps, foreign_key: 'reviewed_object_id', dependent: :destroy, inverse_of: :assignment - has_many :review_mappings, class_name: 'ReviewResponseMap', foreign_key: 'reviewed_object_id', dependent: :destroy, inverse_of: :assignment - has_many :project_topics , class_name: 'ProjectTopic', foreign_key: 'assignment_id', dependent: :destroy - has_many :due_dates,as: :parent, class_name: 'DueDate', dependent: :destroy + has_many :review_mappings, class_name: 'ReviewResponseMap', foreign_key: 'reviewed_object_id', dependent: :destroy, + inverse_of: :assignment + has_many :project_topics, class_name: 'ProjectTopic', foreign_key: 'assignment_id', dependent: :destroy + has_many :due_dates, as: :parent, class_name: 'DueDate', dependent: :destroy + accepts_nested_attributes_for :due_dates, allow_destroy: true has_many :assignments_duties, dependent: :destroy has_many :duties, through: :assignments_duties 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 + validates :name, presence: true + + # Virtual attributes not backed by a DB column + attr_accessor :title, :description + + # Map frontend field names to the actual DB column names + alias_attribute :review_strategy, :review_assignment_strategy + alias_attribute :maximum_number_of_reviews_per_submission, :max_reviews_per_submission + alias_attribute :show_teammate_review, :show_teammate_reviews + alias_attribute :is_review_anonymous, :is_anonymous + alias_attribute :allow_self_reviews, :is_selfreview_enabled + alias_attribute :set_allowed_number_of_reviews_per_reviewer, :num_reviews_allowed + alias_attribute :set_required_number_of_reviews_per_reviewer, :num_reviews_required + alias_attribute :reviews_visible_to_other_reviewers, :reviews_visible_to_all + alias_attribute :review_rubric_varies_by_round, :vary_by_round + alias_attribute :number_of_review_rounds, :rounds_of_reviews + alias_attribute :is_pair_programming, :enable_pair_programming + alias_attribute :allow_participants_to_create_bookmarks, :use_bookmark + alias_attribute :allow_tag_prompts, :is_answer_tagging_allowed + alias_attribute :available_to_students, :availability_flag + # Feature flags added in reimplementation migration. + # Explicit methods instead of alias_attribute so the app loads even before the migration runs + # (Rails 7.1 alias_attribute validates column existence at class-load time). + def review_rubric_varies_by_topic + read_attribute(:vary_by_topic) + end + + def review_rubric_varies_by_topic=(val) + write_attribute(:vary_by_topic, val) + end + + def review_rubric_varies_by_role + read_attribute(:vary_by_role) + end + + def review_rubric_varies_by_role=(val) + write_attribute(:vary_by_role, val) + end + + def auto_assign_mentors + read_attribute(:auto_assign_mentor) + end + + def auto_assign_mentors=(val) + write_attribute(:auto_assign_mentor, val) + end + + def is_role_based + read_attribute(:team_members_have_duties) + end + + def is_role_based=(val) + write_attribute(:team_members_have_duties, val) + end + + def enable_bidding_for_reviews + read_attribute(:bidding_for_reviews_enabled) + end + + def enable_bidding_for_reviews=(val) + write_attribute(:bidding_for_reviews_enabled, val) + end + + def is_review_done_by_teams + read_attribute(:reviewing_is_done_by_teams) + end + + def is_review_done_by_teams=(val) + write_attribute(:reviewing_is_done_by_teams, val) + end + # Frontend field names that map to differently-named existing DB columns + alias_attribute :allow_topic_suggestion_from_students, :allow_suggestions + alias_attribute :allow_reviewer_to_choose_topic_to_review, :can_choose_topic_to_review + alias_attribute :staggered_deadline_assignment, :staggered_deadline + + # Purely virtual flags — no DB column, not persisted + attr_writer :show_template_review, :has_max_review_limit, + :calibration_for_training, :apply_late_policy def review_questionnaire_id Questionnaire.find_by_assignment_id id end def teams? - @has_teams ||= teams.any? + @teams ||= teams.any? end + def num_review_rounds review_rounds = due_dates.where(deadline_type_id: DueDate::REVIEW_DEADLINE_TYPE_ID).count review_rounds.positive? ? review_rounds : (rounds_of_reviews || 0) @@ -39,7 +119,7 @@ def path raise 'The path cannot be created. The assignment must be associated with either a course or an instructor.' end - path_text = if !course_id.nil? && course_id > 0 + path_text = if !course_id.nil? && course_id.positive? "#{Rails.root}/pg_data/#{FileHelper.clean_path(instructor.name)}/#{FileHelper.clean_path(course.directory_path)}/" else "#{Rails.root}/pg_data/#{FileHelper.clean_path(instructor.name)}/" @@ -58,17 +138,17 @@ def add_participant(user_id) # Find the User with the provided user_id user = User.find_by(id: user_id) # Check if the user exists - if user.nil? - raise "The user account does not exist" - end + raise 'The user account does not exist' if user.nil? + # Check if the user is already a participant in the assignment - participant = AssignmentParticipant.find_by(parent_id:id, user_id:user.id) + participant = AssignmentParticipant.find_by(parent_id: id, user_id: user.id) if participant # Raises error if the user is already a participant raise "The user #{user.name} is already a participant." end + # Create a new AssignmentParticipant associated with the assignment and user - new_part = AssignmentParticipant.create(parent_id: self.id, + new_part = AssignmentParticipant.create(parent_id: id, user_id: user.id) # Set the participant's handle new_part.set_handle @@ -76,18 +156,17 @@ def add_participant(user_id) new_part end - # Remove a participant from the assignment based on the provided user_id. # This method finds the AssignmentParticipant with the given assignment_id and user_id, # and then deletes the corresponding record from the database. # No return value; the participant is removed from the assignment. def remove_participant(user_id) # Find the AssignmentParticipant associated with this assignment and user - assignment_participant = AssignmentParticipant.where(parent_id: self.id, user_id: user_id).first + assignment_participant = AssignmentParticipant.where(parent_id: id, user_id: user_id).first # Delete the AssignmentParticipant record - if assignment_participant - assignment_participant.destroy - end + return unless assignment_participant + + assignment_participant.destroy end # Remove the assignment from the associated course. @@ -100,9 +179,7 @@ def remove_assignment_from_course self end - - - # Assign a course to the assignment based on the provided course_id. + # Assign a course to the assignment based on the provided course_id. # If the assignment already belongs to the specified course, an error is raised. # Returns the modified assignment object with the updated course assignment. def assign_course(course_id) @@ -111,41 +188,41 @@ def assign_course(course_id) # Check if the assignment already belongs to the provided course_id if assignment.course_id == course_id # Raises error if the assignment already belongs to the provided course_id - raise "The assignment already belongs to this course id." + raise 'The assignment already belongs to this course id.' end + # Update the assignment's course assignment assignment.course_id = course_id # Return the modified assignment assignment end - # Create a copy of the assignment, including its name, instructor, and course assignment. # The new assignment is named "Copy of [original assignment name]". # Returns the newly created assignment object, which is a copy of the original assignment. def copy copied_assignment = Assignment.new( - name: "Copy of #{self.name}", - course_id: self.course_id - ) + name: "Copy of #{name}", + course_id: course_id + ) # Assign the correct instructor to the copied assignment - copied_assignment.instructor = self.instructor + copied_assignment.instructor = instructor # Save the copied assignment to the database copied_assignment.save copied_assignment - end + def is_calibrated? is_calibrated end - + def pair_programming_enabled? enable_pair_programming end - + def has_badge? has_badge end @@ -154,58 +231,57 @@ def staggered_and_no_topic?(topic_id) staggered_deadline? && topic_id.nil? end - - #This method return the value of the has_topics field for the given assignment object. + # This method return the value of the has_topics field for the given assignment object. # has_topics is of boolean type and is set true if there is any topic associated with the assignment. def topics? - @has_topics ||= project_topics.any? + @topics ||= project_topics.any? end - #This method return if the given assignment is a team assignment. + # This method return if the given assignment is a team assignment. # Checks if the value of max_team_size for the given assignment object is greater than 1 def team_assignment? !max_team_size.nil? && max_team_size > 1 end - #Auxiliary method for checking the validity of the field reviews_allowed for the given assignment object + # Auxiliary method for checking the validity of the field reviews_allowed for the given assignment object # Checks if review_allowed is not null and not negative. def valid_reviews_allowed?(reviews_allowed) reviews_allowed && reviews_allowed != -1 end - #method for checking if reviews_required are smaller than reviews_allowed for the given assignment object. + # method for checking if reviews_required are smaller than reviews_allowed for the given assignment object. def num_reviews_greater?(reviews_required, reviews_allowed) valid_reviews_allowed?(reviews_allowed) and reviews_required > reviews_allowed end - #This method checks if for the given review type, required reviews and allowed reviews have valid order of values + # This method checks if for the given review type, required reviews and allowed reviews have valid order of values # Receives a parameter review_type and return an object with boolean value of 'success' and corresponding message # If the parameter is of a invalid type, the corresponding error message is received. def valid_num_review(review_type) - if review_type=='review' - #checks for reviews - if num_reviews_greater?(num_reviews_required,num_reviews_allowed) - {success: false, message: 'Number of reviews required cannot be greater than number of reviews allowed'} + if review_type == 'review' + # checks for reviews + if num_reviews_greater?(num_reviews_required, num_reviews_allowed) + { success: false, message: 'Number of reviews required cannot be greater than number of reviews allowed' } else - {success: true} + { success: true } end - #checks for meta-reviews + # checks for meta-reviews elsif review_type == 'metareview' - if num_reviews_greater?(num_metareviews_required,num_metareviews_allowed) - {success: false, message: 'Number of metareviews required cannot be greater than number of metareviews allowed'} - else - {success: true} - end + if num_reviews_greater?(num_metareviews_required, num_metareviews_allowed) + { success: false, + message: 'Number of metareviews required cannot be greater than number of metareviews allowed' } + else + { success: true } + end - #for invalid review_type + # for invalid review_type else - {success: false, message: 'Please enter a valid review type.'} + { success: false, message: 'Please enter a valid review type.' } end end - - #This method check if for the given assignment,different type of rubrics are used in different round. + # 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. def varying_rubrics_by_round? @@ -226,5 +302,4 @@ def review_rounds(questionnaireType) end review_rounds end - end diff --git a/app/models/assignment_questionnaire.rb b/app/models/assignment_questionnaire.rb index 7b5f99551..d9f490e08 100644 --- a/app/models/assignment_questionnaire.rb +++ b/app/models/assignment_questionnaire.rb @@ -6,10 +6,12 @@ class AssignmentQuestionnaire < ApplicationRecord 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. + # If the linked questionnaire has scored questions (i.e. not only SectionHeaders), + # any weight is fine. If it has only SectionHeaders, weight must be 0. + # Empty questionnaires (no items yet) are exempt — the questionnaire hasn't been + # configured yet and blocking assignment creation at that stage is premature. def weight_must_be_zero_if_no_scored_questions - return if questionnaire.nil? || questionnaire_weight.nil? || questionnaire_weight.zero? + return if questionnaire.nil? || questionnaire_weight.nil? || questionnaire_weight.zero? || questionnaire.items.none? has_scored = questionnaire.items.where.not(question_type: 'SectionHeader').exists? unless has_scored diff --git a/app/models/assignment_team.rb b/app/models/assignment_team.rb index 8a104864b..b685c584e 100644 --- a/app/models/assignment_team.rb +++ b/app/models/assignment_team.rb @@ -103,7 +103,7 @@ def assign_reviewer(reviewer) assignment = Assignment.find(parent_id) raise 'The assignment cannot be found.' if assignment.nil? - ReviewResponseMap.create(reviewee_id: id, reviewer_id: reviewer.get_reviewer.id, reviewed_object_id: assignment.id, team_reviewing_enabled: assignment.team_reviewing_enabled) + ReviewResponseMap.create(reviewee_id: id, reviewer_id: reviewer.get_reviewer.id, reviewed_object_id: assignment.id, reviewing_is_done_by_teams: assignment.is_review_done_by_teams) end # Returns submitted files for this team. @@ -198,7 +198,7 @@ def assign_reviewer(reviewer) assignment = Assignment.find(parent_id) raise 'The assignment cannot be found.' if assignment.nil? - ReviewResponseMap.create(reviewee_id: id, reviewer_id: reviewer.get_reviewer.id, reviewed_object_id: assignment.id, team_reviewing_enabled: assignment.team_reviewing_enabled) + ReviewResponseMap.create(reviewee_id: id, reviewer_id: reviewer.get_reviewer.id, reviewed_object_id: assignment.id, reviewing_is_done_by_teams: assignment.is_review_done_by_teams) end # Whether the team has submitted work or not diff --git a/app/models/concerns/expertiza_constants.rb b/app/models/concerns/expertiza_constants.rb index 0ee3ab730..da3a70d4c 100644 --- a/app/models/concerns/expertiza_constants.rb +++ b/app/models/concerns/expertiza_constants.rb @@ -9,7 +9,7 @@ module DeadlineTypes SIGNUP = 8 TEAM_FORMATION = 9 - # Maps deadline_type_id to stage name, mirroring the old DeadlineType table + # Maps deadline_type_id to its stage name string. NAMES = { SUBMISSION => 'submission', REVIEW => 'review', diff --git a/app/models/due_date.rb b/app/models/due_date.rb index b81e86a88..68e2a017e 100644 --- a/app/models/due_date.rb +++ b/app/models/due_date.rb @@ -9,9 +9,16 @@ class DueDate < ApplicationRecord LATE_ALLOWED = 2 NOT_ALLOWED = 1 + NAMED_TYPES = [ + ExpertizaConstants::DeadlineTypes::SIGNUP, + ExpertizaConstants::DeadlineTypes::DROP_TOPIC, + ExpertizaConstants::DeadlineTypes::TEAM_FORMATION + ].freeze + belongs_to :parent, polymorphic: true validate :due_at_is_valid_datetime validates :due_at, presence: true + before_save :set_deadline_name attr_accessor :teammate_review_allowed, :submission_allowed, :review_allowed @@ -105,6 +112,16 @@ def self.current_round_number_for(parent, reference_time: Time.current) 0 end + private + + def set_deadline_name + return unless NAMED_TYPES.include?(deadline_type_id) + + self.deadline_name = ExpertizaConstants::DeadlineTypes::NAMES[deadline_type_id] + end + + public + # Creates duplicate due dates and assigns them to a new assignment def copy(new_assignment_id) new_due_date = dup diff --git a/app/models/review_mapping_handler.rb b/app/models/review_mapping_handler.rb index 11a5a5f2d..1144f7d3b 100644 --- a/app/models/review_mapping_handler.rb +++ b/app/models/review_mapping_handler.rb @@ -52,34 +52,33 @@ def assign_dynamic_topic_fairly(reviewer, k: 1) end # ===== CALIBRATION ===== - # calibration for bit turned on - # everyone gets assigned 2 calibration reviews along with the other reviwes - # assign calibration reviews done by instructor in round robin to students + # Assigns calibration reviews to all student participants in round-robin order. + # Calibration reviews are a special case of round-robin assignment: teams are + # filtered to those the instructor has already reviewed with for_calibration: true, + # and each created mapping also carries for_calibration: true. def assign_calibration_reviews_round_robin - # Get all participants (students) - reviewers = AssignmentParticipant.where(parent_id: @assignment.id) + instructor_participant = AssignmentParticipant.find_by( + parent_id: @assignment.id, + user_id: @assignment.instructor_id + ) + return unless instructor_participant - # Get all instructor calibration teams/submissions - calibration_teams = AssignmentTeam.where(parent_id: @assignment.id, is_calibration: true) - return if calibration_teams.empty? + calibration_team_ids = ReviewResponseMap.where( + reviewed_object_id: @assignment.id, + reviewer_id: instructor_participant.id, + for_calibration: true + ).pluck(:reviewee_id) + return if calibration_team_ids.empty? - # Assign in round robin: each reviewer gets 2 calibration reviews - reviewers.each_with_index do |reviewer, index| - 2.times do |i| - team = calibration_teams[(index + i) % calibration_teams.size] - ReviewResponseMap.find_or_create_by!( - reviewer: reviewer, - reviewee: team, - reviewed_object_id: @assignment.id, - calibration: true - ) - end - end - end + teams = AssignmentTeam.where(id: calibration_team_ids) + reviewers = AssignmentParticipant.where(parent_id: @assignment.id) + .where.not(user_id: @assignment.instructor_id) + assign_round_robin(reviewers, teams, reviews_per_reviewer: 2, for_calibration: true) + end def calibration_reviews_for(reviewer) - ReviewResponseMap.where(reviewer: reviewer, calibration: true) + ReviewResponseMap.where(reviewer_id: reviewer.id, for_calibration: true) end # ===== OUTSTANDING REVIEWS ===== @@ -108,6 +107,20 @@ def grade_review(mapping, grade:, comment:) private + def assign_round_robin(reviewers, teams, reviews_per_reviewer: 1, for_calibration: false) + reviewers.each_with_index do |reviewer, index| + reviews_per_reviewer.times do |i| + team = teams[(index + i) % teams.size] + ReviewResponseMap.find_or_create_by!( + reviewer_id: reviewer.id, + reviewee_id: team.id, + reviewed_object_id: @assignment.id, + for_calibration: for_calibration + ) + end + end + end + def create_mapping(reviewer, team) ReviewResponseMap.create!( reviewer: reviewer, diff --git a/app/serializers/assignment_questionnaire_serializer.rb b/app/serializers/assignment_questionnaire_serializer.rb new file mode 100644 index 000000000..f9471a2ca --- /dev/null +++ b/app/serializers/assignment_questionnaire_serializer.rb @@ -0,0 +1,13 @@ +class AssignmentQuestionnaireSerializer < ActiveModel::Serializer + attributes :id, :questionnaire_id, :used_in_round, :questionnaire_weight, + :notification_limit, :dropdown + + def questionnaire + q = object.questionnaire + return nil unless q + + { id: q.id, name: q.name, questionnaire_type: q.questionnaire_type } + end + + attribute :questionnaire +end diff --git a/app/serializers/assignment_serializer.rb b/app/serializers/assignment_serializer.rb index 7124e4bf5..4612d06cc 100644 --- a/app/serializers/assignment_serializer.rb +++ b/app/serializers/assignment_serializer.rb @@ -1,3 +1,39 @@ class AssignmentSerializer < ActiveModel::Serializer - attributes :id, :name, :max_team_size, :course_id + attributes :id, :name, :course_id, :instructor_id, + # General tab + :directory_path, :spec_location, :private, + :require_quiz, :staggered_deadline, :is_calibrated, + :has_teams, :max_team_size, + :show_teammate_review, :is_pair_programming, + :has_topics, + :allow_tag_prompts, :available_to_students, + :allow_participants_to_create_bookmarks, + # Mentors + :has_mentors, :auto_assign_mentors, + :staggered_deadline_assignment, + # Topics / bidding + :allow_topic_suggestion_from_students, + :topics_assigned_by_bidding, + :enable_bidding_for_reviews, + :can_review_same_topic, + :allow_reviewer_to_choose_topic_to_review, + # Review strategy tab + :review_topic_threshold, :maximum_number_of_reviews_per_submission, + :review_strategy, + :review_rubric_varies_by_round, + :review_rubric_varies_by_topic, :review_rubric_varies_by_role, + :is_review_anonymous, :allow_self_reviews, + :reviews_visible_to_other_reviewers, + :is_review_done_by_teams, + :is_role_based, + :set_allowed_number_of_reviews_per_reviewer, + :set_required_number_of_reviews_per_reviewer, + :number_of_review_rounds, + :instructor_grade_min_score, :instructor_grade_max_score, + # Due dates tab + :days_between_submissions, :late_policy_id, + :is_penalty_calculated, :calculate_penalty + + has_many :due_dates, serializer: DueDateSerializer + has_many :assignment_questionnaires, serializer: AssignmentQuestionnaireSerializer end diff --git a/app/serializers/due_date_serializer.rb b/app/serializers/due_date_serializer.rb new file mode 100644 index 000000000..a6226c274 --- /dev/null +++ b/app/serializers/due_date_serializer.rb @@ -0,0 +1,10 @@ +class DueDateSerializer < ActiveModel::Serializer + attributes :id, :deadline_type_id, :due_at, :round, + :submission_allowed_id, :review_allowed_id, :teammate_review_allowed_id + + def deadline_name + ExpertizaConstants::DeadlineTypes::NAMES[object.deadline_type_id] + end + + attribute :deadline_name +end diff --git a/config/routes.rb b/config/routes.rb index a5d3e7e41..005cee54d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -41,6 +41,9 @@ get '/:assignment_id/varying_rubrics_by_round', action: :varying_rubrics_by_round? post '/:assignment_id/create_node',action: :create_node end + member do + get :calibration_submissions + end end resources :bookmarks, except: [:new, :edit] do diff --git a/db/migrate/20260919000001_add_dropdown_to_assignment_questionnaires.rb b/db/migrate/20260919000001_add_dropdown_to_assignment_questionnaires.rb new file mode 100644 index 000000000..7c4e71497 --- /dev/null +++ b/db/migrate/20260919000001_add_dropdown_to_assignment_questionnaires.rb @@ -0,0 +1,5 @@ +class AddDropdownToAssignmentQuestionnaires < ActiveRecord::Migration[7.0] + def change + add_column :assignment_questionnaires, :dropdown, :boolean, default: false, null: false + end +end diff --git a/db/migrate/20260921000001_add_calibrate_to_to_response_maps.rb b/db/migrate/20260921000001_add_calibrate_to_to_response_maps.rb new file mode 100644 index 000000000..f883f220f --- /dev/null +++ b/db/migrate/20260921000001_add_calibrate_to_to_response_maps.rb @@ -0,0 +1,5 @@ +class AddCalibrateToToResponseMaps < ActiveRecord::Migration[7.1] + def change + add_column :response_maps, :for_calibration, :boolean, default: false, null: false + end +end diff --git a/db/migrate/20260921000002_add_missing_assignment_feature_flags.rb b/db/migrate/20260921000002_add_missing_assignment_feature_flags.rb new file mode 100644 index 000000000..141747d06 --- /dev/null +++ b/db/migrate/20260921000002_add_missing_assignment_feature_flags.rb @@ -0,0 +1,12 @@ +class AddMissingAssignmentFeatureFlags < ActiveRecord::Migration[7.1] + def change + add_column :assignments, :vary_by_topic, :boolean, default: false + add_column :assignments, :vary_by_role, :boolean, default: false + add_column :assignments, :has_mentors, :boolean, default: false + add_column :assignments, :auto_assign_mentor, :boolean, default: false + add_column :assignments, :team_members_have_duties, :boolean, default: false + add_column :assignments, :bidding_for_reviews_enabled, :boolean, default: false + add_column :assignments, :topics_assigned_by_bidding, :boolean, default: false + add_column :assignments, :reviewing_is_done_by_teams, :boolean, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index c441fb562..685529ec5 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: 2026_08_17_000001) do +ActiveRecord::Schema[8.0].define(version: 2026_09_21_000002) do create_table "account_requests", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "username" t.string "full_name" @@ -44,6 +44,7 @@ t.datetime "updated_at", null: false t.integer "used_in_round" t.integer "questionnaire_weight" + t.boolean "dropdown", default: false, null: false t.index ["assignment_id"], name: "fk_aq_assignments_id" t.index ["questionnaire_id"], name: "fk_aq_questionnaire_id" end @@ -106,6 +107,14 @@ t.boolean "vary_by_round", default: false, null: false t.integer "instructor_grade_min_score" t.integer "instructor_grade_max_score" + t.boolean "vary_by_topic", default: false + t.boolean "vary_by_role", default: false + t.boolean "has_mentors", default: false + t.boolean "auto_assign_mentor", default: false + t.boolean "team_members_have_duties", default: false + t.boolean "bidding_for_reviews_enabled", default: false + t.boolean "topics_assigned_by_bidding", default: false + t.boolean "reviewing_is_done_by_teams", default: false t.index ["course_id"], name: "index_assignments_on_course_id" t.index ["instructor_id"], name: "index_assignments_on_instructor_id" end @@ -333,6 +342,7 @@ t.datetime "created_at", null: false t.datetime "updated_at", null: false t.string "type" + t.boolean "for_calibration", default: false, null: false t.index ["reviewer_id"], name: "fk_response_map_reviewer" end @@ -468,9 +478,9 @@ add_foreign_key "assignments_duties", "duties" add_foreign_key "courses", "institutions" add_foreign_key "courses", "users", column: "instructor_id" + add_foreign_key "duties", "users", column: "instructor_id" add_foreign_key "invitations", "participants", column: "from_id" add_foreign_key "invitations", "participants", column: "to_id" - add_foreign_key "duties", "users", column: "instructor_id" add_foreign_key "items", "questionnaires" add_foreign_key "participants", "duties" add_foreign_key "participants", "join_team_requests" diff --git a/spec/models/assignment_spec.rb b/spec/models/assignment_spec.rb index dc421bf47..f3c537f4a 100644 --- a/spec/models/assignment_spec.rb +++ b/spec/models/assignment_spec.rb @@ -14,6 +14,97 @@ 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) } + # ----------------------------------------------------------------------- + # Virtual attributes (attr_writer only — no DB column) + # ----------------------------------------------------------------------- + describe 'virtual attr_writer fields' do + it 'apply_late_policy is settable but is not persisted to the DB' do + a = Assignment.create!(name: 'Virtual Late', instructor: instructor) + a.apply_late_policy = true + a.save! + expect(a.reload.is_penalty_calculated).to be_falsy + end + + it 'has_max_review_limit is settable but is not persisted to the DB' do + a = Assignment.create!(name: 'Virtual Max', instructor: instructor, + num_reviews_allowed: 0) + a.has_max_review_limit = true + a.save! + expect(a.reload.num_reviews_allowed).to eq(0) + end + end + + # ----------------------------------------------------------------------- + # is_penalty_calculated — real DB boolean + # ----------------------------------------------------------------------- + describe '#is_penalty_calculated' do + it 'defaults to false' do + a = Assignment.create!(name: 'Default Penalty', instructor: instructor) + expect(a.is_penalty_calculated).to be_falsy + end + + it 'persists true when set on save' do + a = Assignment.create!(name: 'Penalty True', instructor: instructor, + is_penalty_calculated: true) + expect(a.reload.is_penalty_calculated).to be true + end + + it 'can be toggled off after being set' do + a = Assignment.create!(name: 'Toggle Penalty', instructor: instructor, + is_penalty_calculated: true) + a.update!(is_penalty_calculated: false) + expect(a.reload.is_penalty_calculated).to be false + end + end + + # ----------------------------------------------------------------------- + # set_allowed_number_of_reviews_per_reviewer alias (→ num_reviews_allowed) + # ----------------------------------------------------------------------- + describe '#set_allowed_number_of_reviews_per_reviewer' do + it 'reads and writes through the num_reviews_allowed DB column' do + a = Assignment.create!(name: 'Review Limit Model', instructor: instructor, + num_reviews_allowed: 3) + expect(a.set_allowed_number_of_reviews_per_reviewer).to eq(3) + end + + it 'persists when updated via the alias' do + a = Assignment.create!(name: 'Review Limit Update', instructor: instructor, + num_reviews_allowed: 0) + a.update!(set_allowed_number_of_reviews_per_reviewer: 7) + expect(a.reload.num_reviews_allowed).to eq(7) + end + end + + # ----------------------------------------------------------------------- + # Named due_dates — dropdown updates without changing due_at + # ----------------------------------------------------------------------- + describe 'updating named deadline dropdowns without changing due_at' do + it 'preserves due_at when updating only the allowed_id columns' do + a = Assignment.create!(name: 'Dropdown Only', instructor: instructor) + original_due_at = 3.days.from_now + dd = AssignmentDueDate.create!( + parent: a, + due_at: original_due_at, + deadline_type_id: ExpertizaConstants::DeadlineTypes::DROP_TOPIC, + submission_allowed_id: 3, review_allowed_id: 3, teammate_review_allowed_id: 3 + ) + + # Update only the allowed_ids — no due_at in the nested attributes + a.update!(due_dates_attributes: [{ + id: dd.id, + deadline_type_id: ExpertizaConstants::DeadlineTypes::DROP_TOPIC, + submission_allowed_id: 1, + review_allowed_id: 2, + teammate_review_allowed_id: 1 + }]) + + dd.reload + expect(dd.due_at.to_i).to be_within(1).of(original_due_at.to_i) + expect(dd.submission_allowed_id).to eq(1) + expect(dd.review_allowed_id).to eq(2) + end + end + 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) diff --git a/spec/requests/api/v1/assignment_controller_spec.rb b/spec/requests/api/v1/assignment_controller_spec.rb index fe67b223d..00a6b7c9f 100644 --- a/spec/requests/api/v1/assignment_controller_spec.rb +++ b/spec/requests/api/v1/assignment_controller_spec.rb @@ -23,23 +23,23 @@ let!(:user) do User.create!( id: 1, - name: "admin", - full_name: "admin", - email: "admin@gmail.com", - password_digest: "admin", - role_id: 2, # Must exist in DB + name: 'admin', + full_name: 'admin', + email: 'admin@gmail.com', + password_digest: 'admin', + role_id: 2, # Must exist in DB institution_id: institution.id ) end let!(:prof) do User.create!( - name: "profa", - password_digest: "password", + name: 'profa', + password_digest: 'password', role_id: @roles[:instructor].id, - full_name: "Prof A", - email: "testuser@example.com", - mru_directory_path: "/home/testuser" + full_name: 'Prof A', + email: 'testuser@example.com', + mru_directory_path: '/home/testuser' ) end @@ -55,8 +55,7 @@ id: 1, name: 'ECE517', instructor: prof, - institution: institution - ) + institution: institution) end let(:token) { JsonWebToken.encode({ id: prof.id }) } @@ -67,7 +66,7 @@ # ------------------------------------------------------------------------- path '/assignments' do get 'Get assignments' do - tags "Get All Assignments" + tags 'Get All Assignments' produces 'application/json' parameter name: 'Content-Type', in: :header, type: :string let('Content-Type') { 'application/json' } @@ -97,8 +96,8 @@ let('Content-Type') { 'application/json' } response '200', 'participant added successfully' do - let(:user_id) { user.id } # "admin" user - let(:assignment_id) { assignment.id } # assignment1 + let(:user_id) { user.id } # "admin" user + let(:assignment_id) { assignment.id } # assignment1 run_test! do response_json = JSON.parse(response.body) @@ -446,7 +445,6 @@ expect(response).to have_http_status(:ok) expect(data['id']).to eq(assignment.id) expect(data['name']).to eq(assignment.name) - expect(data['has_badge']).to eq(assignment.has_badge?) expect(data['pair_programming_enabled']).to eq(assignment.pair_programming_enabled?) expect(data['is_calibrated']).to eq(assignment.is_calibrated?) expect(data['staggered_and_no_topic']).to eq(assignment.staggered_and_no_topic?(topic_id)) @@ -461,12 +459,91 @@ expect(response).to have_http_status(:not_found) expect(data['error']).to eq('Assignment not found') end - end end end # ------------------------------------------------------------------------- + # GET /assignments/:id — show + # ------------------------------------------------------------------------- + describe 'GET /assignments/:id' do + it 'returns 200 with assignment data for a valid id' do + get "/assignments/#{assignment.id}", headers: { 'Authorization' => Authorization() } + expect(response).to have_http_status(:ok) + data = JSON.parse(response.body) + expect(data['id']).to eq(assignment.id) + expect(data['name']).to eq(assignment.name) + end + + it 'returns 404 when assignment does not exist' do + get '/assignments/999', headers: { 'Authorization' => Authorization() } + expect(response).to have_http_status(:not_found) + end + end + + # ------------------------------------------------------------------------- + # PATCH /assignments/:id — update 404 + # ------------------------------------------------------------------------- + describe 'PATCH /assignments/:id — not found' do + it 'returns 404 when assignment does not exist' do + patch '/assignments/999', + params: { assignment: { name: 'Ghost' } }.to_json, + headers: { 'Content-Type' => 'application/json', 'Authorization' => Authorization() } + expect(response).to have_http_status(:not_found) + end + end + + # ------------------------------------------------------------------------- + # Topic settings — field persistence + # ------------------------------------------------------------------------- + describe 'topic settings field persistence via PATCH /assignments/:id' do + let(:assignment) { Assignment.create!(name: 'Topic Settings Test', instructor_id: prof.id) } + + def patch_assignment(body) + patch "/assignments/#{assignment.id}", + params: { assignment: body }.to_json, + headers: { 'Content-Type' => 'application/json', 'Authorization' => Authorization() } + end + + it 'persists allow_topic_suggestion_from_students' do + patch_assignment(allow_topic_suggestion_from_students: true) + expect(response).to have_http_status(:ok) + expect(assignment.reload.allow_suggestions).to be true + end + + it 'persists topics_assigned_by_bidding' do + patch_assignment(topics_assigned_by_bidding: true) + expect(response).to have_http_status(:ok) + expect(assignment.reload.topics_assigned_by_bidding).to be true + end + + it 'persists can_review_same_topic' do + patch_assignment(can_review_same_topic: true) + expect(response).to have_http_status(:ok) + expect(assignment.reload.can_review_same_topic).to be true + end + + it 'persists allow_reviewer_to_choose_topic_to_review' do + patch_assignment(allow_reviewer_to_choose_topic_to_review: true) + expect(response).to have_http_status(:ok) + expect(assignment.reload.can_choose_topic_to_review).to be true + end + + it 'persists enable_bidding_for_reviews' do + patch_assignment(enable_bidding_for_reviews: true) + expect(response).to have_http_status(:ok) + expect(assignment.reload.bidding_for_reviews_enabled).to be true + end + + it 'resets topic settings to false' do + assignment.update!(allow_suggestions: true, topics_assigned_by_bidding: true) + patch_assignment(allow_topic_suggestion_from_students: false, topics_assigned_by_bidding: false) + expect(response).to have_http_status(:ok) + expect(assignment.reload.allow_suggestions).to be false + expect(assignment.reload.topics_assigned_by_bidding).to be false + end + end + # instructor_grade_min_score / instructor_grade_max_score # ------------------------------------------------------------------------- @@ -482,7 +559,7 @@ it 'includes assignment_questionnaires with nested questionnaire in the response' do questionnaire = Questionnaire.create!(name: 'Review Q', instructor_id: prof.id, - min_question_score: 1, max_question_score: 5) + min_question_score: 1, max_question_score: 5) questionnaire.items.create!(txt: 'Q1', seq: 1, question_type: 'Scale', weight: 1, break_before: true) AssignmentQuestionnaire.create!(assignment: assignment, questionnaire: questionnaire, used_in_round: 1, questionnaire_weight: 100) @@ -517,4 +594,282 @@ end end end + + # ------------------------------------------------------------------------- + # PATCH /assignments/{id} — field persistence for bug-fixed fields + # ------------------------------------------------------------------------- + describe 'PATCH /assignments/:id' do + let(:assignment) { Assignment.create!(name: 'Patch Test', instructor_id: prof.id) } + + def patch_assignment(id, body) + patch "/assignments/#{id}", + params: { assignment: body }.to_json, + headers: { 'Content-Type' => 'application/json', 'Authorization' => Authorization() } + end + + # ------------------------------------------------------------------ + # is_penalty_calculated (the real DB column behind apply_late_policy) + # ------------------------------------------------------------------ + context 'is_penalty_calculated' do + it 'persists true when sent in the payload' do + patch_assignment(assignment.id, { is_penalty_calculated: true }) + expect(response).to have_http_status(:ok) + expect(assignment.reload.is_penalty_calculated).to be true + expect(JSON.parse(response.body)['is_penalty_calculated']).to be true + end + + it 'persists false when sent in the payload' do + assignment.update!(is_penalty_calculated: true) + patch_assignment(assignment.id, { is_penalty_calculated: false }) + expect(response).to have_http_status(:ok) + expect(assignment.reload.is_penalty_calculated).to be false + end + + it 'apply_late_policy (virtual) in payload does not change is_penalty_calculated' do + # Sending the virtual name alone should be a no-op on the DB column. + # The frontend now always sends is_penalty_calculated, not apply_late_policy. + assignment.update!(is_penalty_calculated: false) + patch_assignment(assignment.id, { apply_late_policy: true }) + expect(assignment.reload.is_penalty_calculated).to be false + end + end + + # ------------------------------------------------------------------ + # set_allowed_number_of_reviews_per_reviewer (alias → num_reviews_allowed) + # ------------------------------------------------------------------ + context 'set_allowed_number_of_reviews_per_reviewer' do + it 'persists a positive limit' do + patch_assignment(assignment.id, { set_allowed_number_of_reviews_per_reviewer: 4 }) + expect(response).to have_http_status(:ok) + expect(assignment.reload.num_reviews_allowed).to eq(4) + expect(JSON.parse(response.body)['num_reviews_allowed']).to eq(4) + end + + it 'persists zero to clear the limit' do + assignment.update!(num_reviews_allowed: 5) + patch_assignment(assignment.id, { set_allowed_number_of_reviews_per_reviewer: 0 }) + expect(response).to have_http_status(:ok) + expect(assignment.reload.num_reviews_allowed).to eq(0) + end + + it 'has_max_review_limit (virtual) in payload is accepted without error and is a no-op' do + patch_assignment(assignment.id, { has_max_review_limit: true }) + expect(response).to have_http_status(:ok) + # DB column is unchanged + expect(assignment.reload.num_reviews_allowed).to eq(assignment.num_reviews_allowed) + end + end + + # ------------------------------------------------------------------ + # Named deadline due_dates_attributes (drop_topic, team_formation, signup) + # ------------------------------------------------------------------ + context 'named deadline due_dates_attributes' do + it 'creates a drop_topic deadline when none exists' do + due_at = 7.days.from_now + patch_assignment(assignment.id, { + due_dates_attributes: [{ + deadline_type_id: ExpertizaConstants::DeadlineTypes::DROP_TOPIC, + due_at: due_at.iso8601, + submission_allowed_id: 3, + review_allowed_id: 3, + teammate_review_allowed_id: 3 + }] + }) + expect(response).to have_http_status(:ok) + dd = assignment.due_dates.find_by(deadline_type_id: ExpertizaConstants::DeadlineTypes::DROP_TOPIC) + expect(dd).not_to be_nil + expect(dd.due_at.to_i).to be_within(2).of(due_at.to_i) + end + + it 'updates only allowed_ids on an existing named deadline (no due_at change)' do + original_due_at = 5.days.from_now + dd = AssignmentDueDate.create!( + parent: assignment, + due_at: original_due_at, + deadline_type_id: ExpertizaConstants::DeadlineTypes::DROP_TOPIC, + submission_allowed_id: 3, review_allowed_id: 3, teammate_review_allowed_id: 3 + ) + + patch_assignment(assignment.id, { + due_dates_attributes: [{ + id: dd.id, + deadline_type_id: ExpertizaConstants::DeadlineTypes::DROP_TOPIC, + submission_allowed_id: 1, + review_allowed_id: 2, + teammate_review_allowed_id: 1 + }] + }) + + expect(response).to have_http_status(:ok) + dd.reload + expect(dd.submission_allowed_id).to eq(1) + expect(dd.review_allowed_id).to eq(2) + # due_at is preserved since we omitted it from the payload + expect(dd.due_at.to_i).to be_within(2).of(original_due_at.to_i) + end + + it 'creates a team_formation deadline with the correct deadline_name in the response' do + due_at = 10.days.from_now + patch_assignment(assignment.id, { + due_dates_attributes: [{ + deadline_type_id: ExpertizaConstants::DeadlineTypes::TEAM_FORMATION, + due_at: due_at.iso8601, + submission_allowed_id: 3, + review_allowed_id: 3, + teammate_review_allowed_id: 3 + }] + }) + expect(response).to have_http_status(:ok) + due_dates = JSON.parse(response.body)['due_dates'] + tf = due_dates.find { |d| d['deadline_type_id'] == ExpertizaConstants::DeadlineTypes::TEAM_FORMATION } + expect(tf).not_to be_nil + expect(tf['deadline_name']).to eq('team_formation') + end + end + end + + # ------------------------------------------------------------------------- + # POST /assignments — create + # ------------------------------------------------------------------------- + describe 'POST /assignments' do + def post_assignment(body) + post '/assignments', + params: { assignment: body }.to_json, + headers: { 'Content-Type' => 'application/json', 'Authorization' => Authorization() } + end + + context 'with valid params' do + it 'creates the assignment and returns 201' do + expect do + post_assignment(name: 'Brand New Assignment') + end.to change(Assignment, :count).by(1) + expect(response).to have_http_status(:created) + end + + it 'sets instructor_id to the authenticated user when no course_id is given' do + post_assignment(name: 'Standalone Assignment') + data = JSON.parse(response.body) + expect(data['instructor_id']).to eq(prof.id) + end + + it 'inherits instructor_id from the course when course_id is given' do + post_assignment(name: 'Course Assignment', course_id: course.id) + expect(response).to have_http_status(:created) + data = JSON.parse(response.body) + expect(data['instructor_id']).to eq(course.instructor_id) + expect(data['course_id']).to eq(course.id) + end + end + + context 'with invalid params' do + it 'returns 422 when name is blank' do + post_assignment(name: '') + expect(response).to have_http_status(:unprocessable_entity) + end + end + end + + # ------------------------------------------------------------------------- + # GET /assignments/:id/calibration_submissions + # ------------------------------------------------------------------------- + describe 'GET /assignments/:id/calibration_submissions' do + def get_calibration(id) + get "/assignments/#{id}/calibration_submissions", + headers: { 'Authorization' => Authorization() } + end + + context 'when the assignment exists' do + it 'returns 200 with an empty array when there are no teams' do + get_calibration(assignment.id) + expect(response).to have_http_status(:ok) + expect(JSON.parse(response.body)).to eq([]) + end + + it 'returns a row per team with not_started status when no review map exists' do + team = AssignmentTeam.create!(name: 'Team Alpha', parent_id: assignment.id) + get_calibration(assignment.id) + expect(response).to have_http_status(:ok) + data = JSON.parse(response.body) + expect(data.size).to eq(1) + expect(data.first['id']).to eq(team.id) + expect(data.first['review_status']).to eq('not_started') + expect(data.first['submitted_content']).to include('hyperlinks', 'files') + end + + it 'includes participant names for team members' do + team = AssignmentTeam.create!(name: 'Team Beta', parent_id: assignment.id) + TeamsUser.create!(team: team, user: prof) + get_calibration(assignment.id) + data = JSON.parse(response.body) + expect(data.first['participant_name']).to include(prof.name) + end + end + + context 'when the assignment does not exist' do + it 'returns 404' do + get_calibration(999) + expect(response).to have_http_status(:not_found) + end + end + end + + # ------------------------------------------------------------------------- + # GET /assignments/:assignment_id/varying_rubrics_by_round + # ------------------------------------------------------------------------- + describe 'GET /assignments/:assignment_id/varying_rubrics_by_round' do + def get_varying(assignment_id) + get "/assignments/#{assignment_id}/varying_rubrics_by_round", + headers: { 'Authorization' => Authorization() } + end + + context 'when the assignment does not exist' do + it 'returns 404' do + get_varying(999) + expect(response).to have_http_status(:not_found) + end + end + + context 'when the assignment has no questionnaire' do + it 'returns 404 with a descriptive error' do + get_varying(assignment.id) + expect(response).to have_http_status(:not_found) + data = JSON.parse(response.body) + expect(data['error']).to match(%r{No questionnaire/rubric exists}) + end + end + + context 'when a questionnaire exists' do + let!(:questionnaire) do + Questionnaire.create!(name: 'Review Rubric', instructor_id: prof.id, + min_question_score: 0, max_question_score: 5) + end + + it 'returns false when vary_by_round is false' do + assignment.update!(vary_by_round: false) + AssignmentQuestionnaire.create!(assignment: assignment, questionnaire: questionnaire, + used_in_round: 1, questionnaire_weight: 100) + get_varying(assignment.id) + expect(response).to have_http_status(:ok) + expect(JSON.parse(response.body)).to eq(false) + end + + it 'returns true when vary_by_round is true and per-round questionnaires exist' do + assignment.update!(vary_by_round: true) + AssignmentQuestionnaire.create!(assignment: assignment, questionnaire: questionnaire, + used_in_round: 1, questionnaire_weight: 100) + get_varying(assignment.id) + expect(response).to have_http_status(:ok) + expect(JSON.parse(response.body)).to eq(true) + end + + it 'returns false when vary_by_round is true but no used_in_round is set' do + assignment.update!(vary_by_round: true) + AssignmentQuestionnaire.create!(assignment: assignment, questionnaire: questionnaire, + used_in_round: nil, questionnaire_weight: 100) + get_varying(assignment.id) + expect(response).to have_http_status(:ok) + expect(JSON.parse(response.body)).to eq(false) + end + end + end end diff --git a/spec/requests/api/v1/courses_spec.rb b/spec/requests/api/v1/courses_spec.rb index 241231b43..8c2f1b156 100644 --- a/spec/requests/api/v1/courses_spec.rb +++ b/spec/requests/api/v1/courses_spec.rb @@ -209,6 +209,18 @@ end run_test! end + + response(422, 'unprocessable entity when name is missing') do + let(:institution) { Institution.create(name: "NC State") } + let(:course) { + { institution_id: institution.id, instructor_id: prof.id, directory_path: 'samplepath', name: '' } + } + run_test! do + expect(response).to have_http_status(:unprocessable_entity) + errors = JSON.parse(response.body) + expect(errors).to have_key('name') + end + end end end @@ -233,6 +245,15 @@ end run_test! end + + response(404, 'course not found') do + let(:id) { 999 } + run_test! do + expect(response).to have_http_status(:not_found) + data = JSON.parse(response.body) + expect(data['error']).to match(/not found/i) + end + end end # PATCH /courses/{id} @@ -266,6 +287,30 @@ end run_test! end + + response(404, 'course not found') do + let(:id) { 999 } + let(:course) { { name: 'Updated' } } + run_test! do + expect(response).to have_http_status(:not_found) + data = JSON.parse(response.body) + expect(data['error']).to match(/not found/i) + end + end + + response(422, 'unprocessable entity when name is blank') do + let(:institution2) { Institution.create(name: "NC State 2") } + let(:existing_course) { + Course.create(institution_id: institution2.id, instructor_id: prof.id, directory_path: 'path2', name: 'CS101', info: '') + } + let(:id) { existing_course.id } + let(:course) { { name: '' } } + run_test! do + expect(response).to have_http_status(:unprocessable_entity) + errors = JSON.parse(response.body) + expect(errors).to have_key('name') + end + end end # PUT /courses/{id} @@ -299,6 +344,16 @@ end run_test! end + + response(404, 'course not found') do + let(:id) { 999 } + let(:course) { { name: 'Updated' } } + run_test! do + expect(response).to have_http_status(:not_found) + data = JSON.parse(response.body) + expect(data['error']).to match(/not found/i) + end + end end # DELETE /courses/{id} diff --git a/spec/serializers/assignment_serializer_spec.rb b/spec/serializers/assignment_serializer_spec.rb new file mode 100644 index 000000000..dc9a54dc9 --- /dev/null +++ b/spec/serializers/assignment_serializer_spec.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe AssignmentSerializer, type: :serializer do + include RolesHelper + + before(:each) { @roles = create_roles_hierarchy } + + let(:institution) { Institution.create!(name: 'NCSU Serializer Test') } + let(:instructor) do + User.create!(name: 'serializer_instructor', full_name: 'Serializer Instructor', + email: 'serializer@example.com', password_digest: 'password', + role_id: @roles[:instructor].id, institution_id: institution.id) + end + + def serialized(assignment) + JSON.parse(AssignmentSerializer.new(assignment).to_json) + end + + # ----------------------------------------------------------------------- + # is_penalty_calculated / apply_late_policy + # ----------------------------------------------------------------------- + describe 'late penalty fields' do + it 'includes is_penalty_calculated in the serialized output' do + assignment = Assignment.create!(name: 'Penalty Test', instructor: instructor, + is_penalty_calculated: true) + json = serialized(assignment) + expect(json['is_penalty_calculated']).to be true + end + + it 'does NOT include apply_late_policy (virtual attr_writer has no DB column)' do + assignment = Assignment.create!(name: 'Virtual Penalty', instructor: instructor) + json = serialized(assignment) + expect(json.key?('apply_late_policy')).to be false + end + + it 'is_penalty_calculated persists correctly when updated' do + assignment = Assignment.create!(name: 'Persist Penalty', instructor: instructor, + is_penalty_calculated: false) + assignment.update!(is_penalty_calculated: true) + json = serialized(assignment.reload) + expect(json['is_penalty_calculated']).to be true + end + end + + # ----------------------------------------------------------------------- + # set_allowed_number_of_reviews_per_reviewer / has_max_review_limit + # ----------------------------------------------------------------------- + describe 'review limit fields' do + it 'includes set_allowed_number_of_reviews_per_reviewer in the serialized output' do + assignment = Assignment.create!(name: 'Review Limit', instructor: instructor, + num_reviews_allowed: 5) + json = serialized(assignment) + expect(json['set_allowed_number_of_reviews_per_reviewer']).to eq(5) + end + + it 'does NOT include has_max_review_limit (virtual attr_writer, not a DB column)' do + assignment = Assignment.create!(name: 'No Max Limit Field', instructor: instructor) + json = serialized(assignment) + expect(json.key?('has_max_review_limit')).to be false + end + + it 'set_allowed_number_of_reviews_per_reviewer is 0 when no limit is configured' do + assignment = Assignment.create!(name: 'Zero Limit', instructor: instructor, + num_reviews_allowed: 0) + json = serialized(assignment) + expect(json['set_allowed_number_of_reviews_per_reviewer']).to eq(0) + end + end + + # ----------------------------------------------------------------------- + # due_dates: deadline_name for named deadlines + # ----------------------------------------------------------------------- + describe 'due_dates with named deadlines' do + it 'includes deadline_name = drop_topic for type id 7' do + assignment = Assignment.create!(name: 'Drop Topic', instructor: instructor) + AssignmentDueDate.create!(parent: assignment, + due_at: 1.week.from_now, + deadline_type_id: ExpertizaConstants::DeadlineTypes::DROP_TOPIC, + submission_allowed_id: 3, review_allowed_id: 3) + json = serialized(assignment) + drop = json['due_dates'].find { |d| d['deadline_type_id'] == ExpertizaConstants::DeadlineTypes::DROP_TOPIC } + expect(drop).not_to be_nil + expect(drop['deadline_name']).to eq('drop_topic') + end + + it 'includes deadline_name = team_formation for type id 9' do + assignment = Assignment.create!(name: 'Team Formation', instructor: instructor) + AssignmentDueDate.create!(parent: assignment, + due_at: 1.week.from_now, + deadline_type_id: ExpertizaConstants::DeadlineTypes::TEAM_FORMATION, + submission_allowed_id: 3, review_allowed_id: 3) + json = serialized(assignment) + tf = json['due_dates'].find { |d| d['deadline_type_id'] == ExpertizaConstants::DeadlineTypes::TEAM_FORMATION } + expect(tf).not_to be_nil + expect(tf['deadline_name']).to eq('team_formation') + end + + it 'includes deadline_name = signup for type id 8' do + assignment = Assignment.create!(name: 'Signup', instructor: instructor) + AssignmentDueDate.create!(parent: assignment, + due_at: 1.week.from_now, + deadline_type_id: ExpertizaConstants::DeadlineTypes::SIGNUP, + submission_allowed_id: 3, review_allowed_id: 3) + json = serialized(assignment) + su = json['due_dates'].find { |d| d['deadline_type_id'] == ExpertizaConstants::DeadlineTypes::SIGNUP } + expect(su).not_to be_nil + expect(su['deadline_name']).to eq('signup') + end + end +end