diff --git a/app/controllers/revision_requests_controller.rb b/app/controllers/revision_requests_controller.rb new file mode 100644 index 000000000..a60119f90 --- /dev/null +++ b/app/controllers/revision_requests_controller.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +class RevisionRequestsController < ApplicationController + prepend_before_action :set_assignment, only: :index + prepend_before_action :set_revision_request, only: %i[show update] + + def action_allowed? + case params[:action] + when 'index' + current_user_has_instructor_privileges? && current_user_instructs_assignment?(@assignment) + when 'show' + return false unless @revision_request + + owns_revision_request? || current_user_instructs_assignment?(@revision_request.assignment) + when 'update' + return false unless @revision_request + + current_user_has_instructor_privileges? && current_user_instructs_assignment?(@revision_request.assignment) + else + false + end + end + + def index + return if invalid_status_filter? + + revision_requests = RevisionRequest.where(assignment_id: @assignment.id) + revision_requests = revision_requests.where(status: params[:status]) if params[:status].present? + + render json: revision_requests.order(created_at: :desc).map(&:as_json), status: :ok + end + + def show + return unless @revision_request + + render json: @revision_request.as_json, status: :ok + end + + def update + return render json: { error: 'This revision request has already been processed' }, status: :unprocessable_entity unless @revision_request.status == RevisionRequest::PENDING + return if invalid_update_status? + + if @revision_request.update(update_params) + render json: @revision_request.as_json, status: :ok + else + render json: { error: @revision_request.errors.full_messages.to_sentence }, status: :unprocessable_entity + end + end + + private + + def set_assignment + @assignment = Assignment.find_by(id: params[:assignment_id]) + return if @assignment + + render json: { error: 'Assignment not found' }, status: :not_found + end + + def set_revision_request + @revision_request = RevisionRequest.find_by(id: params[:id]) + return if @revision_request + + render json: { error: 'Revision request not found' }, status: :not_found + end + + def owns_revision_request? + @revision_request.participant.user_id == current_user.id + end + + def invalid_status_filter? + return false if params[:status].blank? || RevisionRequest::STATUSES.include?(params[:status]) + + render json: { error: 'Status must be PENDING, APPROVED, or DECLINED' }, status: :unprocessable_entity + true + end + + def invalid_update_status? + return false if valid_resolved_status? + + render json: { error: 'Status must be APPROVED or DECLINED' }, status: :unprocessable_entity + true + end + + def valid_resolved_status? + [RevisionRequest::APPROVED, RevisionRequest::DECLINED].include?(update_params[:status]) + end + + def update_params + params.require(:revision_request).permit(:status, :response_comment) + end +end diff --git a/app/controllers/student_tasks_controller.rb b/app/controllers/student_tasks_controller.rb index ffb6097a5..66ff18990 100644 --- a/app/controllers/student_tasks_controller.rb +++ b/app/controllers/student_tasks_controller.rb @@ -1,28 +1,52 @@ class StudentTasksController < ApplicationController + before_action :set_student_task, only: %i[show view request_revision] - # List retrieves all student tasks associated with the current logged-in user. def action_allowed? current_user_has_student_privileges? end + + def index + list + end + def list - # Retrieves all tasks that belong to the current user. - @student_tasks = StudentTask.from_user(current_user) - # Render the list of student tasks as JSON. - render json: @student_tasks, status: :ok + render json: StudentTask.from_user(current_user), status: :ok end def show render json: @student_task, status: :ok end - # The view function retrieves a student task based on a participant's ID. - # It is meant to provide an endpoint where tasks can be queried based on participant ID. def view - # Retrieves the student task where the participant's ID matches the provided parameter. - # This function will be used for clicking on a specific student task to "view" its details. - @student_task = StudentTask.from_participant_id(params[:id]) - # Render the found student task as JSON. - render json: @student_task, status: :ok + show end + def request_revision + return render json: { error: 'Revision requests require a team submission' }, status: :unprocessable_entity unless @student_task.team + return render json: { error: 'Revision requests are not available for this task' }, status: :unprocessable_entity unless @student_task.can_request_revision + + revision_request = RevisionRequest.new( + participant: @participant, + team: @student_task.team, + assignment: @participant.assignment, + comments: params[:comments] + ) + + if revision_request.save + @student_task = StudentTask.from_participant(@participant) + render json: { message: 'Revision request submitted successfully', revision_request: revision_request.as_json, student_task: @student_task.as_json }, status: :created + else + render json: { error: revision_request.errors.full_messages.to_sentence }, status: :unprocessable_entity + end + end + + private + + def set_student_task + @participant = AssignmentParticipant.find_by(id: params[:id]) + return render json: { error: 'Student task not found' }, status: :not_found unless @participant + return render json: { error: 'You are not authorized to access this student task' }, status: :forbidden unless @participant.user_id == current_user.id + + @student_task = StudentTask.from_participant(@participant) + end end diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 130fa6837..c3752a5b8 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -14,6 +14,7 @@ class Assignment < ApplicationRecord has_many :due_dates,as: :parent, class_name: 'DueDate', dependent: :destroy has_many :assignments_duties, dependent: :destroy has_many :duties, through: :assignments_duties + has_many :revision_requests, dependent: :destroy belongs_to :course, optional: true belongs_to :instructor, class_name: 'User', inverse_of: :assignments diff --git a/app/models/assignment_participant.rb b/app/models/assignment_participant.rb index f3f1f38b2..26db5836d 100644 --- a/app/models/assignment_participant.rb +++ b/app/models/assignment_participant.rb @@ -6,6 +6,7 @@ class AssignmentParticipant < Participant has_many :review_mappings, class_name: 'ReviewResponseMap', foreign_key: 'reviewee_id' has_many :response_maps, foreign_key: 'reviewee_id' has_many :sent_invitations, class_name: 'Invitation', foreign_key: 'from_id' + has_many :revision_requests, foreign_key: 'participant_id', dependent: :destroy belongs_to :duty, optional: true belongs_to :user validates :handle, presence: true diff --git a/app/models/assignment_team.rb b/app/models/assignment_team.rb index 2bf8e6815..10c1e91b1 100644 --- a/app/models/assignment_team.rb +++ b/app/models/assignment_team.rb @@ -8,6 +8,7 @@ class AssignmentTeam < Team has_many :review_mappings, class_name: 'ReviewResponseMap', foreign_key: 'reviewee_id' has_many :review_response_maps, foreign_key: 'reviewee_id' has_many :responses, through: :review_response_maps, foreign_key: 'map_id' + has_many :revision_requests, foreign_key: 'team_id', dependent: :destroy # Delegation to avoid Law of Demeter violations delegate :path, to: :assignment, prefix: true @@ -224,4 +225,4 @@ def validate_assignment_team_type end end -class TeamFullError < StandardError; end \ No newline at end of file +class TeamFullError < StandardError; end diff --git a/app/models/revision_request.rb b/app/models/revision_request.rb new file mode 100644 index 000000000..05e260af8 --- /dev/null +++ b/app/models/revision_request.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +class RevisionRequest < ApplicationRecord + PENDING = 'PENDING' + APPROVED = 'APPROVED' + DECLINED = 'DECLINED' + STATUSES = [PENDING, APPROVED, DECLINED].freeze + + belongs_to :participant, class_name: 'AssignmentParticipant' + belongs_to :team, class_name: 'AssignmentTeam' + belongs_to :assignment + + validates :comments, presence: true + validates :status, inclusion: { in: STATUSES } + validate :one_pending_request_per_participant_team, on: :create + + scope :pending, -> { where(status: PENDING) } + + def as_json(_options = {}) + { + id: id, + participant_id: participant_id, + team_id: team_id, + assignment_id: assignment_id, + status: status, + comments: comments, + response_comment: response_comment, + created_at: created_at&.iso8601, + updated_at: updated_at&.iso8601 + } + end + + private + + def one_pending_request_per_participant_team + return unless self.class.pending.exists?(participant_id: participant_id, team_id: team_id) + + errors.add(:base, 'A pending revision request already exists for this task') + end +end diff --git a/app/models/student_task.rb b/app/models/student_task.rb index c5bb9332b..0ae33a559 100644 --- a/app/models/student_task.rb +++ b/app/models/student_task.rb @@ -1,50 +1,322 @@ # frozen_string_literal: true class StudentTask - attr_accessor :assignment, :current_stage, :participant, :stage_deadline, :topic, :permission_granted + attr_reader :participant, :assignment, :course, :team, :project_topic, :topic, :current_stage, + :stage_deadline, :permission_granted, :deadlines, :review_grade, :team_members, + :timeline, :feedback, :submission_feedback, :can_request_revision, :revision_request - # Initializes a new instance of the StudentTask class - def initialize(args) - @assignment = args[:assignment] - @current_stage = args[:current_stage] - @participant = args[:participant] - @stage_deadline = args[:stage_deadline] - @topic = args[:topic] - @permission_granted = args[:permission_granted] + def initialize(participant:, assignment:, course:, team:, project_topic:, topic:, current_stage:, + stage_deadline:, permission_granted:, deadlines:, review_grade:, team_members:, + timeline:, feedback:, submission_feedback:, can_request_revision:, revision_request:) + @participant = participant + @assignment = assignment + @course = course + @team = team + @project_topic = project_topic + @topic = topic + @current_stage = current_stage + @stage_deadline = stage_deadline + @permission_granted = permission_granted + @deadlines = deadlines + @review_grade = review_grade + @team_members = team_members + @timeline = timeline + @feedback = feedback + @submission_feedback = submission_feedback + @can_request_revision = can_request_revision + @revision_request = revision_request + end + + def self.from_user(user) + AssignmentParticipant.includes(:user, assignment: :course) + .where(user_id: user.id) + .map { |participant| from_participant(participant) } + .sort_by { |task| [task.stage_deadline.nil? ? 1 : 0, task.stage_deadline || Time.zone.at(0), task.assignment_name] } + end + + def self.from_participant(participant) + assignment = participant.assignment + team = resolve_team(participant) + project_topic = resolve_project_topic(participant, team) + deadlines = resolve_deadlines(assignment, project_topic) + latest_revision_request = resolve_latest_revision_request(participant, team) + + new( + participant: participant, + assignment: assignment, + course: assignment&.course, + team: team, + project_topic: project_topic, + topic: resolve_topic(participant, project_topic), + current_stage: participant.current_stage.presence || 'Unknown', + stage_deadline: resolve_stage_deadline(participant, deadlines), + permission_granted: participant.permission_granted, + deadlines: deadlines, + review_grade: resolve_review_grade(team), + team_members: resolve_team_members(team), + timeline: build_timeline(deadlines, participant.current_stage), + feedback: resolve_feedback(team, assignment), + submission_feedback: resolve_submission_feedback(team), + can_request_revision: can_request_revision?(team, deadlines, latest_revision_request), + revision_request: latest_revision_request + ) + end + + def self.from_participant_id(id) + participant = AssignmentParticipant.find_by(id: id) + return nil unless participant + + from_participant(participant) + end + + def assignment_name + assignment&.name.to_s + end + + def team_name + team&.name.presence || (team && "Team #{team.id}") + end + + def as_json(_options = {}) + { + id: participant.id, + participant_id: participant.id, + assignment_id: assignment&.id, + assignment: assignment_name, + course_id: course&.id, + course: course&.name, + team_id: team&.id, + team_name: team_name, + team_members: team_members, + topic: topic, + topic_details: serialize_topic, + current_stage: current_stage, + stage_deadline: stage_deadline&.iso8601, + permission_granted: permission_granted, + deadlines: deadlines.map { |deadline| serialize_deadline(deadline) }, + timeline: timeline, + feedback: feedback, + submission_feedback: submission_feedback, + can_request_revision: can_request_revision, + revision_request: revision_request&.as_json, + assignment_details: serialize_assignment, + team_details: serialize_team, + review_grade: review_grade + } + end + + class << self + private + + def resolve_team(participant) + participant.team || AssignmentTeam.team(participant) end - # create a new StudentTask instance from a Participant object.cccccccc - def self.create_from_participant(participant) - new( - assignment: participant.assignment.name, # Name of the assignment associated with the student task - topic: participant.topic, # Current stage of the assignment process - current_stage: participant.current_stage, # Participant object - stage_deadline: parse_stage_deadline(participant.stage_deadline), # Deadline for the current stage of the assignment - permission_granted: participant.permission_granted, # Topic of the assignment - participant: participant # Boolean indicating if Publishing Rights is enabled - ) + def resolve_project_topic(participant, team) + return nil unless team + + SignedUpTeam.includes(:project_topic) + .find_by(team_id: team.id, project_topics: { assignment_id: participant.assignment_id }) + &.project_topic + rescue ActiveRecord::StatementInvalid + SignedUpTeam.includes(:project_topic) + .joins(:project_topic) + .find_by(team_id: team.id, project_topics: { assignment_id: participant.assignment_id }) + &.project_topic end + def resolve_deadlines(assignment, project_topic) + return [] unless assignment - # create an array of StudentTask instances for all participants linked to a user, sorted by deadline. - def self.from_user(user) - Participant.where(user_id: user.id) - .map { |participant| StudentTask.create_from_participant(participant) } - .sort_by(&:stage_deadline) + relation = DueDate.where(parent_type: 'Assignment', parent_id: assignment.id) + if project_topic + relation = relation.or(DueDate.where(parent_type: 'ProjectTopic', parent_id: project_topic.id)) + end + + relation.sort_by(&:due_at) end - # create a StudentTask instance from a participant of the provided id - def self.from_participant_id(id) - create_from_participant(Participant.find_by(id: id)) + def resolve_topic(participant, project_topic) + participant.topic.presence || + project_topic&.topic_identifier.presence || + project_topic&.topic_name end - - private - # Parses a date string to a Time object, if parsing fails, set the time to be one year after current - def self.parse_stage_deadline(date_string) - Time.parse(date_string) + def resolve_stage_deadline(participant, deadlines) + parse_deadline(participant.stage_deadline) || + deadlines.find { |deadline| deadline.due_at&.future? }&.due_at || + deadlines.last&.due_at + end + + def parse_deadline(value) + return value.in_time_zone if value.respond_to?(:in_time_zone) + return nil if value.blank? + + Time.zone.parse(value.to_s) + rescue ArgumentError, TypeError + nil + end + + def resolve_review_grade(team) + return nil unless team.respond_to?(:aggregate_review_grade) + + team.aggregate_review_grade rescue StandardError - Time.now + 1.year + nil + end + + def resolve_team_members(team) + return [] unless team + + members = (team.users.to_a + team.participants.includes(:user).map(&:user)).compact.uniq(&:id) + members.sort_by { |user| user.full_name.to_s }.map do |user| + { + id: user.id, + name: user.name, + full_name: user.full_name + } + end + end + + def build_timeline(deadlines, current_stage) + deadlines.map do |deadline| + { + id: deadline.id, + label: deadline.deadline_name.presence || fallback_timeline_label(deadline), + phase: timeline_phase(deadline), + due_at: deadline.due_at&.iso8601, + status: timeline_status(deadline, current_stage) + } + end end - + + def resolve_feedback(team, assignment) + return [] unless team + + Response.joins(:response_map) + .includes(response_map: { reviewer: :user }) + .where(response_maps: { reviewee_id: team.id, reviewed_object_id: assignment&.id }) + .where(is_submitted: true) + .order(updated_at: :desc) + .map do |response| + { + response_id: response.id, + reviewer_name: response.response_map.reviewer&.fullname, + comment: response.additional_comment, + submitted_at: response.updated_at&.iso8601 + } + end + end + + def resolve_submission_feedback(team) + return nil unless team + return nil if team.grade_for_submission.nil? && team.comment_for_submission.blank? + + { + grade_for_submission: team.grade_for_submission, + comment_for_submission: team.comment_for_submission + } + end + + def resolve_latest_revision_request(participant, team) + return nil unless participant && team + + RevisionRequest.where(participant_id: participant.id, team_id: team.id).order(created_at: :desc).first + end + + def can_request_revision?(team, deadlines, latest_revision_request) + return false unless team + return false if [RevisionRequest::PENDING, RevisionRequest::APPROVED].include?(latest_revision_request&.status) + + deadlines.any? do |deadline| + deadline.respond_to?(:resubmission_allowed_id) && + deadline.resubmission_allowed_id.present? && + deadline.resubmission_allowed_id != DueDate::NOT_ALLOWED && + (deadline.due_at.nil? || deadline.due_at.future?) + end + end + + def timeline_phase(deadline) + name = deadline.deadline_name.to_s.downcase + return 'feedback' if name.include?('feedback') + return 'review' if name.include?('review') + + 'submission' + end + + def fallback_timeline_label(deadline) + return "Round #{deadline.round} deadline" if deadline.round.present? + + "Deadline #{deadline.deadline_type_id}" + end + + def timeline_status(deadline, current_stage) + return 'current' if current_stage_matches_phase?(current_stage, timeline_phase(deadline)) + + deadline.due_at&.past? ? 'completed' : 'upcoming' + end + + def current_stage_matches_phase?(current_stage, phase) + stage = current_stage.to_s.downcase + return false if stage.blank? + + case phase + when 'submission' + stage.include?('progress') || stage.include?('start') || stage.include?('submit') + when 'review' + stage.include?('review') + when 'feedback' + stage.include?('feedback') || stage.include?('finish') + else + false + end + end + end + + private + + def serialize_assignment + { + id: assignment&.id, + name: assignment_name, + course_id: course&.id, + course_name: course&.name + } + end + + def serialize_team + { + id: team&.id, + name: team_name, + members: team_members + } + end + + def serialize_topic + return nil unless project_topic || topic.present? + + { + id: project_topic&.id, + identifier: project_topic&.topic_identifier || topic, + name: project_topic&.topic_name || topic + } + end + + def serialize_deadline(deadline) + { + id: deadline.id, + name: deadline.deadline_name.presence || fallback_deadline_name(deadline), + due_at: deadline.due_at&.iso8601, + deadline_type_id: deadline.deadline_type_id, + round: deadline.round, + parent_type: deadline.parent_type, + parent_id: deadline.parent_id + } + end + + def fallback_deadline_name(deadline) + return "Round #{deadline.round} deadline" if deadline.round.present? + + "Deadline #{deadline.deadline_type_id}" + end end diff --git a/backend_implementation_report.txt b/backend_implementation_report.txt new file mode 100644 index 000000000..3d4474301 --- /dev/null +++ b/backend_implementation_report.txt @@ -0,0 +1,155 @@ +Backend implementation report for the Student Task View work described in instructions.txt + +Scope +This report covers only the backend work in this repository. The frontend items from instructions.txt were not part of this repo-level implementation. + +What was implemented + +1. Student task list and detail API hardening +- Implemented and stabilized the student task endpoints in app/controllers/student_tasks_controller.rb. +- Supported endpoints: + - GET /student_tasks + - GET /student_tasks/list + - GET /student_tasks/:id + - GET /student_tasks/view?id=:id +- Added ownership checks so a student can only access their own AssignmentParticipant-backed task data. +- Added clean not-found handling for invalid task IDs. +- Preserved the legacy view endpoint for compatibility. + +2. Composed student task payload +- Implemented task composition in app/models/student_task.rb. +- Built the task response from existing Expertiza relationships instead of a standalone tasks table. +- The task payload includes: + - participant and assignment identifiers + - assignment name and course information + - team id, team name, and team members + - selected topic / project topic data + - current stage and stage deadline + - assignment and topic deadlines + - timeline/stage data derived from deadlines + - review feedback data + - submission feedback from the assignment team + - review grade when available + - revision request state + - can_request_revision flag for frontend/backend action gating + +3. Revision request persistence and associations +- Added a new RevisionRequest model in app/models/revision_request.rb. +- Added the revision_requests table in db/migrate/20260322174500_create_revision_requests.rb. +- Added associations so revision requests connect to: + - AssignmentParticipant + - AssignmentTeam + - Assignment +- Added validation rules: + - comments are required + - status must be one of PENDING, APPROVED, DECLINED + - only one pending revision request can exist per participant/team at a time + +4. Student revision request submission flow +- Implemented POST /student_tasks/:id/request_revision in app/controllers/student_tasks_controller.rb. +- Students can submit a revision request only for their own task. +- The action rejects: + - invalid task IDs + - access to another student’s task + - tasks without a team submission context + - tasks where revision requests are not currently allowed +- On success, the endpoint returns the created revision request and the refreshed student task payload. + +5. Instructor revision request review flow +- Implemented revision request review endpoints in app/controllers/revision_requests_controller.rb. +- Supported endpoints: + - GET /revision_requests?assignment_id=:assignment_id + - GET /revision_requests/:id + - PATCH /revision_requests/:id + - PUT /revision_requests/:id +- Authorization behavior: + - assignment instructors can list revision requests for their assignment + - the owning student can view their own revision request + - the assignment instructor can also view that revision request + - only the assignment instructor can approve or decline a pending request +- Added validation/error handling for: + - invalid assignment IDs + - invalid revision request IDs + - invalid status filters + - invalid resolution statuses + - attempts to re-process an already resolved revision request + +6. Route cleanup +- Updated config/routes.rb so student_tasks only exposes the routes this backend actually supports. +- Added explicit routes for request_revision and revision_requests review endpoints. + +What was tested + +1. Request specs for student task endpoints +- File: spec/requests/api/v1/student_tasks_controller_spec.rb +- Covered: + - successful task list retrieval + - successful task detail retrieval + - empty task list scenario + - unauthorized access without a token + - forbidden access to another student’s task + - invalid student task ID + - legacy /student_tasks/view endpoint + - successful student revision request creation + - duplicate pending revision request rejection + - revision request rejection when resubmission is not allowed + +2. Request specs for revision request review endpoints +- File: spec/requests/api/v1/revision_requests_controller_spec.rb +- Covered: + - instructor retrieval of assignment revision requests + - revision request filtering by status + - invalid assignment ID + - invalid status filter + - forbidden access for the wrong instructor + - forbidden access for students on instructor-only listing endpoints + - owning student access to a single revision request + - instructor access to a single revision request + - forbidden access to another student’s revision request + - invalid revision request ID + - successful instructor approval + - successful instructor decline + - invalid resolution status + - rejection of updates to already processed requests + - unauthorized access without a token + +3. Model specs +- File: spec/models/student_task_spec.rb +- Covered: + - composed task building from an assignment participant + - JSON serialization of the student task payload + - participant lookup by id + - deadline parsing + - can_request_revision rules + +- File: spec/models/revision_request_spec.rb +- Covered: + - comments validation + - duplicate pending request prevention + - allowing a new request after a prior request is no longer pending + +4. Routing specs +- File: spec/routing/student_tasks_routing_spec.rb +- Covered: + - student_tasks index + - show + - list + - view + - request_revision + +- File: spec/routing/revision_requests_routing_spec.rb +- Covered: + - revision_requests index + - show + - update via PATCH + - update via PUT + +Verification command used +docker compose run --rm app bundle exec rspec spec/requests/api/v1/student_tasks_controller_spec.rb spec/requests/api/v1/revision_requests_controller_spec.rb spec/models/student_task_spec.rb spec/models/revision_request_spec.rb spec/routing/student_tasks_routing_spec.rb spec/routing/revision_requests_routing_spec.rb + +Verification result +- 56 examples, 0 failures + +Notes +- This report describes the backend work only. +- The original instructions.txt file was preserved separately from the backend-only work notes. diff --git a/backend_instructions.txt b/backend_instructions.txt new file mode 100644 index 000000000..3e695f7d9 --- /dev/null +++ b/backend_instructions.txt @@ -0,0 +1,83 @@ +E2602. Reimplement Student Task View +Mentor: Koushik Gudipelly + +Backend scope only. +This repository does not contain the frontend implementation, so the work here is limited to the Rails API, authorization, response composition, and automated backend tests. + +Objective +Stabilize and complete the backend for the Student Task View so a logged-in student can retrieve a reliable task list and task detail payload for assignment-related work. + +Required backend outcomes +1. Expose a task list endpoint for the current logged-in student. +2. Expose a task detail endpoint for a single student task. +3. Build student task data from existing Expertiza relationships instead of from a standalone table. +4. Enforce authentication and ownership so students can only access their own tasks. +5. Add backend tests that cover the success and failure paths and prevent regressions. + +Student task endpoint contract +1. `GET /student_tasks/list` +Returns all assignment tasks for the current student. +The response should be sorted consistently and should include enough composed data for a task table and summary cards. + +2. `GET /student_tasks/:id` +Returns the detailed payload for one assignment participant record owned by the current student. + +3. `GET /student_tasks/view?id=:id` +Legacy alias for the detail endpoint. +This should return the same payload as `GET /student_tasks/:id`. + +Required task payload data +1. Participant identity: +`participant_id` + +2. Assignment context: +`assignment_id`, `assignment`, `assignment_details` + +3. Course context: +`course_id`, `course` + +4. Team context: +`team_id`, `team_name`, `team_members`, `team_details` + +5. Topic context: +`topic`, `topic_details` + +6. Status and timing: +`current_stage`, `stage_deadline`, `deadlines`, `timeline` + +7. Permissions and review state: +`permission_granted`, `review_grade`, `can_request_revision` + +8. Feedback: +`feedback`, `submission_feedback` + +Data composition rules +1. Tasks should be derived from `AssignmentParticipant` records for the current user. +2. Assignment, course, team, topic, deadlines, and feedback should be assembled from existing associations and related models. +3. Missing optional data should degrade gracefully instead of raising server errors. +4. Incomplete review or questionnaire data must not crash the endpoint; missing grades should resolve to `nil`. + +Authorization rules +1. Requests without a valid JWT must return `401`. +2. A student may only retrieve tasks that belong to their own `AssignmentParticipant` records. +3. Accessing another student’s task must return `403`. +4. Accessing a non-existent task ID must return `404`. + +Testing requirements +1. Request specs must cover: +successful task list retrieval +successful task detail retrieval +legacy detail endpoint retrieval +empty task list +unauthorized access +forbidden access to another student’s task +invalid task ID + +2. Model specs must cover: +student task payload construction +serialization shape +participant lookup behavior +deadline parsing behavior + +Implementation note +This task is a hardening and completion pass, not a greenfield feature. Prefer fixing endpoint behavior, response consistency, and test coverage over adding speculative new API surface area. diff --git a/config/routes.rb b/config/routes.rb index 57559d007..4e07959f0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -45,13 +45,17 @@ get 'bookmarkratings', to: 'bookmarks#get_bookmark_rating_score' post 'bookmarkratings', to: 'bookmarks#save_bookmark_rating_score' end - end - resources :student_tasks do - collection do - get :list, action: :list - get :view - end - end + end + resources :student_tasks, only: %i[index show] do + member do + post :request_revision + end + collection do + get :list, action: :list + get :view + end + end + resources :revision_requests, only: %i[index show update] resources :courses do collection do diff --git a/db/migrate/20260322174500_create_revision_requests.rb b/db/migrate/20260322174500_create_revision_requests.rb new file mode 100644 index 000000000..7a8bbc1c4 --- /dev/null +++ b/db/migrate/20260322174500_create_revision_requests.rb @@ -0,0 +1,16 @@ +class CreateRevisionRequests < ActiveRecord::Migration[8.0] + def change + create_table :revision_requests do |t| + t.references :participant, null: false, foreign_key: true + t.references :team, null: false, foreign_key: true + t.references :assignment, null: false, foreign_key: true + t.string :status, null: false, default: 'PENDING' + t.text :comments, null: false + t.text :response_comment + + t.timestamps + end + + add_index :revision_requests, %i[participant_id team_id status], name: 'index_revision_requests_on_participant_team_status' + end +end diff --git a/db/schema.rb b/db/schema.rb index cddbe12c6..71dbc4dbb 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_03_13_064334) do +ActiveRecord::Schema[8.0].define(version: 2026_03_22_174500) do create_table "account_requests", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "username" t.string "full_name" @@ -345,6 +345,21 @@ t.index ["map_id"], name: "fk_response_response_map" end + create_table "revision_requests", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| + t.bigint "participant_id", null: false + t.bigint "team_id", null: false + t.bigint "assignment_id", null: false + t.string "status", default: "PENDING", null: false + t.text "comments", null: false + t.text "response_comment" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["assignment_id"], name: "index_revision_requests_on_assignment_id" + t.index ["participant_id", "team_id", "status"], name: "index_revision_requests_on_participant_team_status" + t.index ["participant_id"], name: "index_revision_requests_on_participant_id" + t.index ["team_id"], name: "index_revision_requests_on_team_id" + end + create_table "roles", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "name" t.bigint "parent_id" @@ -456,9 +471,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" @@ -466,6 +481,9 @@ add_foreign_key "participants", "users" add_foreign_key "project_topics", "assignments" add_foreign_key "question_advices", "items", column: "question_id" + add_foreign_key "revision_requests", "assignments" + add_foreign_key "revision_requests", "participants" + add_foreign_key "revision_requests", "teams" add_foreign_key "roles", "roles", column: "parent_id", on_delete: :cascade add_foreign_key "signed_up_teams", "project_topics" add_foreign_key "signed_up_teams", "teams" diff --git a/spec/models/revision_request_spec.rb b/spec/models/revision_request_spec.rb new file mode 100644 index 000000000..9e2f18388 --- /dev/null +++ b/spec/models/revision_request_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe RevisionRequest, type: :model do + include RolesHelper + + let!(:roles) { create_roles_hierarchy } + let!(:institution) { Institution.create!(name: 'NC State') } + let!(:instructor) do + User.create!( + name: 'instructor1', + email: 'instructor1@example.com', + password: 'password', + full_name: 'Instructor One', + institution: institution, + role: roles[:instructor] + ) + end + let!(:student) do + User.create!( + name: 'student1', + email: 'student1@example.com', + password: 'password', + full_name: 'Student One', + institution: institution, + role: roles[:student] + ) + end + let!(:course) do + Course.create!( + name: 'CSC 517', + directory_path: 'csc517', + institution: institution, + instructor: instructor + ) + end + let!(:assignment) do + Assignment.create!( + name: 'Assignment One', + instructor: instructor, + course: course, + directory_path: 'assignment_one' + ) + end + let!(:participant) do + AssignmentParticipant.create!( + user: student, + assignment: assignment, + handle: student.name + ) + end + let!(:team) { AssignmentTeam.create!(name: 'Team Alpha', parent_id: assignment.id) } + let!(:membership) { TeamsParticipant.create!(team: team, participant: participant, user: student) } + + describe 'validations' do + it 'requires comments' do + revision_request = described_class.new( + participant: participant, + team: team, + assignment: assignment, + comments: '' + ) + + expect(revision_request).not_to be_valid + expect(revision_request.errors[:comments]).to include("can't be blank") + end + + it 'allows only one pending request per participant and team' do + described_class.create!( + participant: participant, + team: team, + assignment: assignment, + comments: 'First request' + ) + + duplicate = described_class.new( + participant: participant, + team: team, + assignment: assignment, + comments: 'Second request' + ) + + expect(duplicate).not_to be_valid + expect(duplicate.errors[:base]).to include('A pending revision request already exists for this task') + end + + it 'allows a new request once the previous request is no longer pending' do + described_class.create!( + participant: participant, + team: team, + assignment: assignment, + comments: 'First request', + status: described_class::DECLINED + ) + + next_request = described_class.new( + participant: participant, + team: team, + assignment: assignment, + comments: 'Second request' + ) + + expect(next_request).to be_valid + end + end +end diff --git a/spec/models/student_task_spec.rb b/spec/models/student_task_spec.rb index 94b9e7ffc..2d71f566c 100644 --- a/spec/models/student_task_spec.rb +++ b/spec/models/student_task_spec.rb @@ -2,84 +2,184 @@ require 'rails_helper' - RSpec.describe StudentTask, type: :model do - before(:each) do - @assignment = double(name: "Final Project") - @participant = double( - assignment: @assignment, - topic: "E2442", - current_stage: "finished", - stage_deadline: "2024-04-23", - permission_granted: true - ) + describe '.from_participant' do + let(:course) { instance_double(Course, id: 3, name: 'CSC 517') } + let(:assignment) { instance_double(Assignment, id: 5, name: 'Final Project', course: course) } + let(:team) do + instance_double( + AssignmentTeam, + id: 7, + name: 'Team Alpha', + aggregate_review_grade: nil, + grade_for_submission: 92, + comment_for_submission: 'Please revise the intro.' + ) + end + let(:project_topic) { instance_double(ProjectTopic, id: 11, topic_identifier: 'E2442', topic_name: 'Topic E2442') } + let(:participant_user) { instance_double(User, id: 13, name: 'studenta', full_name: 'Student A') } + let(:participant) do + instance_double( + AssignmentParticipant, + id: 17, + assignment: assignment, + assignment_id: assignment.id, + topic: nil, + current_stage: 'Review', + stage_deadline: '2026-04-23 12:00:00 UTC', + permission_granted: true + ) + end + let(:deadline) do + instance_double( + DueDate, + id: 19, + deadline_name: 'Submission deadline', + due_at: Time.zone.parse('2026-04-25 12:00:00 UTC'), + deadline_type_id: 1, + resubmission_allowed_id: 3, + round: nil, + parent_type: 'Assignment', + parent_id: assignment.id + ) + end + let(:feedback) do + [{ + response_id: 23, + reviewer_name: 'Reviewer One', + comment: 'Looks good.', + submitted_at: '2026-04-22T10:00:00Z' + }] + end + let(:revision_request) do + instance_double( + RevisionRequest, + status: 'PENDING', + as_json: { id: 29, status: 'PENDING', comments: 'Please revise the intro.' } + ) + end + let(:team_members) do + [{ id: 13, name: 'studenta', full_name: 'Student A' }] + end - end + before do + allow(StudentTask).to receive(:resolve_team).with(participant).and_return(team) + allow(StudentTask).to receive(:resolve_project_topic).with(participant, team).and_return(project_topic) + allow(StudentTask).to receive(:resolve_deadlines).with(assignment, project_topic).and_return([deadline]) + allow(StudentTask).to receive(:resolve_latest_revision_request).with(participant, team).and_return(revision_request) + allow(StudentTask).to receive(:resolve_review_grade).with(team).and_return(nil) + allow(StudentTask).to receive(:resolve_team_members).with(team).and_return(team_members) + allow(StudentTask).to receive(:build_timeline).with([deadline], 'Review').and_return([{ label: 'Submission deadline', phase: 'submission' }]) + allow(StudentTask).to receive(:resolve_feedback).with(team, assignment).and_return(feedback) + end - describe ".initialize" do - it "correctly assigns all attributes" do - args = { - assignment: @assignment, - current_stage: "finished", - participant: @participant, - stage_deadline: "2024-04-23", - topic: "E2442", - permission_granted: false - } - - student_task = StudentTask.new(args) - - expect(student_task.assignment.name).to eq("Final Project") - expect(student_task.current_stage).to eq("finished") - expect(student_task.participant).to eq(@participant) - expect(student_task.stage_deadline).to eq("2024-04-23") - expect(student_task.topic).to eq("E2442") - expect(student_task.permission_granted).to be false + it 'builds a composed student task payload from an assignment participant' do + task = described_class.from_participant(participant) + + expect(task.assignment).to eq(assignment) + expect(task.course).to eq(course) + expect(task.team).to eq(team) + expect(task.project_topic).to eq(project_topic) + expect(task.topic).to eq('E2442') + expect(task.current_stage).to eq('Review') + expect(task.stage_deadline).to eq(Time.zone.parse('2026-04-23 12:00:00 UTC')) + expect(task.permission_granted).to be(true) + expect(task.deadlines).to eq([deadline]) + expect(task.timeline).to eq([{ label: 'Submission deadline', phase: 'submission' }]) + expect(task.feedback).to eq(feedback) + expect(task.submission_feedback).to eq( + grade_for_submission: 92, + comment_for_submission: 'Please revise the intro.' + ) + expect(task.can_request_revision).to be(false) + expect(task.revision_request).to eq(revision_request) + end + + it 'serializes the composed payload into frontend-friendly JSON' do + task = described_class.from_participant(participant) + json = task.as_json + + expect(json).to include( + participant_id: 17, + assignment_id: 5, + assignment: 'Final Project', + course_id: 3, + course: 'CSC 517', + team_id: 7, + team_name: 'Team Alpha', + topic: 'E2442', + current_stage: 'Review', + permission_granted: true, + can_request_revision: false, + review_grade: nil + ) + expect(json[:topic_details]).to include(id: 11, identifier: 'E2442', name: 'Topic E2442') + expect(json[:assignment_details]).to include(id: 5, name: 'Final Project', course_id: 3, course_name: 'CSC 517') + expect(json[:team_details]).to include(id: 7, name: 'Team Alpha') + expect(json[:feedback]).to eq(feedback) + expect(json[:submission_feedback]).to eq( + grade_for_submission: 92, + comment_for_submission: 'Please revise the intro.' + ) + expect(json[:revision_request]).to eq(id: 29, status: 'PENDING', comments: 'Please revise the intro.') end end - describe ".from_participant" do - it "creates an instance from a participant instance" do + describe '.from_participant_id' do + it 'returns nil when the participant cannot be found' do + allow(AssignmentParticipant).to receive(:find_by).with(id: 999).and_return(nil) - student_task = StudentTask.create_from_participant(@participant) + expect(described_class.from_participant_id(999)).to be_nil + end - expect(student_task.assignment).to eq(@participant.assignment.name) - expect(student_task.topic).to eq(@participant.topic) - expect(student_task.current_stage).to eq(@participant.current_stage) - expect(student_task.stage_deadline).to eq(Time.parse(@participant.stage_deadline)) - expect(student_task.permission_granted).to be @participant.permission_granted - expect(student_task.participant).to be @participant + it 'looks up assignment participants and delegates task construction' do + participant = instance_double(AssignmentParticipant) + + allow(AssignmentParticipant).to receive(:find_by).with(id: 1).and_return(participant) + allow(described_class).to receive(:from_participant).with(participant).and_return(:task) + + expect(described_class.from_participant_id(1)).to eq(:task) end end - describe ".parse_stage_deadline" do - context "valid date string" do - it "parses the date string into a Time object" do - valid_date = "2024-04-25" - expect(StudentTask.send(:parse_stage_deadline, valid_date)).to eq(Time.parse("2024-04-25")) - end + describe '.parse_deadline' do + it 'parses valid deadline strings' do + expect(described_class.send(:parse_deadline, '2026-04-25 12:00:00 UTC')).to eq(Time.zone.parse('2026-04-25 12:00:00 UTC')) end - context "invalid date string" do - it "returns current time plus one year" do - invalid_date = "invalid input" - # Set the now to be 2024-05-01 for testing purpose - allow(Time).to receive(:now).and_return(Time.new(2024, 5, 1)) - expected_time = Time.new(2025, 5, 1) - expect(StudentTask.send(:parse_stage_deadline, invalid_date)).to eq(expected_time) - end + it 'returns nil for invalid deadline input' do + expect(described_class.send(:parse_deadline, 'not-a-date')).to be_nil end end - describe ".from_participant_id" do - it "fetches a participant by id and creates a student task from it" do - allow(Participant).to receive(:find_by).with(id: 1).and_return(@participant) + describe '.can_request_revision?' do + it 'returns true when resubmission is allowed and there is no pending request' do + deadline = instance_double(DueDate, resubmission_allowed_id: 3, due_at: 1.day.from_now) + team = instance_double(AssignmentTeam) + + expect(described_class.send(:can_request_revision?, team, [deadline], nil)).to be(true) + end - expect(Participant).to receive(:find_by).with(id: 1).and_return(@participant) - expect(StudentTask).to receive(:create_from_participant).with(@participant) + it 'returns false when a pending revision request already exists' do + deadline = instance_double(DueDate, resubmission_allowed_id: 3, due_at: 1.day.from_now) + team = instance_double(AssignmentTeam) + request = instance_double(RevisionRequest, status: RevisionRequest::PENDING) - StudentTask.from_participant_id(1) + expect(described_class.send(:can_request_revision?, team, [deadline], request)).to be(false) end - end -end \ No newline at end of file + it 'returns false when the latest revision request has already been approved' do + deadline = instance_double(DueDate, resubmission_allowed_id: 3, due_at: 1.day.from_now) + team = instance_double(AssignmentTeam) + request = instance_double(RevisionRequest, status: RevisionRequest::APPROVED) + + expect(described_class.send(:can_request_revision?, team, [deadline], request)).to be(false) + end + + it 'returns false when there is no team submission for the task' do + deadline = instance_double(DueDate, resubmission_allowed_id: 3, due_at: 1.day.from_now) + + expect(described_class.send(:can_request_revision?, nil, [deadline], nil)).to be(false) + end + end +end diff --git a/spec/requests/api/v1/revision_requests_controller_spec.rb b/spec/requests/api/v1/revision_requests_controller_spec.rb new file mode 100644 index 000000000..675c72698 --- /dev/null +++ b/spec/requests/api/v1/revision_requests_controller_spec.rb @@ -0,0 +1,338 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'json_web_token' + +RSpec.describe 'RevisionRequests API', type: :request do + include RolesHelper + + let!(:roles) { create_roles_hierarchy } + let!(:institution) { Institution.create!(name: 'NC State') } + + let!(:instructor) do + User.create!( + name: 'instructor1', + email: 'instructor1@example.com', + password: 'password', + full_name: 'Instructor One', + institution: institution, + role: roles[:instructor] + ) + end + + let!(:other_instructor) do + User.create!( + name: 'instructor2', + email: 'instructor2@example.com', + password: 'password', + full_name: 'Instructor Two', + institution: institution, + role: roles[:instructor] + ) + end + + let!(:course) do + Course.create!( + name: 'CSC 517', + directory_path: 'csc517', + institution: institution, + instructor: instructor + ) + end + + let!(:assignment) do + Assignment.create!( + name: 'Assignment One', + instructor: instructor, + course: course, + directory_path: 'assignment_one' + ) + end + + let!(:student) do + User.create!( + name: 'student1', + email: 'student1@example.com', + password: 'password', + full_name: 'Student One', + institution: institution, + role: roles[:student] + ) + end + + let!(:other_student) do + User.create!( + name: 'student2', + email: 'student2@example.com', + password: 'password', + full_name: 'Student Two', + institution: institution, + role: roles[:student] + ) + end + + let!(:third_student) do + User.create!( + name: 'student3', + email: 'student3@example.com', + password: 'password', + full_name: 'Student Three', + institution: institution, + role: roles[:student] + ) + end + + let!(:team_one) { AssignmentTeam.create!(name: 'Team Alpha', parent_id: assignment.id) } + let!(:team_two) { AssignmentTeam.create!(name: 'Team Beta', parent_id: assignment.id) } + + let!(:participant_one) do + AssignmentParticipant.create!( + user: student, + assignment: assignment, + handle: student.name, + current_stage: 'In progress' + ) + end + + let!(:participant_two) do + AssignmentParticipant.create!( + user: other_student, + assignment: assignment, + handle: other_student.name, + current_stage: 'Submitted' + ) + end + + let!(:team_membership_one) do + TeamsParticipant.create!(team: team_one, participant: participant_one, user: student) + end + + let!(:team_membership_two) do + TeamsParticipant.create!(team: team_two, participant: participant_two, user: other_student) + end + + let!(:declined_request) do + RevisionRequest.create!( + participant: participant_two, + team: team_two, + assignment: assignment, + comments: 'Please allow another attempt.', + status: RevisionRequest::DECLINED, + response_comment: 'Use the existing submission for grading.' + ) + end + + let!(:pending_request) do + RevisionRequest.create!( + participant: participant_one, + team: team_one, + assignment: assignment, + comments: 'Please reopen the submission.' + ) + end + + let(:instructor_headers) { auth_headers_for(instructor) } + let(:other_instructor_headers) { auth_headers_for(other_instructor) } + let(:student_headers) { auth_headers_for(student) } + let(:other_student_headers) { auth_headers_for(other_student) } + let(:third_student_headers) { auth_headers_for(third_student) } + + describe 'GET /revision_requests' do + it 'returns assignment revision requests for the assignment instructor' do + get '/revision_requests', params: { assignment_id: assignment.id }, headers: instructor_headers + + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + + expect(body.map { |item| item['id'] }).to eq([pending_request.id, declined_request.id]) + expect(body.first).to include( + 'participant_id' => participant_one.id, + 'team_id' => team_one.id, + 'assignment_id' => assignment.id, + 'status' => 'PENDING', + 'comments' => 'Please reopen the submission.' + ) + end + + it 'filters revision requests by status' do + get '/revision_requests', + params: { assignment_id: assignment.id, status: RevisionRequest::DECLINED }, + headers: instructor_headers + + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + + expect(body.size).to eq(1) + expect(body.first).to include( + 'id' => declined_request.id, + 'status' => 'DECLINED', + 'response_comment' => 'Use the existing submission for grading.' + ) + end + + it 'returns not found for an invalid assignment id' do + get '/revision_requests', params: { assignment_id: 999_999 }, headers: instructor_headers + + expect(response).to have_http_status(:not_found) + expect(JSON.parse(response.body)).to eq('error' => 'Assignment not found') + end + + it 'returns unprocessable entity for an invalid status filter' do + get '/revision_requests', + params: { assignment_id: assignment.id, status: 'INVALID' }, + headers: instructor_headers + + expect(response).to have_http_status(:unprocessable_entity) + expect(JSON.parse(response.body)).to eq('error' => 'Status must be PENDING, APPROVED, or DECLINED') + end + + it 'returns forbidden for a different instructor' do + get '/revision_requests', params: { assignment_id: assignment.id }, headers: other_instructor_headers + + expect(response).to have_http_status(:forbidden) + end + + it 'returns forbidden for a student' do + get '/revision_requests', params: { assignment_id: assignment.id }, headers: student_headers + + expect(response).to have_http_status(:forbidden) + end + + it 'returns unauthorized without a valid token' do + get '/revision_requests', params: { assignment_id: assignment.id } + + expect(response).to have_http_status(:unauthorized) + expect(JSON.parse(response.body)).to eq('error' => 'Not Authorized') + end + end + + describe 'GET /revision_requests/:id' do + it 'returns the revision request for the owning student' do + get "/revision_requests/#{pending_request.id}", headers: student_headers + + expect(response).to have_http_status(:ok) + expect(JSON.parse(response.body)).to include( + 'id' => pending_request.id, + 'participant_id' => participant_one.id, + 'status' => 'PENDING' + ) + end + + it 'returns the revision request for the assignment instructor' do + get "/revision_requests/#{pending_request.id}", headers: instructor_headers + + expect(response).to have_http_status(:ok) + expect(JSON.parse(response.body)).to include( + 'id' => pending_request.id, + 'comments' => 'Please reopen the submission.' + ) + end + + it 'returns forbidden for another student' do + get "/revision_requests/#{pending_request.id}", headers: third_student_headers + + expect(response).to have_http_status(:forbidden) + end + + it 'returns not found for an invalid revision request id' do + get '/revision_requests/999999', headers: instructor_headers + + expect(response).to have_http_status(:not_found) + expect(JSON.parse(response.body)).to eq('error' => 'Revision request not found') + end + + it 'returns unauthorized without a valid token' do + get "/revision_requests/#{pending_request.id}" + + expect(response).to have_http_status(:unauthorized) + expect(JSON.parse(response.body)).to eq('error' => 'Not Authorized') + end + end + + describe 'PATCH /revision_requests/:id' do + it 'allows the assignment instructor to approve a pending revision request' do + patch "/revision_requests/#{pending_request.id}", + params: { revision_request: { status: RevisionRequest::APPROVED, response_comment: 'Approved for one more submission.' } }, + headers: instructor_headers + + expect(response).to have_http_status(:ok) + expect(pending_request.reload).to have_attributes( + status: RevisionRequest::APPROVED, + response_comment: 'Approved for one more submission.' + ) + expect(JSON.parse(response.body)).to include( + 'id' => pending_request.id, + 'status' => 'APPROVED', + 'response_comment' => 'Approved for one more submission.' + ) + end + + it 'allows the assignment instructor to decline a pending revision request' do + patch "/revision_requests/#{pending_request.id}", + params: { revision_request: { status: RevisionRequest::DECLINED, response_comment: 'The current submission will stand.' } }, + headers: instructor_headers + + expect(response).to have_http_status(:ok) + expect(pending_request.reload).to have_attributes( + status: RevisionRequest::DECLINED, + response_comment: 'The current submission will stand.' + ) + end + + it 'rejects an invalid resolution status' do + patch "/revision_requests/#{pending_request.id}", + params: { revision_request: { status: RevisionRequest::PENDING } }, + headers: instructor_headers + + expect(response).to have_http_status(:unprocessable_entity) + expect(JSON.parse(response.body)).to eq('error' => 'Status must be APPROVED or DECLINED') + end + + it 'rejects updates after a request has already been processed' do + patch "/revision_requests/#{declined_request.id}", + params: { revision_request: { status: RevisionRequest::APPROVED, response_comment: 'Changing the decision.' } }, + headers: instructor_headers + + expect(response).to have_http_status(:unprocessable_entity) + expect(JSON.parse(response.body)).to eq('error' => 'This revision request has already been processed') + end + + it 'returns forbidden for a student' do + patch "/revision_requests/#{pending_request.id}", + params: { revision_request: { status: RevisionRequest::APPROVED } }, + headers: student_headers + + expect(response).to have_http_status(:forbidden) + end + + it 'returns forbidden for a different instructor' do + patch "/revision_requests/#{pending_request.id}", + params: { revision_request: { status: RevisionRequest::APPROVED } }, + headers: other_instructor_headers + + expect(response).to have_http_status(:forbidden) + end + + it 'returns not found for an invalid revision request id' do + patch '/revision_requests/999999', + params: { revision_request: { status: RevisionRequest::APPROVED } }, + headers: instructor_headers + + expect(response).to have_http_status(:not_found) + expect(JSON.parse(response.body)).to eq('error' => 'Revision request not found') + end + + it 'returns unauthorized without a valid token' do + patch "/revision_requests/#{pending_request.id}", + params: { revision_request: { status: RevisionRequest::APPROVED } } + + expect(response).to have_http_status(:unauthorized) + expect(JSON.parse(response.body)).to eq('error' => 'Not Authorized') + end + end + + def auth_headers_for(user) + token = JsonWebToken.encode(id: user.id) + { 'Authorization' => "Bearer #{token}" } + end +end diff --git a/spec/requests/api/v1/student_tasks_controller_spec.rb b/spec/requests/api/v1/student_tasks_controller_spec.rb index 481f7b5d1..e368f7422 100644 --- a/spec/requests/api/v1/student_tasks_controller_spec.rb +++ b/spec/requests/api/v1/student_tasks_controller_spec.rb @@ -1,158 +1,457 @@ # frozen_string_literal: true -require 'swagger_helper' +require 'rails_helper' require 'json_web_token' RSpec.describe 'StudentTasks API', type: :request do - before(:all) do - @roles = create_roles_hierarchy - end + include RolesHelper + + let!(:roles) { create_roles_hierarchy } + let!(:institution) { Institution.create!(name: 'NC State') } let!(:instructor) do User.create!( - name: "Instructor", - password_digest: "password", - role_id: @roles[:instructor].id, - full_name: "Instructor Name", - email: "instructor@example.com" + name: 'instructor1', + email: 'instructor1@example.com', + password: 'password', + full_name: 'Instructor One', + institution: institution, + role: roles[:instructor] + ) + end + + let!(:course) do + Course.create!( + name: 'CSC 517', + directory_path: 'csc517', + institution: institution, + instructor: instructor + ) + end + + let!(:student) do + User.create!( + name: 'student1', + email: 'student1@example.com', + password: 'password', + full_name: 'Student One', + institution: institution, + role: roles[:student] ) end - let(:studenta) do + let!(:other_student) do User.create!( - name: "studenta", - password_digest: "password", - role_id: @roles[:student].id, - full_name: "Student A", - email: "testuser@example.com" - ) - end - - let(:token) { JsonWebToken.encode({id: studenta.id}) } - let(:Authorization) { "Bearer #{token}" } - - # ------------------------------------------------------------------------- - # /student_tasks/list - # ------------------------------------------------------------------------- - path '/student_tasks/list' do - get 'student tasks list' do - tags 'StudentTasks' - produces 'application/json' - parameter name: 'Authorization', in: :header, type: :string - - # Just a basic "200" test - response '200', 'authorized request has success response' do - run_test! - end - - # The "proper JSON schema" test - response '200', 'authorized request has proper JSON schema' do - before do - # 1) Create an Assignment - assignment = Assignment.create!( - name: "Sample Assignment", - instructor: instructor - ) - - # 2) Create N Participants for our student, each with different data - 5.times do |i| - AssignmentParticipant.create!( - user_id: studenta.id, - parent_id: assignment.id, - handle: studenta.name, - permission_granted: [true, false].sample, - # store “stage” and “deadline” fields as your Participant model expects - # e.g. might be: - topic: "Topic #{i}", - stage_deadline: (Time.now + (i + 1).days).to_s, - # and if it has “current_stage” or something: - current_stage: "Stage #{i}" - ) - end - end - - run_test! do |response| - data = JSON.parse(response.body) - expect(data).to be_an(Array) - expect(data.size).to eq(5) - - data.each do |task| - # Because StudentTask is just a plain Ruby object, - # we expect the controller to have built it from the Participant - expect(task['assignment']).to be_a(String) - expect(task['current_stage']).to be_a(String) - expect(task['stage_deadline']).to be_a(String) - expect(task['topic']).to be_a(String) - expect(task['permission_granted']).to be_in([true, false]) - end - end - end - - # Unauthorized test - response '401', 'unauthorized request has error response' do - let(:'Authorization') { "Bearer " } - run_test! - end + name: 'student2', + email: 'student2@example.com', + password: 'password', + full_name: 'Student Two', + institution: institution, + role: roles[:student] + ) + end + + let!(:empty_student) do + User.create!( + name: 'student3', + email: 'student3@example.com', + password: 'password', + full_name: 'Student Three', + institution: institution, + role: roles[:student] + ) + end + + let!(:assignment_one) do + Assignment.create!( + name: 'Assignment One', + instructor: instructor, + course: course, + directory_path: 'assignment_one', + has_topics: true + ) + end + + let!(:assignment_two) do + Assignment.create!( + name: 'Assignment Two', + instructor: instructor, + course: course, + directory_path: 'assignment_two' + ) + end + + let!(:team_one) do + AssignmentTeam.create!( + name: 'Team Alpha', + parent_id: assignment_one.id, + grade_for_submission: 88, + comment_for_submission: 'Please revise section 2.' + ) + end + + let!(:team_two) do + AssignmentTeam.create!( + name: 'Team Beta', + parent_id: assignment_two.id + ) + end + + let!(:project_topic) do + ProjectTopic.create!( + assignment: assignment_one, + topic_identifier: 'T1', + topic_name: 'Distributed Scheduler', + max_choosers: 2, + category: 'General' + ) + end + + let!(:signed_up_team) do + SignedUpTeam.create!( + team: team_one, + project_topic: project_topic, + is_waitlisted: false + ) + end + + let!(:assignment_deadline) do + DueDate.create!( + parent: assignment_one, + due_at: Time.zone.parse('2026-04-01 12:00:00'), + submission_allowed_id: 3, + resubmission_allowed_id: 3, + review_allowed_id: 3, + deadline_type_id: 1, + deadline_name: 'Submission deadline' + ) + end + + let!(:topic_deadline) do + DueDate.create!( + parent: project_topic, + due_at: Time.zone.parse('2026-04-05 12:00:00'), + submission_allowed_id: 3, + review_allowed_id: 3, + deadline_type_id: 2, + deadline_name: 'Topic review deadline', + round: 1 + ) + end + + let!(:assignment_two_deadline) do + DueDate.create!( + parent: assignment_two, + due_at: Time.zone.parse('2026-05-01 12:00:00'), + submission_allowed_id: 3, + review_allowed_id: 3, + deadline_type_id: 1, + deadline_name: 'Assignment two submission deadline' + ) + end + + let!(:student_task_participant) do + AssignmentParticipant.create!( + user: student, + assignment: assignment_one, + handle: student.name, + permission_granted: true, + topic: nil, + current_stage: 'In progress', + stage_deadline: Time.zone.parse('2026-04-03 09:00:00') + ) + end + + let!(:second_student_task_participant) do + AssignmentParticipant.create!( + user: student, + assignment: assignment_two, + handle: student.name, + permission_granted: false, + topic: 'Fallback Topic', + current_stage: 'Not started', + stage_deadline: Time.zone.parse('2026-05-02 09:00:00') + ) + end + + let!(:other_student_participant) do + AssignmentParticipant.create!( + user: other_student, + assignment: assignment_one, + handle: other_student.name, + permission_granted: false, + topic: 'Private Topic', + current_stage: 'Submitted', + stage_deadline: Time.zone.parse('2026-04-10 09:00:00') + ) + end + + let!(:student_team_membership) do + TeamsParticipant.create!(team: team_one, participant: student_task_participant, user: student) + end + + let!(:other_student_team_membership) do + TeamsParticipant.create!(team: team_one, participant: other_student_participant, user: other_student) + end + + let!(:second_student_team_membership) do + TeamsParticipant.create!(team: team_two, participant: second_student_task_participant, user: student) + end + + let!(:review_response_map) do + ReviewResponseMap.create!( + reviewed_object_id: assignment_one.id, + reviewer_id: other_student_participant.id, + reviewee_id: team_one.id + ) + end + + let!(:review_response) do + Response.create!( + map_id: review_response_map.id, + additional_comment: 'Strong work overall.', + is_submitted: true + ) + end + + let(:headers) { auth_headers_for(student) } + let(:empty_headers) { auth_headers_for(empty_student) } + + describe 'GET /student_tasks/list' do + it 'returns the current student task list with consistent task data' do + get '/student_tasks/list', headers: headers + + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + + expect(body.map { |task| task['participant_id'] }).to eq([student_task_participant.id, second_student_task_participant.id]) + expect(body.map { |task| task['assignment'] }).to eq(['Assignment One', 'Assignment Two']) + + first_task = body.first + expect(first_task).to include( + 'participant_id' => student_task_participant.id, + 'assignment_id' => assignment_one.id, + 'assignment' => 'Assignment One', + 'course_id' => course.id, + 'course' => 'CSC 517', + 'team_id' => team_one.id, + 'team_name' => 'Team Alpha', + 'topic' => 'T1', + 'current_stage' => 'In progress', + 'permission_granted' => true, + 'can_request_revision' => true, + 'review_grade' => nil + ) + expect(first_task['submission_feedback']).to include( + 'grade_for_submission' => 88, + 'comment_for_submission' => 'Please revise section 2.' + ) + expect(first_task['team_members']).to contain_exactly( + a_hash_including('id' => student.id, 'name' => 'student1', 'full_name' => 'Student One'), + a_hash_including('id' => other_student.id, 'name' => 'student2', 'full_name' => 'Student Two') + ) + expect(first_task['stage_deadline']).to eq('2026-04-03T09:00:00Z') + expect(first_task['deadlines']).to contain_exactly( + a_hash_including( + 'id' => assignment_deadline.id, + 'name' => 'Submission deadline', + 'deadline_type_id' => 1, + 'parent_type' => 'Assignment', + 'parent_id' => assignment_one.id + ), + a_hash_including( + 'id' => topic_deadline.id, + 'name' => 'Topic review deadline', + 'deadline_type_id' => 2, + 'round' => 1, + 'parent_type' => 'ProjectTopic', + 'parent_id' => project_topic.id + ) + ) + expect(first_task['topic_details']).to include( + 'id' => project_topic.id, + 'identifier' => 'T1', + 'name' => 'Distributed Scheduler' + ) + expect(first_task['team_details']).to include( + 'id' => team_one.id, + 'name' => 'Team Alpha' + ) + expect(first_task['assignment_details']).to include( + 'id' => assignment_one.id, + 'name' => 'Assignment One', + 'course_id' => course.id, + 'course_name' => 'CSC 517' + ) + expect(first_task['timeline']).to include( + a_hash_including('label' => 'Submission deadline', 'phase' => 'submission'), + a_hash_including('label' => 'Topic review deadline', 'phase' => 'review') + ) + expect(first_task['feedback']).to include( + a_hash_including( + 'response_id' => review_response.id, + 'reviewer_name' => 'Student Two', + 'comment' => 'Strong work overall.' + ) + ) + end + + it 'returns an empty array when the student has no tasks' do + get '/student_tasks/list', headers: empty_headers + + expect(response).to have_http_status(:ok) + expect(JSON.parse(response.body)).to eq([]) + end + + it 'returns unauthorized without a valid token' do + get '/student_tasks/list' + + expect(response).to have_http_status(:unauthorized) + expect(JSON.parse(response.body)).to eq('error' => 'Not Authorized') + end + end + + describe 'GET /student_tasks/:id' do + it 'returns the requested student task for the owner' do + get "/student_tasks/#{student_task_participant.id}", headers: headers + + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + + expect(body['participant_id']).to eq(student_task_participant.id) + expect(body['assignment']).to eq('Assignment One') + expect(body['topic']).to eq('T1') + expect(body['can_request_revision']).to be(true) + expect(body['submission_feedback']).to include( + 'grade_for_submission' => 88, + 'comment_for_submission' => 'Please revise section 2.' + ) + expect(body['deadlines'].size).to eq(2) + expect(body['feedback']).to include( + a_hash_including( + 'response_id' => review_response.id, + 'reviewer_name' => 'Student Two', + 'comment' => 'Strong work overall.' + ) + ) + expect(body['timeline']).to include( + a_hash_including('label' => 'Submission deadline', 'phase' => 'submission'), + a_hash_including('label' => 'Topic review deadline', 'phase' => 'review') + ) + end + + it 'returns forbidden for another student task' do + get "/student_tasks/#{other_student_participant.id}", headers: headers + + expect(response).to have_http_status(:forbidden) + expect(JSON.parse(response.body)).to eq('error' => 'You are not authorized to access this student task') + end + + it 'returns not found for an invalid task id' do + get '/student_tasks/999999', headers: headers + + expect(response).to have_http_status(:not_found) + expect(JSON.parse(response.body)).to eq('error' => 'Student task not found') + end + + it 'returns unauthorized without a valid token' do + get "/student_tasks/#{student_task_participant.id}" + + expect(response).to have_http_status(:unauthorized) + expect(JSON.parse(response.body)).to eq('error' => 'Not Authorized') + end + end + + describe 'GET /student_tasks/view' do + it 'supports the legacy detail endpoint with the same task payload' do + get '/student_tasks/view', params: { id: student_task_participant.id }, headers: headers + + expect(response).to have_http_status(:ok) + expect(JSON.parse(response.body)).to include( + 'participant_id' => student_task_participant.id, + 'assignment' => 'Assignment One' + ) end end - # ------------------------------------------------------------------------- - # /student_tasks/view - # ------------------------------------------------------------------------- - path '/student_tasks/view' do - get 'Retrieve a specific student task by ID' do - tags 'StudentTasks' - produces 'application/json' - parameter name: 'id', in: :query, type: :Integer, required: true - parameter name: 'Authorization', in: :header, type: :string - - # 200 test - response '200', 'successful retrieval of a student task' do - let!(:assignment) do - Assignment.create!(name: "Test Assignment", instructor: instructor) - end - - # Create *one* participant for the student - let!(:participant) do - AssignmentParticipant.create!( - user_id: studenta.id, - parent_id: assignment.id, - handle: studenta.name, - current_stage: "Review", - stage_deadline: (Time.now + 7.days).to_s, - topic: "Topic XYZ", - permission_granted: true - ) - end - - # This “id” is the participant’s ID to be looked up - let(:id) { participant.id } - - run_test! do |response| - data = JSON.parse(response.body) - expect(data['assignment']).to eq("Test Assignment") - expect(data['current_stage']).to eq("Review") - expect(data['stage_deadline']).to be_a(String) # e.g. "YYYY-MM-DD..." - expect(data['topic']).to eq("Topic XYZ") - expect(data['permission_granted']).to be true - end - end - - response '500', 'participant not found' do - let(:id) { -1 } - run_test! do |response| - expect(response.status).to eq(500) - end - end - - response '401', 'unauthorized request has error response' do - let(:'Authorization') { "Bearer " } - let(:id) { 'any_id' } - run_test! do |response| - data = JSON.parse(response.body) - expect(data["error"]).to eql("Not Authorized") - end - end + describe 'POST /student_tasks/:id/request_revision' do + it 'creates a revision request for the student task owner' do + post "/student_tasks/#{student_task_participant.id}/request_revision", + params: { comments: 'Please allow me to revise the submission based on the feedback.' }, + headers: headers + + expect(response).to have_http_status(:created) + body = JSON.parse(response.body) + + expect(body['message']).to eq('Revision request submitted successfully') + expect(body['revision_request']).to include( + 'participant_id' => student_task_participant.id, + 'team_id' => team_one.id, + 'assignment_id' => assignment_one.id, + 'status' => 'PENDING', + 'comments' => 'Please allow me to revise the submission based on the feedback.' + ) + expect(body['student_task']['can_request_revision']).to be(false) + expect(body['student_task']['revision_request']).to include( + 'status' => 'PENDING', + 'comments' => 'Please allow me to revise the submission based on the feedback.' + ) + end + + it 'rejects duplicate pending revision requests for the same task' do + RevisionRequest.create!( + participant: student_task_participant, + team: team_one, + assignment: assignment_one, + comments: 'Existing revision request' + ) + + post "/student_tasks/#{student_task_participant.id}/request_revision", + params: { comments: 'Another revision request' }, + headers: headers + + expect(response).to have_http_status(:unprocessable_entity) + expect(JSON.parse(response.body)).to eq('error' => 'Revision requests are not available for this task') end + + it 'rejects revision requests when resubmission is not available for the task' do + post "/student_tasks/#{second_student_task_participant.id}/request_revision", + params: { comments: 'Please reopen this submission' }, + headers: headers + + expect(response).to have_http_status(:unprocessable_entity) + expect(JSON.parse(response.body)).to eq('error' => 'Revision requests are not available for this task') + end + + it 'returns forbidden when the task belongs to another student' do + post "/student_tasks/#{other_student_participant.id}/request_revision", + params: { comments: 'Please reopen this submission' }, + headers: headers + + expect(response).to have_http_status(:forbidden) + expect(JSON.parse(response.body)).to eq('error' => 'You are not authorized to access this student task') + end + + it 'returns not found for an invalid task id' do + post '/student_tasks/999999/request_revision', + params: { comments: 'Please reopen this submission' }, + headers: headers + + expect(response).to have_http_status(:not_found) + expect(JSON.parse(response.body)).to eq('error' => 'Student task not found') + end + + it 'returns unauthorized without a valid token' do + post "/student_tasks/#{student_task_participant.id}/request_revision", + params: { comments: 'Please reopen this submission' } + + expect(response).to have_http_status(:unauthorized) + expect(JSON.parse(response.body)).to eq('error' => 'Not Authorized') + end + end + + def auth_headers_for(user) + token = JsonWebToken.encode(id: user.id) + { 'Authorization' => "Bearer #{token}" } end end diff --git a/spec/routing/revision_requests_routing_spec.rb b/spec/routing/revision_requests_routing_spec.rb new file mode 100644 index 000000000..be8103c57 --- /dev/null +++ b/spec/routing/revision_requests_routing_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe RevisionRequestsController, type: :routing do + describe 'routing' do + it 'routes to #index' do + expect(get: '/revision_requests').to route_to('revision_requests#index') + end + + it 'routes to #show' do + expect(get: '/revision_requests/1').to route_to('revision_requests#show', id: '1') + end + + it 'routes to #update via PATCH' do + expect(patch: '/revision_requests/1').to route_to('revision_requests#update', id: '1') + end + + it 'routes to #update via PUT' do + expect(put: '/revision_requests/1').to route_to('revision_requests#update', id: '1') + end + end +end diff --git a/spec/routing/student_tasks_routing_spec.rb b/spec/routing/student_tasks_routing_spec.rb index 1e34d93cb..30e5f9308 100644 --- a/spec/routing/student_tasks_routing_spec.rb +++ b/spec/routing/student_tasks_routing_spec.rb @@ -12,20 +12,16 @@ expect(get: "/student_tasks/1").to route_to("student_tasks#show", id: "1") end - it "routes to #create" do - expect(post: "/student_tasks").to route_to("student_tasks#create") + it "routes to #list" do + expect(get: "/student_tasks/list").to route_to("student_tasks#list") end - it "routes to #update via PUT" do - expect(put: "/student_tasks/1").to route_to("student_tasks#update", id: "1") + it "routes to #view" do + expect(get: "/student_tasks/view").to route_to("student_tasks#view") end - it "routes to #update via PATCH" do - expect(patch: "/student_tasks/1").to route_to("student_tasks#update", id: "1") - end - - it "routes to #destroy" do - expect(delete: "/student_tasks/1").to route_to("student_tasks#destroy", id: "1") + it "routes to #request_revision" do + expect(post: "/student_tasks/1/request_revision").to route_to("student_tasks#request_revision", id: "1") end end end