CSC/ECE 517 Fall 2022 - E2254: Refactor teams controller.rb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
(Finally checked the help page, I added a description of two issues fixed and fixed a sub-bullet)
m (Fixed formatting on code blocks)
Line 29: Line 29:
* Fix #7:  
* Fix #7:  
One of the main changes made to implement this was in <code>Team.rb</code> where the class method <code>copy_teams_to_collection</code> was added.<br>
One of the main changes made to implement this was in <code>Team.rb</code> where the class method <code>copy_teams_to_collection</code> was added.<br>
<code># E2254 : This method copies a list of teams to another collection. It is mainly
<nowiki># E2254 : This method copies a list of teams to another collection. It is mainly
  # used in the Team model for copy_to_assignment and bequeath_all for the purpose of copying
# used in the Team model for copy_to_assignment and bequeath_all for the purpose of copying
  # teams from assignments to courses and vice versa.
# teams from assignments to courses and vice versa.
  def self.copy_teams_to_collection(teams, recipient_collection_id)
def self.copy_teams_to_collection(teams, recipient_collection_id)
    teams.each do |team|
  teams.each do |team|
      team.copy(recipient_collection_id)
    team.copy(recipient_collection_id)
    end
  end
  end</code>
end</nowiki>
This method was called in <code>teams_controller.rb</code> in the <code>copy_to_assignment</code> (method used to be named <code>inherit</code>) and <code>bequeath_all</code> methods.
This method was called in <code>teams_controller.rb</code> in the <code>copy_to_assignment</code> (method used to be named <code>inherit</code>) and <code>bequeath_all</code> methods.
<code>def copy_to_assignment
<nowiki>def copy_to_assignment
  ...
  ...
      if teams.empty?
  if teams.empty?
        flash[:note] = 'No teams were found when trying to copy to assignment.'
    flash[:note] = 'No teams were found when trying to copy to assignment.'
      else
  else
        Team.copy_teams_to_collection(teams, assignment.id)
    Team.copy_teams_to_collection(teams, assignment.id)
      end
  end
  ...
  ...
end</code>
end</nowiki>
<code>def bequeath_all
<nowiki>def bequeath_all
  ...
  ...
      teams = assignment.teams
  teams = assignment.teams
      Team.copy_teams_to_collection(teams, course.id)
  Team.copy_teams_to_collection(teams, course.id)
      flash[:note] = teams.length.to_s + ' teams were successfully copied to "' + course.name + '"'
  flash[:note] = teams.length.to_s + ' teams were successfully copied to "' + course.name + '"'
  ...
  ...
end</code>
end</nowiki>

Revision as of 01:32, 26 October 2022

Introduction

In this project we refactored teams_controller.rb and some tests associated with it. Before we worked on it, teams_controller.rb had some problems that violated some essential Rails design principles. Some methods have been moved to the teams.rb model class. 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. Added more comments and briefly explained the functionality of custom methods like action_allowed, and list
  2. Changed method name of create_teams to a better name: random_teams
  3. Found areas in the code where the DRY principle can be applied and did so
  4. The delete method manipulates a waitlist! This needs to be done in a model class, e.g., SignedUpTeam. And also check if this code is already present in the model or not. Explore the code base to find where else waitlist is manipulated.
    • Changed the delete method to use the existing Waitlist.remove_from_waitlists method to handle removing the deleted team from all waitlists
  5. Removed all extra added gems
  6. Renamed variable signUps to signups because it's a noun
    • Actually ended up removing this variable in the process of doing fix 4 above
  7. Moved list copying code from the inherit method to be a class method of the Team model: copy_teams_to_collection. Also replaced identical code in bequeath_all to also use this class method.
  8. Changed the name of the inherit method to copy_to_assignment so the name is more descriptive of it's purpose
  9. Removed nested if statments and removed other return points from bequeath_all
  10. Removed unnecessary comments for methods like update and added meaningful comments to other methods

Files Changed

  • app/controllers/teams_controller.rb
  • app/models/team.rb
  • app/views/teams/new.html.erb
  • config/locales/en_US.yml
  • config/locales/en_IN.yml
  • config/routes.rb
  • spec/controllers/teams_controller_spec.rb
  • spec/controllers/airbrake_exception_errors_controller_tests_spec.rb

Changes

  • Fix #7:

One of the main changes made to implement this was in Team.rb where the class method copy_teams_to_collection was added.

# E2254 : This method copies a list of teams to another collection. It is mainly
# used in the Team model for copy_to_assignment and bequeath_all for the purpose of copying
# teams from assignments to courses and vice versa.
def self.copy_teams_to_collection(teams, recipient_collection_id)
  teams.each do |team|
    team.copy(recipient_collection_id)
  end
end

This method was called in teams_controller.rb in the copy_to_assignment (method used to be named inherit) and bequeath_all methods.

def copy_to_assignment
  ...
  if teams.empty?
    flash[:note] = 'No teams were found when trying to copy to assignment.'
  else
    Team.copy_teams_to_collection(teams, assignment.id)
  end
  ...
end
def bequeath_all
  ...
  teams = assignment.teams
  Team.copy_teams_to_collection(teams, course.id)
  flash[:note] = teams.length.to_s + ' teams were successfully copied to "' + course.name + '"'
  ...
end