Refactored add_reviewer function- changed function name, split funtio…

…n to 2 parts

app/controllers/review_mapping_controller.rb

@@ -64,7 +64,42 @@ def select_metareviewer
     @mapping = ResponseMap.find(params[:id])
   end
 
-  def add_reviewer
+=begin
+  Used: to assign an new reviewer (who is not already assigned) to a team
+  Implements: checks if a ReviewResponse map exists for that reviewer (user). 
+              Raises error if reviewer is already assigned to the team.
+  Returns: error_msg
+=end
+  def assign_unseen_reviewer_to_team(assignment, user_id)
+    error_msg = ''
+    begin
+      user = User.from_params(params)
+      # contributor_id is team_id
+      regurl = url_for id: assignment.id,
+                       user_id: user.id,
+                       contributor_id: params[:contributor_id]
+
+      # Get the assignment's participant corresponding to the user
+      reviewer = get_reviewer(user, assignment, regurl)
+      # ACS Removed the if condition(and corresponding else) which differentiate assignments as team and individual assignments
+      # to treat all assignments as team assignments
+      if ReviewResponseMap.where(reviewee_id: params[:contributor_id], reviewer_id: reviewer.id).first.nil?
+        ReviewResponseMap.create(reviewee_id: params[:contributor_id], reviewer_id: reviewer.id, reviewed_object_id: assignment.id)
+      else
+        raise 'The reviewer, "' + reviewer.name + '", is already assigned to this contributor.'
+      end
+    rescue StandardError => e
+      error_msg = e.message
+    end
+    error_msg
+  end
+
+=begin
+  Used: to assign a user as a reviewer to a team
+  Implements: checks if a user belongs to one's own team, in which case they cannot self-review.
+               Else, redirects to the SignUpSheet and assigns an unseen reviewer to a team
+=end
+  def add_reviewer_to_team
     assignment = Assignment.find(params[:id])
     topic_id = params[:topic_id]
     user_id = User.where(name: params[:user][:name]).first.id
@@ -75,30 +110,13 @@ def add_reviewer
     else
       # Team lazy initialization
       SignUpSheet.signup_team(assignment.id, user_id, topic_id)
-      msg = ''
-      begin
-        user = User.from_params(params)
-        # contributor_id is team_id
-        regurl = url_for id: assignment.id,
-                         user_id: user.id,
-                         contributor_id: params[:contributor_id]
-
-        # Get the assignment's participant corresponding to the user
-        reviewer = get_reviewer(user, assignment, regurl)
-        # ACS Removed the if condition(and corresponding else) which differentiate assignments as team and individual assignments
-        # to treat all assignments as team assignments
-        if ReviewResponseMap.where(reviewee_id: params[:contributor_id], reviewer_id: reviewer.id).first.nil?
-          ReviewResponseMap.create(reviewee_id: params[:contributor_id], reviewer_id: reviewer.id, reviewed_object_id: assignment.id)
-        else
-          raise 'The reviewer, "' + reviewer.name + '", is already assigned to this contributor.'
-        end
-      rescue StandardError => e
-        msg = e.message
-      end
+      msg = assign_unseen_reviewer_to_team(assignment, user_id)
     end
     redirect_to action: 'list_mappings', id: assignment.id, msg: msg
   end
 
+
+
   # 7/12/2015 -zhewei
   # This method is used for assign submissions to students for peer review.
   # This method is different from 'assignment_reviewer_automatically', which is in 'review_mapping_controller'

app/views/review_mapping/select_reviewer.html.haml

@@ -4,7 +4,7 @@
 %h2
   Assignment: #{@contributor.assignment.name}
 %br/
-= form_tag :action => 'add_reviewer', :id => @contributor.parent_id do
+= form_tag :action => 'add_reviewer_to_team', :id => @contributor.parent_id do
   Enter a user login: #{text_field_with_auto_complete :user, :name, {:size => 41}}
   \#{hidden_field_tag('contributor_id', @contributor.id)}
   %input{:type => "submit", :value => "Add Reviewer"}/

config/routes.rb

@@ -368,7 +368,7 @@
       get :add_calibration_for_instructor
       get :list_mappings
       get :unsubmit_review
-      post :add_reviewer
+      post :add_reviewer_to_team
       post :add_metareviewer
       post :add_user_to_assignment
       post :assign_metareviewer_dynamically

spec/controllers/review_mapping_controller_spec.rb

@@ -60,7 +60,7 @@
     end
   end
 
-  describe '#add_reviewer and #get_reviewer' do
+  describe '#add_reviewer_to_team and #get_reviewer' do
     before(:each) do
       allow(User).to receive_message_chain(:where, :first).with(name: 'expertiza').with(no_args).and_return(double('User', id: 1))
       @params = {
@@ -74,7 +74,7 @@
     context 'when team_user does not exist' do
       it 'shows an error message and redirects to review_mapping#list_mappings page' do
         allow(TeamsUser).to receive(:exists?).with(team_id: '1', user_id: 1).and_return(true)
-        post :add_reviewer, params: @params
+        post :add_reviewer_to_team, params: @params
         expect(response).to redirect_to '/review_mapping/list_mappings?id=1'
       end
     end
@@ -90,15 +90,15 @@
         allow(ReviewResponseMap).to receive_message_chain(:where, :first)
           .with(reviewee_id: '1', reviewer_id: 1).with(no_args).and_return(nil)
         allow(ReviewResponseMap).to receive(:create).with(reviewee_id: '1', reviewer_id: 1, reviewed_object_id: 1).and_return(nil)
-        post :add_reviewer, params: @params
+        post :add_reviewer_to_team, params: @params
         expect(response).to redirect_to '/review_mapping/list_mappings?id=1&msg='
       end
     end
 
     context 'when instructor tries to assign a student their own artifact for reviewing' do
       it 'flashes an error message' do
         allow(TeamsUser).to receive(:exists?).with(team_id: '1', user_id: 1).and_return(true)
-        post :add_reviewer, params: @params
+        post :add_reviewer_to_team, params: @params
         expect(flash[:error]).to eq('You cannot assign this student to review his/her own artifact.')
         expect(response).to redirect_to '/review_mapping/list_mappings?id=1'
       end