Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
7e593fa
Added response type mapping hash, response.kind_name and unit tests
Oct 26, 2025
05ce778
Merge pull request #21 from bestinlalu/feature/response-type-mapping
bestinlalu Oct 26, 2025
21c2f4c
Updated score calculation logic to ensure accurate scoring based on u…
Oct 26, 2025
f903ff4
Merge branch 'main' of https://github.com/bestinlalu/reimplementation…
Oct 26, 2025
dfba845
[#4, #5, #6, #7, #8, #9, #20] Implement ResponseController class with…
bestinlalu Oct 27, 2025
a9732e4
Merge pull request #22 from bestinlalu/issue-4
YuWang1925wy Oct 27, 2025
1067bc9
Merge branch 'main' of https://github.com/bestinlalu/reimplementation…
Oct 27, 2025
a7b4311
Merge branch 'main' of https://github.com/bestinlalu/reimplementation…
Oct 28, 2025
ab3abbc
Updated test for deadline and update link logic
Oct 28, 2025
778de56
Removed the next action api
Oct 28, 2025
4c101db
Added the tests for the update logic
Oct 28, 2025
ea4eaae
removed next action
Oct 29, 2025
77138e3
Merge pull request #23 from bestinlalu/ScoreLogic
surabhi1914 Oct 29, 2025
b0aec87
Removed save_draft endpoint, Removed using Reviewer role, Updated rspec
bestinlalu Oct 29, 2025
a1c620d
Merge branch 'main' into issue-4
bestinlalu Oct 29, 2025
8bb157c
Merge pull request #24 from bestinlalu/issue-4
surabhi1914 Oct 29, 2025
9535b2f
Adding comment for test cases in kind name spec
bestinlalu Oct 29, 2025
ec5e0ab
edited case descriptions
Oct 29, 2025
e5bb8e4
Merge pull request #25 from bestinlalu/feature/response-type-mapping
YuWang1925wy Oct 29, 2025
d268e98
Removing mock data from rails_helper
bestinlalu Oct 29, 2025
1015d4c
Merge pull request #26 from bestinlalu/final-tweaks
bestinlalu Oct 29, 2025
9222eb7
Code clean up, removing output statements
bestinlalu Oct 29, 2025
fdc02a1
Merge pull request #27 from bestinlalu/final-tweaks
bestinlalu Oct 29, 2025
3178c85
Increase puma version to 6.4.2
bestinlalu Oct 29, 2025
d79fe2a
Merge pull request #28 from bestinlalu/final-tweaks
bestinlalu Oct 29, 2025
aff9052
Update the swagger with new response endpoints
bestinlalu Oct 29, 2025
11f4634
Merge pull request #29 from bestinlalu/final-tweaks
bestinlalu Oct 29, 2025
b1de8bd
Added small tweeks to comments for clarity and completeness.
Oct 31, 2025
281f33f
Merge branch 'main' of https://github.com/bestinlalu/reimplementation…
Oct 31, 2025
02d702d
Commented out the 80th line as requested
surabhi1914 Nov 3, 2025
f49b5d3
fix model tests error for PR #227
Nov 4, 2025
fb9b315
Score calculation test cases clean up
bestinlalu Nov 5, 2025
afa1c61
Merge pull request #30 from bestinlalu/final-tweaks
bestinlalu Nov 5, 2025
005c1ac
Changes as per few review comments in the response_controller
bestinlalu Nov 13, 2025
391d244
Merge pull request #31 from bestinlalu/final-tweaks
bestinlalu Nov 13, 2025
7377636
done requested changes related to response type mapping
Nov 14, 2025
6de4fe3
Merge pull request #32 from bestinlalu/pr-fixes
YuWang1925wy Nov 14, 2025
8fcb4ae
Changes as per few review comments in the response_controller
bestinlalu Nov 14, 2025
75bfed7
Merge pull request #33 from bestinlalu/final-tweaks
bestinlalu Nov 14, 2025
a166726
1. updated Response controller specs 2. renamed response map helpers …
Nov 14, 2025
921b75e
Merge pull request #34 from bestinlalu/pr-fixes
YuWang1925wy Nov 14, 2025
d0f2c54
Merge branch 'main' into main
ishani-rajput Nov 30, 2025
d913551
Refactor responses_controller: Add assignment ownership checks and im…
ishani-rajput Nov 30, 2025
8349a11
Auth: allow instructor, TA, and parent-admin; use questionnaire label…
ishani-rajput Dec 3, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ GEM
date
stringio
public_suffix (6.0.2)
puma (6.6.1)
puma (6.4.2)
nio4r (~> 2.0)
racc (1.8.1)
rack (3.2.1)
Expand Down
216 changes: 216 additions & 0 deletions app/controllers/responses_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
# frozen_string_literal: true

class ResponsesController < ApplicationController
before_action :set_response, only: [:update, :submit, :unsubmit, :destroy]

# Authorization: determines if current user can perform the action
def action_allowed?
case action_name
when 'create'
true
when 'update', 'submit'
@response = Response.find(params[:id])
unless response_belongs_to? || current_user_is_parent_of_assignment_instructor_for_response? ||
current_user_is_teaching_staff_for_response_assignment?
render json: { error: 'forbidden' }, status: :forbidden
end
when 'unsubmit', 'destroy'
# Only allow if user is the instructor of the associated assignment or has admin privileges
unless current_user_is_parent_of_assignment_instructor_for_response? ||
current_user_is_teaching_staff_for_response_assignment?
render json: { error: 'forbidden' }, status: :forbidden
end
else
render json: { error: 'forbidden' }, status: :forbidden
end
true
end

# POST /responses
def create
@response_map = ResponseMap.find_by(id: params[:response_map_id] || params[:map_id])
return render json: { error: 'ResponseMap not found' }, status: :not_found unless @response_map

@response = Response.new(
map_id: @response_map.id,
is_submitted: false,
created_at: Time.current
)

if @response.save
render json: { message: "#{response_map_label} submission started successfully", response: @response }, status: :created
else
render json: { error: @response.errors.full_messages.to_sentence }, status: :unprocessable_entity
end
end

# PATCH /responses/:id
# Reviewer edits existing draft (still unsubmitted)
def update
return render json: { error: 'forbidden' }, status: :forbidden if @response.is_submitted?

if @response.update(response_params)
render json: { message: "#{response_map_label} submission saved successfully", response: @response }, status: :ok
else
render json: { error: @response.errors.full_messages.to_sentence }, status: :unprocessable_entity
end
end

# PATCH /responses/:id/submit
# Lock the response and calculate final score
def submit
return render json: { error: 'Submission not found' }, status: :not_found unless @response
if @response.is_submitted?
return render json: { error: 'Submission has already been locked' }, status: :unprocessable_entity
end
# Check deadline
unless submission_window_open?(@response)
return render json: { error: 'Submission deadline has passed' }, status: :forbidden
end

# Lock response
@response.is_submitted = true

# Calculate score via ScorableHelper
total_score = @response.aggregate_questionnaire_score

if @response.save
render json: {
message: "#{response_map_label} submission locked and scored successfully",
response: @response,
total_score: total_score
}, status: :ok
else
render json: { error: @response.errors.full_messages.to_sentence }, status: :unprocessable_entity
end
end

# PATCH /responses/:id/unsubmit
# Instructor/Admin reopens a submitted response for further editing
def unsubmit
return render json: { error: "#{response_map_label} submission not found" }, status: :not_found unless @response

if @response.is_submitted?
@response.update(is_submitted: false)
render json: { message: "#{response_map_label} submission reopened for edits. The reviewer can now make changes.", response: @response }, status: :ok
else
render json: { error: "This #{response_map_label.downcase} submission is not locked, so it cannot be reopened" }, status: :unprocessable_entity
end
end

# DELETE /responses/:id
# Instructor/Admin deletes invalid/test response
def destroy
return render json: { error: 'Submission not found' }, status: :not_found unless @response

@response.destroy
head :no_content
rescue StandardError => e
render json: { error: e.message }, status: :unprocessable_entity
end

private

def set_response
@response = Response.find_by(id: params[:id])
end

def response_params
params.require(:response).permit(
:map_id,
:is_submitted,
:submitted_at,
scores_attributes: [:id, :question_id, :answer, :comment]
)
end

def response_belongs_to?
# Member actions: we have @response from set_response
return @response.map&.reviewer&.id == current_user.id if @response&.map&.reviewer && current_user

# Collection actions (create, next_action): check map ownership
map_id = params[:response_map_id] || params[:map_id]
return false if map_id.blank?

map = ResponseMap.find_by(id: map_id)
return false unless map

map.reviewer == current_user
end

# Checks whether the current_user is the instructor for the assignment
# associated with the response identified by params[:id].
# Uses the shared authorization method from Authorization concern.
def current_user_instructs_response_assignment?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not only the instructor, but also TAs for the course and the admin who created the instructor, need to be able to perform this method.

resp = Response.find_by(id: params[:id])
return false unless resp&.response_map

assignment = resp.response_map&.assignment
return false unless assignment

# Delegate to the shared authorization helper
current_user_instructs_assignment?(assignment)
end

# Returns true if current user is teaching staff for the assignment associated
# with the current response (instructor or TA mapped to the assignment's course)
def current_user_is_teaching_staff_for_response_assignment?
resp = Response.find_by(id: params[:id])
return false unless resp&.response_map

assignment = resp.response_map&.assignment
return false unless assignment

# Uses Authorization concern helper to check instructor OR TA mapping
current_user_teaching_staff_of_assignment?(assignment.id)
end

# Returns true if the current user is the parent (creator) of the instructor
# for the assignment associated with the current response (params[:id]).
def current_user_is_parent_of_assignment_instructor_for_response?
resp = Response.find_by(id: params[:id])
return false unless resp&.response_map

assignment = resp.response_map&.assignment
return false unless assignment

instructor = find_assignment_instructor(assignment)
return false unless instructor

user_logged_in? && instructor.parent_id == current_user.id
end

# Returns the friendly label for the response's map type (e.g., "Review", "Assignment Survey")
# Falls back to a generic "Submission" if the label cannot be determined.
def response_map_label
return 'Submission' unless @response&.response_map

map_label = @response.response_map&.response_map_label
map_label.presence || 'Submission'
end

# Returns true if the assignment's due date is in the future or no due date is set
def submission_window_open?(response)
assignment = response&.response_map&.assignment
return true if assignment.nil?
return true if assignment.due_dates.nil?

# Check if due_date has a future? method, otherwise compare timestamps
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is an "upcoming" method that does what you think future? does.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "upcoming" is a better name.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's E2566, finish DueDates, who is using the upcoming method.

due_dates = assignment.due_dates
# Prefer the `upcoming` API if available
if due_dates.respond_to?(:upcoming)
next_due = due_dates.upcoming.first
return true if next_due.nil?
return next_due.due_at > Time.current
end
# Fallback to legacy `future?` if present
if due_dates.respond_to?(:future?)
return due_dates.first.future?
end

# Fallback: compare timestamps
return true if due_dates.first.nil?

due_dates.first.due_at > Time.current
end
end
3 changes: 2 additions & 1 deletion app/models/concerns/response_map_subclass_titles.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ module ResponseMapSubclassTitles
METAREVIEW_RESPONSE_MAP_TITLE = 'Metareview'
QUIZ_RESPONSE_MAP_TITLE = 'Quiz'
REVIEW_RESPONSE_MAP_TITLE = 'Review'
SURVEY_RESPONSE_MAP_TITLE = 'Survey'
TEAMMATE_REVIEW_RESPONSE_MAP_TITLE = 'Teammate Review'
end
end
45 changes: 38 additions & 7 deletions app/models/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class Response < ApplicationRecord

belongs_to :response_map, class_name: 'ResponseMap', foreign_key: 'map_id', inverse_of: false
has_many :scores, class_name: 'Answer', foreign_key: 'response_id', dependent: :destroy, inverse_of: false
accepts_nested_attributes_for :scores

alias map response_map
delegate :response_assignment, :reviewee, :reviewer, to: :map
Expand All @@ -15,32 +16,62 @@ def questionnaire
response_assignment.assignment_questionnaires.find_by(used_in_round: self.round).questionnaire
end

# returns a string of response name, needed so the front end can tell students which rubric they are filling out
def rubric_label
return 'Response' if map.nil?

if map.respond_to?(:response_map_label)
label = map.response_map_label
return label if label.present?
end

# response type doesn't exist
'Unknown Type'
end

# Returns true if this response's score differs from peers by more than the assignment notification limit
def reportable_difference?
map_class = map.class
# gets all responses made by a reviewee
existing_responses = map_class.assessments_for(map.reviewee)

count = 0
total = 0
count = 0
total_numerator = BigDecimal('0')
total_denominator = BigDecimal('0')
# gets the sum total percentage scores of all responses that are not this response
# (each response can omit questions, so maximum_score may differ and we normalize before averaging)
existing_responses.each do |response|
unless id == response.id # the current_response is also in existing_responses array
count += 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be in the ReviewAggregator mixin?

total += response.aggregate_questionnaire_score.to_f / response.maximum_score
# Accumulate raw sums and divide once to minimize rounding error
total_numerator += BigDecimal(response.aggregate_questionnaire_score.to_s)
total_denominator += BigDecimal(response.maximum_score.to_s)
end
end

# if this response is the only response by the reviewee, there's no grade conflict
return false if count.zero?

# calculates the average score of all other responses
average_score = total / count
# Calculate average of peers by dividing once at the end
average_score = if total_denominator.zero?
0.0
else
(total_numerator / total_denominator).to_f
end

# This score has already skipped the unfilled scorable item(s)
score = aggregate_questionnaire_score.to_f / maximum_score
# Normalize this response similarly, dividing once
this_numerator = BigDecimal(aggregate_questionnaire_score.to_s)
this_denominator = BigDecimal(maximum_score.to_s)
score = if this_denominator.zero?
0.0
else
(this_numerator / this_denominator).to_f
end
questionnaire = questionnaire_by_answer(scores.first)
assignment = map.assignment
assignment_questionnaire = AssignmentQuestionnaire.find_by(assignment_id: assignment.id, questionnaire_id: questionnaire.id)
assignment_questionnaire = AssignmentQuestionnaire.find_by(assignment_id: assignment.id,
questionnaire_id: questionnaire.id)

# notification_limit can be specified on 'Rubrics' tab on assignment edit page.
allowed_difference_percentage = assignment_questionnaire.notification_limit.to_f
Expand Down
Loading