E1909 Refactor review mapping controller

E1909. Refactoring the Review Mapping Controller

About Expertiza

Expertiza is an open source project based on the Ruby on Rails framework. Expertiza provides a platform for instructors to create assignments and for students to set up teams to facilitate early communication and teamwork in these assignments. Expertiza also provides an environment where students can peer review other students, allowing these users to view feedback on their assignments in a timely manner. Expertiza supports submission across various document types, including the URLs and wiki pages.

Refactoring Considerations

The following items were improved in the refactoring of the given file:

• Long blocks of code converted into separate methods
• Ambiguous variable names changed to meaningful variable names
• Code clarity
• Test Coverage

About the Review Mapping Controller

The Review Mapping Controller handles mapping a given group's assignment submission to students working in other groups for peer review. The controller handles items associated with routing submissions to reviewers, handling permissions for instructor review calibration, and controlling drafts or uncompleted reviews. Included in this controller is the ability for the instructor to specify the number of required reviews per student or the number of reviewers per team in order to have group submission automatically sent to all students' review queues.

Examples of Code Modification

in automatic_review_mapping method, the following bits of code were added to separate methods:

participants.each do |participant|
  user = participant.user
  next if TeamsUser.team_id(assignment_id, user.id)
  team = AssignmentTeam.create_team_and_node(assignment_id)
  ApplicationController.helpers.create_team_users(user, team.id)
  teams << team
end

The above was moved into the team_assignment method that assigns users to a new team.

# check for exit paths first
  if student_review_num == 0 and submission_review_num == 0
    flash[:error] = "Please choose either the number of reviews per student or the number of reviewers per team (student)."
  elsif student_review_num != 0 and submission_review_num != 0
    flash[:error] = "Please choose either the number of reviews per student or the number of reviewers per team (student), not both."
  elsif student_review_num >= teams.size
    # Exception detection: If instructor wants to assign too many reviews done
    # by each student, there will be an error msg.
    flash[:error] = 'You cannot set the number of reviews done ' \
                    'by each student to be greater than or equal to total number of teams ' \
                    '[or "participants" if it is an individual assignment].'
  else
    # REVIEW: mapping strategy
    automatic_review_mapping_strategy(assignment_id, participants, teams, student_review_num, submission_review_num)
  end

Again, the above is a unnecessarily large 'if' block that can be moved to another method, one that was named validate_review_selection

The following is the final block of code moved to another method from automatic_review_mapping:

teams_with_calibrated_artifacts = []
teams_with_uncalibrated_artifacts = []
ReviewResponseMap.where(reviewed_object_id: assignment_id, calibrate_to: 1).each do |response_map|
  teams_with_calibrated_artifacts << AssignmentTeam.find(response_map.reviewee_id)
end
teams_with_uncalibrated_artifacts = teams - teams_with_calibrated_artifacts
# REVIEW: mapping strategy
automatic_review_mapping_strategy(assignment_id,participants,teams_with_calibrated_artifacts.shuffle!,calibrated_artifacts_num, 0)
# REVIEW: mapping strategy
# since after first mapping, participants (delete_at) will be nil
participants = AssignmentParticipant.where(parent_id: params[:id].to_i).to_a.select(&:can_review).shuffle!
automatic_review_mapping_strategy(assignment_id,participants,teams_with_uncalibrated_artifacts.shuffle!,uncalibrated_artifacts_num, 0)