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
- 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 functioncopy_teams
- Refactored function
copy_teams
that callschoose_copy_type
, which callsbequeath_copy
orinherit_copy
based on bequeath
- 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
- Made changes to
hi_IN.yml
file, by adding more annotations and some grammatical errors - 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.
# 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
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, new test cases were added, some test cases had to be removed, while some of them had to be modified to correction.
Test Coverage
Coverage has slightly improved, as the number of lines in team.rb file has decreased as a result of refactoring.
Added Tests
1. In the test create teams method
, we added a test for incorrect parameters passed into the method. Below is the change
Modified Tests
2. In the test create method
, there was a test case which looked redundant, as it wasn't working earlier as well and it was already covered in the test file.
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 called when assignment belongs to course and team is not empty
. Similarly, bequeath_copy
was tested under called when assignment belongs to course but team is empty
.
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.