diff --git a/Gemfile.lock b/Gemfile.lock index 1570d2590..ff4ac5ceb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -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) diff --git a/app/controllers/responses_controller.rb b/app/controllers/responses_controller.rb new file mode 100644 index 000000000..ab3abf92c --- /dev/null +++ b/app/controllers/responses_controller.rb @@ -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? + 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 + 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 diff --git a/app/models/concerns/response_map_subclass_titles.rb b/app/models/concerns/response_map_subclass_titles.rb index 8c1fee2f3..32c86824f 100644 --- a/app/models/concerns/response_map_subclass_titles.rb +++ b/app/models/concerns/response_map_subclass_titles.rb @@ -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 \ No newline at end of file +end diff --git a/app/models/response.rb b/app/models/response.rb index 1dd9ba045..d2a2e55d9 100644 --- a/app/models/response.rb +++ b/app/models/response.rb @@ -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 @@ -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 - 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 diff --git a/app/models/response_map.rb b/app/models/response_map.rb index 5657741cf..d66ed1286 100644 --- a/app/models/response_map.rb +++ b/app/models/response_map.rb @@ -8,19 +8,45 @@ class ResponseMap < ApplicationRecord alias map_id id + # Shared helper for Response#rubric_label; looks up the declarative constant so each map advertises its UI label + def response_map_label + const_name = "#{self.class.name.demodulize.underscore.upcase}_TITLE" + if ResponseMapSubclassTitles.const_defined?(const_name) + ResponseMapSubclassTitles.const_get(const_name).presence + end + end + def questionnaire Questionnaire.find_by(id: reviewed_object_id) end - # returns the assignment related to the response map def response_assignment - # reviewer will always be the Assignment Participant so finding Assignment based on reviewer_id. - return reviewer.assignment + Participant.find(reviewer_id).assignment + end + + # Decide whether the reviewer should see an "Update" button (something new to review) + # or the default "Edit" button (no changes since the last submitted review). + def needs_update_link? + # Most recent submitted review for this mapping + last = Response.where(map_id: id, is_submitted: true).order(Arel.sql('created_at DESC')).first + return true if last.nil? + + last_created_at = last.created_at + + # Latest time the reviewee (or their team) made a submission + latest_submission = latest_submission_at_for_reviewee + return true if latest_submission.present? && latest_submission > last_created_at + + # Check if a later review round has passed since the last submitted review + last_round = (last.respond_to?(:round, true) ? last.round : 0).to_i + curr_round = current_round_from_due_dates.to_i + return true if curr_round.positive? && curr_round > last_round + + false end def self.assessments_for(team) responses = [] - # stime = Time.now if team array_sort = [] sort_to = [] @@ -30,13 +56,11 @@ def self.assessments_for(team) all_resp = Response.where(map_id: map.map_id).last if map.type.eql?('ReviewResponseMap') - # If its ReviewResponseMap then only consider those response which are submitted. array_sort << all_resp if all_resp.is_submitted else array_sort << all_resp end - # sort all versions in descending order and get the latest one. - sort_to = array_sort.sort # { |m1, m2| (m1.updated_at and m2.updated_at) ? m2.updated_at <=> m1.updated_at : (m1.version_num ? -1 : 1) } + sort_to = array_sort.sort responses << sort_to[0] unless sort_to[0].nil? array_sort.clear sort_to.clear @@ -46,37 +70,56 @@ def self.assessments_for(team) responses end - # Computes the average score (as a fraction between 0 and 1) across the latest submitted responses - # from each round for this ReviewResponseMap. - def aggregate_reviewers_score - # Return nil if there are no responses for this map - return nil if responses.empty? - - # Group all responses by round, then select the latest one per round based on the most recent created one (i.e., most recent revision in that round) - latest_responses_by_round = responses - .group_by(&:round) - .transform_values { |resps| resps.max_by(&:updated_at) } - - response_score = 0.0 # Sum of actual scores obtained - total_score = 0.0 # Sum of maximum possible scores - submitted_found = false #flag to track if any submitted response exists - - # For each latest response in each round, if the response was submitted, sum up its earned score and its maximum possible score. - latest_responses_by_round.each_value do |response| - # Only consider responses that were submitted - next unless response.is_submitted - - submitted_found = true # true if a submitted response is found - - # Accumulate the obtained and maximum scores - response_score += response.aggregate_questionnaire_score - total_score += response.maximum_score + def survey? + false + end + + # Best-effort timestamp of when the reviewee (or their team) last touched the work. + def latest_submission_at_for_reviewee + return nil unless reviewee + + candidates = [] + # Use respond_to? because legacy records might not have timestamps (or reviewee could be a Team instead of Participant) + candidates << reviewee.updated_at if reviewee.respond_to?(:updated_at) && reviewee.updated_at.present? + + # Check team-related timestamps if the reviewee has a team + if reviewee.respond_to?(:team) && reviewee.team + team = reviewee.team + candidates << team.updated_at if team.respond_to?(:updated_at) && team.updated_at.present? + + # Also gather timestamps from join records (teams_participants) so collaborator edits count as activity + if team.respond_to?(:teams_participants) + team.teams_participants.each do |tp| + candidates << tp.updated_at if tp.respond_to?(:updated_at) && tp.updated_at.present? + end + end end - # If no submitted responses at all, return nil - return nil unless submitted_found + candidates.compact.max + end + + # Infer the current review round from due dates when the assignment doesn’t provide it directly. + def current_round_from_due_dates + return 0 unless assignment - # Return the normalized score (as a float), or 0 if no valid total score - total_score > 0 ? (response_score.to_f / total_score) : 0 + # Gather all due dates with round and due_at + + due_dates = Array(assignment.due_dates).select do |d| + d.respond_to?(:round) && d.round.present? && + d.respond_to?(:due_at) && d.due_at.present? + end + return 0 if due_dates.empty? + + # Find the latest due date that is in the past (or the earliest if none are in the past) + past = due_dates.select { |d| d.due_at <= Time.current } + + # Use the latest past due date if available, otherwise the earliest future due date + reference = + if past.any? + past.sort_by(&:due_at).last + else + due_dates.sort_by(&:due_at).first + end + reference.round.to_i end end diff --git a/config/routes.rb b/config/routes.rb index 25642363c..6101b2d86 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -149,4 +149,10 @@ get '/:participant_id/instructor_review', to: 'grades#instructor_review' end end -end \ No newline at end of file + resources :responses do + member do + patch :submit # PATCH /responses/:id/submit + patch :unsubmit # PATCH /responses/:id/unsubmit + end + end +end diff --git a/spec/controllers/responses_controller_spec.rb b/spec/controllers/responses_controller_spec.rb new file mode 100644 index 000000000..8bcacc63b --- /dev/null +++ b/spec/controllers/responses_controller_spec.rb @@ -0,0 +1,307 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ResponsesController, type: :controller do + let(:user) { instance_double('User', id: 10) } + + before do + # Prevent before_action from blocking tests: stub the authorize before_action defined in ApplicationController + allow(controller).to receive(:authorize).and_return(true) + + # Provide a current_user for controller methods that rely on it + # Set a fake Authorization header and stub token decoding + User.find so authenticate_request! succeeds without DB + request.headers['Authorization'] = 'Bearer faketoken' + allow(JsonWebToken).to receive(:decode).and_return({ id: user.id }) + allow(User).to receive(:find).with(user.id).and_return(user) + # NOTE: not stubbing authenticate_request! so the controller's JwtToken behavior runs but uses the above stubs + + # Also provide current_user directly to be safe + allow(controller).to receive(:current_user).and_return(user) + + # Stub role helpers used by ResponsesController + allow(controller).to receive(:has_role?).and_return(true) + allow(controller).to receive(:action_allowed?).and_return(true) + end + + describe 'POST #create' do + let(:response_map) { double('ResponseMap', id: 123) } + let(:response_double) { double('Response') } + + before do + allow(ResponseMap).to receive(:find_by).and_return(response_map) + allow(Response).to receive(:new).and_return(response_double) + allow(response_double).to receive(:as_json).and_return({ 'id' => 1 }) + end + + context 'when save succeeds' do + before do + allow(response_double).to receive(:save).and_return(true) + end + + it 'creates a draft and returns 201' do + expect(Response).to receive(:new).with(hash_including(map_id: response_map.id, is_submitted: false)) + post :create, params: { response_map_id: response_map.id } + expect(response).to have_http_status(:created) + body = JSON.parse(response.body) + expect(body['message']).to eq('Response draft created successfully') + expect(body['response']).to eq({ 'id' => 1 }) + end + end + + context 'when save fails' do + before do + allow(response_double).to receive(:save).and_return(false) + allow(response_double).to receive_message_chain(:errors, :full_messages).and_return(['err']) + end + + it 'returns 422 with error message' do + post :create, params: { response_map_id: response_map.id } + expect(response).to have_http_status(:unprocessable_entity) + body = JSON.parse(response.body) + expect(body['error']).to include('err') + end + end + + context 'when response_map not found' do + before do + allow(ResponseMap).to receive(:find_by).and_return(nil) + end + + it 'returns 404' do + post :create, params: { response_map_id: 9999 } + expect(response).to have_http_status(:not_found) + body = JSON.parse(response.body) + expect(body['error']).to eq('ResponseMap not found') + end + end + end + + describe 'PATCH #update' do + let(:response_double) { double('Response') } + + before do + allow(controller).to receive(:set_response) { controller.instance_variable_set(:@response, response_double) } + allow(response_double).to receive(:as_json).and_return({ 'id' => 1 }) + allow(response_double).to receive(:update) + end + + context 'when response already submitted' do + before do + allow(response_double).to receive(:is_submitted?).and_return(true) + end + + it 'returns forbidden' do + patch :update, params: { id: 1, response: { content: 'x' } } + expect(response).to have_http_status(:forbidden) + body = JSON.parse(response.body) + expect(body['error']).to eq('forbidden') + end + end + + context 'when update succeeds' do + before do + allow(response_double).to receive(:is_submitted?).and_return(false) + allow(response_double).to receive(:update).and_return(true) + end + + it 'returns ok and success message' do + patch :update, params: { id: 1, response: { content: 'x' } } + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + expect(body['message']).to eq('Draft updated successfully') + end + end + + context 'when update fails' do + before do + allow(response_double).to receive(:is_submitted?).and_return(false) + allow(response_double).to receive(:update).and_return(false) + allow(response_double).to receive_message_chain(:errors, :full_messages).and_return(['bad']) + end + + it 'returns unprocessable_entity with errors' do + patch :update, params: { id: 1, response: { content: 'x' } } + expect(response).to have_http_status(:unprocessable_entity) + body = JSON.parse(response.body) + expect(body['error']).to include('bad') + end + end + end + + describe 'PATCH #submit' do + let(:response_double) { double('Response') } + + before do + allow(controller).to receive(:set_response) { controller.instance_variable_set(:@response, response_double) } + end + + context 'when response not found' do + before do + allow(controller).to receive(:set_response) { controller.instance_variable_set(:@response, nil) } + end + + it 'returns 404' do + patch :submit, params: { id: 1 } + expect(response).to have_http_status(:not_found) + end + end + + context 'when already submitted' do + before do + allow(response_double).to receive(:is_submitted?).and_return(true) + end + + it 'returns 422 with already submitted message' do + patch :submit, params: { id: 1 } + expect(response).to have_http_status(:unprocessable_entity) + body = JSON.parse(response.body) + expect(body['error']).to eq('User has already submitted the response') + end + end + + context 'when rubric incomplete' do + before do + allow(response_double).to receive(:is_submitted?).and_return(false) + allow(controller).to receive(:submission_window_open?).with(response_double).and_return(true) + allow(response_double).to receive(:scores).and_return([double('Score', answer: nil)]) + end + + it 'returns 422 with rubric error' do + patch :submit, params: { id: 1 } + expect(response).to have_http_status(:unprocessable_entity) + body = JSON.parse(response.body) + expect(body['error']).to eq('All rubric items must be answered') + end + end + + context 'when deadline has passed' do + before do + allow(response_double).to receive(:is_submitted?).and_return(false) + allow(controller).to receive(:submission_window_open?).with(response_double).and_return(false) + end + + it 'returns forbidden with deadline message' do + patch :submit, params: { id: 1 } + expect(response).to have_http_status(:forbidden) + body = JSON.parse(response.body) + expect(body['error']).to eq('Deadline has passed') + end + end + + context 'when submitting twice (duplicate submission)' do + before do + # first call: not submitted, second call: already submitted + allow(controller).to receive(:submission_window_open?).with(response_double).and_return(true) + allow(response_double).to receive(:is_submitted?).and_return(false, true) + allow(response_double).to receive(:scores).and_return([]) + allow(response_double).to receive(:aggregate_questionnaire_score).and_return(42) + allow(response_double).to receive(:save).and_return(true) + allow(response_double).to receive(:as_json).and_return({ 'id' => 99 }) + allow(response_double).to receive(:is_submitted=).with(true) + end + + it 'allows the first submission and rejects the second' do + patch :submit, params: { id: 1 } + expect(response).to have_http_status(:ok) + + # second attempt should be blocked as already submitted + patch :submit, params: { id: 1 } + expect(response).to have_http_status(:unprocessable_entity) + body = JSON.parse(response.body) + expect(body['error']).to eq('User has already submitted the response') + end + end + end + + describe 'DELETE #destroy' do + let(:response_double) { double('Response') } + + before do + allow(controller).to receive(:set_response) { controller.instance_variable_set(:@response, response_double) } + end + + context 'when response not found' do + before do + allow(controller).to receive(:set_response) { controller.instance_variable_set(:@response, nil) } + end + + it 'returns 404' do + delete :destroy, params: { id: 1 } + expect(response).to have_http_status(:not_found) + end + end + + context 'when destroy succeeds' do + before do + allow(response_double).to receive(:destroy).and_return(true) + end + + it 'returns no content' do + delete :destroy, params: { id: 1 } + expect(response).to have_http_status(:no_content) + end + end + + context 'when destroy raises' do + before do + allow(response_double).to receive(:destroy).and_raise(StandardError.new('boom')) + end + + it 'returns unprocessable_entity with error message' do + delete :destroy, params: { id: 1 } + expect(response).to have_http_status(:unprocessable_entity) + body = JSON.parse(response.body) + expect(body['error']).to include('boom') + end + end + end + + describe 'PATCH #unsubmit' do + let(:response_double) { double('Response') } + + before do + allow(controller).to receive(:set_response) { controller.instance_variable_set(:@response, response_double) } + allow(controller).to receive(:has_role?).and_return(true) + end + + context 'when response not found' do + before do + allow(controller).to receive(:set_response) { controller.instance_variable_set(:@response, nil) } + end + + it 'returns 404' do + patch :unsubmit, params: { id: 1 } + expect(response).to have_http_status(:not_found) + end + end + + context 'when response is submitted' do + before do + allow(response_double).to receive(:is_submitted?).and_return(true) + allow(response_double).to receive(:update).with(is_submitted: false).and_return(true) + allow(response_double).to receive(:as_json).and_return({ 'id' => 1 }) + end + + it 'reopens the response and returns ok' do + patch :unsubmit, params: { id: 1 } + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + expect(body['message']).to eq('User response has been unsubmitted') + end + end + + context 'when response already unsubmitted' do + before do + allow(response_double).to receive(:is_submitted?).and_return(false) + end + + it 'returns unprocessable_entity' do + patch :unsubmit, params: { id: 1 } + expect(response).to have_http_status(:unprocessable_entity) + body = JSON.parse(response.body) + expect(body['error']).to eq('User response already unsubmitted') + end + end + end +end diff --git a/spec/factories/reponse.rb b/spec/factories/reponse.rb new file mode 100644 index 000000000..5959cf890 --- /dev/null +++ b/spec/factories/reponse.rb @@ -0,0 +1,8 @@ +FactoryBot.define do + factory :response do + map_id { 1 } + is_submitted { false } + created_at { Time.current } + updated_at { Time.current } + end +end diff --git a/spec/factories/response_maps.rb b/spec/factories/response_maps.rb new file mode 100644 index 000000000..678d7c3ce --- /dev/null +++ b/spec/factories/response_maps.rb @@ -0,0 +1,12 @@ +FactoryBot.define do + factory :response_map do + reviewed_object_id { 1 } + reviewer_id { 1 } + reviewee_id { 1 } + type { 'ResponseMap' } + end + + factory :review_response_map, class: 'ReviewResponseMap', parent: :response_map do + type { 'ReviewResponseMap' } + end +end diff --git a/spec/models/response_map_label_spec.rb b/spec/models/response_map_label_spec.rb new file mode 100644 index 000000000..28369de13 --- /dev/null +++ b/spec/models/response_map_label_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ResponseMap, type: :model do + describe '#response_map_label' do + subject(:label) { map.response_map_label } + + context 'when the subclass defines a title constant' do + let(:map) { TeammateReviewResponseMap.new } + + it { is_expected.to eq(ResponseMapSubclassTitles::TEAMMATE_REVIEW_RESPONSE_MAP_TITLE) } + end + + context 'when no title information is available' do + let(:map) { ResponseMap.new } + + it { is_expected.to be_nil } + end + end +end diff --git a/spec/models/response_map_spec.rb b/spec/models/response_map_spec.rb index 0a84ef6de..721d8b4e3 100644 --- a/spec/models/response_map_spec.rb +++ b/spec/models/response_map_spec.rb @@ -1,6 +1,134 @@ -# frozen_string_literal: true - +# spec/models/response_map_spec.rb require 'rails_helper' +require 'securerandom' RSpec.describe ResponseMap, type: :model do + describe '#needs_update_link?' do + def create_role(label) + Role.create!(name: "#{label} #{SecureRandom.hex(4)}") + end + + def create_user(prefix, role, institution) + User.create!( + name: "#{prefix}_#{SecureRandom.hex(4)}", + email: "#{prefix}_#{SecureRandom.hex(4)}@example.com", + password: 'password', + full_name: "#{prefix.capitalize} User", + role: role, + institution: institution + ) + end + + let(:institution) { Institution.create!(name: "Institution #{SecureRandom.hex(4)}") } + let(:student_role) { create_role('Student') } + let(:instructor_role) { create_role('Instructor') } + + let(:instructor_user) { create_user('instructor', instructor_role, institution) } + + let(:assignment) do + Assignment.create!( + name: "Assignment #{SecureRandom.hex(4)}", + directory_path: "dir_#{SecureRandom.hex(4)}", + instructor: instructor_user + ) + end + + let(:team) do + AssignmentTeam.create!( + name: "Team #{SecureRandom.hex(4)}", + type: 'AssignmentTeam', + max_team_size: 5, + parent_id: assignment.id + ) + end + + let(:reviewer_user) { create_user('reviewer', student_role, institution) } + let(:reviewee_user) { create_user('reviewee', student_role, institution) } + + let(:reviewer_participant) do + AssignmentParticipant.create!( + user: reviewer_user, + assignment: assignment, + parent_id: assignment.id, + handle: reviewer_user.name + ) + end + + let(:reviewee_participant) do + AssignmentParticipant.create!( + user: reviewee_user, + assignment: assignment, + parent_id: assignment.id, + handle: reviewee_user.name, + team: team + ) + end + + let!(:teams_participant_record) do + TeamsParticipant.create!(participant: reviewee_participant, team: team, user: reviewee_user) + end + + let!(:response_map) do + ResponseMap.create!( + assignment: assignment, + reviewer: reviewer_participant, + reviewee: reviewee_participant + ) + end + + let(:base_time) { Time.zone.now - 5.days } + let(:response_time) { base_time + 1.day } + + before do + reviewee_participant.update_column(:updated_at, base_time) + team.update_column(:updated_at, base_time) + teams_participant_record.update_column(:updated_at, base_time) + end + + it 'returns true when no submitted response exists yet' do + expect(response_map.needs_update_link?).to be true + end + + it 'returns false when the last submitted response is the most recent activity' do + Response.create!(map_id: response_map.id, is_submitted: true, created_at: response_time, updated_at: response_time) + + expect(response_map.needs_update_link?).to be false + end + + it 'returns true when the reviewee participant updates after the last submitted response' do + Response.create!(map_id: response_map.id, is_submitted: true, created_at: response_time, updated_at: response_time) + reviewee_participant.update_column(:updated_at, response_time + 2.days) + + expect(response_map.needs_update_link?).to be true + end + + it 'returns true when the reviewee team updates after the last submitted response' do + Response.create!(map_id: response_map.id, is_submitted: true, created_at: response_time, updated_at: response_time) + team.update_column(:updated_at, response_time + 2.days) + + expect(response_map.needs_update_link?).to be true + end + + it 'returns true when a later review round has passed since the last response' do + Response.create!(map_id: response_map.id, is_submitted: true, created_at: response_time, updated_at: response_time) + + assignment.due_dates.create!( + due_at: response_time + 1.day, + deadline_type_id: 1, + submission_allowed_id: 1, + review_allowed_id: 1, + round: 1 + ) + + assignment.due_dates.create!( + due_at: Time.zone.now - 1.day, + deadline_type_id: 1, + submission_allowed_id: 1, + review_allowed_id: 1, + round: 2 + ) + + expect(response_map.needs_update_link?).to be true + end + end end diff --git a/spec/models/response_rubric_label_spec.rb b/spec/models/response_rubric_label_spec.rb new file mode 100644 index 000000000..fcf95ec33 --- /dev/null +++ b/spec/models/response_rubric_label_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Response, type: :model do + describe '#rubric_label' do + subject(:label) { response.rubric_label } + + let(:response) { described_class.new } + + context 'when no response_map is set' do + before { response.response_map = nil } + + it { is_expected.to eq('Response') } + end + + context 'when the map exposes a title constant' do + before { response.response_map = TeammateReviewResponseMap.new } + + it { is_expected.to eq(ResponseMapSubclassTitles::TEAMMATE_REVIEW_RESPONSE_MAP_TITLE) } + end + + context 'when the map provides no title information' do + before do + stub_const('MysteryResponseMap', Class.new(ResponseMap)) + response.response_map = MysteryResponseMap.new + end + + it { is_expected.to eq('Unknown Type') } + end + end +end diff --git a/spec/models/response_spec.rb b/spec/models/response_spec.rb index 976128828..9dd906526 100644 --- a/spec/models/response_spec.rb +++ b/spec/models/response_spec.rb @@ -114,4 +114,4 @@ expect(review_response_map.response_assignment).to eq(assignment) end end -end +end \ No newline at end of file diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 4e51c9933..56c2eb94f 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -79,8 +79,12 @@ # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures # config.fixture_path = Rails.root.join('spec/fixtures') - # Since we're using Factory Bot instead of fixtures, we don't need fixture_path - # config.fixture_path is deprecated in newer RSpec versions anyway + if config.respond_to?(:fixture_paths=) + config.fixture_paths = [Rails.root.join('spec/fixtures').to_s] + else + # fallback for older Rails / rspec-rails + config.fixture_path = Rails.root.join('spec/fixtures') + end # We're using DatabaseCleaner instead of transactional fixtures # config.use_transactional_fixtures = false diff --git a/swagger/v1/swagger.yaml b/swagger/v1/swagger.yaml index cc0294e73..1ff54191c 100644 --- a/swagger/v1/swagger.yaml +++ b/swagger/v1/swagger.yaml @@ -1326,6 +1326,131 @@ paths: responses: '200': description: A specific student task + "/responses": + post: + summary: Create a new Response + tags: + - Responses + parameters: [] + responses: + '201': + description: Response created successfully + '422': + description: Invalid request payload + requestBody: + content: + application/json: + schema: + type: object + properties: + user_id: + type: integer + questionnaire_id: + type: integer + content: + type: string + required: + - user_id + - questionnaire_id + - content + + "/responses/{id}": + parameters: + - name: id + in: path + description: ID of the response + required: true + schema: + type: integer + patch: + summary: Update a Response + tags: + - Responses + responses: + '200': + description: Response updated successfully + '403': + description: Forbidden + '404': + description: Response not found + '422': + description: Invalid request payload + requestBody: + content: + application/json: + schema: + type: object + properties: + content: + type: string + delete: + summary: Delete a Response + tags: + - Responses + responses: + '204': + description: Response deleted successfully + '404': + description: Response not found + "/responses/{id}/submit": + parameters: + - name: id + in: path + description: ID of the response + required: true + schema: + type: integer + patch: + summary: Submit a Response + tags: + - Responses + responses: + '200': + description: Response updated successfully + '403': + description: Deadline has passed + '404': + description: Response not found + '422': + description: All rubric items must be answered + requestBody: + content: + application/json: + schema: + type: object + properties: + content: + type: string + "/responses/:id/unsubmit": + parameters: + - name: id + in: path + description: ID of the response + required: true + schema: + type: integer + patch: + summary: Unsubmit a Response + tags: + - Responses + responses: + '200': + description: Response reopened for revision + '403': + description: Forbidden + '404': + description: Response not found + '422': + description: Response already unsubmitted + requestBody: + content: + application/json: + schema: + type: object + properties: + content: + type: string +