E2218. Refactor response_controller.rb (#2327)

* Refactor create, action_allowed? and update methods in response controller

* Refactoring changes for create_answers, update and save method in response controller

* Added ResponseHelper Class, Fixed Code Climate Issues

* Before action method to initialize respone and map variables

* Before action method for view and delete methods to remove common code

* Changes for defining new method in award badge model

* Moved cake score calculation logic to Cake class, Removed calibration_results_info method, Added ResponseHelper methods

* Arranged Methods in Response Controller, Added Comments

* Fixed method name in before_action

* Comment with project ID for set_response method

* Updated the response_spec.rb test case file to reflect method name change

* Changes to resolve test case failures in response controller

* Chnages to authorization method used to display calibration results for a reviewer

* added comments in response_controller.rb

* comments for delete and action_allowed methods

* Fixed failed test cases in response views

* Changes to fix rubocop errors

* Fix for failing test cases

* remove unfunctional code

* address rubocop

Co-authored-by: Vamshi Chidara <schidar@vclvm176-217.vcl.ncsu.edu>
Co-authored-by: Samson Reddy Mulkur <mulkursamson@gmail.com>
Co-authored-by: akhilkumarmengani <akhilkumarmengani@gmail.com>
Co-authored-by: John Bumgardner <john.bumgardner@icloud.com>

app/controllers/response_controller.rb

@@ -1,22 +1,31 @@
 class ResponseController < ApplicationController
   include AuthorizationHelper
+  include ResponseHelper
 
   helper :submitted_content
   helper :file
 
+  before_action :authorize_show_calibration_results, only: %i[show_calibration_results_for_student]
+  before_action :set_response, only: %i[update delete view]
+
+  # E2218: Method to check if that action is allowed for the user.
   def action_allowed?
     response = user_id = nil
     action = params[:action]
+    # Initialize response and user id if action is edit or delete or update or view.
     if %w[edit delete update view].include?(action)
       response = Response.find(params[:id])
       user_id = response.map.reviewer.user_id if response.map.reviewer
     end
     case action
-    when 'edit' # If response has been submitted, no further editing allowed
+    when 'edit'
+      # If response has been submitted, no further editing allowed.
       return false if response.is_submitted
 
+      # Else, return true if the user is a reviewer for that response.
       return current_user_is_reviewer?(response.map, user_id)
-      # Deny access to anyone except reviewer & author's team
+
+    # Deny access to anyone except reviewer & author's team
     when 'delete', 'update'
       return current_user_is_reviewer?(response.map, user_id)
     when 'view'
@@ -26,10 +35,16 @@ def action_allowed?
     end
   end
 
-  # E-1973 - helper method to check if the current user is the reviewer
-  # if the reviewer is an assignment team, we have to check if the current user is on the team
-  def current_user_is_reviewer?(map, _reviewer_id)
-    map.reviewer.current_user_is_reviewer? current_user.try(:id)
+  # E2218: Method to authorize if the reviewer can view the calibration results
+  # When user manipulates the URL, the user should be authorized
+  def authorize_show_calibration_results
+    response_map = ResponseMap.find(params[:review_response_map_id])
+    user_id = response_map.reviewer.user_id if response_map.reviewer
+    # Deny access to the calibration result page if the current user is not a reviewer.
+    unless current_user_is_reviewer?(response_map, user_id)
+      flash[:error] = 'You are not allowed to view this calibration result'
+      redirect_to controller: 'student_review', action: 'list', id: user_id
+    end
   end
 
   # GET /response/json?response_id=xx
@@ -39,10 +54,9 @@ def json
     render json: response
   end
 
+  # E2218: Method to delete a response.
   def delete
     # The locking was added for E1973, team-based reviewing. See lock.rb for details
-    @response = Response.find(params[:id])
-    @map = @response.map
     if @map.reviewer_is_team
       @response = Lock.get_lock(@response, current_user, Lock::DEFAULT_TIMEOUT)
       if @response.nil?
@@ -62,6 +76,7 @@ def delete
   # If so, edit that version otherwise create a new version.
 
   # Prepare the parameters when student clicks "Edit"
+  # response questions with answers and scores are rendered in the edit page based on the version number
   def edit
     assign_action_parameters
     @prev = Response.where(map_id: @map.id)
@@ -90,7 +105,7 @@ def edit
     # set more handy variables for the view
     set_content
     @review_scores = []
-    @questions.each do |question|
+    @review_questions.each do |question|
       @review_scores << Answer.where(response_id: @response.response_id, question_id: question.id).first
     end
     @questionnaire = questionnaire_from_response
@@ -100,22 +115,23 @@ def edit
   # Update the response and answers when student "edit" existing response
   def update
     render nothing: true unless action_allowed?
-
     msg = ''
     begin
       # the response to be updated
       # Locking functionality added for E1973, team-based reviewing
-      @response = Response.find(params[:id])
-      @map = @response.map
       if @map.reviewer_is_team && !Lock.lock_between?(@response, current_user)
         response_lock_action
         return
       end
+
       @response.update_attribute('additional_comment', params[:review][:comments])
       @questionnaire = questionnaire_from_response
       questions = sort_questions(@questionnaire.questions)
-      create_answers(params, questions) unless params[:responses].nil? # for some rubrics, there might be no questions but only file submission (Dr. Ayala's rubric)
+
+      # for some rubrics, there might be no questions but only file submission (Dr. Ayala's rubric)
+      create_answers(params, questions) unless params[:responses].nil?
       @response.update_attribute('is_submitted', true) if params['isSubmit'] && params['isSubmit'] == 'Yes'
+
       @response.notify_instructor_on_difference if (@map.is_a? ReviewResponseMap) && @response.is_submitted && @response.significant_difference?
     rescue StandardError
       msg = "Your response was not saved. Cause:189 #{$ERROR_INFO}"
@@ -135,7 +151,7 @@ def new
     # So do the answers, otherwise the response object can't find the questionnaire when the user hasn't saved his new review and closed the window.
     # A new response has to be created when there hasn't been any reviews done for the current round,
     # or when there has been a submission after the most recent review in this round.
-    @response = @response.populate_new_response(@map, @current_round)
+    @response = @response.create_or_get_response(@map, @current_round)
     questions = sort_questions(@questionnaire.questions)
     store_total_cake_score
     init_answers(questions)
@@ -159,8 +175,6 @@ def new_feedback
 
   # view response
   def view
-    @response = Response.find(params[:id])
-    @map = @response.map
     set_content
   end
 
@@ -179,22 +193,21 @@ def create
     # Hence we need to pick the latest response.
     @response = Response.where(map_id: @map.id, round: @round.to_i).order(created_at: :desc).first
     if @response.nil?
-      @response = Response.create(
-        map_id: @map.id,
-        additional_comment: params[:review][:comments],
-        round: @round.to_i,
-        is_submitted: is_submitted
-      )
+      @response = Response.create(map_id: @map.id, additional_comment: params[:review][:comments],
+                                  round: @round.to_i, is_submitted: is_submitted)
     end
     was_submitted = @response.is_submitted
-    @response.update(additional_comment: params[:review][:comments], is_submitted: is_submitted) # ignore if autoupdate try to save when the response object is not yet created.
+
+    # ignore if autoupdate try to save when the response object is not yet created.s
+    @response.update(additional_comment: params[:review][:comments], is_submitted: is_submitted)
 
     # :version_num=>@version)
     # Change the order for displaying questions for editing response views.
     questions = sort_questions(@questionnaire.questions)
     create_answers(params, questions) if params[:responses]
     msg = 'Your response was successfully saved.'
     error_msg = ''
+
     # only notify if is_submitted changes from false to true
     if (@map.is_a? ReviewResponseMap) && (!was_submitted && @response.is_submitted) && @response.significant_difference?
       @response.notify_instructor_on_difference
@@ -208,18 +221,6 @@ def save
     @map = ResponseMap.find(params[:id])
     @return = params[:return]
     @map.save
-    participant = Participant.find_by(id: @map.reviewee_id)
-    # E1822: Added logic to insert a student suggested 'Good Teammate' or 'Good Reviewer' badge in the awarded_badges table.
-    if @map.assignment.badge?
-      if @map.is_a?(TeammateReviewResponseMap) && (params[:review][:good_teammate_checkbox] == 'on')
-        badge_id = Badge.get_id_from_name('Good Teammate')
-        AwardedBadge.where(participant_id: participant.id, badge_id: badge_id, approval_status: 0).first_or_create
-      end
-      if @map.is_a?(FeedbackResponseMap) && (params[:review][:good_reviewer_checkbox] == 'on')
-        badge_id = Badge.get_id_from_name('Good Reviewer')
-        AwardedBadge.where(participant_id: participant.id, badge_id: badge_id, approval_status: 0).first_or_create
-      end
-    end
     ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, 'Response was successfully saved')
     redirect_to action: 'redirect', id: @map.map_id, return: params.permit(:return)[:return], msg: params.permit(:msg)[:msg], error_msg: params.permit(:error_msg)[:error_msg]
   end
@@ -256,14 +257,14 @@ def redirect
     end
   end
 
-  # This method controls what is shown students when they view results from a calibration.
-  # Most of the business logic lives in the model, where the :calibration_response_map_id and :review_response_map_id are used
-  # to find the appropriate references to calibration responses, review responses as well as the response questions
+  # This method set the appropriate values to the instance variables used in the 'show_calibration_results_for_student' page
+  # Responses are fetched using calibration_response_map_id and review_response_map_id params passed in the URL
+  # Questions are fetched by querying AssignmentQuestionnaire table to get the valid questions
   def show_calibration_results_for_student
     @assignment = Assignment.find(params[:assignment_id])
-    @calibration_response,
-    @review_response,
-    @questions = Response.calibration_results_info(params[:calibration_response_map_id], params[:review_response_map_id], params[:assignment_id])
+    @calibration_response = ReviewResponseMap.find(params[:calibration_response_map_id]).response[0]
+    @review_response = ReviewResponseMap.find(params[:review_response_map_id]).response[0]
+    @review_questions = AssignmentQuestionnaire.get_questions_by_assignment_id(params[:assignment_id])
   end
 
   def toggle_permission
@@ -291,39 +292,19 @@ def toggle_permission
 
   private
 
+  # E2218: Method to initialize response and response map for update, delete and view methods
+  def set_response
+    @response = Response.find(params[:id])
+    @map = @response.map
+  end
+
   # Added for E1973, team-based reviewing:
   # http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2019_-_Project_E1973._Team_Based_Reviewing
   # Taken if the response is locked and cannot be edited right now
   def response_lock_action
     redirect_to action: 'redirect', id: @map.map_id, return: 'locked', error_msg: 'Another user is modifying this response or has modified this response. Try again later.'
   end
 
-  # new_response if a flag parameter indicating that if user is requesting a new rubric to fill
-  # if true: we figure out which questionnaire to use based on current time and records in assignment_questionnaires table
-  # e.g. student click "Begin" or "Update" to start filling out a rubric for others' work
-  # if false: we figure out which questionnaire to display base on @response object
-  # e.g. student click "Edit" or "View"
-  def set_content(new_response = false)
-    @title = @map.get_title
-    if @map.survey?
-      @survey_parent = @map.survey_parent
-    else
-      @assignment = @map.assignment
-    end
-    @participant = @map.reviewer
-    @contributor = @map.contributor
-    new_response ? questionnaire_from_response_map : questionnaire_from_response
-    set_dropdown_or_scale
-    @questions = sort_questions(@questionnaire.questions)
-    @min = @questionnaire.min_question_score
-    @max = @questionnaire.max_question_score
-    # The new response is created here so that the controller has access to it in the new method
-    # This response object is populated later in the new method
-    if new_response
-      @response = Response.create(map_id: @map.id, additional_comment: '', round: @current_round, is_submitted: 0)
-    end
-  end
-
   # This method is called within the Edit or New actions
   # It will create references to the objects that the controller will need when a user creates a new response or edits an existing one.
   def assign_action_parameters
@@ -387,11 +368,6 @@ def set_dropdown_or_scale
     @dropdown_or_scale = (use_dropdown ? 'dropdown' : 'scale')
   end
 
-  # sorts by sequence number
-  def sort_questions(questions)
-    questions.sort_by(&:seq)
-  end
-
   # For each question in the list, starting with the first one, you update the comment and score
   def create_answers(params, questions)
     params[:responses].each_pair do |k, v|
@@ -402,24 +378,13 @@ def create_answers(params, questions)
     end
   end
 
+  # This method initialize answers for the questions in the response
+  # Iterates over each questions and create corresponding answer for that
   def init_answers(questions)
     questions.each do |q|
       # it's unlikely that these answers exist, but in case the user refresh the browser some might have been inserted.
       answer = Answer.where(response_id: @response.id, question_id: q.id).first
       Answer.create(response_id: @response.id, question_id: q.id, answer: nil, comments: '') if answer.nil?
     end
   end
-
-  # Creates a table to store total contribution for Cake question across all reviewers
-  def store_total_cake_score
-    @total_score = {}
-    @questions.each do |question|
-      next unless question.instance_of? Cake
-
-      reviewee_id = ResponseMap.select(:reviewee_id, :type).where(id: @response.map_id.to_s).first
-      total_score = question.get_total_score_for_question(reviewee_id.type, question.id, @participant.id, @assignment.id, reviewee_id.reviewee_id).to_s
-      total_score = 0 if total_score.nil?
-      @total_score[question.id] = total_score
-    end
-  end
 end

app/helpers/response_helper.rb

@@ -0,0 +1,51 @@
+module ResponseHelper
+  # E2218: this module contains methods that are used in response_controller class
+
+  # E-1973 - helper method to check if the current user is the reviewer
+  # if the reviewer is an assignment team, we have to check if the current user is on the team
+  def current_user_is_reviewer?(map, _reviewer_id)
+    map.reviewer.current_user_is_reviewer? current_user.try(:id)
+  end
+
+  # sorts the questions passed by sequence number in ascending order
+  def sort_questions(questions)
+    questions.sort_by(&:seq)
+  end
+
+  # Assigns total contribution for cake question across all reviewers to a hash map
+  # Key : question_id, Value : total score for cake question
+  def store_total_cake_score
+    reviewee = ResponseMap.select(:reviewee_id, :type).where(id: @response.map_id.to_s).first
+    @total_score = Cake.get_total_score_for_questions(reviewee.type,
+                                                      @review_questions,
+                                                      @participant.id,
+                                                      @assignment.id,
+                                                      reviewee.reviewee_id)
+  end
+
+  # new_response if a flag parameter indicating that if user is requesting a new rubric to fill
+  # if true: we figure out which questionnaire to use based on current time and records in assignment_questionnaires table
+  # e.g. student click "Begin" or "Update" to start filling out a rubric for others' work
+  # if false: we figure out which questionnaire to display base on @response object
+  # e.g. student click "Edit" or "View"
+  def set_content(new_response = false)
+    @title = @map.get_title
+    if @map.survey?
+      @survey_parent = @map.survey_parent
+    else
+      @assignment = @map.assignment
+    end
+    @participant = @map.reviewer
+    @contributor = @map.contributor
+    new_response ? questionnaire_from_response_map : questionnaire_from_response
+    set_dropdown_or_scale
+    @review_questions = sort_questions(@questionnaire.questions)
+    @min = @questionnaire.min_question_score
+    @max = @questionnaire.max_question_score
+    # The new response is created here so that the controller has access to it in the new method
+    # This response object is populated later in the new method
+    if new_response
+      @response = Response.create(map_id: @map.id, additional_comment: '', round: @current_round, is_submitted: 0)
+    end
+  end
+end

app/models/assignment_questionnaire.rb

@@ -13,4 +13,13 @@ def self.get_latest_assignment(questionnaire_id)
     record = includes(:assignment).where(questionnaire_id: questionnaire_id).order('assignments.created_at').last
     return record.assignment, record.used_in_round unless record.nil?
   end
+
+  # E2218
+  # @param assignment_id [Integer]
+  # @return questions corresponding to the assignment_id and review questionnaire questions that are not headers
+  def self.get_questions_by_assignment_id(assignment_id)
+    AssignmentQuestionnaire.find_by(['assignment_id = ? and questionnaire_id IN (?)',
+                                     Assignment.find(assignment_id).id, ReviewQuestionnaire.select('id')])
+                           .questionnaire.questions.reject { |q| q.is_a?(QuestionnaireHeader) }
+  end
 end

app/models/awarded_badge.rb

@@ -5,4 +5,11 @@ class AwardedBadge < ApplicationRecord
   def approved?
     approval_status == 1
   end
+
+  # E2218: This method is called when a badge is awarded to a participant
+  # This method is called from response controller
+  def award_badge(participant_id, badge_name)
+    badge_id = Badge.get_id_from_name(badge_name: badge_name)
+    AwardedBadge.where(participant_id: participant_id, badge_id: badge_id, approval_status: 0).first_or_create
+  end
 end

app/models/cake.rb

@@ -72,6 +72,26 @@ def view_completed_question(count, answer)
     safe_join([''.html_safe, ''.html_safe], html.html_safe)
   end
 
+  # E2218: This Method returns the total cake score for each question.
+  # Its called from the model new method in response controller.
+  # @param review_type  [String]
+  # @param questions [Array]
+  # @param participant_id [Integer]
+  # @param assignment_id [Integer]
+  # @param reviewee_id [Integer]
+  # @return total_scores - a hash with key as question_id and values as total score for each cake question
+  def self.get_total_score_for_questions(review_type, questions, participant_id, assignment_id, reviewee_id)
+    total_scores = {}
+    questions.each do |question|
+      next unless question.instance_of? Cake
+
+      total_score = question.get_total_score_for_question(review_type, question.id, participant_id, assignment_id, reviewee_id).to_s
+      total_score = 0 if total_score.nil?
+      total_scores[question.id] = total_score
+    end
+    total_scores
+  end
+
   # Finds all teammates and calculates the total contribution of all members for the question
   def get_total_score_for_question(review_type, question_id, participant_id, assignment_id, reviewee_id)
     # get the reviewer's team id for the currently answered question