CSC/ECE 517 Spring 2023 - E2303 Refactor teams controller.rb

Expertiza

Expertiza is a Ruby on Rails based open source project. Instructors have the ability to add new projects, assignments, etc., as well as edit existing ones. Later on, they can view student submissions and grade them. Students can also use Expertiza to organize into teams to work on different projects and assignments and submit their work. They can also review other students' submissions.

Introduction

In this project we refactored teams_controller.rb and some tests associated with it. Some duplicate code was removed. We added some comments for readability. And, we fixed tests to be more consistent with the program and execution.

Issues Fixed

1. Refactored code in teams_controller.rb
   • Added more comments and briefly explained the functionality of all methods
   • Found areas in the code where the DRY principle can be applied
   • Refactored the function bequeath_all to improve readability by including function copy_teams
   • Refactored function copy_teams that calls choose_copy_type, which calls bequeath_copy or inherit_copy based on bequeath
2. Refactored code in teams.rb
   • Added more comments to some methods in the code
   • Found areas in the code where the DRY principle can be applied
3. Made changes to hi_IN.yml file, by adding more annotations and some grammatical errors
4. Added more coverage to test by modifying teams_controller_spec.rb

Files Changed

• app/controllers/teams_controller.rb
• app/models/teams.rb
• config/locales/hi_IN.yml
• spec/controllers/teams_controller_spec.rb
• expertiza/app/views/teams/list.html.erb

Changes

Fix #1

The code in teams_controller.rb for function bequeath_all was too long, which would make it unreadable. To achieve it, a function copy_teams was added to the code.

  # Handovers all teams to the course that contains the corresponding assignment
  # The team and team members are all copied.
  def bequeath_all
    if session[:team_type] == Team.allowed_types[1]
      flash[:error] = 'Invalid team type for bequeath all'
      redirect_to controller: 'teams', action: 'list', id: params[:id]
    else
      copy_teams(Team.team_operation[:bequeath])
    end
  end
 

The function copy_teams is refactored further to improve it. The function choose_copy_type is refactored further to either perform bequeath_copy or inherit_copy based on team operation.

# Method to abstract the functionality to copy teams.
  def copy_teams(operation)
    assignment = Assignment.find(params[:id])
    if assignment.course_id
      choose_copy_type(assignment, operation)
    else
      flash[:error] = 'No course was found for this assignment.'
    end
    redirect_to controller: 'teams', action: 'list', id: assignment.id
  end

# Abstraction over different methods
  def choose_copy_type(assignment, operation)
    course = Course.find(assignment.course_id)
    if operation == Team.team_operation[:bequeath]
      bequeath_copy(assignment, course)
    else
      inherit_copy(assignment, course)
    end
  end

  # Method to perform a copy of assignment teams to course
  def bequeath_copy(assignment, course)
    teams = assignment.teams
    if course.course_teams.any?
      flash[:error] = 'The course already has associated teams'
    else
      Team.copy_content(teams, course)
      flash[:note] = teams.length.to_s + ' teams were successfully copied to "' + course.name + '"'
    end
  end

  # Method to inherit teams from course by copying
  def inherit_copy(assignment, course)
    teams = course.course_teams
    if teams.empty?
      flash[:error] = 'No teams were found when trying to inherit.'
    else
      Team.copy_content(teams, assignment)
      flash[:note] = teams.length.to_s + ' teams were successfully copied to "' + assignment.name + '"'
    end
  end

Fix #2

The file team.rb had an area where DRY principle could be applied. Function randomize_all_by_parent was modified to decrease the number of lines of code. The snippet to find teams still need team members and users who are not in any team has been modified, below is the code snippet.

   # find teams still need team members and users who are not in any team
   teams = Team.where(parent_id: parent.id, type: parent.class.to_s + 'Team').to_a
   teams.each { |team| TeamsUser.where(team_id: team.id).each { |teams_user| users.delete(User.find(teams_user.user_id)) } }
   teams.reject! { |team| Team.size(team.id) >= min_team_size }

Fix #3

The file hi_IN.yml has some formatting in annotations that needed to be taken care of. For example, in Hindi, a sentence ending is indicated by the symbol "|", but it is given as "." everywhere. This was taken care of in the code, wherever Hindi annotations are available. Also, some additional annotations were added for datetime->distance_in_words section, where the annotations were modified to maintain uniformity throughout this section.

Test Plan

Since this project has only refactoring to do, no concrete Test Plan has been devised for it. However, the test coverage has improved a little.

Test Coverage

Coverage has slightly improved, as the number of lines in team.rb file has decreased as a result of refactoring.

Contributors

This project was done as part of Dr. Edward Gehringer's "CSC/ECE 517: Object-Oriented Design and Development" class, Spring 2023. The contributors were: Sasank Marabattula, Srilekha Gudipati and Varun Deepak Gudhe. Our project mentor was Divyang Doshi.