diff --git a/.gitignore b/.gitignore index b5973ad4c..82d93b6b9 100644 --- a/.gitignore +++ b/.gitignore @@ -32,3 +32,9 @@ coverage/ rsa_keys.yml pg_data/ + +# Local-only files not intended for the repository +app/controllers/demo/ +app/services/demo/ +app/controllers/transcript +wiki.txt diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb new file mode 100644 index 000000000..f3681269c --- /dev/null +++ b/app/controllers/reports_controller.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +# Reports live in their own controller per Expertiza convention: a single +# controller responsible only for rendering reports, dispatching to per-report +# actions. Feature controllers (e.g. ReviewMappingsController) should not +# embed report logic, since that violates SRP and hides the code from other +# consumers who need the same information. +class ReportsController < ApplicationController + include Authorization + before_action :set_assignment + + # Reports are viewable only by teaching staff for the assignment (instructor + # of the assignment, the course instructor, or a TA mapped to the course). + def action_allowed? + current_user_teaching_staff_of_assignment?(params[:assignment_id]) + end + + # GET /assignments/:assignment_id/reports/calibration/:map_id + # + # Renders the calibration comparison report for one instructor map. + # All assembly logic (rubric items, student responses, per-item score + # histograms, submitted content) lives in Reports::CalibrationReport, + # which follows the iterator pattern from Reports::Base — responses are + # walked one at a time via find_each, never preloaded into an ad-hoc array. + def calibration + instructor_map = ReviewResponseMap.find_by!( + id: params[:map_id], + reviewed_object_id: @assignment.id, + for_calibration: true + ) + render json: Reports::CalibrationReport.new(instructor_map).render, status: :ok + rescue ActiveRecord::RecordNotFound + render_error('Calibration review map not found', :not_found) + rescue Reports::CalibrationReport::InstructorResponseMissing, + Reports::CalibrationReport::RubricMissing => e + render_error(e.message, :unprocessable_entity) + end + + private + + def set_assignment + @assignment = Assignment.find(params[:assignment_id]) + end + + def render_error(message, status) + render json: { error: message }, status: status + end +end diff --git a/app/controllers/review_mappings_controller.rb b/app/controllers/review_mappings_controller.rb index 486840270..587e95e83 100644 --- a/app/controllers/review_mappings_controller.rb +++ b/app/controllers/review_mappings_controller.rb @@ -62,6 +62,58 @@ def assign_calibration_artifacts render json: { status: "ok", message: "Calibration reviews assigned to all reviewers" } end + # ===== CALIBRATION PARTICIPANTS ===== + # The "Calibration" tab on the assignment edit view designates one or more + # users as calibration submitters. The instructor types a username into the + # text box and that user is added as an AssignmentParticipant, a team is + # created for them (so submissions flow through the normal team-based + # infrastructure), and a ReviewResponseMap with for_calibration = true is + # created with the instructor as reviewer. The instructor later opens the + # calibration report for this map to enter/compare the calibration review. + + # GET /assignments/:assignment_id/review_mappings/calibration_participants + def list_calibration_participants + render json: { + assignment_id: @assignment.id, + calibration_participants: @assignment.calibration_participant_rows + }, status: :ok + end + + # POST /assignments/:assignment_id/review_mappings/calibration_participants + # Body: { username: "unctlt1" } + def add_calibration_participant + username = (params[:username] || params.dig(:calibration_participant, :username)).to_s.strip + return render json: { error: 'username is required' }, status: :bad_request if username.blank? + + user = User.find_by(name: username) || User.find_by(email: username) + return render json: { error: "User '#{username}' not found" }, status: :not_found unless user + + row = @assignment.add_calibration_submitter!(user) + render json: row, status: :created + rescue ArgumentError, ActiveRecord::RecordInvalid => e + render json: { error: e.message }, status: :unprocessable_entity + rescue StandardError => e + render json: { error: e.message }, status: :unprocessable_entity + end + + # NOTE: The demo-only "mock instructor response" endpoint that used to live + # here has moved to Demo::CalibrationInstructorResponsesController so this + # controller stays focused on real review-mapping behaviour. See + # Demo::CalibrationInstructorSeeder for the demo's removal checklist. + + # DELETE /assignments/:assignment_id/review_mappings/calibration_participants/:participant_id + def remove_calibration_participant + participant = AssignmentParticipant.find_by(id: params[:participant_id], parent_id: @assignment.id) + return render json: { error: 'Calibration participant not found' }, status: :not_found unless participant + + team = AssignmentTeam.team(participant) + return render json: { error: 'Participant has no team' }, status: :unprocessable_entity unless team + + ReviewResponseMap.calibration_for(@assignment).where(reviewee_id: team.id).destroy_all + + render json: { message: "Calibration participant #{participant.id} removed." }, status: :ok + end + # ===== DELETE ===== def destroy handler = ReviewMappingHandler.new(@assignment) @@ -85,6 +137,23 @@ def grade_review render json: { status: "ok", message: "Review graded" } end + # Actions that designate calibration submitters require teaching staff + # privileges; everything else defaults to allowed and relies on + # ApplicationController's own checks. We reuse the shared + # `current_user_teaching_staff_of_assignment?` helper from the Authorization + # concern instead of duplicating the logic here. + CALIBRATION_PARTICIPANT_ACTIONS = %w[ + list_calibration_participants + add_calibration_participant + remove_calibration_participant + ].freeze + + def action_allowed? + return current_user_teaching_staff_of_assignment?(params[:assignment_id]) if CALIBRATION_PARTICIPANT_ACTIONS.include?(params[:action]) + + true + end + private def set_assignment diff --git a/app/models/Item.rb b/app/models/Item.rb index 0b6535228..08ec7c214 100644 --- a/app/models/Item.rb +++ b/app/models/Item.rb @@ -33,6 +33,21 @@ def as_json(options = {}) end end + # JSON representation used by calibration reports. Kept on the item itself + # (Information Expert) so other report/view code does not re-implement this + # shape. + def as_calibration_json + { + id: id, + txt: txt, + seq: seq, + question_type: question_type, + weight: weight, + min_score: questionnaire.min_question_score, + max_score: questionnaire.max_question_score + } + end + def strategy case question_type when 'dropdown' diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 130fa6837..9487b3815 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -76,6 +76,53 @@ def add_participant(user_id) end + # ===== CALIBRATION PARTICIPANT MANAGEMENT ===== + + # Idempotent version of add_participant: returns the existing + # AssignmentParticipant if one already exists, otherwise creates one. + # Used by calibration flows where a user may already be a participant + # (e.g. the instructor, or a re-added submitter). + def find_or_add_participant!(user) + AssignmentParticipant.find_by(parent_id: id, user_id: user.id) || add_participant(user.id) + end + + # Orchestrates adding a calibration submitter in one atomic transaction: + # 1. Ensure the user is an AssignmentParticipant (idempotent). + # 2. Ensure the user has a team (creates an AssignmentTeam if not). + # 3. Ensure the instructor is an AssignmentParticipant (idempotent). + # 4. Find or create the for_calibration ReviewResponseMap (instructor → team). + # + # Returns the JSON row hash for the new calibration participant, ready to + # render directly from the controller. + def add_calibration_submitter!(user) + ActiveRecord::Base.transaction do + submitter = find_or_add_participant!(user) + team = AssignmentTeam.team(submitter) || Team.create_team_for_participant(submitter) + instructor_p = find_or_add_participant!(instructor) + map = ReviewResponseMap.find_or_create_by!( + reviewer_id: instructor_p.id, + reviewee_id: team.id, + reviewed_object_id: id, + for_calibration: true + ) + map.calibration_participant_json(instructor_user_id: instructor_id) + end + end + + # Returns the list of calibration participant rows for the assignment editor's + # Calibration tab. Delegates query + serialization to ReviewResponseMap so + # the controller stays a thin delegate. + def calibration_participant_rows + ReviewResponseMap + .calibration_for(self) + .order(:id) + .group_by(&:reviewee_id) + .filter_map do |_team_id, team_maps| + instructor_map = team_maps.find { |m| m.reviewer&.user_id == instructor_id } || team_maps.first + instructor_map.calibration_participant_json(instructor_user_id: instructor_id) + end + 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. diff --git a/app/models/assignment_team.rb b/app/models/assignment_team.rb index 2bf8e6815..0a18fa0fc 100644 --- a/app/models/assignment_team.rb +++ b/app/models/assignment_team.rb @@ -16,6 +16,39 @@ def hyperlinks submitted_hyperlinks.blank? ? [] : YAML.safe_load(submitted_hyperlinks) end + # SubmissionRecords for files this team has submitted against its assignment. + # Lives on the team (Information Expert) so callers don't have to know about + # SubmissionRecord's schema. + def submitted_file_records + SubmissionRecord.files.where(team_id: id, assignment_id: parent_id).order(created_at: :asc) + end + + # Submitted content in the simple shape expected by reports: hyperlinks and + # file paths only. Used by calibration/report views where we just need the + # URLs. + def submitted_content + { + hyperlinks: hyperlinks, + files: submitted_file_records.pluck(:content) + } + end + + # Submitted content in the rich shape expected by the calibration + # participants listing (per-file id, name, path, timestamps, submitter). + def submitted_content_detail + files = submitted_file_records.map do |record| + { + id: record.id, + name: File.basename(record.content.to_s), + path: record.content, + submitted_at: record.created_at, + submitted_by: record.user + } + end + + { hyperlinks: hyperlinks, files: files } + end + def submit_hyperlink(hyperlink) hyperlink.strip! raise 'The hyperlink cannot be empty!' if hyperlink.empty? diff --git a/app/models/questionnaire.rb b/app/models/questionnaire.rb index b7eeee017..e78de026c 100644 --- a/app/models/questionnaire.rb +++ b/app/models/questionnaire.rb @@ -85,6 +85,16 @@ def as_json(options = {}) DEFAULT_MIN_QUESTION_SCORE = 0 # The lowest score that a reviewer can assign to any questionnaire question DEFAULT_MAX_QUESTION_SCORE = 5 # The highest score that a reviewer can assign to any questionnaire question + + # Returns the integer Range of valid scores for this questionnaire's items, + # falling back to the class-level defaults when the DB columns are unset. + # Consumers (e.g. report histogram builders) should call this rather than + # hard-coding the 0..5 range themselves. + def score_range + min = (min_question_score || DEFAULT_MIN_QUESTION_SCORE).to_i + max = (max_question_score || DEFAULT_MAX_QUESTION_SCORE).to_i + min..max + end DEFAULT_QUESTIONNAIRE_URL = 'http://www.courses.ncsu.edu/csc517'.freeze QUESTIONNAIRE_TYPES = ['ReviewQuestionnaire', 'MetareviewQuestionnaire', diff --git a/app/models/response.rb b/app/models/response.rb index f99db770f..aaa4b0926 100644 --- a/app/models/response.rb +++ b/app/models/response.rb @@ -62,6 +62,45 @@ def aggregate_questionnaire_score sum end + # Returns the Answer this response recorded for a specific rubric item, or + # nil if no answer was given. Centralises item-lookup here (Information Expert) + # so callers don't scatter `scores.find { |a| a.item_id == item.id }` everywhere. + def answer_for(item) + scores.find { |a| a.item_id == item.id } + end + + # Ordered rubric items used for this response, or [] if the rubric cannot be + # resolved (e.g. no AssignmentQuestionnaire for this round). Belongs here + # rather than in a controller because the response knows which questionnaire + # applies to it. + def rubric_items + questionnaire.items.order(:seq) + rescue NoMethodError + [] + end + + # JSON representation of this response for calibration reports. Keeps the + # serialization next to the class that owns the data (Information Expert), + # so any consumer that needs a response rendered for a calibration view can + # ask the response for it. + def as_calibration_json + { + id: id, + map_id: map_id, + reviewer_id: map.reviewer_id, + reviewer_name: map.reviewer&.fullname, + is_submitted: is_submitted, + updated_at: updated_at, + answers: scores.map do |answer| + { + item_id: answer.item_id, + score: answer.answer, + comments: answer.comments + } + end + } + end + # Returns the maximum possible score for this response def maximum_score # only count the scorable questions, only when the answer is not nil (we accept nil as diff --git a/app/models/response_map.rb b/app/models/response_map.rb index 185d9dd77..e2dbaf0d0 100644 --- a/app/models/response_map.rb +++ b/app/models/response_map.rb @@ -20,6 +20,24 @@ def response_assignment return reviewer.assignment end + # Most recently submitted response on this map, or nil. Callers that need + # "the map's current submitted response" should ask the map directly rather + # than re-query Response in a controller. + def latest_submitted_response + responses.where(is_submitted: true).order(updated_at: :desc).first + end + + # Classify the progress of this review map: + # :not_started -- no Response rows exist yet + # :in_progress -- at least one Response, none submitted + # :submitted -- at least one submitted Response + # Lives here (Information Expert) because the map owns its responses. + def review_status + return :not_started if responses.empty? + return :submitted if responses.where(is_submitted: true).exists? + :in_progress + end + def self.assessments_for(team) responses = [] # stime = Time.now diff --git a/app/models/review_mapping_handler.rb b/app/models/review_mapping_handler.rb index 11a5a5f2d..6b0951577 100644 --- a/app/models/review_mapping_handler.rb +++ b/app/models/review_mapping_handler.rb @@ -1,3 +1,13 @@ +# ReviewMappingHandler contains the **business logic** for creating, deleting, +# and managing ReviewResponseMap records on an assignment. It is deliberately +# separate from ReviewMappingsController (the HTTP layer): the controller +# parses params, authorises the request, and renders JSON; the handler +# performs the actual domain work using pluggable Strategy objects. +# +# Strategy classes live in ReviewMappingStrategies/ and encapsulate the +# pairing algorithm (round-robin, random, CSV, topic-fair, etc.). The handler +# is not tied to any particular strategy — it just calls +# `strategy.each_review_pair` and creates the resulting maps. class ReviewMappingHandler DEFAULT_OUTSTANDING_LIMIT = 2 @@ -59,8 +69,12 @@ def assign_calibration_reviews_round_robin # Get all participants (students) reviewers = AssignmentParticipant.where(parent_id: @assignment.id) - # Get all instructor calibration teams/submissions - calibration_teams = AssignmentTeam.where(parent_id: @assignment.id, is_calibration: true) + # Get teams that already have an instructor calibration map. + calibration_team_ids = ReviewResponseMap + .where(reviewed_object_id: @assignment.id, for_calibration: true) + .distinct + .pluck(:reviewee_id) + calibration_teams = AssignmentTeam.where(id: calibration_team_ids) return if calibration_teams.empty? # Assign in round robin: each reviewer gets 2 calibration reviews @@ -71,7 +85,7 @@ def assign_calibration_reviews_round_robin reviewer: reviewer, reviewee: team, reviewed_object_id: @assignment.id, - calibration: true + for_calibration: true ) end end @@ -79,7 +93,7 @@ def assign_calibration_reviews_round_robin def calibration_reviews_for(reviewer) - ReviewResponseMap.where(reviewer: reviewer, calibration: true) + ReviewResponseMap.where(reviewer: reviewer, reviewed_object_id: @assignment.id, for_calibration: true) end # ===== OUTSTANDING REVIEWS ===== diff --git a/app/models/review_response_map.rb b/app/models/review_response_map.rb index ce38793dc..023c42f00 100644 --- a/app/models/review_response_map.rb +++ b/app/models/review_response_map.rb @@ -3,6 +3,12 @@ class ReviewResponseMap < ResponseMap include ResponseMapSubclassTitles belongs_to :reviewee, class_name: 'Team', foreign_key: 'reviewee_id', inverse_of: false + # All for_calibration maps on a given assignment. + # Centralises the for_calibration condition so callers don't repeat it. + scope :calibration_for, ->(assignment) { + where(reviewed_object_id: assignment.id, for_calibration: true) + } + # returns the assignment related to the response map def response_assignment return assignment @@ -11,7 +17,7 @@ def response_assignment def questionnaire_type 'Review' end - + def get_title REVIEW_RESPONSE_MAP_TITLE end @@ -20,4 +26,49 @@ def get_title def review_map_type 'ReviewResponseMap' end + + # Iterator over the latest submitted peer calibration Response for each + # student calibration map that shares the same reviewee as `instructor_map`. + # + # Uses find_each (batch loading) so large calibration cohorts don't load + # all maps into memory at once. Yields one Response at a time. + # + # Without a block, returns an Enumerator so callers can chain lazily. + def self.peer_calibration_responses_each(instructor_map) + return enum_for(:peer_calibration_responses_each, instructor_map) unless block_given? + + peer_maps = where( + reviewed_object_id: instructor_map.reviewed_object_id, + reviewee_id: instructor_map.reviewee_id, + for_calibration: true + ).where.not(id: instructor_map.id) + + peer_maps.find_each do |map| + response = map.latest_submitted_response + yield response if response + end + end + + # Serializes this calibration map into the JSON row expected by the + # assignment editor's Calibration tab. + # Lives here (Information Expert) — the map knows its reviewee team, + # its reviewer, and can ask itself for review_status. + def calibration_participant_json(instructor_user_id:) + team = reviewee + submitter = team.participants.where(type: 'AssignmentParticipant').first + return nil unless submitter + + { + participant_id: submitter.id, + user_id: submitter.user_id, + username: submitter.user&.name, + full_name: submitter.user&.full_name, + handle: submitter.handle, + team_id: team.id, + team_name: team.name, + instructor_review_map_id: id, + instructor_review_status: review_status, + submissions: team.submitted_content_detail + } + end end diff --git a/app/models/team.rb b/app/models/team.rb index e2c39fbaa..51273b988 100644 --- a/app/models/team.rb +++ b/app/models/team.rb @@ -22,6 +22,59 @@ class Team < ApplicationRecord after_update :release_topics_if_empty + # Factory method that creates the appropriate subclass of Team for the given + # participant and enrolls that participant as a member. + # + # - AssignmentParticipant => AssignmentTeam (parent = assignment) + # - CourseParticipant => CourseTeam (parent = course) + # + # This is the mechanism used to back individual submitters (including the + # phantom "calibration submitters" that an instructor adds on the Calibration + # tab). Expertiza routes every submission through a team, so even a single + # submitter needs a team behind the scenes. + # + # Params: + # participant - an AssignmentParticipant or CourseParticipant + # name: - optional team name; if omitted a stable unique name is + # generated from the user's handle/name. + # + # Returns the newly-created team (AssignmentTeam or CourseTeam). + # Raises ArgumentError if the participant type is not supported. + def self.create_team_for_participant(participant, name: nil) + raise ArgumentError, 'participant is required' if participant.nil? + + team_class, parent = + case participant + when AssignmentParticipant + [AssignmentTeam, participant.assignment] + when CourseParticipant + [CourseTeam, participant.course] + else + raise ArgumentError, "Unsupported participant type: #{participant.class}" + end + + team_name = name.presence || generate_team_name_for(participant, parent) + + team = team_class.create!(parent_id: parent.id, name: team_name) + result = team.add_member(participant) + + unless result.is_a?(Hash) && result[:success] + team.destroy + error = result.is_a?(Hash) ? result[:error] : 'Unable to add participant to new team' + raise ActiveRecord::RecordInvalid.new(team), error + end + + team + end + + def self.generate_team_name_for(participant, parent) + base = (participant.user&.handle.presence || participant.user&.name.presence || "participant_#{participant.id}").to_s + candidate = "#{base}_team" + return candidate unless Team.exists?(parent_id: parent.id, name: candidate) + + "#{candidate}_#{SecureRandom.hex(3)}" + end + def has_member?(user) participants.exists?(user_id: user.id) end diff --git a/app/services/calibration_per_item_summary.rb b/app/services/calibration_per_item_summary.rb new file mode 100644 index 000000000..b10044dc9 --- /dev/null +++ b/app/services/calibration_per_item_summary.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +# CalibrationPerItemSummary builds a per-rubric-item summary (score buckets + +# instructor score) from a pre-fetched collection of Response objects. +# +# NOTE: This class is superseded for the HTTP report endpoint by +# Reports::CalibrationReport, which uses an iterator-based pipeline +# (Template Method pattern) so responses are never bulk-loaded into memory. +# CalibrationPerItemSummary is retained for use by the calibration demo rake +# task (lib/tasks/calibration_demo.rake), which works with small in-memory +# arrays and does not need the streaming approach. +class CalibrationPerItemSummary + # Convenience factory: constructs an instance and immediately calls build. + # Equivalent to `new(...).build`. + def self.build(items:, instructor_response:, student_responses:) + new( + items: items, + instructor_response: instructor_response, + student_responses: student_responses + ).build + end + + def initialize(items:, instructor_response:, student_responses:) + @items = Array(items) + @instructor_response = instructor_response + @student_responses = Array(student_responses).compact + end + + # Returns an Array of per-item summary hashes sorted by item sequence number. + # Each hash contains the instructor score, instructor comment, per-score + # bucket counts, and the number of student responses that contributed. + def build + instructor_answers = answers_by_item(@instructor_response) + latest_student_responses = latest_submitted_student_responses + student_answers = latest_student_responses.to_h { |response| [response.id, answers_by_item(response)] } + + # Sort by seq so the caller receives items in rubric display order. + @items.sort_by(&:seq).map do |item| + { + item_id: item.id, + item_label: item.txt, + item_seq: item.seq, + instructor_score: instructor_answers[item.id]&.answer, + instructor_comment: instructor_answers[item.id]&.comments, + bucket_counts: bucket_counts_for(item, latest_student_responses, student_answers), + student_response_count: latest_student_responses.count + } + end + end + + private + + # For each calibration map, keep only the most recently updated submitted + # response. Older submitted versions and any drafts are discarded. + def latest_submitted_student_responses + @student_responses + .select(&:is_submitted) + .group_by(&:map_id) + .values + .map { |responses| responses.max_by(&:updated_at) } + .compact + end + + # Returns a Hash mapping item_id → Answer for all answers in the response, + # or {} when the response is nil (e.g. instructor has no response yet). + def answers_by_item(response) + return {} unless response + + response.scores.each_with_object({}) do |answer, by_item| + by_item[answer.item_id] = answer + end + end + + # Tallies how many student responses scored each possible value for `item`. + # Returns a Hash of { score_string => count } covering every integer in the + # questionnaire's min..max range so the caller always gets a full histogram. + def bucket_counts_for(item, responses, answers_by_response) + buckets = score_range_for(item).each_with_object({}) do |score, counts| + counts[score.to_s] = 0 + end + + # `responses` is already reduced to the latest submitted response for each + # student calibration map, so this counts one effective calibration review + # per reviewer for the shared calibration artifact. + responses.each do |response| + score = answers_by_response.fetch(response.id).fetch(item.id, nil)&.answer + next if score.nil? + + key = score.to_i.to_s + buckets[key] ||= 0 + buckets[key] += 1 + end + + buckets + end + + def score_range_for(item) + min_score = item.questionnaire&.min_question_score || 0 + max_score = item.questionnaire&.max_question_score || 5 + + min_score.to_i..max_score.to_i + end +end diff --git a/app/services/reports/base.rb b/app/services/reports/base.rb new file mode 100644 index 000000000..d5849e962 --- /dev/null +++ b/app/services/reports/base.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module Reports + # Reports::Base is the skeleton shared by every report in the system + # (review, author-feedback, teammate, bookmark, calibration, survey, quiz). + # + # It defines the fixed algorithm: setup → iterate one response at a time → + # assemble the payload. Subclasses fill in each step without altering the + # overall sequence. + # + # Two design rules drive what does -- and does not -- live here: + # + # 1. Iterate over responses, never preload them into an ad-hoc array. + # Subclasses implement `each_response` as an *iterator* that yields one + # Response at a time. The base loops once and dispatches; nothing in + # the base ever needs the whole collection in memory. + # + # 2. No specific metrics in the base. + # Different reports want different summaries (averages for one, score + # histograms for another, per-tag counts for answer-tagging). The + # base therefore exposes only the skeleton -- setup, accumulate, + # payload -- and lets each subclass define its own state and metric logic. + # + # Subclass contract: + # * setup -- prepare any state needed before iteration + # (rubric items, output buffers, etc.). Optional. + # * each_response -- yield Responses one at a time. REQUIRED. + # * accumulate(r) -- update subclass-owned state from a single response. + # REQUIRED. + # * payload -- return the final JSON-ready Hash. REQUIRED. + class Base + def render + setup + each_response { |response| accumulate(response) } + payload + end + + private + + def setup; end + + def each_response + raise NotImplementedError, "#{self.class} must implement #each_response as an iterator" + end + + def accumulate(_response) + raise NotImplementedError, "#{self.class} must implement #accumulate(response)" + end + + def payload + raise NotImplementedError, "#{self.class} must implement #payload" + end + end +end diff --git a/app/services/reports/base_report.rb b/app/services/reports/base_report.rb new file mode 100644 index 000000000..d5849e962 --- /dev/null +++ b/app/services/reports/base_report.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module Reports + # Reports::Base is the skeleton shared by every report in the system + # (review, author-feedback, teammate, bookmark, calibration, survey, quiz). + # + # It defines the fixed algorithm: setup → iterate one response at a time → + # assemble the payload. Subclasses fill in each step without altering the + # overall sequence. + # + # Two design rules drive what does -- and does not -- live here: + # + # 1. Iterate over responses, never preload them into an ad-hoc array. + # Subclasses implement `each_response` as an *iterator* that yields one + # Response at a time. The base loops once and dispatches; nothing in + # the base ever needs the whole collection in memory. + # + # 2. No specific metrics in the base. + # Different reports want different summaries (averages for one, score + # histograms for another, per-tag counts for answer-tagging). The + # base therefore exposes only the skeleton -- setup, accumulate, + # payload -- and lets each subclass define its own state and metric logic. + # + # Subclass contract: + # * setup -- prepare any state needed before iteration + # (rubric items, output buffers, etc.). Optional. + # * each_response -- yield Responses one at a time. REQUIRED. + # * accumulate(r) -- update subclass-owned state from a single response. + # REQUIRED. + # * payload -- return the final JSON-ready Hash. REQUIRED. + class Base + def render + setup + each_response { |response| accumulate(response) } + payload + end + + private + + def setup; end + + def each_response + raise NotImplementedError, "#{self.class} must implement #each_response as an iterator" + end + + def accumulate(_response) + raise NotImplementedError, "#{self.class} must implement #accumulate(response)" + end + + def payload + raise NotImplementedError, "#{self.class} must implement #payload" + end + end +end diff --git a/app/services/reports/calibration_report.rb b/app/services/reports/calibration_report.rb new file mode 100644 index 000000000..ebfc19aa0 --- /dev/null +++ b/app/services/reports/calibration_report.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +module Reports + # Reports::CalibrationReport renders the comparison view for one instructor + # calibration map: the instructor's submitted response, the rubric items, + # the (latest submitted) student peer responses for the same reviewee, and + # a per-item bucket-count summary suitable for a stacked chart. + # + # Design notes (mirroring Reports::Base): + # * Responses are walked one-at-a-time via an iterator on + # ReviewResponseMap. We never build a giant Array of all peer responses + # in memory; we ask the model to yield the latest submitted Response + # for each peer calibration map. + # * The metric this report produces (per-item score histograms) lives + # here, NOT in the base, because other reports (averages, tag counts, + # survey aggregates) want completely different shapes. + class CalibrationReport < Base + InstructorResponseMissing = Class.new(StandardError) + RubricMissing = Class.new(StandardError) + + def initialize(instructor_map) + @instructor_map = instructor_map + end + + private + + def setup + @instructor_response = @instructor_map.latest_submitted_response + raise InstructorResponseMissing, 'Submitted instructor calibration response not found' if @instructor_response.nil? + + @rubric_items = @instructor_response.rubric_items + raise RubricMissing, 'Review rubric not found' if @rubric_items.empty? + + # @bucket_counts is a two-level Hash: + # { item_id => { "0" => count, "1" => count, ..., "N" => count } } + # The inner Hash covers every integer in the questionnaire's score range so + # the presenter always receives a complete histogram (zero-filled buckets + # included) without needing to know the min/max score. + @bucket_counts = empty_buckets_for(@rubric_items) + @student_responses = [] + end + + # Iterator: ask ReviewResponseMap for the latest submitted student + # calibration Response per peer map and yield them one at a time. + def each_response(&block) + ReviewResponseMap.peer_calibration_responses_each(@instructor_map, &block) + end + + # State updated per response. The metric here (score histograms) is + # CalibrationReport-specific; that is exactly why it does not belong in + # the base. + def accumulate(response) + @student_responses << response + response.scores.each do |answer| + next if answer.answer.nil? + next unless @bucket_counts.key?(answer.item_id) + + bucket_key = answer.answer.to_i.to_s + @bucket_counts[answer.item_id][bucket_key] ||= 0 + @bucket_counts[answer.item_id][bucket_key] += 1 + end + end + + def payload + { + map_id: @instructor_map.id, + assignment_id: @instructor_map.reviewed_object_id, + reviewee_id: @instructor_map.reviewee_id, + rubric_items: @rubric_items.map(&:as_calibration_json), + instructor_response: @instructor_response.as_calibration_json, + student_responses: @student_responses.map(&:as_calibration_json), + per_item_summary: @rubric_items.sort_by(&:seq).map { |item| per_item_summary(item) }, + submitted_content: submitted_content_for(@instructor_map.reviewee) + } + end + + def empty_buckets_for(items) + items.each_with_object({}) do |item, hash| + # Delegate the valid score range to the Questionnaire (Information Expert). + # The fallback to class-level defaults lives in Questionnaire#score_range, + # not here, so calibration code does not carry knowledge of score bounds. + range = item.questionnaire&.score_range || (0..5) + hash[item.id] = range.each_with_object({}) { |score, b| b[score.to_s] = 0 } + end + end + + def per_item_summary(item) + instructor_answer = @instructor_response.answer_for(item) + { + item_id: item.id, + item_label: item.txt, + item_seq: item.seq, + instructor_score: instructor_answer&.answer, + instructor_comment: instructor_answer&.comments, + bucket_counts: @bucket_counts[item.id], + student_response_count: @student_responses.size + } + end + + def submitted_content_for(reviewee) + return { hyperlinks: [], files: [] } unless reviewee.respond_to?(:submitted_content) + reviewee.submitted_content + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 57559d007..8a95c9ccb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -38,6 +38,22 @@ get '/:assignment_id/varying_rubrics_by_round', action: :varying_rubrics_by_round? post '/:assignment_id/create_node',action: :create_node end + + resources :review_mappings, only: [] do + collection do + get :calibration_participants, action: :list_calibration_participants + post :calibration_participants, action: :add_calibration_participant + delete 'calibration_participants/:participant_id', action: :remove_calibration_participant + end + end + + # Reports are owned by ReportsController, not by feature controllers. + # Adding a new report type only requires a new action here. + resources :reports, only: [] do + collection do + get 'calibration/:map_id', action: :calibration, as: :calibration + end + end end resources :bookmarks, except: [:new, :edit] do diff --git a/db/migrate/20260423010050_add_for_calibration_to_response_maps.rb b/db/migrate/20260423010050_add_for_calibration_to_response_maps.rb new file mode 100644 index 000000000..30a930d4a --- /dev/null +++ b/db/migrate/20260423010050_add_for_calibration_to_response_maps.rb @@ -0,0 +1,6 @@ +class AddForCalibrationToResponseMaps < ActiveRecord::Migration[8.0] + def change + add_column :response_maps, :for_calibration, :boolean, default: false, null: false + add_index :response_maps, :for_calibration + end +end diff --git a/db/schema.rb b/db/schema.rb index cddbe12c6..10b0ceb64 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_04_23_010050) do create_table "account_requests", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "username" t.string "full_name" @@ -331,6 +331,8 @@ 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 ["for_calibration"], name: "index_response_maps_on_for_calibration" t.index ["reviewer_id"], name: "fk_response_map_reviewer" end @@ -456,9 +458,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/lib/tasks/calibration_demo.rake b/lib/tasks/calibration_demo.rake new file mode 100644 index 000000000..292719d2b --- /dev/null +++ b/lib/tasks/calibration_demo.rake @@ -0,0 +1,244 @@ +# frozen_string_literal: true + +namespace :demo do + desc 'Create or refresh calibration report demo data with six student results' + task calibration_report: :environment do + role_for = lambda do |name| + Role.find_by!(name: name) + end + + institution = Institution.first || Institution.create!(name: 'North Carolina State University') + + ensure_user = lambda do |model_class:, name:, email:, role_name:, full_name:, password: 'password123', parent: nil| + user = User.find_by(name: name) || User.find_by(email: email) + + if user + user.update!( + email: email, + full_name: full_name, + role: role_for.call(role_name), + institution: institution, + parent: parent + ) + user.password = password if user.authenticate(password).nil? + user.save! if user.changed? + model_class.find(user.id) + else + model_class.create!( + name: name, + email: email, + password: password, + full_name: full_name, + role: role_for.call(role_name), + institution: institution, + parent: parent + ) + end + end + + ensure_participant = lambda do |assignment:, user:, handle:| + AssignmentParticipant.find_or_create_by!(parent_id: assignment.id, user_id: user.id) do |participant| + participant.handle = handle + end + end + + ensure_item = lambda do |questionnaire:, txt:, seq:| + item = questionnaire.items.find_or_initialize_by(txt: txt) + item.assign_attributes( + seq: seq, + weight: 1, + question_type: 'Scale', + break_before: true + ) + item.save! + item + end + + update_response = lambda do |map:, scores:| + response = Response.where(map_id: map.id, is_submitted: true).order(updated_at: :desc).first + + unless response + response = Response.create!( + response_map: map, + round: 1, + version_num: Response.where(map_id: map.id).maximum(:version_num).to_i + 1, + is_submitted: true + ) + end + + response.update!(is_submitted: true, updated_at: Time.current) + + scores.each do |item, score| + answer = Answer.find_or_initialize_by(response_id: response.id, item_id: item.id) + answer.update!( + answer: score, + comments: '' + ) + end + + response + end + + instructor = ensure_user.call( + model_class: Instructor, + name: 'calibration_demo_instructor', + email: 'calibration_demo_instructor@example.com', + role_name: 'Instructor', + full_name: 'Calibration Demo Instructor' + ) + + assignment = Assignment.find_by(id: 8) || Assignment.find_by(name: 'Calibration Demo Assignment') + assignment ||= Assignment.create!( + name: 'Calibration Demo Assignment', + instructor: instructor, + has_teams: true, + private: false + ) + assignment.update!(instructor: instructor, has_teams: true, private: false) + + reviewee_team = + if assignment.id == 8 && ReviewResponseMap.exists?(id: 8, reviewed_object_id: assignment.id, for_calibration: true) + ReviewResponseMap.find(8).reviewee + else + assignment.teams.find_or_create_by!(name: 'Calibration Demo Reviewee Team') + end + + questionnaire = + assignment.assignment_questionnaires.find_by(used_in_round: 1)&.questionnaire || + Questionnaire.find_by(name: 'Calibration Demo Rubric', instructor_id: instructor.id) + + questionnaire ||= Questionnaire.create!( + name: 'Calibration Demo Rubric', + private: false, + min_question_score: 0, + max_question_score: 5, + instructor: instructor + ) + + AssignmentQuestionnaire.find_or_create_by!( + assignment: assignment, + questionnaire: questionnaire, + used_in_round: 1 + ) + + items = [ + ensure_item.call(questionnaire: questionnaire, txt: 'Code quality', seq: 1), + ensure_item.call(questionnaire: questionnaire, txt: 'Documentation', seq: 2), + ensure_item.call(questionnaire: questionnaire, txt: 'Testing', seq: 3) + ] + + instructor_participant = ensure_participant.call( + assignment: assignment, + user: instructor, + handle: instructor.name + ) + + instructor_map = + ReviewResponseMap.find_by(id: 8, reviewed_object_id: assignment.id, for_calibration: true) || + ReviewResponseMap.find_or_create_by!( + reviewed_object_id: assignment.id, + reviewer_id: instructor_participant.id, + reviewee_id: reviewee_team.id, + for_calibration: true + ) + + update_response.call( + map: instructor_map, + scores: { + items[0] => 4, + items[1] => 5, + items[2] => 3 + } + ) + + score_sets = [ + [4, 5, 3], + [4, 4, 3], + [4, 4, 2], + [3, 4, 1], + [3, 2, 1], + [1, 1, 0] + ] + + student_maps = score_sets.each_with_index.map do |scores, index| + student = ensure_user.call( + model_class: User, + name: "calibration_demo_student_#{index + 1}", + email: "calibration_demo_student_#{index + 1}@example.com", + role_name: 'Student', + full_name: "Calibration Demo Student #{index + 1}", + parent: instructor + ) + + participant = ensure_participant.call( + assignment: assignment, + user: student, + handle: student.name + ) + + map = ReviewResponseMap.find_or_create_by!( + reviewed_object_id: assignment.id, + reviewer_id: participant.id, + reviewee_id: reviewee_team.id, + for_calibration: true + ) + + update_response.call( + map: map, + scores: { + items[0] => scores[0], + items[1] => scores[1], + items[2] => scores[2] + } + ) + + map + end + + ReviewResponseMap.where( + reviewed_object_id: assignment.id, + reviewee_id: reviewee_team.id, + for_calibration: true + ).where.not(id: [instructor_map.id, *student_maps.map(&:id)]).find_each(&:destroy!) + + reviewee_team.update!(submitted_hyperlinks: YAML.dump(['https://example.com/submission'])) + SubmissionRecord.find_or_create_by!( + record_type: 'file', + content: 'submission/report.pdf', + operation: 'Submit File', + team_id: reviewee_team.id, + user: instructor.name, + assignment_id: assignment.id + ) + + student_responses = student_maps.map do |map| + Response.where(map_id: map.id, is_submitted: true).order(updated_at: :desc).first + end + instructor_response = Response.where(map_id: instructor_map.id, is_submitted: true).order(updated_at: :desc).first + + summaries = CalibrationPerItemSummary.build( + items: items, + instructor_response: instructor_response, + student_responses: student_responses + ) + + distribution = summaries.map do |summary| + { + item: summary[:item_label], + agree: summary[:bucket_counts][summary[:instructor_score].to_s], + near: summary[:bucket_counts].select { |score, _count| (score.to_i - summary[:instructor_score].to_i).abs == 1 }.values.sum, + disagree: summary[:bucket_counts].select { |score, _count| (score.to_i - summary[:instructor_score].to_i).abs > 1 }.values.sum + } + end + + puts({ + assignment_id: assignment.id, + map_id: instructor_map.id, + reviewee_id: reviewee_team.id, + student_count: student_maps.length, + rubric_items: items.map(&:txt), + summaries: distribution, + frontend_url: "http://localhost:3000/assignments/edit/#{assignment.id}/calibration/#{instructor_map.id}" + }.inspect) + end +end diff --git a/spec/factories/review_response_maps.rb b/spec/factories/review_response_maps.rb index 1cee9aace..fd1dc712f 100644 --- a/spec/factories/review_response_maps.rb +++ b/spec/factories/review_response_maps.rb @@ -1,8 +1,12 @@ FactoryBot.define do factory :review_response_map do association :assignment - association :reviewer, factory: :user - association :reviewee, factory: :team - reviewed_object_id { 1 } + reviewer { association :assignment_participant, assignment: assignment } + reviewee { association :assignment_team, assignment: assignment } + reviewed_object_id { assignment.id } + + trait :for_calibration do + for_calibration { true } + end end -end \ No newline at end of file +end diff --git a/spec/factories/teams.rb b/spec/factories/teams.rb index 8533a6b0e..903e32eb3 100644 --- a/spec/factories/teams.rb +++ b/spec/factories/teams.rb @@ -24,18 +24,18 @@ course { create(:course) } end + assignment { create(:assignment, course: course) } + parent_id { assignment.id } + after(:build) do |team, evaluator| - if team.assignment.nil? - team.course = evaluator.course - else - team.course = team.assignment.course - end + team.assignment ||= create(:assignment, course: evaluator.course) + team.parent_id = team.assignment.id end trait :with_assignment do after(:build) do |team, evaluator| team.assignment = create(:assignment, course: evaluator.course) - team.course = team.assignment.course + team.parent_id = team.assignment.id end end end diff --git a/spec/models/response_map_spec.rb b/spec/models/response_map_spec.rb index 0a84ef6de..58b44fee9 100644 --- a/spec/models/response_map_spec.rb +++ b/spec/models/response_map_spec.rb @@ -3,4 +3,27 @@ require 'rails_helper' RSpec.describe ResponseMap, type: :model do + describe '#review_status' do + let(:map) { create(:review_response_map) } + + it 'returns :not_started when no responses have been created' do + expect(map.review_status).to eq(:not_started) + end + + it 'returns :in_progress when a draft (unsubmitted) response exists' do + Response.create!(response_map: map, round: 1, version_num: 1, is_submitted: false) + expect(map.review_status).to eq(:in_progress) + end + + it 'returns :submitted when at least one submitted response exists' do + Response.create!(response_map: map, round: 1, version_num: 1, is_submitted: true) + expect(map.review_status).to eq(:submitted) + end + + it 'returns :submitted even when a later draft also exists' do + Response.create!(response_map: map, round: 1, version_num: 1, is_submitted: true) + Response.create!(response_map: map, round: 1, version_num: 2, is_submitted: false) + expect(map.review_status).to eq(:submitted) + end + end end diff --git a/spec/models/review_mapping_handler_spec.rb b/spec/models/review_mapping_handler_spec.rb new file mode 100644 index 000000000..d2501832c --- /dev/null +++ b/spec/models/review_mapping_handler_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ReviewMappingHandler do + describe '#calibration_reviews_for' do + it 'returns only calibration review maps for the handler assignment and reviewer' do + assignment = create(:assignment) + other_assignment = create(:assignment) + reviewer = create(:assignment_participant, assignment: assignment) + + calibration_map = create(:review_response_map, :for_calibration, assignment: assignment, reviewer: reviewer) + create(:review_response_map, assignment: assignment, reviewer: reviewer) + create(:review_response_map, :for_calibration, assignment: other_assignment) + + result = described_class.new(assignment).calibration_reviews_for(reviewer) + + expect(result).to contain_exactly(calibration_map) + end + end + + describe '#assign_calibration_reviews_round_robin' do + it 'creates calibration maps against existing calibration reviewees' do + assignment = create(:assignment) + reviewers = create_list(:assignment_participant, 2, assignment: assignment) + calibration_teams = create_list(:assignment_team, 2, assignment: assignment) + + create(:review_response_map, :for_calibration, assignment: assignment, reviewee: calibration_teams.first) + create(:review_response_map, :for_calibration, assignment: assignment, reviewee: calibration_teams.second) + + described_class.new(assignment).assign_calibration_reviews_round_robin + + reviewers.each do |reviewer| + maps = ReviewResponseMap.where( + reviewer: reviewer, + reviewed_object_id: assignment.id, + for_calibration: true + ) + + expect(maps.count).to eq(2) + expect(maps.pluck(:reviewee_id)).to match_array(calibration_teams.map(&:id)) + end + end + end +end diff --git a/spec/requests/api/v1/reports_calibration_spec.rb b/spec/requests/api/v1/reports_calibration_spec.rb new file mode 100644 index 000000000..82980313d --- /dev/null +++ b/spec/requests/api/v1/reports_calibration_spec.rb @@ -0,0 +1,355 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'json_web_token' + +RSpec.describe 'Reports calibration', type: :request do + let!(:super_admin_role) { Role.create!(name: 'Super Administrator') } + let!(:admin_role) { Role.create!(name: 'Administrator') } + let!(:instructor_role) { Role.create!(name: 'Instructor') } + let!(:student_role) { Role.create!(name: 'Student') } + let(:institution) { create(:institution) } + let(:instructor) { create_user(role: instructor_role, name: 'calibration_instructor') } + let(:student_user) { create_user(role: student_role, name: 'calibration_student') } + let(:assignment) { Assignment.create!(name: 'Calibration Assignment', instructor: instructor) } + let(:questionnaire) { create_questionnaire } + let(:code_quality) { create_item('Code quality', 1) } + let(:documentation) { create_item('Documentation', 2) } + let(:reviewee_team) { create(:assignment_team, assignment: assignment) } + let(:instructor_participant) { create(:assignment_participant, assignment: assignment, user: instructor) } + let(:student_participant) { create(:assignment_participant, assignment: assignment, user: student_user) } + let(:instructor_map) do + create( + :review_response_map, + :for_calibration, + assignment: assignment, + reviewer: instructor_participant, + reviewee: reviewee_team + ) + end + let(:student_map) do + create( + :review_response_map, + :for_calibration, + assignment: assignment, + reviewer: student_participant, + reviewee: reviewee_team + ) + end + let(:instructor_headers) { auth_headers_for(instructor) } + + before do + AssignmentQuestionnaire.create!( + assignment: assignment, + questionnaire: questionnaire, + used_in_round: 1 + ) + code_quality + documentation + end + + # --------------------------------------------------------------------------- + # Calibration participant management (add / list / remove) + # --------------------------------------------------------------------------- + describe 'POST /assignments/:assignment_id/review_mappings/calibration_participants' do + let(:submitter_user) do + User.create!(name: 'unctlt1', full_name: 'UNC TLT 1', + email: 'unctlt1@example.com', password: 'password', + role: student_role, institution: institution) + end + let(:participants_path) { "/assignments/#{assignment.id}/review_mappings/calibration_participants" } + + it 'atomically creates participant, team, and for_calibration map and returns 201' do + submitter_user + expect { + post participants_path, params: { username: 'unctlt1' }, headers: instructor_headers + }.to change { AssignmentParticipant.where(parent_id: assignment.id, user_id: submitter_user.id).count }.by(1) + .and change { AssignmentTeam.where(parent_id: assignment.id).count }.by(1) + .and change { ReviewResponseMap.where(reviewed_object_id: assignment.id, for_calibration: true).count }.by(1) + + expect(response).to have_http_status(:created) + json = JSON.parse(response.body) + expect(json).to include('username' => 'unctlt1', 'instructor_review_status' => 'not_started') + end + + it 'returns 404 when the username does not exist' do + post participants_path, params: { username: 'nobody' }, headers: instructor_headers + expect(response).to have_http_status(:not_found) + end + + it 'is idempotent — re-adding the same username does not create duplicate maps or teams' do + submitter_user + post participants_path, params: { username: 'unctlt1' }, headers: instructor_headers + expect(response).to have_http_status(:created) + + maps_before = ReviewResponseMap.where(reviewed_object_id: assignment.id, for_calibration: true).count + teams_before = AssignmentTeam.where(parent_id: assignment.id).count + + post participants_path, params: { username: 'unctlt1' }, headers: instructor_headers + + expect(response).to have_http_status(:created) + expect(ReviewResponseMap.where(reviewed_object_id: assignment.id, for_calibration: true).count).to eq(maps_before) + expect(AssignmentTeam.where(parent_id: assignment.id).count).to eq(teams_before) + end + end + + describe 'GET /assignments/:assignment_id/reports/calibration/:map_id' do + it 'returns report JSON for a calibration map' do + create_response( + map: instructor_map, + submitted: true, + scores: { + code_quality => { answer: 4, comments: 'Clear implementation' }, + documentation => { answer: 5, comments: 'Complete docs' } + } + ) + create_response( + map: student_map, + submitted: true, + scores: { + code_quality => { answer: 3, comments: 'Mostly clear' }, + documentation => { answer: 5, comments: 'Strong docs' } + } + ) + SubmissionRecord.create!( + record_type: 'file', + content: 'submission/report.pdf', + operation: 'Submit File', + team_id: reviewee_team.id, + user: instructor.name, + assignment_id: assignment.id + ) + reviewee_team.update!(submitted_hyperlinks: YAML.dump(['https://example.com/submission'])) + + get calibration_report_path(instructor_map), headers: instructor_headers + + expect(response).to have_http_status(:ok) + + json = JSON.parse(response.body) + expect(json['map_id']).to eq(instructor_map.id) + expect(json['assignment_id']).to eq(assignment.id) + expect(json['reviewee_id']).to eq(reviewee_team.id) + expect(json['rubric_items'].map { |item| item['txt'] }).to eq(['Code quality', 'Documentation']) + expect(json['instructor_response']['answers']).to include( + hash_including('item_id' => code_quality.id, 'score' => 4, 'comments' => 'Clear implementation') + ) + expect(json['student_responses'].length).to eq(1) + expect(json['per_item_summary']).to include( + hash_including( + 'item_id' => code_quality.id, + 'item_label' => 'Code quality', + 'instructor_score' => 4, + 'bucket_counts' => hash_including('3' => 1) + ) + ) + expect(json['submitted_content']).to eq( + 'hyperlinks' => ['https://example.com/submission'], + 'files' => ['submission/report.pdf'] + ) + end + + it 'returns only the latest submitted student response for each calibration map' do + create_response( + map: instructor_map, + submitted: true, + scores: { + code_quality => { answer: 4, comments: 'Instructor score' }, + documentation => { answer: 5, comments: 'Instructor docs' } + } + ) + create_response( + map: student_map, + submitted: true, + updated_at: 2.days.ago, + scores: { + code_quality => { answer: 2, comments: 'Older response' }, + documentation => { answer: 3, comments: 'Older docs' } + } + ) + create_response( + map: student_map, + submitted: true, + updated_at: 1.day.ago, + scores: { + code_quality => { answer: 5, comments: 'Latest response' }, + documentation => { answer: 4, comments: 'Latest docs' } + } + ) + + get calibration_report_path(instructor_map), headers: instructor_headers + + expect(response).to have_http_status(:ok) + + json = JSON.parse(response.body) + expect(json['student_responses'].length).to eq(1) + expect(json['student_responses'].first['answers']).to include( + hash_including('item_id' => code_quality.id, 'score' => 5, 'comments' => 'Latest response'), + hash_including('item_id' => documentation.id, 'score' => 4, 'comments' => 'Latest docs') + ) + expect(json['per_item_summary']).to include( + hash_including( + 'item_id' => code_quality.id, + 'bucket_counts' => hash_including('5' => 1, '2' => 0) + ) + ) + end + + it 'returns 404 when the calibration map does not exist' do + get "/assignments/#{assignment.id}/reports/calibration/0", headers: instructor_headers + + expect(response).to have_http_status(:not_found) + expect(JSON.parse(response.body)['error']).to eq('Calibration review map not found') + end + + it 'returns 422 when the instructor response has not been submitted' do + create_response(map: instructor_map, submitted: false, scores: { code_quality => { answer: 4 } }) + + get calibration_report_path(instructor_map), headers: instructor_headers + + expect(response).to have_http_status(:unprocessable_entity) + expect(JSON.parse(response.body)['error']).to eq('Submitted instructor calibration response not found') + end + + it 'returns 403 for a student who is not teaching staff for the assignment' do + create_response(map: instructor_map, submitted: true, scores: { code_quality => { answer: 4 } }) + + get calibration_report_path(instructor_map), headers: auth_headers_for(student_user) + + expect(response).to have_http_status(:forbidden) + end + end + + def calibration_report_path(map) + "/assignments/#{assignment.id}/reports/calibration/#{map.id}" + end + + def auth_headers_for(user) + { 'Authorization' => "Bearer #{JsonWebToken.encode(id: user.id)}" } + end + + def create_user(role:, name:) + User.create!( + name: name, + email: "#{name}@example.com", + password: 'password', + full_name: name.titleize, + role: role, + institution: institution + ) + end + + def create_questionnaire + Questionnaire.create!( + name: 'Calibration Rubric', + private: false, + min_question_score: 0, + max_question_score: 5, + instructor: Instructor.find(instructor.id) + ) + end + + def create_item(txt, seq) + item = Item.create!( + questionnaire: questionnaire, + txt: txt, + seq: seq, + weight: 1, + question_type: 'Scale', + break_before: true + ) + item.update!(seq: seq) + item + end + + def create_response(map:, submitted:, scores:, updated_at: Time.current) + response = Response.create!( + response_map: map, + round: 1, + version_num: 1, + is_submitted: submitted, + created_at: updated_at, + updated_at: updated_at + ) + + scores.each do |item, score| + Answer.create!( + response: response, + item: item, + answer: score[:answer], + comments: score[:comments] + ) + end + + response + end +end + +# --------------------------------------------------------------------------- +# Service unit tests for Reports::CalibrationReport (bucket logic in isolation) +# --------------------------------------------------------------------------- +RSpec.describe Reports::CalibrationReport do + let(:assignment) { create(:assignment) } + let(:reviewee_team) { create(:assignment_team, assignment: assignment) } + let(:instructor_participant) { create(:assignment_participant, assignment: assignment) } + let(:instructor_map) do + create(:review_response_map, :for_calibration, + assignment: assignment, reviewer: instructor_participant, reviewee: reviewee_team) + end + let(:questionnaire) do + Questionnaire.create!( + name: "Calibration Rubric #{SecureRandom.hex(4)}", + private: false, min_question_score: 0, max_question_score: 5, + instructor: Instructor.find(instructor_participant.user_id) + ) + end + let!(:code_quality) { create(:item, questionnaire: questionnaire, txt: 'Code quality', seq: 1) } + let!(:documentation) { create(:item, questionnaire: questionnaire, txt: 'Documentation', seq: 2) } + + before { AssignmentQuestionnaire.create!(assignment: assignment, questionnaire: questionnaire, used_in_round: 1) } + + def make_response(map:, submitted:, scores:, updated_at: Time.current) + r = Response.create!(response_map: map, round: 1, version_num: 1, + is_submitted: submitted, created_at: updated_at, updated_at: updated_at) + scores.each { |item, score| Answer.create!(response: r, item: item, answer: score[:answer], comments: score[:comments]) } + r + end + + it 'accumulates bucket counts from submitted student responses' do + make_response(map: instructor_map, submitted: true, + scores: { code_quality => { answer: 4 }, documentation => { answer: 5 } }) + student_map = create(:review_response_map, :for_calibration, assignment: assignment, + reviewer: create(:assignment_participant, assignment: assignment), reviewee: reviewee_team) + make_response(map: student_map, submitted: true, + scores: { code_quality => { answer: 3, comments: 'ok' }, documentation => { answer: 5 } }) + + result = described_class.new(instructor_map).render + cq = result[:per_item_summary].find { |s| s[:item_id] == code_quality.id } + expect(cq[:instructor_score]).to eq(4) + expect(cq[:bucket_counts]['3']).to eq(1) + expect(cq[:bucket_counts]['4']).to eq(0) + end + + it 'uses only the latest submitted response per student map' do + make_response(map: instructor_map, submitted: true, + scores: { code_quality => { answer: 4 }, documentation => { answer: 5 } }) + student_map = create(:review_response_map, :for_calibration, assignment: assignment, + reviewer: create(:assignment_participant, assignment: assignment), reviewee: reviewee_team) + make_response(map: student_map, submitted: true, updated_at: 2.days.ago, + scores: { code_quality => { answer: 1 }, documentation => { answer: 2 } }) + make_response(map: student_map, submitted: true, updated_at: 1.day.ago, + scores: { code_quality => { answer: 3 }, documentation => { answer: 2 } }) + make_response(map: student_map, submitted: false, updated_at: 1.hour.ago, + scores: { code_quality => { answer: 5 }, documentation => { answer: 5 } }) + + result = described_class.new(instructor_map).render + cq = result[:per_item_summary].find { |s| s[:item_id] == code_quality.id } + expect(cq[:bucket_counts]['3']).to eq(1) + expect(cq[:bucket_counts]['1']).to eq(0) + expect(cq[:bucket_counts]['5']).to eq(0) + end + + it 'raises InstructorResponseMissing when the instructor has not submitted' do + expect { described_class.new(instructor_map).render } + .to raise_error(Reports::CalibrationReport::InstructorResponseMissing) + end +end