CSC/ECE 517 Fall 2022 - E2274: Refactor teams controller.rb

Introduction

Background

In Expertiza, teams are used to allow students to work together in group projects. The teams controller is responsible for assigning students to teams, as well as performing CRUD operations. In this project we will refactor teams_controller.rb and add tests to increase test coverage. We aim to follow the DRY principle in our refactoring by removing code that exists in multiple places. This project is a continuation of our previous work as described below, and we will build our existing code changes.

Previous Work

A team in spring 2022 attempted to refactor the controller as shown here. This revision modified a few methods such as delete, inherit, and bequeath_all. It also added new methods like init_team_type, get_parent_by_id, get_parent_from_child.

More recently, we began a new refactor which can be found here. This revision involved renaming methods, improving how teams were removed from the waitlist, and drying code. Some code was also moved to the Team model (team.rb).

Project Overview

There are several issues we plan to address as part of this project.

• More comments need to be added to the controller to briefly explain functionality of custom methods.
• Find and fix any code where the DRY principle can be applied. Two places that will be taken a look at for this are the create and update methods.
• Try to refactor the update method to make it cleaner.
• There are few methods such as team_size, team_parent which will be removed.
• Fix the code and Hindi text in config/locales/hi_IN.yml.
• Add tests to increase test coverage by 10-20% if able.

Issue 1: Add Comments

We plan to add comments describing the functionality of the following methods.

• list

• new

• update

• edit

• delete_all

• delete

Issue 2: Dry Code

Here's one fix that needs to be done that can be used to enfore the DRY principle in these the list, create, and delete_all methods in teams_controller.rb. To fix this, we can extract the Object.const_get(session[:team_type] + ... ) to its own method that can be used in all instances. By moving this code to its own method and replacing with the previous code, it will be following the DRY princple because there's less code being repeated.

To address this code, we added a new method:

This method replaced the repeating code from in the program, ending up with the result seen below. This code is cleaner and properly applies the DRY principle.

Issue 3: Refactor Update

We have analyzed the update function in teams_controller.rb to see how we could refactor it to improve functionality. The method currently uses check_for_existing_teams to validate the new team name as shown below.

We have refactored to move the error catching code to the check_for_existing_teams method. This cleans up the update method a bit. The same change was applied to the create method as well.

Issue 4: Remove Methods

We will analyze teams_controller to find unused or one-line methods such as team_size and team_parent and remove or refactor them to clean up the code. The goal of this task is to remove unused or extra code to make the controller easier to read and understand. After analyzing the code, we could see that the team_size method only had one line, and was being used only in one place. So, the method was removed and the call to it (boxed in blue) was replaced with the line of code from the method. The code for team_parent was found to be redundant, and so every call made for team_parent was replaced with what would be the result from it (boxedin red). Also, because the ID of the team parent is the same as the params[:id], all instances of team_parent.id were replaced with params[:id] (boxed in green).

Issue 5: Update Translations

The code that was changed is highlighted below. The translation needs to be ensured that it is proper and the frontend needs to make sure that it's mapped to the correct phrase. The English file was also updated to reflect new language introduced in our last project, E2254, which is linked above.

Issue 6: Test Coverage

Before starting this project, the teams controller was 79.8% covered. If all of the below tests were added, the controller would reach at 100% coverage. We added some of these tests as part of this project and brought coverage up to 94.9%, which is a 15.1% increase. We also discovered a couple bugs when implementing the tests.

Create team when team exists

Parts of an existing test were uncommented and fixed to test this case and show coverage.

Update team when team exists

A new test was added to ensure the code properly handles the TeamExistsError which occurs if an existing team name is used.

Update team when team doesn't exist

Parts of an existing test were uncommented and fixed to test this case and show coverage.

Delete all when child nodes exist

Attempting to write a test for this method revealed that the delete_all method is currently broken in Expertiza. The code before we made any changes called Team.destroy_all to delete the teams for an assignment. However, this will delete all teams, not just the teams for a assignment or course. This code was commented out and a note was added to document the broken method. A skipped test stub was added, but the delete_all method will need to implemented properly before the full test is written.

Copy to assignment when no course

An existing test case was fixed to test this code. The test should have been failing, but it passed because of improper checking. The test code was updated to check the flash message to ensure an error is shown when an invalid course id is given. The code was updated to try to actually find a course with the given id rather than just checking if the id is present.

Create when team parent is a Course

This test was not added, but adding it should d cover an additional 2 lines in the team_type method.

Diagrams

Overview Diagram

The following diagram shows an overview of the different routes in the controller and some of their basic functionality.

Use Case Diagram

The teams controller is mainly responsible for dealing with the Team model. It can perform CRUD operations with teams and manage them on assignments and courses. It also has methods that can move the teams between them.

Test Plan

RSpec Tests

As mentioned for Issue 6, we plan to write several RSpec tests to increase test coverage of the teams_controller file by 10-20%. Specific test cases are list in the issue's section.

Manual Testing

Randomize Teams Testing

Given you are logged in as an instructor

• Username: instructor6 Password password

And you are on the Manage Assignments page

When you click on create teams for an assignment with participants

And you click "Create Team"

And enter appropriate information

Then teams should be randomized and created

Bequeath all Testing

Given a team has been created for an assignment

And I am on the create teams page for an assignment

When I click "Bequeath All Teams"

Then I should be able to move the team from an assignment to a course

Course teams before bequeath all:

Course teams after bequeath all: