CSC/ECE 517 Spring 2025 - E2509 Refactoring and Enhancing the Feedback Response Map Controller

From Expertiza_Wiki
Jump to navigation Jump to search

Introduction

Project Overview

This project aims to refactor the Feedback Response Map controller in the system and to improve the current Feedback Response Map into a more robust and maintainable Response Map and its corresponding controller. Within Expertiza, when a user submits a review, an instance of the Response class is created, associated with a ResponseMap. When a user writes and submits a review in Expertiza, an instance of Response class is created and associated with a ResponseMap. Every ResponseMap connects the reviewed item (reviewed_object_id), the reviewer (reviewer_id), and the reviewee (reviewee_id). Feedback provided by the reviewee on their received review is represented by FeedbackResponseMap, a subclass of ResponseMap. This subclass ensures that the reviewee's comments are appropriately linked to their reviews and this is what the project will mainly focus on.

The system architecture follows an inheritance structure where the ResponseMap class acts as a base class, and both ReviewResponseMap and FeedbackResponseMap are its subclasses. This object-oriented design allows for modular handling of different types of responses in the system. While ReviewResponseMap manages the mapping between reviewers and reviewees for peer reviews, FeedbackResponseMap is specifically responsible for capturing feedback from reviewees on the reviews they received. The inheritance ensures shared functionality resides in the ResponseMap superclass, while each subclass handles its specific behavior, promoting code reuse and easier maintenance.

Objectives

  • Refactoring to Response Map - Refactor the existing the Feedback Response Map to make a more generalized Response Map controller by restructuring the methods in the controller and integrate new functionalities in it to support a wider range of response types.
  • Thorough Testing - Develop a strategy to cover all features of the new refactored Response Map and controller. To ensure that all functionalities are working, unit tests, integration tests and end-to-end tests need to be included.
  • Develop and Test Email Functionality - The email notification functionality has to be included in this project but as separate logic to ensure that the Single Responsibility Principle is followed and there is modularity and clear separation of concerns.
  • Code Quality Improvement - The existing codebase should be reviewed and enhanced to improve readability and maintainability. This can be done by updating comments, class names, and variable names. It is important to ensure that all the modifications are well documented and clear.
  • Best Practices Implementation - Apply SOLID principles, DRY (Don’t Repeat Yourself) philosophy, and appropriate design patterns to enhance the modularity and testability of the code and justify each implementation choice.
  • Originality and Integrity - All the code should be original and should meet the ethical coding standards.

Current Implementation

Overview of ResponseMap Class

The ResponseMap class is an ActiveRecord model in a Ruby on Rails application, primarily responsible for managing the relationship between reviewers and reviewees within the context of an assignment. It acts as a bridge linking participants (reviewers and reviewees) with their respective responses in a peer-review system.

This class is tightly coupled with several other models:

  • Reviewer and Reviewee: Both are instances of the Participant class, representing users assigned to review or be reviewed.
  • Assignment: The object being reviewed, associated through the reviewed_object_id.
  • Response: The actual evaluation or review content provided by a reviewer.

Key Functionalities

  • Relationship Management: Utilizes has_many and belongs_to associations to link responses, participants, and assignments.
  • Alias Declaration: The model ID is aliased as map_id for internal referencing.
  • Custom Methods:
    • response_assignment: Fetches the assignment associated with the reviewer.
    • assessments_for(team): Gathers the most recent and relevant responses for a given team, filtering based on submission status and response type.

Logic and Behavior

The assessments_for class method exemplifies custom business logic that:

  • Iterates over response maps for a specific team.
  • Filters out empty or unsubmitted responses (especially in the case of ReviewResponseMap types).
  • Sorts and selects the latest version of each valid response.
  • Returns the final result sorted by reviewer name.

This class is integral to handling the complex mappings and evaluations common in peer-reviewed academic or collaborative environments.

Design Patterns in ResponseMap

The ResponseMap class in this Ruby on Rails application exhibits the use of established object-oriented design patterns. Two notable patterns implemented in the class are the Iterator and Delegation patterns.

Iterator Pattern

The assessments_for class method makes use of Ruby's built-in each construct to iterate over collections of ResponseMap instances and associated Response records. This reflects the Iterator design pattern, which provides a uniform interface for traversing elements in a collection. In this context, it allows the code to abstract away the internal structure of the response map list, enabling clean and consistent traversal without exposing collection details to the caller.

Delegation Pattern

The method response_assignment illustrates the Delegation pattern, where the task of retrieving the associated Assignment is delegated to the Participant object representing the reviewer. Rather than the ResponseMap owning the logic for determining the assignment, it calls Participant.find(self.reviewer_id).assignment, passing responsibility to the related model. Although implemented in a direct and somewhat manual way, it reflects the core idea of delegation—passing behavior to another object that is better suited to handle it.

Defects in existing code

While the ResponseMap class makes use of several object-oriented principles, some design patterns appear to have been applied in a limited or suboptimal manner. Two key patterns that could be improved in this implementation are the Strategy and Delegation patterns.

Strategy Pattern

The assessments_for method includes logic that changes behavior based on the type of response map, such as conditionally handling instances of ReviewResponseMap. This suggests the need for the Strategy pattern, where each type of map could encapsulate its own assessment retrieval behavior. However, instead of using polymorphism or encapsulated strategies, the method relies on conditional checks (if map.type.eql?('ReviewResponseMap')), leading to tightly coupled and hard-to-extend logic.

Delegation Pattern

The method response_assignment performs manual delegation by directly retrieving the reviewer's assignment through a chain of method calls (Participant.find(self.reviewer_id).assignment). This approach bypasses more maintainable delegation mechanisms provided by the framework, such as Ruby’s delegate method. As a result, the model becomes more tightly coupled to other classes and less readable, reducing flexibility and violating the principle of clear responsibility separation.

Implementation Plan

Email Functionality

To adhere to the Single Responsibility Principle, the email functionality needs to be extracted from the FeedbackResponseMap model and moved to a separate class called the FeedbackEmailService. This will ensure that the model remains focused solely on managing feedback mappings, while all concerns related to emails are handled independently. Separating this logic will improve code clarity, testability, and maintainability. While a simple service object is sufficient for the current need, other design patterns like Strategy or Visitor could also be considered depending on how the notification logic evolves, especially if multiple types of emails or behaviors need to be supported in the future.

ResponseMap Controller

The ResponseMapController serves as the base controller for all types of response maps within the system. It encapsulates common functionalities shared across different response map types, significantly reducing code duplication. By centralizing shared logic, the controller promotes a modular architecture, making the codebase easier to maintain, extend, and test. This refactoring effort aligns with the architectural improvements discussed with the professor and forms the foundation for scalable enhancements in the future.

FeedbackResponseMap Controller

The FeedbackResponseMapController extends the base ResponseMapController and overrides specific functionalities that differ from the generic response map logic. In addition to the overridden methods, this controller implements new features exclusive to feedback response maps, such as feedback_response_report. These additions are tailored to meet the unique requirements of handling feedback responses and ensure specialized behavior without compromising the integrity of the base class design.

Testing

We are planning to implement a comprehensive testing strategy to ensure the reliability and correctness of the refactored response map system. Key elements of this approach include:

  • Object Creation Based on Model Relationships: All dependent objects will be instantiated in accordance with the established model relationships before creating a response map object.
  • Edge Case Handling: We are addressing previously unhandled edge cases, such as creating response maps for rounds that have not yet occurred (e.g., assigning a round attribute value higher than the current one).
  • High Coverage: Our test suite includes unit, integration, and end-to-end tests to verify both base and specialized behaviors across the system.
  • Documentation: Each test will be accompanied by detailed documentation to facilitate understanding and reproducibility.

New Implementation

Response Model – Refactored

The Response model was comprehensively refactored to enhance modularity, readability, and extendability. A new submit method and a corresponding after_save callback were introduced to handle post-submission actions, such as triggering email notifications through a clean separation of concerns. The aggregate score computation was optimized by replacing manual iterations with ActiveRecord joins, improving performance and readability. A new maximum_score method was added to compute the highest possible score based on the questionnaire's rubric. Additional helper methods like questionnaire_by_answer and active_scored_questions were added for clarity and reuse. Validations were introduced to enforce data integrity, and inline documentation was added throughout to improve maintainability.

  • Note: Lines prefixed with + represent the new implementation, while lines with - indicate removed or replaced code from the old version.

Legacy Response Model (Expertiza) vs Refactored Response Model

class Response < ApplicationRecord
   include ScorableHelper
   include MetricHelper

+  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
+
+  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_notification_email
+  end

   def reportable_difference?
     map_class = map.class
-    existing_responses = map_class.assessments_for(map.reviewee)
-
-    count = 0
-    total = 0
-    existing_responses.each do |response|
-      unless id == response.id
-        count += 1
-        total +=  response.aggregate_questionnaire_score.to_f / response.maximum_score
-      end
-    end
-
-    return false if count.zero?
-
-    average_score = total / count
-    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)
-    allowed_difference_percentage = assignment_questionnaire.notification_limit.to_f
-
-    (average_score - score).abs * 100 > allowed_difference_percentage
+    existing_responses = map_class.assessments_for(map.reviewee)
+    count = 0
+    total = 0
+
+    existing_responses.each do |response|
+      next if id == response.id
+      count += 1
+      total += response.aggregate_questionnaire_score.to_f / response.maximum_score
+    end
+
+    return false if count.zero?
+
+    average_score = total / count
+    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
+    )
+    difference_threshold = assignment_questionnaire.try(:notification_limit) || 0.0
+    (score - average_score).abs * 100 > difference_threshold
   end

   def aggregate_questionnaire_score
-    sum = 0
-    scores.each do |s|
-      item = Item.find(s.question_id)
-      sum += s.answer * item.weight unless s.answer.nil? || !item.scorable?
-    end
-    sum
+    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
+  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

ResponseMap Model – Modernized and Modularized

The ResponseMap model was redesigned to centralize core responsibilities of the reviewer-reviewee relationship across all response types. Legacy logic and methods like response_assignment and assessments_for were removed to reduce clutter and responsibility duplication. In their place, new modular methods such as title, contributor, latest_response, and has_submitted_response? were introduced to provide clean and reusable abstractions. Reporting logic was added via the response_report class method, which groups responses either by review round or latest version. Private helper methods like group_responses_by_round and group_latest_responses were implemented to improve encapsulation and support report generation use cases.

Legacy ResponseMap Model (Expertiza) vs Refactored ResponseMap Model

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

-  def response_assignment
-    return Participant.find(self.reviewer_id).assignment
-  end
-
-  def self.assessments_for(team)
-    responses = []
-    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')
-          array_sort << all_resp if all_resp.is_submitted
-        else
-          array_sort << all_resp
-        end
-        sort_to = array_sort.sort
-        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 }
-    end
-    responses
-  end
+
+  def title
+    self.class.name.sub("ResponseMap", "")
+  end
+
+  def questionnaire
+    self.assignment.questionnaires
+  end
+
+  def contributor
+    self.reviewee
+  end
+
+  def round
+    self.responses.order(created_at: :desc).first&.round
+  end
+
+  def latest_response
+    self.responses.order(created_at: :desc).first
+  end
+
+  def has_submitted_response?
+    self.responses.where(is_submitted: true).exists?
+  end
+
+  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
+  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
  • Note: Lines prefixed with + represent the new implementation, while lines with - indicate removed or replaced code from the old version.

ResponseMapsController – New Middleware Controller

A completely new ResponseMapsController was developed as a centralized middleware for handling RESTful operations on ResponseMap entities. This controller includes full CRUD support—index, show, create, update, and destroy—while also introducing a dedicated submit_response action that persists a response, marks it as submitted, and triggers notification emails using the FeedbackEmailService. The controller handles parameter whitelisting, submission workflows, and error handling in a modular way through private utility methods like persist_and_respond and set_response_map. Additionally, it supports a report generation endpoint to fetch summarized feedback analytics by assignment. 

# 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]

  # 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
      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]
    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,
      :is_submitted,
      scores_attributes: [:answer, :comments, :question_id]
    )
  end

  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
    end
  end
end

FeedbackResponseMapsController – Feedback-Specific Extension

The FeedbackResponseMapsController was introduced as a subclass of ResponseMapsController to specialize behavior for feedback-specific operations while reusing core functionalities. It overrides the set_response_map method to scope queries strictly to FeedbackResponseMap instances. Several new endpoints were added, including assignment_feedback, which lists all feedback maps under a given assignment, and reviewer_feedback, which returns a reviewer's complete feedback history along with responses. Additionally, a feedback_response_rate endpoint computes completion statistics such as total feedbacks, completed ones, and overall response rate percentage. The controller ensures type-specific map creation and parameter handling, enabling clean separation of feedback functionality. 

# app/controllers/api/v1/feedback_response_maps_controller.rb
# Handles operations specific to feedback response maps
class Api::V1::FeedbackResponseMapsController < ResponseMapsController
  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

  # POST /api/v1/feedback_response_maps
  def create
    @response_map = FeedbackResponseMap.new(response_map_params)
    persist_and_respond(@response_map, :created)
  end
end


Email Functionality

To follow the Single Responsibility Principle, the email notification logic was extracted from the FeedbackResponseMap model and encapsulated in a dedicated service called FeedbackEmailService, keeping the model focused purely on data and improving testability. This service, designed using the Service Object Pattern, is solely responsible for sending feedback-related emails. It is initialized with a response_map and an assignment, and exposes a call method as its public interface. The service constructs email details via a private build_defn method, which retrieves related data such as the original review response, reviewer participant, and recipient user. It then uses Mailer.sync_message to dispatch the notification. This separation of concerns not only improves maintainability and clarity but also simplifies testing by allowing isolated unit tests without involving the database or a mail server.


Testing

Unit Testing of ResponseMapController

The ResponseMapsController in the application has been unit tested to ensure the stability and correctness of key API functionalities related to peer reviews. The testing focuses on verifying that the controller handles data retrieval and creation operations accurately, and that valid HTTP responses are returned under expected conditions. Three core functionalities have been validated through successful unit tests. First, the GET /api/v1/response_maps endpoint is tested to confirm that it retrieves all existing response maps from the database, ensuring the index action behaves as expected. Second, the GET /api/v1/response_maps/:id test confirms that a specific response map can be fetched reliably using its ID, and also handles the edge case of returning null if the record does not exist. Third, the POST /api/v1/response_maps endpoint is tested to verify that a new response map can be created successfully when valid reviewer and reviewee participants are provided. These tests—returns all response maps, returns the requested response map or 'null' if not found, and creates a new response map and returns it—serve as essential checks to confirm that the controller functions as intended and handles basic CRUD operations gracefully. This has increased the coverage of the code to 7%.

Unit Testing of FeedbackEmailService

The overall goal of the test is to ensure that the FeedbackEmailService correctly constructs the email content and interacts with the Mailer class to send the notification, verifying that the key fields in the email definition are accurate. The test sets up several test doubles to simulate the behavior of the models involved (such as Response, ResponseMap, AssignmentParticipant, and User). These test doubles allow the test to isolate and focus on the behavior of the service without actually querying the database. The before block stubs the find methods of ActiveRecord models to return predefined doubles, and it also mocks the Mailer class to intercept the email sending process. In the test, the service is instantiated, and it is verified that the email definition is built correctly, with the expected values for the recipient's email, the email type, the user's full name, and the assignment name. The test concludes by calling the call method to simulate sending the email, ensuring the Mailer.sync_message method is invoked with the correct parameters.

On testing the email functionality, we get this

Relevant Links

Team

Mentor
  • Aniruddha Rajnekar (aarajnek@ncsu.edu)
Members
  • Riya Bihani (rbihani@ncsu.edu)
  • Nayan Taori (ntaori@ncsu.edu)
  • Chandrakant Koneti (ckoneti@ncsu.edu)
Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Spring_2025_-_E2509_Refactoring_and_Enhancing_the_Feedback_Response_Map_Controller&oldid=165244"