#1907 Refactor response_controller

app/controllers/response_controller.rb

@@ -17,20 +17,19 @@ def action_allowed?
     when 'delete', 'update'
       return current_user_id?(user_id)
     when 'view'
-      return edit_allowed?(response.map, user_id)
+      return view_allowed?(response.map, user_id)
     else
       current_user
     end
   end
 
-  def edit_allowed?(map, user_id)
-    assignment = map.reviewer.assignment
-    # if it is a review response map, all the members of reviewee team should be able to view the reponse (can be done from heat map)
+  def view_allowed?(map, user_id)
+    assignment = map.reviewer.assignment # if it is a review response map, all the members of reviewee team should be able to view the response
     if map.is_a? ReviewResponseMap
       reviewee_team = AssignmentTeam.find(map.reviewee_id)
       return current_user_id?(user_id) || reviewee_team.user?(current_user) || current_user.role.name == 'Administrator' ||
         (current_user.role.name == 'Instructor' and assignment.instructor_id == current_user.id) ||
-        (current_user.role.name == 'Teaching Assistant' and TaMapping.exists?(ta_id: current_user.id, course_id: assignment.course.id))
+          (current_user.role.name == 'Teaching Assistant' and TaMapping.exists?(ta_id: current_user.id, course_id: assignment.course.id))
     else
       current_user_id?(user_id)
     end
@@ -165,7 +164,7 @@ def create
     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.
 
-    # ,:version_num=>@version)
+    # :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]
@@ -201,26 +200,30 @@ def save
   end
 
   def redirect
-    flash[:error] = params[:error_msg] unless params[:error_msg] and params[:error_msg].empty?
-    flash[:note] = params[:msg] unless params[:msg] and params[:msg].empty?
+    error_id = params[:error_msg]
+    message_id = params[:msg]
+    flash[:error] = error_id unless error_id and error_id.empty?
+    flash[:note] = message_id unless message_id and message_id.empty?
     @map = Response.find_by(map_id: params[:id])
-    if params[:return] == "feedback"
+    case params[:return]
+    when "feedback"
       redirect_to controller: 'grades', action: 'view_my_scores', id: @map.reviewer.id
-    elsif params[:return] == "teammate"
+    when "teammate"
       redirect_to view_student_teams_path student_id: @map.reviewer.id
-    elsif params[:return] == "instructor"
+    when "instructor"
       redirect_to controller: 'grades', action: 'view', id: @map.response_map.assignment.id
-    elsif params[:return] == "assignment_edit"
+    when "assignment_edit"
       redirect_to controller: 'assignments', action: 'edit', id: @map.response_map.assignment.id
-    elsif params[:return] == "selfreview"
+    when "selfreview"
       redirect_to controller: 'submitted_content', action: 'edit', id: @map.response_map.reviewer_id
-    elsif params[:return] == "survey"
-      redirect_to controller: 'response', action: 'pending_surveys'
+    when "survey"
+      redirect_to controller: 'survey_deployment', action: 'pending_surveys'
     else
       redirect_to controller: 'student_review', action: 'list', id: @map.reviewer.id
     end
   end
 
+  # assigning variables for the expert reviews
   def show_calibration_results_for_student
     calibration_response_map = ReviewResponseMap.find(params[:calibration_response_map_id])
     review_response_map = ReviewResponseMap.find(params[:review_response_map_id])
@@ -232,40 +235,6 @@ def show_calibration_results_for_student
     @questions = @assignment_questionnaire.questionnaire.questions.reject {|q| q.is_a?(QuestionnaireHeader) }
   end
 
-  # This method should be moved to survey_deployment_contoller.rb
-  def pending_surveys
-    unless session[:user] # Check for a valid user
-      redirect_to '/'
-      return
-    end
-
-    # Get all the course survey deployments for this user
-    @surveys = []
-    [CourseParticipant, AssignmentParticipant].each do |participant_type|
-      # Get all the participant(course or assignment) entries for this user
-      participants = participant_type.where(user_id: session[:user].id)
-      next unless participants
-      participants.each do |p|
-        survey_deployment_type = (participant_type == CourseParticipant ? CourseSurveyDeployment : AssignmentSurveyDeployment)
-        survey_deployments = survey_deployment_type.where(parent_id: p.parent_id)
-        next unless survey_deployments
-        survey_deployments.each do |survey_deployment|
-          next unless survey_deployment && Time.now > survey_deployment.start_date && Time.now < survey_deployment.end_date
-          @surveys <<
-              [
-                'survey' => Questionnaire.find(survey_deployment.questionnaire_id),
-                'survey_deployment_id' => survey_deployment.id,
-                'start_date' => survey_deployment.start_date,
-                'end_date' => survey_deployment.end_date,
-                'parent_id' => p.parent_id,
-                'participant_id' => p.id,
-                'global_survey_id' => survey_deployment.global_survey_id
-              ]
-        end
-      end
-    end
-  end
-
   private
 
   # new_response if a flag parameter indicating that if user is requesting a new rubric to fill
@@ -288,7 +257,6 @@ def set_content(new_response = false)
     @min = @questionnaire.min_question_score
     @max = @questionnaire.max_question_score
   end
-
   # assigning the instance variables for Edit and New actions
   def assign_instance_vars
     case params[:action]
@@ -307,7 +275,8 @@ def assign_instance_vars
     end
     @return = params[:return]
   end
-
+  # identifying the questionnaire type
+  # updating the current round for the reviewer's responses
   def set_questionnaire_for_new_response
     case @map.type
     when "ReviewResponseMap", "SelfReviewResponseMap"
@@ -324,7 +293,7 @@ def set_questionnaire_for_new_response
       @questionnaire = @map.questionnaire
     end
   end
-
+  # stores the first instance of the score for each question
   def scores
     @review_scores = []
     @questions.each do |question|
@@ -342,19 +311,20 @@ def set_questionnaire
     @questionnaire = @response.questionnaire_by_answer(answer)
   end
 
+  # checks if the questionnaire is nil and opens drop down or rating accordingly
   def set_dropdown_or_scale
     use_dropdown = AssignmentQuestionnaire.where(assignment_id: @assignment.try(:id),
                                                  questionnaire_id: @questionnaire.try(:id))
                                           .first.try(:dropdown)
     @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)
-    # create score if it is not found. If it is found update it otherwise update it
     params[:responses].each_pair do |k, v|
       score = Answer.where(response_id: @response.id, question_id: questions[k.to_i].id).first
       score ||= Answer.create(response_id: @response.id, question_id: questions[k.to_i].id, answer: v[:score], comments: v[:comment])

app/controllers/survey_deployment_controller.rb

@@ -125,7 +125,7 @@ def generate_statistics
     end
   end
 
-  # Allows for children to rediect to this controller
+  # Allows for children to redirect to this controller
   def self.inherited(child)
     child.instance_eval do
       def model_name
@@ -135,7 +135,35 @@ def model_name
     super
   end
 
-  # This method should be moved to survey_deployment_contoller.rb
+  def pending_surveys
+    unless session[:user] # Check for a valid user
+      redirect_to '/'
+      return
+    end
+    @surveys = [] # Get all the course survey deployments for this user
+    [CourseParticipant, AssignmentParticipant].each do |participant_type| # Get all the participant(course or assignment) entries for this user
+      participants = participant_type.where(user_id: session[:user].id)
+      next unless participants
+      participants.each do |p|
+        survey_deployment_type = (participant_type == CourseParticipant ? CourseSurveyDeployment : AssignmentSurveyDeployment)
+        survey_deployments = survey_deployment_type.where(parent_id: p.parent_id)
+        next unless survey_deployments
+        survey_deployments.each do |survey_deployment|
+          next unless survey_deployment && Time.zone.now > survey_deployment.start_date && Time.zone.now < survey_deployment.end_date
+          @surveys <<
+              ['survey' => Questionnaire.find(survey_deployment.questionnaire_id),
+               'survey_deployment_id' => survey_deployment.id,
+               'start_date' => survey_deployment.start_date,
+               'end_date' => survey_deployment.end_date,
+               'parent_id' => p.parent_id,
+               'participant_id' => p.id,
+               'global_survey_id' => survey_deployment.global_survey_id]
+        end
+      end
+    end
+  end
+
+  # This method should be moved to survey_deployment_controller.rb
   def view_responses
     sd = SurveyDeployment.find(params[:id])
     @questionnaire = Questionnaire.find(sd.questionnaire_id)

config/routes.rb

@@ -1,3 +1,4 @@
+
 Expertiza::Application.routes.draw do
   ###
   # Please insert new routes alphabetically!
@@ -275,7 +276,6 @@
       get :redirect
       get :show_calibration_results_for_student
       post :custom_create
-      get :pending_surveys
       get :json
     end
   end
@@ -411,6 +411,7 @@
     collection do
       get :list
       get :reminder_thread
+      get :pending_surveys
     end
   end
 

spec/controllers/response_controller_spec.rb

@@ -286,10 +286,10 @@
     end
 
     context 'when params[:return] is survey' do
-      it 'redirects to response#pending_surveys page' do
+      it 'redirects to survey_deployment#pending_surveys page' do
         @params[:return] = 'survey'
         get :redirect, @params
-        expect(response).to redirect_to('/response/pending_surveys')
+        expect(response).to redirect_to('/survey_deployment/pending_surveys')
       end
     end
 
@@ -302,32 +302,9 @@
     end
   end
 
-  describe '#pending_surveys' do
-    context 'when session[:user] is nil' do
-      it 'redirects to root path (/)' do
-        params = {}
-        session = {}
-        get :pending_surveys, params, session
-        expect(response).to redirect_to('/')
-      end
-    end
-
-    context 'when session[:user] is not nil' do
-      it 'renders pending_surveys page' do
-        allow(CourseParticipant).to receive(:where).with(user_id: 6).and_return([double('CourseParticipant', id: 8, parent_id: 1)])
-        allow(AssignmentParticipant).to receive(:where).with(user_id: 6).and_return([participant])
-        survey_deployment = double('SurveyDeployment', id: 1, questionnaire_id: 1, global_survey_id: 1,
-                                                       start_date: DateTime.now.in_time_zone - 1.day, end_date: DateTime.now.in_time_zone + 1.day)
-        allow(Questionnaire).to receive(:find).with(1).and_return(questionnaire)
-        allow(CourseSurveyDeployment).to receive(:where).with(parent_id: 1).and_return([survey_deployment])
-        participant.parent_id = 1
-        allow(AssignmentSurveyDeployment).to receive(:where).with(parent_id: 1).and_return([survey_deployment])
-        params = {}
-        session = {user: instructor}
-        get :pending_surveys, params, session
-        expect(controller.instance_variable_get(:@surveys).size).to eq(2)
-        expect(response).to render_template(:pending_surveys)
-      end
-    end
-  end
+  # describe '#json' do
+  #  allow(Response).to receive(:find).with(response_id: 1).and_return([review_response])
+  #  allow(response).to receive(:response_id).and_return(1)
+  #  expect(response).to respond_with_content_type(:json)
+  # end
 end