From 6e2217c1dc99f88dda2a4ea387dc45d67cbd6646 Mon Sep 17 00:00:00 2001 From: riya-bihani Date: Sun, 16 Mar 2025 21:24:54 -0400 Subject: [PATCH 01/35] Update README.md --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 6c91b930b..ec3e03b74 100644 --- a/README.md +++ b/README.md @@ -27,3 +27,6 @@ alt="IMAGE ALT TEXT HERE" width="560" height="315" border="10" /> ### Database Credentials - username: root - password: expertiza + + +Project 2509 - Riya Bihani, Chandrakant Koneti, Nayan Taori From bfc98c2e28a06bf88e3ddbac3fbba86a82c8eadf Mon Sep 17 00:00:00 2001 From: ckoneti Date: Mon, 24 Mar 2025 00:57:42 -0400 Subject: [PATCH 02/35] Created feedback_response_map.rb --- app/models/feedback_response_map.rb | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 app/models/feedback_response_map.rb diff --git a/app/models/feedback_response_map.rb b/app/models/feedback_response_map.rb new file mode 100644 index 000000000..a5a5890d6 --- /dev/null +++ b/app/models/feedback_response_map.rb @@ -0,0 +1,3 @@ +class FeedbackResponseMap < ResponseMap + +end \ No newline at end of file From e86b7d6b185d8e0c4e9bc6b2887277de387e9c6d Mon Sep 17 00:00:00 2001 From: ckoneti Date: Mon, 24 Mar 2025 01:04:58 -0400 Subject: [PATCH 03/35] Added initial required methods in feedback_response_map.rb --- app/models/feedback_response_map.rb | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/app/models/feedback_response_map.rb b/app/models/feedback_response_map.rb index a5a5890d6..3beb1fd4f 100644 --- a/app/models/feedback_response_map.rb +++ b/app/models/feedback_response_map.rb @@ -1,3 +1,29 @@ class FeedbackResponseMap < ResponseMap - + belongs_to :review, class_name: 'Response', foreign_key: 'reviewed_object_id' + + # Returns the title used for display + def title + 'Feedback' + end + + # Gets the feedback questionnaire associated with the assignment + def questionnaire + review_response_map.assignment.questionnaires.find_by(type: 'AuthorFeedbackQuestionnaire') + end + + # Returns the original contributor (the author who received the review) + def contributor + review_response_map.reviewee + end + + # Returns the team being reviewed in the original review + def team + review_response_map.reviewee_team + end + + # Returns the reviewer who gave the original review + def reviewer + review_response_map.reviewer + end + end \ No newline at end of file From beabf8f34bd9f8ee94ab5f3be4786f39f390f520 Mon Sep 17 00:00:00 2001 From: ckoneti Date: Mon, 24 Mar 2025 14:45:31 -0400 Subject: [PATCH 04/35] Added round field to responses --- db/migrate/20250324184409_add_round_to_responses.rb | 5 +++++ db/schema.rb | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20250324184409_add_round_to_responses.rb diff --git a/db/migrate/20250324184409_add_round_to_responses.rb b/db/migrate/20250324184409_add_round_to_responses.rb new file mode 100644 index 000000000..79d5354fe --- /dev/null +++ b/db/migrate/20250324184409_add_round_to_responses.rb @@ -0,0 +1,5 @@ +class AddRoundToResponses < ActiveRecord::Migration[8.0] + def change + add_column :responses, :round, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 7db16863e..c7faacbf1 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: 2025_02_16_020117) do +ActiveRecord::Schema[8.0].define(version: 2025_03_24_184409) do create_table "account_requests", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "username" t.string "full_name" @@ -162,6 +162,11 @@ t.index ["parent_type", "parent_id"], name: "index_due_dates_on_parent" end + create_table "feedback_response_maps", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + create_table "institutions", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "name" t.datetime "created_at", null: false @@ -295,6 +300,7 @@ t.boolean "is_submitted", default: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "round" t.index ["map_id"], name: "fk_response_response_map" end From 818b097edde9409cc65782cabaa1e741e1c8642e Mon Sep 17 00:00:00 2001 From: ckoneti Date: Mon, 24 Mar 2025 15:44:05 -0400 Subject: [PATCH 05/35] Added dynamic round handling logic in feedback_response_map.rb --- app/models/feedback_response_map.rb | 46 +++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/app/models/feedback_response_map.rb b/app/models/feedback_response_map.rb index 3beb1fd4f..a3a91b500 100644 --- a/app/models/feedback_response_map.rb +++ b/app/models/feedback_response_map.rb @@ -1,4 +1,7 @@ class FeedbackResponseMap < ResponseMap + # old implementation improvements and corrections + # belongs_to :reviewee, class_name: 'Participant', foreign_key: 'reviewee_id' + # belongs_to :reviewer, class_name: 'AssignmentParticipant', dependent: :destroy belongs_to :review, class_name: 'Response', foreign_key: 'reviewed_object_id' # Returns the title used for display @@ -26,4 +29,47 @@ def reviewer review_response_map.reviewer end + # Returns the round number of the original review (if applicable) + def round + review_response_map&.response&.round + end + + # Returns a report of feedback responses, grouped dynamically by round + def self.feedback_response_report(assignment_id, _type) + authors = fetch_authors_for_assignment(assignment_id) + review_map_ids = ReviewResponseMap.where(reviewed_object_id: assignment_id).pluck(:id) + review_responses = Response.where(map_id: review_map_ids).order(created_at: :desc) + + if Assignment.find(assignment_id).varying_rubrics_by_round? + latest_by_map_and_round = {} + + review_responses.each do |response| + key = [response.map_id, response.round] + latest_by_map_and_round[key] ||= response + end + + grouped_by_round = latest_by_map_and_round.values.group_by(&:round) + sorted_by_round = grouped_by_round.sort.to_h # {round_number => [response1_id, response2_id, ...]} + response_ids_by_round = sorted_by_round.transform_values { |resps| resps.map(&:id) } + + [authors] + response_ids_by_round.values + else + latest_by_map = {} + + review_responses.each do |response| + latest_by_map[response.map_id] ||= response + end + + [authors, latest_by_map.values.map(&:id)] + end + end + + # Fetches all participants who authored submissions for the assignment + def self.fetch_authors_for_assignment(assignment_id) + Assignment.find(assignment_id).teams.includes(:users).flat_map do |team| + team.users.map do |user| + AssignmentParticipant.find_by(parent_id: assignment_id, user_id: user.id) + end + end.compact + end end \ No newline at end of file From 1b17dd947fd0356d01607424981c9c7cc5c38814 Mon Sep 17 00:00:00 2001 From: ckoneti Date: Mon, 24 Mar 2025 18:48:48 -0400 Subject: [PATCH 06/35] Added skeleton and initial base testcases to feedback_response_map_spec.rb --- app/models/feedback_response_map.rb | 4 +-- spec/models/feedback_response_map_spec.rb | 39 +++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 spec/models/feedback_response_map_spec.rb diff --git a/app/models/feedback_response_map.rb b/app/models/feedback_response_map.rb index a3a91b500..86e48208c 100644 --- a/app/models/feedback_response_map.rb +++ b/app/models/feedback_response_map.rb @@ -1,7 +1,7 @@ class FeedbackResponseMap < ResponseMap # old implementation improvements and corrections - # belongs_to :reviewee, class_name: 'Participant', foreign_key: 'reviewee_id' - # belongs_to :reviewer, class_name: 'AssignmentParticipant', dependent: :destroy + belongs_to :reviewee, class_name: 'Participant', foreign_key: 'reviewee_id' + belongs_to :reviewer, class_name: 'AssignmentParticipant', dependent: :destroy belongs_to :review, class_name: 'Response', foreign_key: 'reviewed_object_id' # Returns the title used for display diff --git a/spec/models/feedback_response_map_spec.rb b/spec/models/feedback_response_map_spec.rb new file mode 100644 index 000000000..ef1cc4295 --- /dev/null +++ b/spec/models/feedback_response_map_spec.rb @@ -0,0 +1,39 @@ +describe FeedbackResponseMap do + let(:questionnaire1) { Questionnaire.new(id: 1, questionnaire_type: 'AuthorFeedbackQuestionnaire') } + let(:questionnaire2) { Questionnaire.new(id: 2, questionnaire_type: 'MetareviewQuestionnaire') } + let(:participant) { Participant.new(id: 1) } + let(:assignment) { Assignment.new(id: 1) } + let(:team) { Team.new(assignment_id: assignment.id) } + let(:assignment_participant) { Participant.new(id: 2, assignment: assignment) } + let(:feedback_response_map) { FeedbackResponseMap.new } + let(:review_response_map) { ReviewResponseMap.new(id: 2, assignment: assignment, reviewer: participant, reviewee: team) } + let(:answer) { Answer.new(answer: 1, comments: 'Answer text', question_id: 1) } + let(:response) { Response.new(id: 1, map_id: 1, response_map: review_response_map, scores: [answer]) } + let(:user1) { User.new(name: 'abc', fullname: 'abc bbc', email: 'abcbbc@gmail.com', password: '123456789', password_confirmation: '123456789') } + + before(:each) do + questionnaires = [questionnaire1, questionnaire2] + allow(feedback_response_map).to receive(:reviewee).and_return(participant) + allow(feedback_response_map).to receive(:review).and_return(response) + allow(feedback_response_map).to receive(:reviewer).and_return(assignment_participant) + allow(response).to receive(:map).and_return(review_response_map) + allow(response).to receive(:reviewee).and_return(assignment_participant) + allow(review_response_map).to receive(:assignment).and_return(assignment) + allow(feedback_response_map).to receive(:assignment).and_return(assignment) + allow(assignment).to receive(:questionnaires).and_return(questionnaires) + # allow(questionnaires).to receive(:find_by).with(type: 'AuthorFeedbackQuestionnaire').and_return([questionnaire1]) + end + + describe '#assignment' do + it 'returns the assignment associated with this FeedbackResponseMap' do + expect(feedback_response_map.assignment).to eq(assignment) + end + end + + describe '#title' do + it 'returns "Feedback"' do + expect(feedback_response_map.title).to eq('Feedback') + end + end + end + \ No newline at end of file From 2a7b7368205a3c7e0366073e94828b591e6ff9f2 Mon Sep 17 00:00:00 2001 From: ckoneti Date: Mon, 24 Mar 2025 19:39:21 -0400 Subject: [PATCH 07/35] Corrected few codes and added questionnaire testcase in feedback_response_map_spec.rb --- app/models/feedback_response_map.rb | 10 +++++----- spec/models/feedback_response_map_spec.rb | 9 ++++++++- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/app/models/feedback_response_map.rb b/app/models/feedback_response_map.rb index 86e48208c..e6d15b28e 100644 --- a/app/models/feedback_response_map.rb +++ b/app/models/feedback_response_map.rb @@ -11,27 +11,27 @@ def title # Gets the feedback questionnaire associated with the assignment def questionnaire - review_response_map.assignment.questionnaires.find_by(type: 'AuthorFeedbackQuestionnaire') + self.assignment.questionnaires end # Returns the original contributor (the author who received the review) def contributor - review_response_map.reviewee + self.reviewee end # Returns the team being reviewed in the original review def team - review_response_map.reviewee_team + self.reviewee_team end # Returns the reviewer who gave the original review def reviewer - review_response_map.reviewer + self.reviewer end # Returns the round number of the original review (if applicable) def round - review_response_map&.response&.round + self&.response&.round end # Returns a report of feedback responses, grouped dynamically by round diff --git a/spec/models/feedback_response_map_spec.rb b/spec/models/feedback_response_map_spec.rb index ef1cc4295..415b944c3 100644 --- a/spec/models/feedback_response_map_spec.rb +++ b/spec/models/feedback_response_map_spec.rb @@ -21,7 +21,7 @@ allow(review_response_map).to receive(:assignment).and_return(assignment) allow(feedback_response_map).to receive(:assignment).and_return(assignment) allow(assignment).to receive(:questionnaires).and_return(questionnaires) - # allow(questionnaires).to receive(:find_by).with(type: 'AuthorFeedbackQuestionnaire').and_return([questionnaire1]) + # allow(questionnaires).to receive(:find_by).with(questionnaire_type: 'AuthorFeedbackQuestionnaire').and_return([questionnaire1]) end describe '#assignment' do @@ -35,5 +35,12 @@ expect(feedback_response_map.title).to eq('Feedback') end end + + describe '#questionnaire' do + it 'returns an AuthorFeedbackQuestionnaire' do + # Assuming `feedback_response_map.questionnaire` returns the actual questionnaire object + expect(feedback_response_map.questionnaire).to eq([questionnaire1, questionnaire2]) + end + end end \ No newline at end of file From b7d2d4644b2c7a8e6a268ecaab1e98a49b9ec316 Mon Sep 17 00:00:00 2001 From: ckoneti Date: Mon, 24 Mar 2025 19:42:03 -0400 Subject: [PATCH 08/35] Added testcases for other methods in feedback_response_map_spec.rb --- app/models/feedback_response_map.rb | 8 +++--- spec/models/feedback_response_map_spec.rb | 31 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/app/models/feedback_response_map.rb b/app/models/feedback_response_map.rb index e6d15b28e..c2e744c10 100644 --- a/app/models/feedback_response_map.rb +++ b/app/models/feedback_response_map.rb @@ -19,10 +19,10 @@ def contributor self.reviewee end - # Returns the team being reviewed in the original review - def team - self.reviewee_team - end + # # Returns the team being reviewed in the original review + # def team + # self.reviewee_team + # end # Returns the reviewer who gave the original review def reviewer diff --git a/spec/models/feedback_response_map_spec.rb b/spec/models/feedback_response_map_spec.rb index 415b944c3..153b701fc 100644 --- a/spec/models/feedback_response_map_spec.rb +++ b/spec/models/feedback_response_map_spec.rb @@ -42,5 +42,36 @@ expect(feedback_response_map.questionnaire).to eq([questionnaire1, questionnaire2]) end end + + describe '#contributor' do + it 'returns the reviewee (the original contributor)' do + expect(feedback_response_map.contributor).to eq(participant) + end + end + + # describe '#team' do + # it 'returns the team being reviewed' do + # expect(feedback_response_map.team).to eq(team) + # end + # end + + describe '#reviewer' do + it 'returns the reviewer who gave the original review' do + expect(feedback_response_map.reviewer).to eq(assignment_participant) + end + end + + describe '#round' do + it 'returns the round number of the original review' do + # Mock the response round number + allow(feedback_response_map).to receive(:round).and_return(1) + expect(feedback_response_map.round).to eq(1) + end + + it 'returns nil if the round number is not present' do + allow(feedback_response_map).to receive(:round).and_return(nil) + expect(feedback_response_map.round).to be_nil + end + end end \ No newline at end of file From 0019a7f85a66b1d2913a41d6991f412316ef6977 Mon Sep 17 00:00:00 2001 From: NDT2000 Date: Mon, 24 Mar 2025 22:14:51 -0400 Subject: [PATCH 09/35] Controler and CRUD Operations created --- .../v1/feedback_response_maps_controller.rb | 59 ++++++++++ config/routes.rb | 6 + swagger/v1/swagger.yaml | 107 +++++++++++++++++- 3 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 app/controllers/api/v1/feedback_response_maps_controller.rb diff --git a/app/controllers/api/v1/feedback_response_maps_controller.rb b/app/controllers/api/v1/feedback_response_maps_controller.rb new file mode 100644 index 000000000..a5d28840b --- /dev/null +++ b/app/controllers/api/v1/feedback_response_maps_controller.rb @@ -0,0 +1,59 @@ +class Api::V1::FeedbackResponseMapsController < ApplicationController + before_action :set_feedback_response_map, only: [:show, :update, :destroy] + + # GET /api/v1/feedback_response_maps + def index + @feedback_response_maps = FeedbackResponseMap.all + render json: @feedback_response_maps + end + + # GET /api/v1/feedback_response_maps/:id + def show + render json: @feedback_response_map + end + + # POST /api/v1/feedback_response_maps + def create + @feedback_response_map = FeedbackResponseMap.new(feedback_response_map_params) + + if @feedback_response_map.save + render json: @feedback_response_map, status: :created + else + render json: @feedback_response_map.errors, status: :unprocessable_entity + end + end + + # PATCH/PUT /api/v1/feedback_response_maps/:id + def update + if @feedback_response_map.update(feedback_response_map_params) + render json: @feedback_response_map + else + render json: @feedback_response_map.errors, status: :unprocessable_entity + end + end + + # DELETE /api/v1/feedback_response_maps/:id + def destroy + @feedback_response_map.destroy + head :no_content + end + + # GET /api/v1/feedback_response_maps/response_report/:assignment_id + def response_report + assignment_id = params[:assignment_id] + report = FeedbackResponseMap.feedback_response_report(assignment_id, params[:type]) + render json: report + end + + private + + def set_feedback_response_map + @feedback_response_map = FeedbackResponseMap.find(params[:id]) + rescue ActiveRecord::RecordNotFound + render json: { error: 'Feedback response map not found' }, status: :not_found + end + + def feedback_response_map_params + params.require(:feedback_response_map).permit(:reviewee_id, :reviewer_id, :reviewed_object_id) + end +end \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index 33df803e7..38b10ec9e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -120,6 +120,12 @@ delete '/:id', to: 'participants#destroy' end end + + resources :feedback_response_maps do + collection do + get 'response_report/:assignment_id', action: :response_report + end + end end end end \ No newline at end of file diff --git a/swagger/v1/swagger.yaml b/swagger/v1/swagger.yaml index de8081625..fdc9db07a 100644 --- a/swagger/v1/swagger.yaml +++ b/swagger/v1/swagger.yaml @@ -1326,10 +1326,113 @@ paths: responses: '200': description: A specific student task + "/api/v1/feedback_response_maps": + get: + summary: List all feedback response maps + tags: + - Feedback Response Maps + responses: + '200': + description: successful + post: + summary: Create feedback response map + tags: + - Feedback Response Maps + parameters: [] + responses: + '201': + description: created + '422': + description: unprocessable entity + requestBody: + content: + application/json: + schema: + type: object + properties: + feedback_response_map: + type: object + properties: + reviewee_id: + type: integer + reviewer_id: + type: integer + reviewed_object_id: + type: integer + required: + - reviewee_id + - reviewer_id + - reviewed_object_id + "/api/v1/feedback_response_maps/{id}": + parameters: + - name: id + in: path + required: true + schema: + type: integer + get: + summary: Show feedback response map + tags: + - Feedback Response Maps + responses: + '200': + description: successful + '404': + description: not found + put: + summary: Update feedback response map + tags: + - Feedback Response Maps + parameters: [] + responses: + '200': + description: successful + '404': + description: not found + '422': + description: unprocessable entity + requestBody: + content: + application/json: + schema: + type: object + properties: + feedback_response_map: + type: object + properties: + reviewee_id: + type: integer + reviewer_id: + type: integer + reviewed_object_id: + type: integer + delete: + summary: Delete feedback response map + tags: + - Feedback Response Maps + responses: + '204': + description: successful + '404': + description: not found - - + "/api/v1/feedback_response_maps/response_report/{assignment_id}": + parameters: + - name: assignment_id + in: path + required: true + schema: + type: integer + get: + summary: Get feedback response report for assignment + tags: + - Feedback Response Maps + responses: + '200': + description: successful + '404': + description: not found servers: - url: http://{defaultHost} From 55d72338e261d42a71677965296c0192ce72c261 Mon Sep 17 00:00:00 2001 From: Nayan Taori <64462883+NDT2000@users.noreply.github.com> Date: Mon, 24 Mar 2025 23:03:11 -0400 Subject: [PATCH 10/35] Update database.yml --- config/database.yml | 47 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/config/database.yml b/config/database.yml index b9f5aa055..092986fc7 100644 --- a/config/database.yml +++ b/config/database.yml @@ -1,18 +1,55 @@ +# MySQL. Versions 5.5.8 and up are supported. +# +# Install the MySQL driver +# gem install mysql2 +# +# Ensure the MySQL gem is defined in your Gemfile +# gem "mysql2" +# +# And be sure to use new-style password hashing: +# https://dev.mysql.com/doc/refman/5.7/en/password-hashing.html +# default: &default adapter: mysql2 encoding: utf8mb4 pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %> - port: 3306 - socket: /var/run/mysqld/mysqld.sock + username: root + password: expertiza + development: <<: *default - url: <%= ENV['DATABASE_URL'].gsub('?', '_development?') %> + database: reimplementation_development +# Warning: The database defined as "test" will be erased and +# re-generated from your development database when you run "rake". +# Do not set this db to the same as development or production. test: <<: *default - url: <%= ENV['DATABASE_URL'].gsub('?', '_test?') %> + database: reimplementation_test +# As with config/credentials.yml, you never want to store sensitive information, +# like your database password, in your source code. If your source code is +# ever seen by anyone, they now have access to your database. +# +# Instead, provide the password or a full connection URL as an environment +# variable when you boot the app. For example: +# +# DATABASE_URL="mysql2://myuser:mypass@localhost/somedatabase" +# +# If the connection URL is provided in the special DATABASE_URL environment +# variable, Rails will automatically merge its configuration values on top of +# the values provided in this file. Alternatively, you can specify a connection +# URL environment variable explicitly: +# +# production: +# url: <%= ENV["MY_APP_DATABASE_URL"] %> +# +# Read https://guides.rubyonrails.org/configuring.html#configuring-a-database +# for a full overview on how database connection configuration can be specified. +# production: <<: *default - url: <%= ENV['DATABASE_URL'].gsub('?', '_production?') %> \ No newline at end of file + database: reimplementation_production + username: reimplementation + password: <%= ENV["REIMPLEMENTATION_DATABASE_PASSWORD"] %> From 059e18a1c3e4457a5ce35890f0aae78acd6bcb84 Mon Sep 17 00:00:00 2001 From: ckoneti Date: Mon, 24 Mar 2025 23:51:57 -0400 Subject: [PATCH 11/35] Modified testcases logic on feedback_response_map_spec.rb --- app/models/feedback_response_map.rb | 5 +- spec/models/feedback_response_map_spec.rb | 158 +++++++++++++--------- 2 files changed, 96 insertions(+), 67 deletions(-) diff --git a/app/models/feedback_response_map.rb b/app/models/feedback_response_map.rb index c2e744c10..1af557e7b 100644 --- a/app/models/feedback_response_map.rb +++ b/app/models/feedback_response_map.rb @@ -37,8 +37,9 @@ def round # Returns a report of feedback responses, grouped dynamically by round def self.feedback_response_report(assignment_id, _type) authors = fetch_authors_for_assignment(assignment_id) - review_map_ids = ReviewResponseMap.where(reviewed_object_id: assignment_id).pluck(:id) - review_responses = Response.where(map_id: review_map_ids).order(created_at: :desc) + review_map_ids = review_map_ids = ReviewResponseMap.where(["reviewed_object_id = ?", assignment_id]).pluck("id") + review_responses = Response.where(["map_id IN (?)", review_map_ids]) + review_responses = review_responses.order("created_at DESC") if review_responses.respond_to?(:order) if Assignment.find(assignment_id).varying_rubrics_by_round? latest_by_map_and_round = {} diff --git a/spec/models/feedback_response_map_spec.rb b/spec/models/feedback_response_map_spec.rb index 153b701fc..a0ea83e36 100644 --- a/spec/models/feedback_response_map_spec.rb +++ b/spec/models/feedback_response_map_spec.rb @@ -1,77 +1,105 @@ describe FeedbackResponseMap do - let(:questionnaire1) { Questionnaire.new(id: 1, questionnaire_type: 'AuthorFeedbackQuestionnaire') } - let(:questionnaire2) { Questionnaire.new(id: 2, questionnaire_type: 'MetareviewQuestionnaire') } - let(:participant) { Participant.new(id: 1) } - let(:assignment) { Assignment.new(id: 1) } - let(:team) { Team.new(assignment_id: assignment.id) } - let(:assignment_participant) { Participant.new(id: 2, assignment: assignment) } - let(:feedback_response_map) { FeedbackResponseMap.new } - let(:review_response_map) { ReviewResponseMap.new(id: 2, assignment: assignment, reviewer: participant, reviewee: team) } - let(:answer) { Answer.new(answer: 1, comments: 'Answer text', question_id: 1) } - let(:response) { Response.new(id: 1, map_id: 1, response_map: review_response_map, scores: [answer]) } - let(:user1) { User.new(name: 'abc', fullname: 'abc bbc', email: 'abcbbc@gmail.com', password: '123456789', password_confirmation: '123456789') } - - before(:each) do - questionnaires = [questionnaire1, questionnaire2] - allow(feedback_response_map).to receive(:reviewee).and_return(participant) - allow(feedback_response_map).to receive(:review).and_return(response) - allow(feedback_response_map).to receive(:reviewer).and_return(assignment_participant) - allow(response).to receive(:map).and_return(review_response_map) - allow(response).to receive(:reviewee).and_return(assignment_participant) - allow(review_response_map).to receive(:assignment).and_return(assignment) - allow(feedback_response_map).to receive(:assignment).and_return(assignment) - allow(assignment).to receive(:questionnaires).and_return(questionnaires) - # allow(questionnaires).to receive(:find_by).with(questionnaire_type: 'AuthorFeedbackQuestionnaire').and_return([questionnaire1]) - end - - describe '#assignment' do - it 'returns the assignment associated with this FeedbackResponseMap' do - expect(feedback_response_map.assignment).to eq(assignment) - end - end - - describe '#title' do - it 'returns "Feedback"' do - expect(feedback_response_map.title).to eq('Feedback') - end + let(:questionnaire1) { Questionnaire.new(id: 1, questionnaire_type: 'AuthorFeedbackQuestionnaire') } + let(:questionnaire2) { Questionnaire.new(id: 2, questionnaire_type: 'MetareviewQuestionnaire') } + let(:participant) { Participant.new(id: 1) } + let(:assignment) { Assignment.new(id: 1) } + let(:team) { Team.new(id: 1) } + let(:assignment_participant) { Participant.new(id: 2, assignment: assignment) } + let(:feedback_response_map) { FeedbackResponseMap.new } + let(:review_response_map) { ReviewResponseMap.new(id: 2, assignment: assignment, reviewer: participant, reviewee: team) } + let(:answer) { Answer.new(answer: 1, comments: 'Answer text', question_id: 1) } + let(:response) { Response.new(id: 1, map_id: 1, response_map: review_response_map, scores: [answer]) } + let(:user1) { User.new(name: 'abc', full_name: 'abc bbc', email: 'abcbbc@gmail.com', password: '123456789', password_confirmation: '123456789') } + + before(:each) do + questionnaires = [questionnaire1, questionnaire2] + allow(feedback_response_map).to receive(:reviewee).and_return(participant) + allow(feedback_response_map).to receive(:review).and_return(response) + allow(feedback_response_map).to receive(:reviewer).and_return(assignment_participant) + allow(response).to receive(:map).and_return(review_response_map) + allow(response).to receive(:reviewee).and_return(assignment_participant) + allow(review_response_map).to receive(:assignment).and_return(assignment) + allow(feedback_response_map).to receive(:assignment).and_return(assignment) + allow(assignment).to receive(:questionnaires).and_return(questionnaires) + end + + describe '#assignment' do + it 'returns the assignment associated with this FeedbackResponseMap' do + expect(feedback_response_map.assignment).to eq(assignment) end + end - describe '#questionnaire' do - it 'returns an AuthorFeedbackQuestionnaire' do - # Assuming `feedback_response_map.questionnaire` returns the actual questionnaire object - expect(feedback_response_map.questionnaire).to eq([questionnaire1, questionnaire2]) - end + describe '#title' do + it 'returns "Feedback"' do + expect(feedback_response_map.title).to eq('Feedback') end - - describe '#contributor' do - it 'returns the reviewee (the original contributor)' do - expect(feedback_response_map.contributor).to eq(participant) - end + end + + describe '#questionnaire' do + it 'returns an AuthorFeedbackQuestionnaire' do + expect(feedback_response_map.questionnaire).to eq([questionnaire1, questionnaire2]) end + end - # describe '#team' do - # it 'returns the team being reviewed' do - # expect(feedback_response_map.team).to eq(team) - # end - # end + describe '#contributor' do + it 'returns the reviewee' do + expect(feedback_response_map.contributor).to eq(participant) + end + end - describe '#reviewer' do - it 'returns the reviewer who gave the original review' do - expect(feedback_response_map.reviewer).to eq(assignment_participant) - end + describe '#reviewer' do + it 'returns the reviewer' do + expect(feedback_response_map.reviewer).to eq(assignment_participant) end + end - describe '#round' do - it 'returns the round number of the original review' do - # Mock the response round number - allow(feedback_response_map).to receive(:round).and_return(1) - expect(feedback_response_map.round).to eq(1) - end + describe '#round' do + it 'returns the round number of the original review' do + # Mock the response round number + allow(feedback_response_map).to receive(:round).and_return(1) + expect(feedback_response_map.round).to eq(1) + end - it 'returns nil if the round number is not present' do - allow(feedback_response_map).to receive(:round).and_return(nil) - expect(feedback_response_map.round).to be_nil - end + it 'returns nil if the round number is not present' do + allow(feedback_response_map).to receive(:round).and_return(nil) + expect(feedback_response_map.round).to be_nil end end - \ No newline at end of file + + # describe '#feedback_response_report' do + # it 'returns a report' do + # maps = [review_response_map] + # allow(ReviewResponseMap).to receive(:where).with(['reviewed_object_id = ?', 1]).and_return(maps) + # allow(maps).to receive(:pluck).with('id').and_return(review_response_map.id) + # allow(Team).to receive_message_chain(:includes, :where).and_return([team]) + # allow(team).to receive(:users).and_return([user1]) + # allow(user1).to receive(:id).and_return(1) + # allow(AssignmentParticipant).to receive(:where).with(parent_id: 1, user_id: 1).and_return([participant]) + + # response1 = instance_double('Response', round: 1, additional_comment: '') + # response2 = instance_double('Response', round: 2, additional_comment: 'LGTM') + # response3 = instance_double('Response', round: 3, additional_comment: 'Bad') + # rounds = [response1, response2, response3] + + # # Mock `Response.where` to return rounds + # allow(Response).to receive(:where).with(['map_id IN (?)', 2]).and_return(rounds) + # allow(Response).to receive_message_chain(:where, :order).with(['map_id IN (?)', 2], 'created_at DESC').and_return(['map_id IN (?)', 2]) + # allow(Assignment).to receive(:find).with(1).and_return(assignment) + # # allow(assignment).to receive(:varying_rubrics_by_round).and_return(true) + + # # Mock necessary methods for `response` objects + # allow(response1).to receive(:map_id).and_return(1) + # allow(response2).to receive(:map_id).and_return(2) + # allow(response3).to receive(:map_id).and_return(3) + # allow(response1).to receive(:id).and_return(1) + # allow(response2).to receive(:id).and_return(2) + # allow(response3).to receive(:id).and_return(3) + + # report = FeedbackResponseMap.feedback_response_report(1, nil) + # expect(report[0]).to eq([participant]) + # expect(report[1]).to eq([1, 2, 3]) + # expect(report[2]).to eq(nil) + # expect(report[3]).to eq(nil) + # end + # end +end \ No newline at end of file From 55f366368656527ad03d1e339dec2fe13a60de18 Mon Sep 17 00:00:00 2001 From: riya-bihani Date: Thu, 17 Apr 2025 11:18:08 -0400 Subject: [PATCH 12/35] new way to separate email functionality --- app/services/feedback_email_service.rb | 32 ++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 app/services/feedback_email_service.rb diff --git a/app/services/feedback_email_service.rb b/app/services/feedback_email_service.rb new file mode 100644 index 000000000..327db9148 --- /dev/null +++ b/app/services/feedback_email_service.rb @@ -0,0 +1,32 @@ +class FeedbackEmailService + def initialize(response_map, assignment) + @response_map = response_map + @assignment = assignment + end + + # public API + def call + Mailer.sync_message(build_defn).deliver + end + + private + + def build_defn + # find the original review response + response = Response.find(@response_map.reviewed_object_id) + # find the ResponseMap that created that review + original_map = ResponseMap.find(response.map_id) + # find the participant who wrote that review + participant = AssignmentParticipant.find(original_map.reviewer_id) + user = User.find(participant.user_id) + + { + to: user.email, + body: { + type: 'Author Feedback', + first_name: user.fullname, + obj_name: @assignment.name + } + } + end +end From ba2dfd3e06787bc5f32edb9068f1e5876c80c91f Mon Sep 17 00:00:00 2001 From: riya-bihani Date: Thu, 17 Apr 2025 14:46:18 -0400 Subject: [PATCH 13/35] calling the email in the feedback map --- app/models/feedback_response_map.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/models/feedback_response_map.rb b/app/models/feedback_response_map.rb index 1af557e7b..bf01b7a12 100644 --- a/app/models/feedback_response_map.rb +++ b/app/models/feedback_response_map.rb @@ -73,4 +73,9 @@ def self.fetch_authors_for_assignment(assignment_id) end end.compact end + + def send_feedback_email(assignment) + FeedbackEmailService.new(self, assignment).call + end + end \ No newline at end of file From 68faab2fa459b2291422ecec4d25940d8e68a136 Mon Sep 17 00:00:00 2001 From: riya-bihani Date: Fri, 18 Apr 2025 10:48:03 -0400 Subject: [PATCH 14/35] testing the separated email functionality --- spec/factories/roles.rb | 2 +- spec/services/feedback_email_service_spec.rb | 46 ++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 spec/services/feedback_email_service_spec.rb diff --git a/spec/factories/roles.rb b/spec/factories/roles.rb index 1783617b6..7e1ee56ba 100644 --- a/spec/factories/roles.rb +++ b/spec/factories/roles.rb @@ -3,7 +3,7 @@ factory :role do sequence(:name) { |n| "Role #{n}" } - initialize_with { Role.find_or_create_by(id: id) } + # initialize_with { Role.find_or_create_by(id: id) } trait :student do id { Role::STUDENT } diff --git a/spec/services/feedback_email_service_spec.rb b/spec/services/feedback_email_service_spec.rb new file mode 100644 index 000000000..5540e1292 --- /dev/null +++ b/spec/services/feedback_email_service_spec.rb @@ -0,0 +1,46 @@ +# spec/services/feedback_email_service_spec.rb +require 'rails_helper' + +RSpec.describe FeedbackEmailService, type: :service do + describe '#call' do + let(:assignment) { double('Assignment', name: 'Cool Project') } + let(:feedback_map) { double('FeedbackResponseMap', reviewed_object_id: response_id) } + let(:response_id) { 77 } + let(:response) { double('Response', id: response_id, map_id: map_id) } + let(:map_id) { 99 } + let(:response_map) { double('ResponseMap', reviewer_id: participant_id) } + let(:participant_id){ 123 } + let(:participant) { double('AssignmentParticipant', user_id: user_id) } + let(:user_id) { 456 } + let(:user) { double('User', email: 'rev@example.com', fullname: 'Reviewer') } + + before do + # Stub all the ActiveRecord lookups + allow(Response).to receive(:find).with(response_id).and_return(response) + allow(ResponseMap).to receive(:find).with(map_id).and_return(response_map) + allow(AssignmentParticipant).to receive(:find).with(participant_id).and_return(participant) + allow(User).to receive(:find).with(user_id).and_return(user) + + # Create a dummy Mailer class that implements .sync_message + mailer_klass = Class.new do + def self.sync_message(_defn) + double(deliver: true) + end + end + stub_const('Mailer', mailer_klass) + end + + it 'builds the correct definition and tells the mailer to deliver it' do + service = described_class.new(feedback_map, assignment) + + expect(Mailer).to receive(:sync_message) do |defn| + expect(defn[:to]).to eq 'rev@example.com' + expect(defn[:body][:type]).to eq 'Author Feedback' + expect(defn[:body][:first_name]).to eq 'Reviewer' + expect(defn[:body][:obj_name]).to eq 'Cool Project' + end.and_return(double(deliver: true)) + + service.call + end + end +end From 6943d2d9cf4021642ef08c7766e655bd043e8de9 Mon Sep 17 00:00:00 2001 From: riya-bihani Date: Fri, 18 Apr 2025 13:04:01 -0400 Subject: [PATCH 15/35] working on separating the email functionality from feedback_response_map.rb --- spec/factories/roles.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/factories/roles.rb b/spec/factories/roles.rb index 7e1ee56ba..1783617b6 100644 --- a/spec/factories/roles.rb +++ b/spec/factories/roles.rb @@ -3,7 +3,7 @@ factory :role do sequence(:name) { |n| "Role #{n}" } - # initialize_with { Role.find_or_create_by(id: id) } + initialize_with { Role.find_or_create_by(id: id) } trait :student do id { Role::STUDENT } From 520b9a0c8bc79b1ba9e3b32ac96bdd2bc9d4f91a Mon Sep 17 00:00:00 2001 From: riya-bihani Date: Fri, 18 Apr 2025 13:05:23 -0400 Subject: [PATCH 16/35] testing the separated email functionality --- spec/services/feedback_email_service_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/services/feedback_email_service_spec.rb b/spec/services/feedback_email_service_spec.rb index 5540e1292..2112cf147 100644 --- a/spec/services/feedback_email_service_spec.rb +++ b/spec/services/feedback_email_service_spec.rb @@ -1,4 +1,3 @@ -# spec/services/feedback_email_service_spec.rb require 'rails_helper' RSpec.describe FeedbackEmailService, type: :service do From 1297ec8f94300dbef10845e495b137d10ebfb8e9 Mon Sep 17 00:00:00 2001 From: ckoneti Date: Fri, 18 Apr 2025 15:05:08 -0400 Subject: [PATCH 17/35] Added intial methods in response map controller --- .../api/v1/response_maps_controller.rb | 68 +++++++++++++++++++ spec/requests/api/v1/response_maps_spec.rb | 7 ++ 2 files changed, 75 insertions(+) create mode 100644 app/controllers/api/v1/response_maps_controller.rb create mode 100644 spec/requests/api/v1/response_maps_spec.rb diff --git a/app/controllers/api/v1/response_maps_controller.rb b/app/controllers/api/v1/response_maps_controller.rb new file mode 100644 index 000000000..b56816288 --- /dev/null +++ b/app/controllers/api/v1/response_maps_controller.rb @@ -0,0 +1,68 @@ +class ResponseMapsController < ApplicationController + before_action :set_response_map, only: [:show, :update, :destroy, :send_email] + + # GET /api/v1/response_maps + def index + @response_maps = ResponseMap.all + render json: @response_maps + end + + # GET /api/v1/response_maps/:id + def show + render json: @response_map + end + + # POST /api/v1/response_maps + def create + @response_map = ResponseMap.new(response_map_params) + + if @response_map.save + render json: @response_map, status: :created + else + render json: @response_map.errors, status: :unprocessable_entity + end + end + + # PATCH/PUT /api/v1/response_maps/:id + def update + if @response_map.update(response_map_params) + render json: @response_map + else + render json: @response_map.errors, status: :unprocessable_entity + end + end + + # DELETE /api/v1/response_maps/:id + def destroy + @response_map.destroy + head :no_content + end + + # POST /api/v1/response_maps/:id/send_email + def send_email + assignment = Assignment.find(params[:assignment_id]) + @response_map.send_email(assignment) + render json: { message: 'Email sent successfully' }, status: :ok + rescue StandardError => e + render json: { error: e.message }, status: :unprocessable_entity + end + + # GET /api/v1/response_maps/response_report/:assignment_id + def response_report + assignment_id = params[:assignment_id] + report = ResponseMap.response_report(assignment_id, params[:type]) + render json: report + end + + private + + def set_response_map + @response_map = ResponseMap.find(params[:id]) + rescue ActiveRecord::RecordNotFound + render json: { error: 'Response map not found' }, status: :not_found + end + + def response_map_params + params.require(:response_map).permit(:reviewee_id, :reviewer_id, :reviewed_object_id) + end +end diff --git a/spec/requests/api/v1/response_maps_spec.rb b/spec/requests/api/v1/response_maps_spec.rb new file mode 100644 index 000000000..5620a40b1 --- /dev/null +++ b/spec/requests/api/v1/response_maps_spec.rb @@ -0,0 +1,7 @@ +require 'rails_helper' + +RSpec.describe "ResponseMaps", type: :request do + describe "GET /index" do + pending "add some examples (or delete) #{__FILE__}" + end +end From d2e6a265816247264e43f0d763c6960b46c32828 Mon Sep 17 00:00:00 2001 From: ckoneti Date: Fri, 18 Apr 2025 17:14:49 -0400 Subject: [PATCH 18/35] Integrated email functionality into ResponseMapController using the email service --- .../api/v1/response_maps_controller.rb | 123 +++++++++--------- 1 file changed, 59 insertions(+), 64 deletions(-) diff --git a/app/controllers/api/v1/response_maps_controller.rb b/app/controllers/api/v1/response_maps_controller.rb index b56816288..7b7eaa57e 100644 --- a/app/controllers/api/v1/response_maps_controller.rb +++ b/app/controllers/api/v1/response_maps_controller.rb @@ -1,68 +1,63 @@ -class ResponseMapsController < ApplicationController - before_action :set_response_map, only: [:show, :update, :destroy, :send_email] - - # GET /api/v1/response_maps - def index - @response_maps = ResponseMap.all - render json: @response_maps - end - - # GET /api/v1/response_maps/:id - def show - render json: @response_map - end - - # POST /api/v1/response_maps - def create - @response_map = ResponseMap.new(response_map_params) - - if @response_map.save - render json: @response_map, status: :created - else - render json: @response_map.errors, status: :unprocessable_entity +# app/controllers/api/v1/response_maps_controller.rb +class Api::V1::ResponseMapsController < ApplicationController + before_action :set_response_map, only: [:show, :update, :destroy] + + # GET /api/v1/response_maps + def index + render json: ResponseMap.all end - end - - # PATCH/PUT /api/v1/response_maps/:id - def update - if @response_map.update(response_map_params) + + # GET /api/v1/response_maps/:id + def show render json: @response_map - else - render json: @response_map.errors, status: :unprocessable_entity + end + + # POST /api/v1/response_maps + def create + @response_map = ResponseMap.new(response_map_params) + persist_and_respond(@response_map, :created) + end + + # PATCH /api/v1/response_maps/:id + def update + @response_map.assign_attributes(response_map_params) + persist_and_respond(@response_map, :ok) + end + + # DELETE /api/v1/response_maps/:id + def destroy + @response_map.destroy + head :no_content + end + + private + + def persist_and_respond(record, success_status) + if record.save + send_feedback_email_if_submitted(record) + render json: record, status: success_status + else + render json: record.errors, status: :unprocessable_entity + end + end + + def send_feedback_email_if_submitted(response_map) + return unless ActiveModel::Type::Boolean.new.cast(params[:submitted]) + + # Use your FeedbackEmailService to build & deliver the mail + FeedbackEmailService.new(response_map, response_map.assignment).call + rescue StandardError => e + Rails.logger.error "FeedbackEmailService failed: #{e.message}" + end + + def set_response_map + @response_map = ResponseMap.find(params[:id]) + rescue ActiveRecord::RecordNotFound + render json: { error: 'Response map not found' }, status: :not_found + end + + def response_map_params + params.require(:response_map).permit(:reviewee_id, :reviewer_id, :reviewed_object_id) end end - - # DELETE /api/v1/response_maps/:id - def destroy - @response_map.destroy - head :no_content - end - - # POST /api/v1/response_maps/:id/send_email - def send_email - assignment = Assignment.find(params[:assignment_id]) - @response_map.send_email(assignment) - render json: { message: 'Email sent successfully' }, status: :ok - rescue StandardError => e - render json: { error: e.message }, status: :unprocessable_entity - end - - # GET /api/v1/response_maps/response_report/:assignment_id - def response_report - assignment_id = params[:assignment_id] - report = ResponseMap.response_report(assignment_id, params[:type]) - render json: report - end - - private - - def set_response_map - @response_map = ResponseMap.find(params[:id]) - rescue ActiveRecord::RecordNotFound - render json: { error: 'Response map not found' }, status: :not_found - end - - def response_map_params - params.require(:response_map).permit(:reviewee_id, :reviewer_id, :reviewed_object_id) - end -end + \ No newline at end of file From 4bef586470b8e42b0857e60ab31e37ffe2618e85 Mon Sep 17 00:00:00 2001 From: ckoneti Date: Sat, 19 Apr 2025 21:38:06 -0400 Subject: [PATCH 19/35] Modified response model structure --- app/models/response.rb | 81 +++++++++++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 25 deletions(-) diff --git a/app/models/response.rb b/app/models/response.rb index 9e07fd79d..ba55b5b73 100644 --- a/app/models/response.rb +++ b/app/models/response.rb @@ -10,50 +10,81 @@ class Response < ApplicationRecord alias map response_map delegate :questionnaire, :reviewee, :reviewer, to: :map + validates :map_id, presence: true + + after_save :handle_response_submission + + def submit + update(is_submitted: true) + end + + def handle_response_submission + return unless is_submitted_changed? && is_submitted? + + # Send email notification through the response map + send_notification_email + end + 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 - # gets the sum total percentage scores of all responses that are not this response + 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 - end + next if id == response.id + count += 1 + total += response.aggregate_questionnaire_score.to_f / response.maximum_score 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 - - # This score has already skipped the unfilled scorable item(s) score = aggregate_questionnaire_score.to_f / maximum_score questionnaire = questionnaire_by_answer(scores.first) assignment = map.assignment - 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 + assignment_questionnaire = AssignmentQuestionnaire.find_by( + assignment_id: assignment.id, + questionnaire_id: questionnaire.id + ) - # the range of average_score_on_same_artifact_from_others and score is [0,1] - # the range of allowed_difference_percentage is [0, 100] - (average_score - score).abs * 100 > allowed_difference_percentage + difference_threshold = assignment_questionnaire.try(:notification_limit) || 0.0 + (score - average_score).abs * 100 > difference_threshold end def aggregate_questionnaire_score - # only count the scorable questions, only when the answer is not nil - # we accept nil as answer for scorable questions, and they will not be counted towards the total score - sum = 0 - scores.each do |s| - item = Item.find(s.question_id) - # For quiz responses, the weights will be 1 or 0, depending on if correct - sum += s.answer * item.weight unless s.answer.nil? || !item.scorable? + scores.joins(:question) + .where(questions: { scorable: true }) + .sum('answers.answer * questions.weight') + end + + def maximum_score + return 0 if scores.empty? + + questionnaire = questionnaire_by_answer(scores.first) + questionnaire.max_question_score * active_scored_questions.size + end + + private + + def send_notification_email + return unless map.assignment.present? + + if map.is_a?(FeedbackResponseMap) + FeedbackEmailService.new(map, map.assignment).call end - sum + # Add other response map type email services as needed + end + + def active_scored_questions + return [] if scores.empty? + + questionnaire = questionnaire_by_answer(scores.first) + questionnaire.items.select(&:scorable?) + end + + def questionnaire_by_answer(answer) + answer&.question&.questionnaire end end From f9cd7e7c6e262063e54776bd1309806557670d35 Mon Sep 17 00:00:00 2001 From: ckoneti Date: Sat, 19 Apr 2025 21:42:39 -0400 Subject: [PATCH 20/35] Refactored response_map model to follow a centralized pattern for all response_maps --- app/models/response_map.rb | 87 ++++++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 32 deletions(-) diff --git a/app/models/response_map.rb b/app/models/response_map.rb index d27124d76..9b399a186 100644 --- a/app/models/response_map.rb +++ b/app/models/response_map.rb @@ -1,41 +1,64 @@ class ResponseMap < ApplicationRecord - has_many :response, foreign_key: 'map_id', dependent: :destroy, inverse_of: false + has_many :responses, foreign_key: 'map_id', dependent: :destroy, inverse_of: false belongs_to :reviewer, class_name: 'Participant', foreign_key: 'reviewer_id', inverse_of: false belongs_to :reviewee, class_name: 'Participant', foreign_key: 'reviewee_id', inverse_of: false belongs_to :assignment, class_name: 'Assignment', foreign_key: 'reviewed_object_id', inverse_of: false alias map_id id - # returns the assignment related to the response map - def response_assignment - return Participant.find(self.reviewer_id).assignment - end - - def self.assessments_for(team) - responses = [] - # stime = Time.now - if team - array_sort = [] - sort_to = [] - maps = where(reviewee_id: team.id) - maps.each do |map| - next if map.response.empty? - - 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) } - responses << sort_to[0] unless sort_to[0].nil? - array_sort.clear - sort_to.clear - end - responses = responses.sort { |a, b| a.map.reviewer.fullname <=> b.map.reviewer.fullname } + # Returns the title used for display - should be overridden by subclasses + def title + self.class.name.sub("ResponseMap", "") + end + + # Gets the questionnaire associated with the assignment + def questionnaire + self.assignment.questionnaires + end + + # Returns the original contributor + def contributor + self.reviewee + end + + # Returns the round number of the latest response + def round + self.responses.order(created_at: :desc).first&.round + end + + # Returns the latest response + def latest_response + self.responses.order(created_at: :desc).first + end + + # Returns if this response map has any submitted responses + def has_submitted_response? + self.responses.where(is_submitted: true).exists? + end + + # Generate a report for responses grouped by rounds + def self.response_report(assignment_id, type = nil) + responses = Response.joins(:response_map) + .where(response_maps: { reviewed_object_id: assignment_id }) + .order(created_at: :desc) + + if Assignment.find(assignment_id).varying_rubrics_by_round? + group_responses_by_round(responses) + else + group_latest_responses(responses) end - responses end -end + + private + + def self.group_responses_by_round(responses) + responses.group_by(&:round) + .transform_values { |resps| resps.map(&:id) } + end + + def self.group_latest_responses(responses) + responses.group_by { |r| r.map_id } + .transform_values { |resps| resps.first.id } + .values + end +end \ No newline at end of file From 5cc285028c63b9dce9f855b56f73960a0f406e20 Mon Sep 17 00:00:00 2001 From: ckoneti Date: Sun, 20 Apr 2025 15:39:36 -0400 Subject: [PATCH 21/35] feat: call FeedbackEmailService in submit_response --- .../api/v1/response_maps_controller.rb | 136 ++++++++++-------- 1 file changed, 78 insertions(+), 58 deletions(-) diff --git a/app/controllers/api/v1/response_maps_controller.rb b/app/controllers/api/v1/response_maps_controller.rb index 7b7eaa57e..012be1a77 100644 --- a/app/controllers/api/v1/response_maps_controller.rb +++ b/app/controllers/api/v1/response_maps_controller.rb @@ -1,63 +1,83 @@ # app/controllers/api/v1/response_maps_controller.rb class Api::V1::ResponseMapsController < ApplicationController - before_action :set_response_map, only: [:show, :update, :destroy] - - # GET /api/v1/response_maps - def index - render json: ResponseMap.all - end - - # GET /api/v1/response_maps/:id - def show - render json: @response_map - end - - # POST /api/v1/response_maps - def create - @response_map = ResponseMap.new(response_map_params) - persist_and_respond(@response_map, :created) - end - - # PATCH /api/v1/response_maps/:id - def update - @response_map.assign_attributes(response_map_params) - persist_and_respond(@response_map, :ok) - end - - # DELETE /api/v1/response_maps/:id - def destroy - @response_map.destroy - head :no_content - end - - private - - def persist_and_respond(record, success_status) - if record.save - send_feedback_email_if_submitted(record) - render json: record, status: success_status - else - render json: record.errors, status: :unprocessable_entity - end - end - - def send_feedback_email_if_submitted(response_map) - return unless ActiveModel::Type::Boolean.new.cast(params[:submitted]) - - # Use your FeedbackEmailService to build & deliver the mail - FeedbackEmailService.new(response_map, response_map.assignment).call - rescue StandardError => e - Rails.logger.error "FeedbackEmailService failed: #{e.message}" - end - - def set_response_map - @response_map = ResponseMap.find(params[:id]) - rescue ActiveRecord::RecordNotFound - render json: { error: 'Response map not found' }, status: :not_found + before_action :set_response_map, only: [:show, :update, :destroy, :submit_response] + + # GET /api/v1/response_maps + def index + @response_maps = ResponseMap.all + render json: @response_maps + end + + # GET /api/v1/response_maps/:id + def show + render json: @response_map + end + + # POST /api/v1/response_maps + def create + @response_map = ResponseMap.new(response_map_params) + persist_and_respond(@response_map, :created) + end + + # PATCH/PUT /api/v1/response_maps/:id + def update + @response_map.assign_attributes(response_map_params) + persist_and_respond(@response_map, :ok) + end + + # DELETE /api/v1/response_maps/:id + def destroy + @response_map.destroy + head :no_content + end + + # POST /api/v1/response_maps/:id/submit_response + def submit_response + @response = @response_map.responses.find_or_initialize_by(id: params[:response_id]) + @response.assign_attributes(response_params) + @response.is_submitted = true + + if @response.save + # send feedback email now that itโ€™s marked submitted + FeedbackEmailService.new(@response_map, @response_map.assignment).call + render json: { message: 'Response submitted successfully, email sent' }, status: :ok + else + render json: { errors: @response.errors }, status: :unprocessable_entity end - - def response_map_params - params.require(:response_map).permit(:reviewee_id, :reviewer_id, :reviewed_object_id) + end + + # GET /api/v1/response_maps/response_report/:assignment_id + def response_report + assignment_id = params[:assignment_id] + report = ResponseMap.response_report(assignment_id, params[:type]) + render json: report + end + + private + + def set_response_map + @response_map = ResponseMap.find(params[:id]) + rescue ActiveRecord::RecordNotFound + render json: { error: 'Response map not found' }, status: :not_found + end + + def response_map_params + params.require(:response_map).permit(:reviewee_id, :reviewer_id, :reviewed_object_id) + end + + def response_params + params.require(:response).permit( + :additional_comment, + :round, + scores_attributes: [:answer, :comments, :question_id] + ) + end + + def persist_and_respond(record, success_status) + if record.save + render json: record, status: success_status + else + render json: record.errors, status: :unprocessable_entity end end - \ No newline at end of file +end \ No newline at end of file From 5ae94ac061a32af0c8ecabbb32f200059fe17f46 Mon Sep 17 00:00:00 2001 From: ckoneti Date: Sun, 20 Apr 2025 15:41:47 -0400 Subject: [PATCH 22/35] chore: allow submitted flag in strong params --- app/controllers/api/v1/response_maps_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/api/v1/response_maps_controller.rb b/app/controllers/api/v1/response_maps_controller.rb index 012be1a77..90b74497c 100644 --- a/app/controllers/api/v1/response_maps_controller.rb +++ b/app/controllers/api/v1/response_maps_controller.rb @@ -69,6 +69,7 @@ def response_params params.require(:response).permit( :additional_comment, :round, + :is_submitted, scores_attributes: [:answer, :comments, :question_id] ) end From ef1486ba5e3c1efe8cb076cfa7b49a6818cfa139 Mon Sep 17 00:00:00 2001 From: ckoneti Date: Sun, 20 Apr 2025 15:44:04 -0400 Subject: [PATCH 23/35] refactor: DRY out email logic into private method --- app/controllers/api/v1/response_maps_controller.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/controllers/api/v1/response_maps_controller.rb b/app/controllers/api/v1/response_maps_controller.rb index 90b74497c..13edfba10 100644 --- a/app/controllers/api/v1/response_maps_controller.rb +++ b/app/controllers/api/v1/response_maps_controller.rb @@ -41,11 +41,20 @@ def submit_response # send feedback email now that itโ€™s marked submitted FeedbackEmailService.new(@response_map, @response_map.assignment).call render json: { message: 'Response submitted successfully, email sent' }, status: :ok + handle_submission(@response_map) else render json: { errors: @response.errors }, status: :unprocessable_entity end end + def handle_submission(map) + FeedbackEmailService.new(map, map.assignment).call + render json: { message: 'Response submitted successfully, email sent' }, status: :ok + rescue StandardError => e + Rails.logger.error "FeedbackEmail failed: #{e.message}" + render json: { message: 'Response submitted, but email failed' }, status: :ok + end + # GET /api/v1/response_maps/response_report/:assignment_id def response_report assignment_id = params[:assignment_id] From beb697ca8502410e7713b60c1450e766761d4a7c Mon Sep 17 00:00:00 2001 From: ckoneti Date: Sun, 20 Apr 2025 15:44:40 -0400 Subject: [PATCH 24/35] feat: send email on create/update when submitted flag set --- app/controllers/api/v1/response_maps_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/api/v1/response_maps_controller.rb b/app/controllers/api/v1/response_maps_controller.rb index 13edfba10..3291bbf70 100644 --- a/app/controllers/api/v1/response_maps_controller.rb +++ b/app/controllers/api/v1/response_maps_controller.rb @@ -85,6 +85,7 @@ def response_params def persist_and_respond(record, success_status) if record.save + handle_submission(record) if record.is_submitted? render json: record, status: success_status else render json: record.errors, status: :unprocessable_entity From 4c20c2ee05f4b07f0233fcdbbc1185d56742acdc Mon Sep 17 00:00:00 2001 From: NDT2000 Date: Sun, 20 Apr 2025 21:13:47 -0400 Subject: [PATCH 25/35] first unit test case added --- config/routes.rb | 1 + .../api/response_map_controller_spec.rb | 77 +++++++++++++++++++ spec/factories/response_maps.rb | 6 ++ spec/factories/roles.rb | 2 +- 4 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 spec/controllers/api/response_map_controller_spec.rb create mode 100644 spec/factories/response_maps.rb diff --git a/config/routes.rb b/config/routes.rb index 38b10ec9e..e995e7ead 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -46,6 +46,7 @@ post 'bookmarkratings', to: 'bookmarks#save_bookmark_rating_score' end end + resources :response_maps, only: [:index] resources :student_tasks do collection do get :list, action: :list diff --git a/spec/controllers/api/response_map_controller_spec.rb b/spec/controllers/api/response_map_controller_spec.rb new file mode 100644 index 000000000..0bd19862a --- /dev/null +++ b/spec/controllers/api/response_map_controller_spec.rb @@ -0,0 +1,77 @@ +require 'rails_helper' + +RSpec.describe "Api::V1::ResponseMaps", type: :request do + controller_class = Api::V1::ResponseMapsController + + before(:each) do + # ๐Ÿ” Skip any authentication callbacks + controller_class._process_action_callbacks + .select { |callback| callback.kind == :before } + .map(&:filter) + .each do |filter| + begin + controller_class.skip_before_action(filter, raise: false) + rescue => e + puts "Could not skip filter #{filter}: #{e.message}" + end + end + end + + describe "GET /api/v1/response_maps" do + it "returns all response maps" do + instructor_role = Role.create!(name: "Instructor") + + user1 = User.create!( + name: "Instructor", + full_name: "Instructor User", + email: "instructor@example.com", + password: "password123", + role: instructor_role + ) + + user2 = User.create!( + name: "Reviewer", + full_name: "Reviewer User", + email: "reviewer@example.com", + password: "password123", + role: instructor_role + ) + + user3 = User.create!( + name: "Reviewee", + full_name: "Reviewee User", + email: "reviewee@example.com", + password: "password123", + role: instructor_role + ) + + assignment = Assignment.create!( + name: "Test Assignment", + directory_path: "test_assignment", + instructor: user1, + num_reviews: 1, + num_reviews_required: 1, + num_reviews_allowed: 1, + num_metareviews_required: 1, + num_metareviews_allowed: 1, + rounds_of_reviews: 1, + is_calibrated: false, + has_badge: false, + enable_pair_programming: false, + staggered_deadline: false, + show_teammate_reviews: false, + is_coding_assignment: false + ) + + reviewer = Participant.create!(handle: "rev", user: user2, assignment: assignment) + reviewee = Participant.create!(handle: "ree", user: user3, assignment: assignment) + + ResponseMap.create!(reviewer: reviewer, reviewee: reviewee, assignment: assignment) + + get "/api/v1/response_maps" + + expect(response).to have_http_status(:ok) + expect(JSON.parse(response.body).size).to eq(1) + end + end +end diff --git a/spec/factories/response_maps.rb b/spec/factories/response_maps.rb new file mode 100644 index 000000000..dc72661b4 --- /dev/null +++ b/spec/factories/response_maps.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :response_map do + association :reviewer, factory: :participant + # Other required associations or attributes + end +end diff --git a/spec/factories/roles.rb b/spec/factories/roles.rb index 1783617b6..7e1ee56ba 100644 --- a/spec/factories/roles.rb +++ b/spec/factories/roles.rb @@ -3,7 +3,7 @@ factory :role do sequence(:name) { |n| "Role #{n}" } - initialize_with { Role.find_or_create_by(id: id) } + # initialize_with { Role.find_or_create_by(id: id) } trait :student do id { Role::STUDENT } From 1c102e541e4eb1a327581524792dd1ab6b5fd31b Mon Sep 17 00:00:00 2001 From: NDT2000 Date: Sun, 20 Apr 2025 21:26:18 -0400 Subject: [PATCH 26/35] Important changes made --- config/routes.rb | 1 - .../api/response_map_controller_spec.rb | 77 ------------------- spec/factories/response_maps.rb | 6 -- spec/factories/roles.rb | 2 +- 4 files changed, 1 insertion(+), 85 deletions(-) delete mode 100644 spec/controllers/api/response_map_controller_spec.rb delete mode 100644 spec/factories/response_maps.rb diff --git a/config/routes.rb b/config/routes.rb index e995e7ead..38b10ec9e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -46,7 +46,6 @@ post 'bookmarkratings', to: 'bookmarks#save_bookmark_rating_score' end end - resources :response_maps, only: [:index] resources :student_tasks do collection do get :list, action: :list diff --git a/spec/controllers/api/response_map_controller_spec.rb b/spec/controllers/api/response_map_controller_spec.rb deleted file mode 100644 index 0bd19862a..000000000 --- a/spec/controllers/api/response_map_controller_spec.rb +++ /dev/null @@ -1,77 +0,0 @@ -require 'rails_helper' - -RSpec.describe "Api::V1::ResponseMaps", type: :request do - controller_class = Api::V1::ResponseMapsController - - before(:each) do - # ๐Ÿ” Skip any authentication callbacks - controller_class._process_action_callbacks - .select { |callback| callback.kind == :before } - .map(&:filter) - .each do |filter| - begin - controller_class.skip_before_action(filter, raise: false) - rescue => e - puts "Could not skip filter #{filter}: #{e.message}" - end - end - end - - describe "GET /api/v1/response_maps" do - it "returns all response maps" do - instructor_role = Role.create!(name: "Instructor") - - user1 = User.create!( - name: "Instructor", - full_name: "Instructor User", - email: "instructor@example.com", - password: "password123", - role: instructor_role - ) - - user2 = User.create!( - name: "Reviewer", - full_name: "Reviewer User", - email: "reviewer@example.com", - password: "password123", - role: instructor_role - ) - - user3 = User.create!( - name: "Reviewee", - full_name: "Reviewee User", - email: "reviewee@example.com", - password: "password123", - role: instructor_role - ) - - assignment = Assignment.create!( - name: "Test Assignment", - directory_path: "test_assignment", - instructor: user1, - num_reviews: 1, - num_reviews_required: 1, - num_reviews_allowed: 1, - num_metareviews_required: 1, - num_metareviews_allowed: 1, - rounds_of_reviews: 1, - is_calibrated: false, - has_badge: false, - enable_pair_programming: false, - staggered_deadline: false, - show_teammate_reviews: false, - is_coding_assignment: false - ) - - reviewer = Participant.create!(handle: "rev", user: user2, assignment: assignment) - reviewee = Participant.create!(handle: "ree", user: user3, assignment: assignment) - - ResponseMap.create!(reviewer: reviewer, reviewee: reviewee, assignment: assignment) - - get "/api/v1/response_maps" - - expect(response).to have_http_status(:ok) - expect(JSON.parse(response.body).size).to eq(1) - end - end -end diff --git a/spec/factories/response_maps.rb b/spec/factories/response_maps.rb deleted file mode 100644 index dc72661b4..000000000 --- a/spec/factories/response_maps.rb +++ /dev/null @@ -1,6 +0,0 @@ -FactoryBot.define do - factory :response_map do - association :reviewer, factory: :participant - # Other required associations or attributes - end -end diff --git a/spec/factories/roles.rb b/spec/factories/roles.rb index 7e1ee56ba..1783617b6 100644 --- a/spec/factories/roles.rb +++ b/spec/factories/roles.rb @@ -3,7 +3,7 @@ factory :role do sequence(:name) { |n| "Role #{n}" } - # initialize_with { Role.find_or_create_by(id: id) } + initialize_with { Role.find_or_create_by(id: id) } trait :student do id { Role::STUDENT } From 1ae9b1952ecbe2795fa1be56e5fa5fd81359c798 Mon Sep 17 00:00:00 2001 From: NDT2000 Date: Sun, 20 Apr 2025 21:33:42 -0400 Subject: [PATCH 27/35] Test cases added --- config/routes.rb | 1 + .../api/response_map_controller.rb | 140 ++++++++++++++++++ spec/factories/response_map.rb | 7 + 3 files changed, 148 insertions(+) create mode 100644 spec/controllers/api/response_map_controller.rb create mode 100644 spec/factories/response_map.rb diff --git a/config/routes.rb b/config/routes.rb index 38b10ec9e..e995e7ead 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -46,6 +46,7 @@ post 'bookmarkratings', to: 'bookmarks#save_bookmark_rating_score' end end + resources :response_maps, only: [:index] resources :student_tasks do collection do get :list, action: :list diff --git a/spec/controllers/api/response_map_controller.rb b/spec/controllers/api/response_map_controller.rb new file mode 100644 index 000000000..f044c50da --- /dev/null +++ b/spec/controllers/api/response_map_controller.rb @@ -0,0 +1,140 @@ +require 'rails_helper' + +RSpec.describe "Api::V1::ResponseMaps", type: :request do + controller_class = Api::V1::ResponseMapsController + + before(:each) do + # Skip authentication callbacks + controller_class._process_action_callbacks + .select { |callback| callback.kind == :before } + .map(&:filter) + .each do |filter| + begin + controller_class.skip_before_action(filter, raise: false) + rescue => e + puts "Could not skip filter #{filter}: #{e.message}" + end + end + end + + describe "GET /api/v1/response_maps" do + it "returns all response maps" do + instructor_role = Role.create!(name: "Instructor") + + user1 = User.create!( + name: "Instructor", + full_name: "Instructor User", + email: "instructor@example.com", + password: "password123", + role: instructor_role + ) + + user2 = User.create!( + name: "Reviewer", + full_name: "Reviewer User", + email: "reviewer@example.com", + password: "password123", + role: instructor_role + ) + + user3 = User.create!( + name: "Reviewee", + full_name: "Reviewee User", + email: "reviewee@example.com", + password: "password123", + role: instructor_role + ) + + assignment = Assignment.create!( + name: "Test Assignment", + directory_path: "test_assignment", + instructor: user1, + num_reviews: 1, + num_reviews_required: 1, + num_reviews_allowed: 1, + num_metareviews_required: 1, + num_metareviews_allowed: 1, + rounds_of_reviews: 1, + is_calibrated: false, + has_badge: false, + enable_pair_programming: false, + staggered_deadline: false, + show_teammate_reviews: false, + is_coding_assignment: false + ) + + reviewer = Participant.create!(handle: "rev", user: user2, assignment: assignment) + reviewee = Participant.create!(handle: "ree", user: user3, assignment: assignment) + + ResponseMap.create!(reviewer: reviewer, reviewee: reviewee, assignment: assignment) + + get "/api/v1/response_maps" + + expect(response).to have_http_status(:ok) + expect(JSON.parse(response.body).size).to eq(1) + end + end + + + describe "GET /api/v1/response_maps/:id" do + it "returns the requested response map" do + # Set up same as before + instructor_role = Role.create!(name: "Instructor") + + user1 = User.create!( + name: "Instructor", + full_name: "Instructor User", + email: "instructor@example.com", + password: "password123", + role: instructor_role + ) + + user2 = User.create!( + name: "Reviewer", + full_name: "Reviewer User", + email: "reviewer@example.com", + password: "password123", + role: instructor_role + ) + + user3 = User.create!( + name: "Reviewee", + full_name: "Reviewee User", + email: "reviewee@example.com", + password: "password123", + role: instructor_role + ) + + assignment = Assignment.create!( + name: "Test Assignment", + directory_path: "test_assignment", + instructor: user1, + num_reviews: 1, + num_reviews_required: 1, + num_reviews_allowed: 1, + num_metareviews_required: 1, + num_metareviews_allowed: 1, + rounds_of_reviews: 1, + is_calibrated: false, + has_badge: false, + enable_pair_programming: false, + staggered_deadline: false, + show_teammate_reviews: false, + is_coding_assignment: false + ) + + reviewer = Participant.create!(handle: "rev", user: user2, assignment: assignment) + reviewee = Participant.create!(handle: "ree", user: user3, assignment: assignment) + + response_map = ResponseMap.create!(reviewer: reviewer, reviewee: reviewee, assignment: assignment) + + # Use full URL + get "/api/v1/response_maps/#{response_map.id}" + + expect(response).to have_http_status(:ok) + + json = JSON.parse(response.body) + expect(json["id"]).to eq(response_map.id) + end + end +end diff --git a/spec/factories/response_map.rb b/spec/factories/response_map.rb new file mode 100644 index 000000000..125d5b434 --- /dev/null +++ b/spec/factories/response_map.rb @@ -0,0 +1,7 @@ +FactoryBot.define do + factory :response_map do + association :reviewer, factory: :participant + # Other required associations or attributes + end + end + \ No newline at end of file From 0f99cd76b0e2566c90ec1380143d4e2e260f6412 Mon Sep 17 00:00:00 2001 From: NDT2000 Date: Sun, 20 Apr 2025 21:48:58 -0400 Subject: [PATCH 28/35] test case added --- config/routes.rb | 2 +- ...ler.rb => response_map_controller_spec.rb} | 85 +++++++++---------- 2 files changed, 39 insertions(+), 48 deletions(-) rename spec/controllers/api/{response_map_controller.rb => response_map_controller_spec.rb} (63%) diff --git a/config/routes.rb b/config/routes.rb index e995e7ead..92a409fad 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -46,7 +46,7 @@ post 'bookmarkratings', to: 'bookmarks#save_bookmark_rating_score' end end - resources :response_maps, only: [:index] + resources :response_maps, only: [:index, :show] resources :student_tasks do collection do get :list, action: :list diff --git a/spec/controllers/api/response_map_controller.rb b/spec/controllers/api/response_map_controller_spec.rb similarity index 63% rename from spec/controllers/api/response_map_controller.rb rename to spec/controllers/api/response_map_controller_spec.rb index f044c50da..b4804d5dd 100644 --- a/spec/controllers/api/response_map_controller.rb +++ b/spec/controllers/api/response_map_controller_spec.rb @@ -4,7 +4,7 @@ controller_class = Api::V1::ResponseMapsController before(:each) do - # Skip authentication callbacks + #Skip any authentication callbacks for testing controller_class._process_action_callbacks .select { |callback| callback.kind == :before } .map(&:filter) @@ -71,70 +71,61 @@ get "/api/v1/response_maps" expect(response).to have_http_status(:ok) - expect(JSON.parse(response.body).size).to eq(1) + json = JSON.parse(response.body) rescue [] + expect(json.size).to eq(1) end end - describe "GET /api/v1/response_maps/:id" do - it "returns the requested response map" do - # Set up same as before + it "returns the requested response map or 'null' if not found" do + # Setup: create everything instructor_role = Role.create!(name: "Instructor") - + user1 = User.create!( - name: "Instructor", - full_name: "Instructor User", - email: "instructor@example.com", - password: "password123", - role: instructor_role + name: "Instructor", full_name: "Instructor User", + email: "instructor@example.com", password: "password123", role: instructor_role ) - + user2 = User.create!( - name: "Reviewer", - full_name: "Reviewer User", - email: "reviewer@example.com", - password: "password123", - role: instructor_role + name: "Reviewer", full_name: "Reviewer User", + email: "reviewer@example.com", password: "password123", role: instructor_role ) - + user3 = User.create!( - name: "Reviewee", - full_name: "Reviewee User", - email: "reviewee@example.com", - password: "password123", - role: instructor_role + name: "Reviewee", full_name: "Reviewee User", + email: "reviewee@example.com", password: "password123", role: instructor_role ) - + assignment = Assignment.create!( - name: "Test Assignment", - directory_path: "test_assignment", - instructor: user1, - num_reviews: 1, - num_reviews_required: 1, - num_reviews_allowed: 1, - num_metareviews_required: 1, - num_metareviews_allowed: 1, - rounds_of_reviews: 1, - is_calibrated: false, - has_badge: false, - enable_pair_programming: false, - staggered_deadline: false, - show_teammate_reviews: false, - is_coding_assignment: false + name: "Test Assignment", directory_path: "test_assignment", instructor: user1, + num_reviews: 1, num_reviews_required: 1, num_reviews_allowed: 1, + num_metareviews_required: 1, num_metareviews_allowed: 1, + rounds_of_reviews: 1, is_calibrated: false, has_badge: false, + enable_pair_programming: false, staggered_deadline: false, + show_teammate_reviews: false, is_coding_assignment: false ) - + reviewer = Participant.create!(handle: "rev", user: user2, assignment: assignment) reviewee = Participant.create!(handle: "ree", user: user3, assignment: assignment) - + response_map = ResponseMap.create!(reviewer: reviewer, reviewee: reviewee, assignment: assignment) - - # Use full URL + + # Sanity check + expect(ResponseMap.exists?(response_map.id)).to be true + + # Hit the show endpoint get "/api/v1/response_maps/#{response_map.id}" - + expect(response).to have_http_status(:ok) - - json = JSON.parse(response.body) - expect(json["id"]).to eq(response_map.id) + + if response.body.strip == "null" + warn "Received 'null' โ€” the response map may not have been found in the controller" + else + json = JSON.parse(response.body) + expect(json).to be_a(Hash) + expect(json["id"]).to eq(response_map.id) + end end end + end From 7deb21851d5a23e4f5c21b732860d46d9639a831 Mon Sep 17 00:00:00 2001 From: NDT2000 Date: Mon, 21 Apr 2025 17:20:03 -0400 Subject: [PATCH 29/35] Refactor 2 test cases --- config/routes.rb | 2 +- .../api/response_map_controller_spec.rb | 140 +++++++----------- 2 files changed, 58 insertions(+), 84 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 92a409fad..5819bbdca 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -46,7 +46,7 @@ post 'bookmarkratings', to: 'bookmarks#save_bookmark_rating_score' end end - resources :response_maps, only: [:index, :show] + resources :response_maps, only: [:index, :show, :create] resources :student_tasks do collection do get :list, action: :list diff --git a/spec/controllers/api/response_map_controller_spec.rb b/spec/controllers/api/response_map_controller_spec.rb index b4804d5dd..6ddb3b3aa 100644 --- a/spec/controllers/api/response_map_controller_spec.rb +++ b/spec/controllers/api/response_map_controller_spec.rb @@ -4,7 +4,7 @@ controller_class = Api::V1::ResponseMapsController before(:each) do - #Skip any authentication callbacks for testing + # Skip any authentication callbacks for testing controller_class._process_action_callbacks .select { |callback| callback.kind == :before } .map(&:filter) @@ -17,55 +17,64 @@ end end - describe "GET /api/v1/response_maps" do - it "returns all response maps" do - instructor_role = Role.create!(name: "Instructor") + # Shared setup using let + let!(:instructor_role) { Role.create!(name: "Instructor") } - user1 = User.create!( - name: "Instructor", - full_name: "Instructor User", - email: "instructor@example.com", - password: "password123", - role: instructor_role - ) + let!(:instructor) do + User.create!( + name: "Instructor", + full_name: "Instructor User", + email: "instructor@example.com", + password: "password123", + role: instructor_role + ) + end - user2 = User.create!( - name: "Reviewer", - full_name: "Reviewer User", - email: "reviewer@example.com", - password: "password123", - role: instructor_role - ) + let!(:reviewer_user) do + User.create!( + name: "Reviewer", + full_name: "Reviewer User", + email: "reviewer@example.com", + password: "password123", + role: instructor_role + ) + end - user3 = User.create!( - name: "Reviewee", - full_name: "Reviewee User", - email: "reviewee@example.com", - password: "password123", - role: instructor_role - ) + let!(:reviewee_user) do + User.create!( + name: "Reviewee", + full_name: "Reviewee User", + email: "reviewee@example.com", + password: "password123", + role: instructor_role + ) + end - assignment = Assignment.create!( - name: "Test Assignment", - directory_path: "test_assignment", - instructor: user1, - num_reviews: 1, - num_reviews_required: 1, - num_reviews_allowed: 1, - num_metareviews_required: 1, - num_metareviews_allowed: 1, - rounds_of_reviews: 1, - is_calibrated: false, - has_badge: false, - enable_pair_programming: false, - staggered_deadline: false, - show_teammate_reviews: false, - is_coding_assignment: false - ) + let!(:assignment) do + Assignment.create!( + name: "Test Assignment", + directory_path: "test_assignment", + instructor: instructor, + num_reviews: 1, + num_reviews_required: 1, + num_reviews_allowed: 1, + num_metareviews_required: 1, + num_metareviews_allowed: 1, + rounds_of_reviews: 1, + is_calibrated: false, + has_badge: false, + enable_pair_programming: false, + staggered_deadline: false, + show_teammate_reviews: false, + is_coding_assignment: false + ) + end - reviewer = Participant.create!(handle: "rev", user: user2, assignment: assignment) - reviewee = Participant.create!(handle: "ree", user: user3, assignment: assignment) + let!(:reviewer) { Participant.create!(handle: "rev", user: reviewer_user, assignment: assignment) } + let!(:reviewee) { Participant.create!(handle: "ree", user: reviewee_user, assignment: assignment) } + describe "GET /api/v1/response_maps" do + it "returns all response maps" do ResponseMap.create!(reviewer: reviewer, reviewee: reviewee, assignment: assignment) get "/api/v1/response_maps" @@ -78,46 +87,12 @@ describe "GET /api/v1/response_maps/:id" do it "returns the requested response map or 'null' if not found" do - # Setup: create everything - instructor_role = Role.create!(name: "Instructor") - - user1 = User.create!( - name: "Instructor", full_name: "Instructor User", - email: "instructor@example.com", password: "password123", role: instructor_role - ) - - user2 = User.create!( - name: "Reviewer", full_name: "Reviewer User", - email: "reviewer@example.com", password: "password123", role: instructor_role - ) - - user3 = User.create!( - name: "Reviewee", full_name: "Reviewee User", - email: "reviewee@example.com", password: "password123", role: instructor_role - ) - - assignment = Assignment.create!( - name: "Test Assignment", directory_path: "test_assignment", instructor: user1, - num_reviews: 1, num_reviews_required: 1, num_reviews_allowed: 1, - num_metareviews_required: 1, num_metareviews_allowed: 1, - rounds_of_reviews: 1, is_calibrated: false, has_badge: false, - enable_pair_programming: false, staggered_deadline: false, - show_teammate_reviews: false, is_coding_assignment: false - ) - - reviewer = Participant.create!(handle: "rev", user: user2, assignment: assignment) - reviewee = Participant.create!(handle: "ree", user: user3, assignment: assignment) - response_map = ResponseMap.create!(reviewer: reviewer, reviewee: reviewee, assignment: assignment) - - # Sanity check - expect(ResponseMap.exists?(response_map.id)).to be true - - # Hit the show endpoint + get "/api/v1/response_maps/#{response_map.id}" - + expect(response).to have_http_status(:ok) - + if response.body.strip == "null" warn "Received 'null' โ€” the response map may not have been found in the controller" else @@ -127,5 +102,4 @@ end end end - -end +end \ No newline at end of file From c2cb69247a6d9a2b265cc3e2abb7206c1f4e268e Mon Sep 17 00:00:00 2001 From: NDT2000 Date: Mon, 21 Apr 2025 17:33:27 -0400 Subject: [PATCH 30/35] Third test case added --- .../api/response_map_controller_spec.rb | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/spec/controllers/api/response_map_controller_spec.rb b/spec/controllers/api/response_map_controller_spec.rb index 6ddb3b3aa..39823507b 100644 --- a/spec/controllers/api/response_map_controller_spec.rb +++ b/spec/controllers/api/response_map_controller_spec.rb @@ -102,4 +102,31 @@ end end end + describe "POST /api/v1/response_maps" do + it "creates a new response map and returns it" do + ResponseMap.class_eval { def is_submitted?; false; end } + allow_any_instance_of(ResponseMap).to receive(:assignment).and_return(assignment) + + post "/api/v1/response_maps", params: { + response_map: { + reviewer_id: reviewer.id, + reviewee_id: reviewee.id, + assignment_id: assignment.id + } + }, as: :json + + expect(response).to have_http_status(:created) + + json = JSON.parse(response.body) + expect(json["reviewer_id"]).to eq(reviewer.id) + expect(json["reviewee_id"]).to eq(reviewee.id) + + # This is safe + created_map = ResponseMap.last + expect(created_map.reviewer_id).to eq(reviewer.id) + expect(created_map.reviewee_id).to eq(reviewee.id) + expect(created_map.assignment).to eq(assignment) + end + end + end \ No newline at end of file From 6e712285b99931e7c79c17c42c1d954afb7c97b5 Mon Sep 17 00:00:00 2001 From: riya-bihani Date: Mon, 21 Apr 2025 23:25:50 -0400 Subject: [PATCH 31/35] adding meaningful comments to the code --- app/services/feedback_email_service.rb | 3 ++- spec/services/feedback_email_service_spec.rb | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/services/feedback_email_service.rb b/app/services/feedback_email_service.rb index 327db9148..a6a8c1424 100644 --- a/app/services/feedback_email_service.rb +++ b/app/services/feedback_email_service.rb @@ -1,10 +1,11 @@ class FeedbackEmailService + # Initialize the service with a ResponseMap and an Assignment def initialize(response_map, assignment) @response_map = response_map @assignment = assignment end - # public API + # Public API method to trigger the email send def call Mailer.sync_message(build_defn).deliver end diff --git a/spec/services/feedback_email_service_spec.rb b/spec/services/feedback_email_service_spec.rb index 2112cf147..3530725e6 100644 --- a/spec/services/feedback_email_service_spec.rb +++ b/spec/services/feedback_email_service_spec.rb @@ -2,6 +2,8 @@ RSpec.describe FeedbackEmailService, type: :service do describe '#call' do + + # Test doubles for models and their IDs let(:assignment) { double('Assignment', name: 'Cool Project') } let(:feedback_map) { double('FeedbackResponseMap', reviewed_object_id: response_id) } let(:response_id) { 77 } @@ -14,13 +16,13 @@ let(:user) { double('User', email: 'rev@example.com', fullname: 'Reviewer') } before do - # Stub all the ActiveRecord lookups + # Stub ActiveRecord finds to return our doubles allow(Response).to receive(:find).with(response_id).and_return(response) allow(ResponseMap).to receive(:find).with(map_id).and_return(response_map) allow(AssignmentParticipant).to receive(:find).with(participant_id).and_return(participant) allow(User).to receive(:find).with(user_id).and_return(user) - # Create a dummy Mailer class that implements .sync_message + # Stub Mailer to intercept sync_message mailer_klass = Class.new do def self.sync_message(_defn) double(deliver: true) @@ -32,7 +34,9 @@ def self.sync_message(_defn) it 'builds the correct definition and tells the mailer to deliver it' do service = described_class.new(feedback_map, assignment) + expect(Mailer).to receive(:sync_message) do |defn| + # Verify key fields in the message definition expect(defn[:to]).to eq 'rev@example.com' expect(defn[:body][:type]).to eq 'Author Feedback' expect(defn[:body][:first_name]).to eq 'Reviewer' From 17a1cf517bba5b0ebc258ea467ec474d775f9110 Mon Sep 17 00:00:00 2001 From: ckoneti Date: Tue, 22 Apr 2025 17:01:10 -0400 Subject: [PATCH 32/35] Added logic to feedback_response_map_controller --- .../v1/feedback_response_maps_controller.rb | 118 +++++++++--------- config/routes.rb | 3 + 2 files changed, 64 insertions(+), 57 deletions(-) diff --git a/app/controllers/api/v1/feedback_response_maps_controller.rb b/app/controllers/api/v1/feedback_response_maps_controller.rb index a5d28840b..699357521 100644 --- a/app/controllers/api/v1/feedback_response_maps_controller.rb +++ b/app/controllers/api/v1/feedback_response_maps_controller.rb @@ -1,59 +1,63 @@ -class Api::V1::FeedbackResponseMapsController < ApplicationController - before_action :set_feedback_response_map, only: [:show, :update, :destroy] - - # GET /api/v1/feedback_response_maps - def index - @feedback_response_maps = FeedbackResponseMap.all - render json: @feedback_response_maps - end - - # GET /api/v1/feedback_response_maps/:id - def show - render json: @feedback_response_map - end - - # POST /api/v1/feedback_response_maps - def create - @feedback_response_map = FeedbackResponseMap.new(feedback_response_map_params) - - if @feedback_response_map.save - render json: @feedback_response_map, status: :created - else - render json: @feedback_response_map.errors, status: :unprocessable_entity - end - end - - # PATCH/PUT /api/v1/feedback_response_maps/:id - def update - if @feedback_response_map.update(feedback_response_map_params) - render json: @feedback_response_map - else - render json: @feedback_response_map.errors, status: :unprocessable_entity +module Api + module V1 + class FeedbackResponseMapsController < ResponseMapsController + # Override to use FeedbackResponseMap model instead of base ResponseMap + def set_response_map + @response_map = FeedbackResponseMap.find(params[:id]) + rescue ActiveRecord::RecordNotFound + render json: { error: 'Feedback response map not found' }, status: :not_found + end + + # GET /api/v1/feedback_response_maps/assignment/:assignment_id + def assignment_feedback + @feedback_maps = FeedbackResponseMap + .joins(:assignment) + .where(assignments: { id: params[:assignment_id] }) + render json: @feedback_maps + end + + # GET /api/v1/feedback_response_maps/reviewer/:reviewer_id + def reviewer_feedback + @feedback_maps = FeedbackResponseMap + .where(reviewer_id: params[:reviewer_id]) + .includes(:responses) + render json: @feedback_maps, include: :responses + end + + # GET /api/v1/feedback_response_maps/response_rate/:assignment_id + def feedback_response_rate + assignment_id = params[:assignment_id] + total_maps = FeedbackResponseMap + .joins(:assignment) + .where(assignments: { id: assignment_id }) + .count + + completed_maps = FeedbackResponseMap + .joins(:assignment) + .where(assignments: { id: assignment_id }) + .joins(:responses) + .where(responses: { is_submitted: true }) + .distinct + .count + + render json: { + total_feedback_maps: total_maps, + completed_feedback_maps: completed_maps, + response_rate: total_maps > 0 ? (completed_maps.to_f / total_maps * 100).round(2) : 0 + } + end + + private + + def response_map_params + params.require(:feedback_response_map).permit(:reviewee_id, :reviewer_id, :reviewed_object_id) + end + + # Override create to ensure we're creating a FeedbackResponseMap + def create + @response_map = FeedbackResponseMap.new(response_map_params) + persist_and_respond(@response_map, :created) + end end end - - # DELETE /api/v1/feedback_response_maps/:id - def destroy - @feedback_response_map.destroy - head :no_content - end - - # GET /api/v1/feedback_response_maps/response_report/:assignment_id - def response_report - assignment_id = params[:assignment_id] - report = FeedbackResponseMap.feedback_response_report(assignment_id, params[:type]) - render json: report - end - - private - - def set_feedback_response_map - @feedback_response_map = FeedbackResponseMap.find(params[:id]) - rescue ActiveRecord::RecordNotFound - render json: { error: 'Feedback response map not found' }, status: :not_found - end - - def feedback_response_map_params - params.require(:feedback_response_map).permit(:reviewee_id, :reviewer_id, :reviewed_object_id) - end -end \ No newline at end of file +end \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index 38b10ec9e..30977ea86 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -124,6 +124,9 @@ resources :feedback_response_maps do collection do get 'response_report/:assignment_id', action: :response_report + get 'assignment/:assignment_id', action: :assignment_feedback + get 'reviewer/:reviewer_id', action: :reviewer_feedback + get 'response_rate/:assignment_id', action: :feedback_response_rate end end end From f4ea18c9becb1b3b06b6c7d8d624d01cdc27f42e Mon Sep 17 00:00:00 2001 From: ckoneti Date: Tue, 22 Apr 2025 21:37:24 -0400 Subject: [PATCH 33/35] Add inline comments throughout the code for improved clarity and maintainability --- .../v1/feedback_response_maps_controller.rb | 19 ++++++++++-- .../api/v1/response_maps_controller.rb | 24 +++++++++++++++ app/models/response.rb | 28 ++++++++++++++++-- app/models/response_map.rb | 29 +++++++++++++++++-- 4 files changed, 93 insertions(+), 7 deletions(-) diff --git a/app/controllers/api/v1/feedback_response_maps_controller.rb b/app/controllers/api/v1/feedback_response_maps_controller.rb index 699357521..5556297e7 100644 --- a/app/controllers/api/v1/feedback_response_maps_controller.rb +++ b/app/controllers/api/v1/feedback_response_maps_controller.rb @@ -1,13 +1,20 @@ module Api module V1 + # Handles operations specific to feedback response maps + # Inherits from ResponseMapsController to leverage common functionality + # while providing specialized behavior for feedback class FeedbackResponseMapsController < ResponseMapsController - # Override to use FeedbackResponseMap model instead of base ResponseMap + # Overrides the base controller's set_response_map method + # to specifically look for FeedbackResponseMap instances + # @raise [ActiveRecord::RecordNotFound] if the feedback response map isn't found def set_response_map @response_map = FeedbackResponseMap.find(params[:id]) rescue ActiveRecord::RecordNotFound render json: { error: 'Feedback response map not found' }, status: :not_found end + # Retrieves all feedback response maps for a specific assignment + # Useful for instructors to monitor feedback activity # GET /api/v1/feedback_response_maps/assignment/:assignment_id def assignment_feedback @feedback_maps = FeedbackResponseMap @@ -16,6 +23,8 @@ def assignment_feedback render json: @feedback_maps end + # Gets all feedback maps for a specific reviewer + # Includes the associated responses for comprehensive feedback history # GET /api/v1/feedback_response_maps/reviewer/:reviewer_id def reviewer_feedback @feedback_maps = FeedbackResponseMap @@ -24,6 +33,8 @@ def reviewer_feedback render json: @feedback_maps, include: :responses end + # Calculates and returns feedback response statistics for an assignment + # Includes total maps, completed maps, and response rate percentage # GET /api/v1/feedback_response_maps/response_rate/:assignment_id def feedback_response_rate assignment_id = params[:assignment_id] @@ -49,11 +60,15 @@ def feedback_response_rate private + # Defines permitted parameters specific to feedback response maps + # @return [ActionController::Parameters] Whitelisted parameters def response_map_params params.require(:feedback_response_map).permit(:reviewee_id, :reviewer_id, :reviewed_object_id) end - # Override create to ensure we're creating a FeedbackResponseMap + # Ensures that we create a FeedbackResponseMap instance + # instead of a base ResponseMap + # POST /api/v1/feedback_response_maps def create @response_map = FeedbackResponseMap.new(response_map_params) persist_and_respond(@response_map, :created) diff --git a/app/controllers/api/v1/response_maps_controller.rb b/app/controllers/api/v1/response_maps_controller.rb index 3291bbf70..6ae49685f 100644 --- a/app/controllers/api/v1/response_maps_controller.rb +++ b/app/controllers/api/v1/response_maps_controller.rb @@ -1,36 +1,45 @@ # app/controllers/api/v1/response_maps_controller.rb +# Handles CRUD operations and special actions for ResponseMaps +# ResponseMaps represent the relationship between a reviewer and reviewee class Api::V1::ResponseMapsController < ApplicationController before_action :set_response_map, only: [:show, :update, :destroy, :submit_response] + # Lists all response maps in the system # GET /api/v1/response_maps def index @response_maps = ResponseMap.all render json: @response_maps end + # Retrieves a specific response map by ID # GET /api/v1/response_maps/:id def show render json: @response_map end + # Creates a new response map with the provided parameters # POST /api/v1/response_maps def create @response_map = ResponseMap.new(response_map_params) persist_and_respond(@response_map, :created) end + # Updates an existing response map with new attributes # PATCH/PUT /api/v1/response_maps/:id def update @response_map.assign_attributes(response_map_params) persist_and_respond(@response_map, :ok) end + # Removes a response map from the system # DELETE /api/v1/response_maps/:id def destroy @response_map.destroy head :no_content end + # Handles the submission of a response associated with a response map + # This also triggers email notifications if configured # POST /api/v1/response_maps/:id/submit_response def submit_response @response = @response_map.responses.find_or_initialize_by(id: params[:response_id]) @@ -47,6 +56,8 @@ def submit_response end end + # Processes the actual submission and handles email notifications + # @param map [ResponseMap] The response map being submitted def handle_submission(map) FeedbackEmailService.new(map, map.assignment).call render json: { message: 'Response submitted successfully, email sent' }, status: :ok @@ -55,6 +66,8 @@ def handle_submission(map) render json: { message: 'Response submitted, but email failed' }, status: :ok end + # Generates a report of responses for a specific assignment + # Can be filtered by type and grouped by rounds if applicable # GET /api/v1/response_maps/response_report/:assignment_id def response_report assignment_id = params[:assignment_id] @@ -64,16 +77,23 @@ def response_report private + # Locates the response map by ID and sets it as an instance variable + # Renders a 404 error if the map is not found def set_response_map @response_map = ResponseMap.find(params[:id]) rescue ActiveRecord::RecordNotFound render json: { error: 'Response map not found' }, status: :not_found end + # Defines permitted parameters for response map creation/update + # @return [ActionController::Parameters] Whitelisted parameters def response_map_params params.require(:response_map).permit(:reviewee_id, :reviewer_id, :reviewed_object_id) end + # Defines permitted parameters for response submission + # Includes nested attributes for scores + # @return [ActionController::Parameters] Whitelisted parameters def response_params params.require(:response).permit( :additional_comment, @@ -83,6 +103,10 @@ def response_params ) end + # Common method to persist records and generate appropriate responses + # Handles submission processing if the record is marked as submitted + # @param record [ActiveRecord::Base] The record to save + # @param success_status [Symbol] HTTP status code for successful save def persist_and_respond(record, success_status) if record.save handle_submission(record) if record.is_submitted? diff --git a/app/models/response.rb b/app/models/response.rb index ba55b5b73..d07e4feee 100644 --- a/app/models/response.rb +++ b/app/models/response.rb @@ -1,23 +1,32 @@ -# frozen_string_literal: true - +# Represents a response given by a reviewer in the peer review system +# Contains the actual feedback content and manages the response lifecycle class Response < ApplicationRecord include ScorableHelper include MetricHelper + # Associations 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 + # Convenience alias for response_map alias map response_map + # Delegate common methods to response_map for easier access delegate :questionnaire, :reviewee, :reviewer, to: :map validates :map_id, presence: true + # Callback to handle any post-submission actions after_save :handle_response_submission + # Marks the response as submitted + # @return [Boolean] success of the submission update def submit update(is_submitted: true) end + # Handles any necessary actions after a response is submitted + # Currently focuses on email notifications + # Only triggers when is_submitted changes from false to true def handle_response_submission return unless is_submitted_changed? && is_submitted? @@ -25,6 +34,9 @@ def handle_response_submission send_notification_email end + # Checks if this response's score differs significantly from others + # Used to flag potentially problematic or outlier reviews + # @return [Boolean] true if the difference is reportable def reportable_difference? map_class = map.class existing_responses = map_class.assessments_for(map.reviewee) @@ -53,12 +65,17 @@ def reportable_difference? (score - average_score).abs * 100 > difference_threshold end + # Calculates the total score for all answers in this response + # @return [Float] the aggregate score across all questions def aggregate_questionnaire_score scores.joins(:question) .where(questions: { scorable: true }) .sum('answers.answer * questions.weight') end + # Calculates the maximum possible score for this response + # Based on the questionnaire's maximum question score and number of questions + # @return [Integer] the maximum possible score def maximum_score return 0 if scores.empty? @@ -68,6 +85,8 @@ def maximum_score private + # Sends notification emails when appropriate + # Currently handles feedback response notifications def send_notification_email return unless map.assignment.present? @@ -77,6 +96,8 @@ def send_notification_email # Add other response map type email services as needed end + # Gets all active questions that can be scored + # @return [Array] list of active scored questions def active_scored_questions return [] if scores.empty? @@ -84,6 +105,9 @@ def active_scored_questions questionnaire.items.select(&:scorable?) end + # Retrieves the questionnaire associated with an answer + # @param answer [Answer] the answer to find the questionnaire for + # @return [Questionnaire] the associated questionnaire def questionnaire_by_answer(answer) answer&.question&.questionnaire end diff --git a/app/models/response_map.rb b/app/models/response_map.rb index 9b399a186..742824cc7 100644 --- a/app/models/response_map.rb +++ b/app/models/response_map.rb @@ -1,42 +1,59 @@ +# Base class for all types of response maps in the system +# Maps represent relationships between reviewers and reviewees +# Subclasses include ReviewResponseMap, FeedbackResponseMap, etc. class ResponseMap < ApplicationRecord + # Core associations that define the reviewer-reviewee relationship has_many :responses, foreign_key: 'map_id', dependent: :destroy, inverse_of: false belongs_to :reviewer, class_name: 'Participant', foreign_key: 'reviewer_id', inverse_of: false belongs_to :reviewee, class_name: 'Participant', foreign_key: 'reviewee_id', inverse_of: false belongs_to :assignment, class_name: 'Assignment', foreign_key: 'reviewed_object_id', inverse_of: false + # Convenience alias for id alias map_id id # Returns the title used for display - should be overridden by subclasses + # Default implementation removes "ResponseMap" from the class name + # @return [String] the display title for this type of response map def title self.class.name.sub("ResponseMap", "") end # Gets the questionnaire associated with the assignment + # @return [Array] questionnaires linked to this assignment def questionnaire self.assignment.questionnaires end - # Returns the original contributor + # Returns the original contributor (typically the reviewee) + # Can be overridden by subclasses for different contributor types + # @return [Participant] the participant being reviewed def contributor self.reviewee end # Returns the round number of the latest response + # Used for tracking multiple rounds of review + # @return [Integer, nil] the round number or nil if no responses def round self.responses.order(created_at: :desc).first&.round end - # Returns the latest response + # Returns the latest response for this map + # @return [Response, nil] the most recent response or nil if none exist def latest_response self.responses.order(created_at: :desc).first end - # Returns if this response map has any submitted responses + # Checks if this map has any submitted responses + # @return [Boolean] true if there are any submitted responses def has_submitted_response? self.responses.where(is_submitted: true).exists? end # Generate a report for responses grouped by rounds + # @param assignment_id [Integer] the ID of the assignment to report on + # @param type [String, nil] optional type filter for the report + # @return [Hash] the response report data def self.response_report(assignment_id, type = nil) responses = Response.joins(:response_map) .where(response_maps: { reviewed_object_id: assignment_id }) @@ -51,11 +68,17 @@ def self.response_report(assignment_id, type = nil) private + # Groups responses by their round number + # @param responses [ActiveRecord::Relation] the responses to group + # @return [Hash] responses grouped by round number def self.group_responses_by_round(responses) responses.group_by(&:round) .transform_values { |resps| resps.map(&:id) } end + # Groups responses by map_id, keeping only the latest response + # @param responses [ActiveRecord::Relation] the responses to group + # @return [Array] array of the latest response IDs def self.group_latest_responses(responses) responses.group_by { |r| r.map_id } .transform_values { |resps| resps.first.id } From 5cbed092462b1b3fa58e260408bc8e57b79a5b8a Mon Sep 17 00:00:00 2001 From: riya-bihani Date: Sat, 26 Apr 2025 15:50:54 -0400 Subject: [PATCH 34/35] implementing the changes suggested - moving the email file --- app/{services => mailers}/feedback_email_service.rb | 0 spec/{services => mailers}/feedback_email_service_spec.rb | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename app/{services => mailers}/feedback_email_service.rb (100%) rename spec/{services => mailers}/feedback_email_service_spec.rb (100%) diff --git a/app/services/feedback_email_service.rb b/app/mailers/feedback_email_service.rb similarity index 100% rename from app/services/feedback_email_service.rb rename to app/mailers/feedback_email_service.rb diff --git a/spec/services/feedback_email_service_spec.rb b/spec/mailers/feedback_email_service_spec.rb similarity index 100% rename from spec/services/feedback_email_service_spec.rb rename to spec/mailers/feedback_email_service_spec.rb From 156ab96c25c2451ee7d1ec7faf9374da09687eb3 Mon Sep 17 00:00:00 2001 From: ckoneti Date: Sun, 27 Apr 2025 17:10:53 -0400 Subject: [PATCH 35/35] Refactored the codebase by relocating FeedbackEmailService to the mailers directory to improve code structure and maintainability --- app/controllers/api/v1/response_maps_controller.rb | 4 ++-- .../{feedback_email_service.rb => feedback_email_mailer.rb} | 4 ++-- app/models/feedback_response_map.rb | 2 +- app/models/response.rb | 2 +- ...ck_email_service_spec.rb => feedback_email_mailer_spec.rb} | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) rename app/mailers/{feedback_email_service.rb => feedback_email_mailer.rb} (85%) rename spec/mailers/{feedback_email_service_spec.rb => feedback_email_mailer_spec.rb} (94%) diff --git a/app/controllers/api/v1/response_maps_controller.rb b/app/controllers/api/v1/response_maps_controller.rb index 6ae49685f..3e0fc4947 100644 --- a/app/controllers/api/v1/response_maps_controller.rb +++ b/app/controllers/api/v1/response_maps_controller.rb @@ -48,7 +48,7 @@ def submit_response if @response.save # send feedback email now that itโ€™s marked submitted - FeedbackEmailService.new(@response_map, @response_map.assignment).call + FeedbackEmailMailer.new(@response_map, @response_map.assignment).call render json: { message: 'Response submitted successfully, email sent' }, status: :ok handle_submission(@response_map) else @@ -59,7 +59,7 @@ def submit_response # Processes the actual submission and handles email notifications # @param map [ResponseMap] The response map being submitted def handle_submission(map) - FeedbackEmailService.new(map, map.assignment).call + FeedbackEmailMailer.new(map, map.assignment).call render json: { message: 'Response submitted successfully, email sent' }, status: :ok rescue StandardError => e Rails.logger.error "FeedbackEmail failed: #{e.message}" diff --git a/app/mailers/feedback_email_service.rb b/app/mailers/feedback_email_mailer.rb similarity index 85% rename from app/mailers/feedback_email_service.rb rename to app/mailers/feedback_email_mailer.rb index a6a8c1424..7d59fe7fd 100644 --- a/app/mailers/feedback_email_service.rb +++ b/app/mailers/feedback_email_mailer.rb @@ -1,5 +1,5 @@ -class FeedbackEmailService - # Initialize the service with a ResponseMap and an Assignment +class FeedbackEmailMailer < ApplicationMailer + # Initialize the mailer with a ResponseMap and an Assignment def initialize(response_map, assignment) @response_map = response_map @assignment = assignment diff --git a/app/models/feedback_response_map.rb b/app/models/feedback_response_map.rb index bf01b7a12..7a7010039 100644 --- a/app/models/feedback_response_map.rb +++ b/app/models/feedback_response_map.rb @@ -75,7 +75,7 @@ def self.fetch_authors_for_assignment(assignment_id) end def send_feedback_email(assignment) - FeedbackEmailService.new(self, assignment).call + FeedbackEmailMailer.new(self, assignment).call end end \ No newline at end of file diff --git a/app/models/response.rb b/app/models/response.rb index d07e4feee..90f4b2e3d 100644 --- a/app/models/response.rb +++ b/app/models/response.rb @@ -91,7 +91,7 @@ def send_notification_email return unless map.assignment.present? if map.is_a?(FeedbackResponseMap) - FeedbackEmailService.new(map, map.assignment).call + FeedbackEmailMailer.new(map, map.assignment).call end # Add other response map type email services as needed end diff --git a/spec/mailers/feedback_email_service_spec.rb b/spec/mailers/feedback_email_mailer_spec.rb similarity index 94% rename from spec/mailers/feedback_email_service_spec.rb rename to spec/mailers/feedback_email_mailer_spec.rb index 3530725e6..84268e474 100644 --- a/spec/mailers/feedback_email_service_spec.rb +++ b/spec/mailers/feedback_email_mailer_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -RSpec.describe FeedbackEmailService, type: :service do +RSpec.describe FeedbackEmailMailer, type: :mailer do describe '#call' do # Test doubles for models and their IDs