CSC/ECE 517 Spring 2025 - E2512. Reimplement responses controller

Introduction

The ResponseController in Expertiza is responsible for handling responses to quizzes, peer reviews, and surveys. However, the current implementation has several design issues, including violations of Rails naming conventions, excessive responsibilities, and redundant logic. The goal of this project is to reimplement (not refactor) a new response_controller.rb as responses_controller.rb. This includes CRUD operations (create, read, update, delete) and the functionality previously found in the old controller, but done in a clean, modern, and conventional way

Motivation

The Responses controller currently does not follow modern idioms and the updated Expertiza style guide. It also contains too many extra pieces of logic and needs to move logic to other mailers/helpers/services/models.

Tasks Identified

• Rename response_controller.rb to responses_controller.rb to adhere to Rails naming conventions.
• Limit the controller to CRUD operations, moving non-CRUD logic to models, helpers, or services.
• Ensure questions (items) are displayed in the correct sequence when rendering responses.
• Move the sorting logic from the edit method to the Response model for better organization.
• Rename sort_questions to sort_items for improved clarity.
• Use polymorphism to determine if the reviewer is a team, replacing existing if statements.
• Consolidate redundant parameter-setting methods (assign_action_parameters, set_content) into a single, more efficient method.
• Refactor questionnaire_from_response_map to simplify logic and support topic-based rubrics.
• Identify and remove unused (dead) methods to clean up the codebase.
• Extract email generation logic from the controller and move it to a dedicated helper.

Our Reimplementation

Move Sorting Logic & Display In Correct Order

Previously, the sorting logic was inside the update method of the Response controller:

 if @prev.present?
     @sorted = @review_scores.sort do |m1, m2|
       if m1.version_num.to_i && m2.version_num.to_i
         m2.version_num.to_i <=> m1.version_num.to_i
       else
         m1.version_num ? -1 : 1
       end
     end
     @largest_version_num = @sorted[0]
   end

This was re-implemented in the Response model:

 # Sort responses by version number, descending
 def self.sort_by_version
   review_scores = Response.where(map_id: @map.id).to_a
   return [] if review_scores.empty?
   sorted = review_scores.sort_by { |response| response.version_num.to_i }.reverse
   sorted[0]
 end

The new logic is very similar, but cleaner and available for use in other contexts.

Similarly, sort_questions was re-implemented in ResponsesHelper:

 #Renamed to sort_items from sort_questions
 def sort_items(questions)
   questions.sort_by(&:seq)
 end

Consolidate Parameter Setting Methods

The old ResponseController had separate methods to set key Response parameters: assign_action_parameters and set_content. These methods are highly duplicative of each other and add more complexity to the code base. They are also non-CRUD methods in the controller, so we re-implemented them into one helper method: prepare_response_content

def prepare_response_content(map, action_params = nil, new_response = false)

       # Set title and other initial content based on the map
       title = map.get_title
       survey_parent = nil
       assignment = nil
       participant = map.reviewer
       contributor = map.contributor
 
       if map.survey?
         survey_parent = map.survey_parent
       else
         assignment = map.assignment
       end
 
       # Get the questionnaire and sort questions
       questionnaire = questionnaire_from_response_map(map, contributor, assignment)
       review_questions = Response.sort_by_version(questionnaire.questions)
       min = questionnaire.min_question_score
       max = questionnaire.max_question_score

       # Initialize response if new_response is true
       response = nil
       if new_response
         response = Response.where(map_id: map.id).order(updated_at: :desc).first
         if response.nil?
           response = Response.create(map_id: map.id, additional_comment: , is_submitted: 0)
         end
       end
       # Set up dropdowns or scales
       set_dropdown_or_scale(questionnaire, assignment)
       # Process the action parameters if provided
       if action_params
         case action_params[:action]
         when 'edit'
           header = 'Edit'
           next_action = 'update'
           response = Response.find(action_params[:id])
           contributor = map.contributor
         when 'new'
           header = 'New'
           next_action = 'create'
           feedback = action_params[:feedback]
           modified_object = map.id
         end
       end 

By using parameters to manage cases around the action being 'new', we were able to combine these methods into one method. This allows for one method to perform this singular function, and the final controller code can be simplified with this in mind.

Refactor Questionnaire_from_response_map

The old questionnaire_from_response_map method did not support topic based reviewing and resided in the controller:

 def questionnaire_from_response_map
   case @map.type
   when 'ReviewResponseMap', 'SelfReviewResponseMap'
     reviewees_topic = SignedUpTeam.topic_id_by_team_id(@contributor.id)
     @current_round = @assignment.number_of_current_round(reviewees_topic)
     @questionnaire = @map.questionnaire(@current_round, reviewees_topic)
   when
     'MetareviewResponseMap',
     'TeammateReviewResponseMap',
     'FeedbackResponseMap',
     'CourseSurveyResponseMap',
     'AssignmentSurveyResponseMap',
     'GlobalSurveyResponseMap',
     'BookmarkRatingResponseMap'
     if @assignment.duty_based_assignment?
       # E2147 : gets questionnaire of a particular duty in that assignment rather than generic questionnaire
       @questionnaire = @map.questionnaire_by_duty(@map.reviewee.duty_id)
     else
       @questionnaire = @map.questionnaire
     end
   end
 end

To remedy this, the method was re-implemented in the ResponsesHelper file, along with some helper methods :

 def questionnaire_from_response_map(map, contributor, assignment)
   if ['ReviewResponseMap', 'SelfReviewResponseMap'].include?(map.type)
     get_questionnaire_by_contributor(map, contributor, assignment)
   else
     get_questionnaire_by_duty(map, assignment)
   end
   end
   def get_questionnaire_by_contributor(map, contributor, assignment)       
     reviewees_topic = SignedUpTeam.find_by(team_id: contributor.id)&.sign_up_topic_id
     current_round = DueDate.next_due_date(reviewees_topic).round
     map.questionnaire(current_round, reviewees_topic)
   end
   def get_questionnaire_by_duty(map, assignment)
     if assignment.duty_based_assignment?
       # Gets questionnaire of a particular duty in that assignment rather than generic questionnaire
       map.questionnaire_by_duty(map.reviewee.duty_id)
   else
       map.questionnaire
   end
 end

This logic is much more extendable to new implementations than the legacy logic, and easier to read and maintain than the legacy logic.

Refactor toggle_permissions

toggle_permissions was updated to improve readability using the Single Responsibility principle. Below is the original code for toggle_permissions.

 def toggle_permission
   render nothing: true unless action_allowed?

   # the response to be updated
   @response = Response.find(params[:id])

   # Error message placeholder
   error_msg = 

   begin
     @map = @response.map

     # Updating visibility for the response object, by E2022 @SujalAhrodia -->
     visibility = params[:visibility]
     unless visibility.nil?
       @response.update_attribute('visibility', visibility)
     end
   rescue StandardError
     error_msg = "Your response was not saved. Cause:189 #{$ERROR_INFO}"
   end
   redirect_to action: 'redirect', id: @map.map_id, return: params[:return], msg: params[:msg], error_msg: error_msg
 end

In the new iteration, the method was updated to use a helper method to update the visibility attribute.

 # responses_controller.rb
 def toggle_permission
   return head :no_content unless action_allowed?

   error = update_visibility(params[:visibility])
   if error == 
     map_id = @response.map.map_id
   else
     map_id = nil
   end
   redirect_to action: 'redirect', id: map_id, return: params[:return], msg: params[:msg], error_msg: error
 end

 # responses_helper.rb
 def update_visibility(visibility)
     error = ""
     begin
       unless visibility.nil?
         @response.update(visibility: visibility)
       end
     rescue StandardError
       error = "Your response was not saved. Cause:189 #{$ERROR_INFO}"
     end
     error
 end

Refactor new_feedback

new_feedback was refactored using the prepare_response_content helper function to shorten the number of lines dedicated to instance assignments. Previous method:

 def new_feedback
   review = Response.find(params[:id]) unless params[:id].nil?
   if review
     reviewer = AssignmentParticipant.where(user_id: session[:user].id, parent_id: review.map.assignment.id).first
     map = FeedbackResponseMap.where(reviewed_object_id: review.id, reviewer_id: reviewer.id).first
     if map.nil?
       # if no feedback exists by dat user den only create for dat particular response/review
       map = FeedbackResponseMap.create(reviewed_object_id: review.id, reviewer_id: reviewer.id, reviewee_id: review.map.reviewer.id)
     end
     redirect_to action: 'new', id: map.id, return: 'feedback'
   else
     redirect_back fallback_location: root_path
   end
 end

New implementation:

 # responses_controller.rb
 def new_feedback
   if Response.find(params[:id])
     @map = Response.find(params[:id]).map
     response_content = prepare_response_content(@map)

     # Assign variables from response_content hash
     response_content.each { |key, value| instance_variable_set("@#{key}", value) }
     if @response
       @reviewer = AssignmentParticipant.where(user_id: current_user.id, parent_id: @response.map.assignment.id).first
       map = find_or_create_feedback
       redirect_to action: 'new', id: map.id, return: 'feedback'
     end
   else
     redirect_back fallback_location: root_path
   end
 end

 #responses_helper.rb
 def find_or_create_feedback
   map = FeedbackResponseMap.where(reviewed_object_id: @response.id, reviewer_id: @reviewer.id).first
   if map.nil?
     map = FeedbackResponseMap.create(reviewed_object_id: @response.id, reviewer_id: @reviewer.id, reviewee_id: @response.map.reviewer.id)
   end
   map
 end

Extract email generation logic and move to model

Previously, the email generation logic resided in the controller and was designed around one type of email:

 def send_email
   subject = params['send_email']['subject']
   body = params['send_email']['email_body']
   response = params['response']
   email = params['email']

   respond_to do |format|
     if subject.blank? || body.blank?
       flash[:error] = 'Please fill in the subject and the email content.'
       format.html { redirect_to controller: 'response', action: 'author', response: response, email: email }
       format.json { head :no_content }
     else
       # make a call to method invoking the email process
       MailerHelper.send_mail_to_author_reviewers(subject, body, email)
       flash[:success] = 'Email sent to the author.'
       format.html { redirect_to controller: 'student_task', action: 'list' }
       format.json { head :no_content }
     end
   end
 end

This was re-implemented in the ResponseMailer class:

   # Send an email to authors from a reviewer
   def send_response_email(response)
       @body = response.params[:send_email][:email_body]
       @subject = params[:send_email][:subject]
       Rails.env.development? || Rails.env.test? ? @email = 'expertiza.mailer@gmail.com' : @email = response.params[:email]
       mail(to: @email, body: @body, subject: @subject, content_type: 'text/html',)
   end

And sent in the Response model:

 # Send response email from reviewer to author
 def self.send_response_email
   ResponseMailer.with(response: self)
     .send_response_email
     .deliver_later
 end

This mailer implementation is idiomatic to modern Rails and also used in other places within the Expertiza backend re-implementation. This format makes it easy to manage the emails sent by Expertiza by isolating them and their content from the actual location in the codebase where they are sent, allowing for a clean division of responsibilities and easy future maintenance.

Create Operation

Using the helper methods implemented and already discussed, the code for the new and create methods is simplified:

 def new
   @map = ResponseMap.find(params[:id])
   attributes = prepare_response_content(map, 'New', true)
   attributes.each do |key, value|
     instance_variable_set("@#{key}", value)
   end
   if @assignment
     @stage = @assignment.current_stage(SignedUpTeam.topic_id(@participant.parent_id, @participant.user_id))
   end
   questions = sort_questions(@questionnaire.questions)
   @total_score = total_cake_score
   init_answers(@response, questions)
   render action: 'response'
 end

Read Operation

Thanks to simplification in the new Responses controller, we were able to use the Rails scaffold methods for index and show in our re-implemented controller.

 # GET /api/v1/responses
 def index
   @responses = Response.all

   render json: @responses
 end

 # GET /api/v1/responses/1
 def show
   if @response
     render json: @response, status: :ok
   else
     render json: { error: 'Response not found' }, status: :not_found
   end
 end

Update Operation

The update and edit methods had the most change, with simple code that makes heavy use of helper functions:

 def edit 
   action_params = { action: 'edit', id: params[:id], return: params[:return] }
   response_content = prepare_response_content(@map, params[:round], action_params)
   # Assign variables from response_content hash
   response_content.each { |key, value| instance_variable_set("@#{key}", value) }
   @largest_version_num  = Response.sort_by_version(@review_questions)
   @review_scores = @review_questions.map do |question|
     Answer.where(response_id: @response.response_id, question_id: question.id).first
   end
   render action: 'response'
 end

This is in contrast to the legacy implementation, which integrates sorting, locking, and display ordering or responses directly in the function.

Destroy Operation

As our team was told to not re-implement lock functionality, the new destory operation follows the pattern given in Rails scaffold

New:

 def delete
   if @response.delete
     render json: @response, status: :deleted, location: @response
   else
     render json: @response.errors, status: :unprocessable_entity
   end
 end

 # DELETE /api/v1/responses/1
 def destroy
   @response.destroy!
 end

Old:

 def delete
   # The locking was added for E1973, team-based reviewing. See lock.rb for details
   if @map.team_reviewing_enabled
     @response = Lock.get_lock(@response, current_user, Lock::DEFAULT_TIMEOUT)
     if @response.nil?
       response_lock_action
       return
     end
   end

   # user cannot delete other people's responses. Needs to be authenticated.
   map_id = @response.map.id
   # The lock will be automatically destroyed when the response is destroyed
   @response.delete
   redirect_to action: 'redirect', id: map_id, return: params[:return], msg: 'The response was deleted.'
 end

Impact Analysis

The new ResponsesController is more streamlined, easier to read, and more extensible for system improvements than the old controller. By following modern Rails standards and Expertiza standards, the new code is implemented in a clean fashion that is easier to rigorously verify and less prone to failures in future operation as the use of Expertiza shifts and grows from the original use cases.

Future Improvements

• Improve the redirect method. Our team was not able to improve this to a sufficient level within the initial time constraints, but this method would benefit from substantial re-implementation to allow for the easy implementation of new user types and map types.

• One test is failing because there is not a root_path set in routes. Much of the app is unimplemented at this point, so we made the decision to leave that as is for when that is eventually set. If it will not be set, another value will need to be added.

• We feel fairly confident that the controller works with the current implementation of authentication, but a deeper understanding of how authentication and authorization are implemented could lead to further improvements.

Affected Classes

Changed existing classes

• controllers/responses_controller.rb
• models/responses.rb
• models/cake.rb
• models/lock.rb

New modules

• response_helper.rb
• response_mailer.rb

Pull Request

Here is our Pull Request: https://github.com/expertiza/reimplementation-back-end/pull/165.

Running the Project Locally

The project could be run locally by cloning the Github repository Expertiza Reimplementation and then running the following commands sequentially.

bundle install
rake db:create:all
rake db:migrate
rails server

Tests can be run with the following:

rspec test spec/controllers/api/v1/responses_controller_spec.rb
rspec test spec/helpers/response_helper_spec.rb