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 associated files with it. Some duplicate code was removed. We added some comments for readability. Also, the hindi yml file was modified to enhance it.

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 team.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/team.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 teams.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.

   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 key, where the annotations were modified to maintain uniformity throughout this key.

Test Plan

The teams controller testing document has been covered well by the previous teams, however after detailed analysis of the test file, we found more edge cases that need to be tested, hence we added two new test cases. There was a test which was commented, it is found to be not required, and hence it was removed, while some of them had to be modified to correction.

Test Coverage

Coverage has slightly improved, as evident from the below image.Firstly, we have added new tests to cover more edge cases and Secondly, the number of lines in team.rb file has decreased as a result of refactoring.

Added Tests

1. In the test edit method, we added a test for when team with given id that exists. We also added a code when team with given id does not exist.

Also, for test create teams method, added a test case for when incorrect parameters are passed.

Modified Tests

2. In the test create method, there was a test case which was redundant, because a create method will ideally be called with a team that doesn't exist. Therefore, we removed this test.

3. There was some confusion about the function choose_copy_type, bequeath_copy and inherit_copy. In the test inherit method, the function inherit_copy was tested under inherit method (lines 180 - 220). Similarly, bequeath_copy was tested under #bequeath_all (lines 222 - 266).

4. In the test delete method, some redundant comments were made, that was removed as well.

5. Some minor changes were made to some of the test cases.

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.