E1771. [TLD] Refactor team.rb

Gemfile.lock

@@ -520,6 +520,3 @@ DEPENDENCIES
   uglifier
   will_paginate
   zip-zip
-
-BUNDLED WITH
-   1.15.1

app/controllers/grades_controller.rb

@@ -195,7 +195,7 @@ def redirect_when_disallowed
     if @participant.assignment.max_team_size > 1
       team = @participant.team
       unless team.nil?
-        unless team.has_user session[:user]
+        unless team.user? session[:user]
           flash[:error] = 'You are not on the team that wrote this feedback'
           redirect_to '/'
           return true

app/controllers/response_controller.rb

@@ -19,7 +19,9 @@ def action_allowed?
       # if it is a review response map, all the members of reviewee team should be able to view the reponse (can be done from heat map)
       if map.is_a? ReviewResponseMap
         reviewee_team = AssignmentTeam.find(map.reviewee_id)
-        return current_user_id?(response.map.reviewer.user_id) || reviewee_team.has_user(current_user) || current_user.role.name == 'Administrator' || (current_user.role.name == 'Instructor' and assignment.instructor_id == current_user.id) || (current_user.role.name == 'Teaching Assistant' and TaMapping.exists?(ta_id: current_user.id, course_id: assignment.course.id))
+        return current_user_id?(response.map.reviewer.user_id) || reviewee_team.user?(current_user) || current_user.role.name == 'Administrator' ||
+            (current_user.role.name == 'Instructor' and assignment.instructor_id == current_user.id) || (current_user.role.name == 'Teaching Assistant' and
+            TaMapping.exists?(ta_id: current_user.id, course_id: assignment.course.id))
       else
         return current_user_id?(response.map.reviewer.user_id)
       end

app/helpers/manage_team_helper.rb

@@ -15,7 +15,7 @@ def create_team_users(user, team_id)
   end
 
   # check if the user specified by 'user' already belongs to team specified by 'team_id'
-  def has_user(user, team_id)
+  def user?(user, team_id)
     if TeamsUser.where(team_id: team_id, user_id:  user.id).first
       return true
     else

app/models/assignment_team.rb

@@ -71,7 +71,7 @@ def participants
     users = self.users
     participants = []
     users.each do |user|
-      participant = AssignmentParticipant.where(user_id: user.id, parent_id: self.parent_id).first
+      participant = AssignmentParticipant.find_by(user_id: user.id, parent_id: self.parent_id)
       participants << participant unless participant.nil?
     end
     participants
@@ -147,7 +147,9 @@ def copy(course_id)
 
   # Add Participants to the current Assignment Team
   def add_participant(assignment_id, user)
-    AssignmentParticipant.create(parent_id: assignment_id, user_id: user.id, permission_granted: user.master_permission_granted) if AssignmentParticipant.where(parent_id: assignment_id, user_id: user.id).first.nil?
+    if AssignmentParticipant.find_by(parent_id: assignment_id, user_id: user.id).nil?
+      AssignmentParticipant.create(parent_id: assignment_id, user_id: user.id, permission_granted: user.master_permission_granted)
+    end
   end
 
   # Return the parent Assignment

app/models/course_team.rb

@@ -39,7 +39,7 @@ def copy(assignment_id)
 
   # deprecated: the functionality belongs to course
   def add_participant(course_id, user)
-    if CourseParticipant.where(parent_id: course_id, user_id: user.id).first.nil?
+    if CourseParticipant.find_by(parent_id: course_id, user_id: user.id).nil?
       CourseParticipant.create(parent_id: course_id, user_id: user.id, permission_granted: user.master_permission_granted)
     end
   end
@@ -71,12 +71,12 @@ def self.export_fields(options)
 
   # Add member to the course team
   def add_member(user)
-    if has_user(user)
+    if user?(user)
       raise "The user \"" + user.name + "\" is already a member of the team, \"" + self.name + "\""
     end
 
     t_user = TeamsUser.create(user_id: user.id, team_id: self.id)
-    parent = TeamNode.find_by_node_object_id(self.id)
+    parent = TeamNode.find_by(node_object_id: self.id)
     TeamUserNode.create(parent_id: parent.id, node_object_id: t_user.id)
     add_participant(self.parent_id, user)
   end

app/models/review_assignment.rb

@@ -144,7 +144,7 @@ def reject_previously_reviewed_submissions(contributor_set, reviewer)
   end
 
   def reject_own_submission(contributor_set, reviewer)
-    contributor_set.reject! {|contributor| contributor.has_user(User.find(reviewer.user_id)) }
+    contributor_set.reject! {|contributor| contributor.user?(User.find(reviewer.user_id)) }
     contributor_set
   end
 

app/models/team.rb

@@ -19,19 +19,19 @@ def responses
 
   # Delete the given team
   def delete
-    TeamsUser.where(team_id: self.id).each{ |teams_user| teams_user.destroy }
+    TeamsUser.where(team_id: self.id).each(&:destroy)
     node = TeamNode.find_by(node_object_id: self.id)
     node.destroy if node
     self.destroy
   end
 
   # Get the node type of the tree structure
-  def get_node_type
+  def node_type
     "TeamNode"
   end
 
   # Get the names of the users
-  def get_author_names
+  def author_names
     names = []
     users.each do |user|
       names << user.fullname
@@ -40,7 +40,7 @@ def get_author_names
   end
 
   # Check if the user exist
-  def has_user(user)
+  def user?(user)
     users.include? user
   end
 
@@ -54,14 +54,14 @@ def full?
 
   # Add memeber to the team
   def add_member(user, _assignment_id = nil)
-    if has_user(user)
+    if user?(user)
       raise "The user #{user.name} is already a member of the team #{self.name}"
     end
     can_add_member = false
     unless full?
       can_add_member = true
       t_user = TeamsUser.create(user_id: user.id, team_id: self.id)
-      parent = TeamNode.find_by_node_object_id(self.id)
+      parent = TeamNode.find_by(node_object_id: self.id)
       TeamUserNode.create(parent_id: parent.id, node_object_id: t_user.id)
       add_participant(self.parent_id, user)
     end
@@ -114,43 +114,56 @@ def self.randomize_all_by_parent(parent, team_type, min_team_size)
       end
     end
     # sort teams by decreasing team size
-    teams.sort_by {|team| Team.size(team.id) }.reverse!
+    teams = sort_teams_by_members_reverse(teams)
     # insert users who are not in any team to teams still need team members
     if !users.empty? and !teams.empty?
-      teams.each do |team|
-        curr_team_size = Team.size(team.id)
-        member_num_difference = min_team_size - curr_team_size
-        for i in (1..member_num_difference).to_a
-          team.add_member(users.first, parent.id)
-          users.delete(users.first)
-          break if users.empty?
-        end
-        break if users.empty?
-      end
+      assign_single_users_to_teams(min_team_size, parent, teams, users)
     end
     # If all the existing teams are fill to the min_team_size and we still have more users, create teams for them.
     unless users.empty?
-      num_of_teams = users.length.fdiv(min_team_size).ceil
-      nextTeamMemberIndex = 0
-      for i in (1..num_of_teams).to_a
-        team = Object.const_get(team_type + 'Team').create(name: "Team" + i.to_s, parent_id: parent.id)
-        TeamNode.create(parent_id: parent.id, node_object_id: team.id)
-        min_team_size.times do
-          break if nextTeamMemberIndex >= users.length
-          user = users[nextTeamMemberIndex]
-          team.add_member(user, parent.id)
-          nextTeamMemberIndex += 1
-        end
+      create_team_from_single_users(min_team_size, parent, team_type, users)
+    end
+  end
+
+  def self.create_team_from_single_users(min_team_size, parent, team_type, users)
+    num_of_teams = users.length.fdiv(min_team_size).ceil
+    next_team_member_index = 0
+    for i in (1..num_of_teams).to_a
+      team = Object.const_get(team_type + 'Team').create(name: "Team" + i.to_s, parent_id: parent.id)
+      TeamNode.create(parent_id: parent.id, node_object_id: team.id)
+      min_team_size.times do
+        break if next_team_member_index >= users.length
+        user = users[next_team_member_index]
+        team.add_member(user, parent.id)
+        next_team_member_index += 1
       end
     end
   end
 
+  def self.assign_single_users_to_teams(min_team_size, parent, teams, users)
+    teams.each do |team|
+      curr_team_size = Team.size(team.id)
+      member_num_difference = min_team_size - curr_team_size
+      while member_num_difference > 0
+        team.add_member(users.first, parent.id)
+        users.delete(users.first)
+        member_num_difference -= 1
+        break if users.empty?
+      end
+      break if users.empty?
+    end
+  end
+
+  def self.sort_teams_by_members_reverse(teams)
+    teams.sort_by { |team| Team.size(team.id) }.reverse!
+  end
+
   # Generate the team name
   def self.generate_team_name(team_name_prefix)
     counter = 1
     loop do
       team_name = team_name_prefix + "_Team#{counter}"
-      return team_name unless Team.find_by_name(team_name)
+      return team_name unless Team.find_by(name: team_name)
       counter += 1
     end
   end
@@ -159,11 +172,11 @@ def self.generate_team_name(team_name_prefix)
   def import_team_members(starting_index, row)
     index = starting_index
     while index < row.length
-      user = User.find_by_name(row[index].to_s.strip)
+      user = User.find_by(name: row[index].to_s.strip)
       if user.nil?
         raise ImportError, "The user #{row[index].to_s.strip} was not found. <a href='/users/new'>Create</a> this user?"
       else
-        if TeamsUser.where(team_id: id, user_id: user.id).first.nil?
+        if TeamsUser.find_by(team_id: id, user_id: user.id).nil?
           add_member(user)
         end
       end
@@ -178,7 +191,7 @@ def self.import(row, id, options, teamtype)
 
     if options[:has_column_names] == "true"
       name = row[0].to_s.strip
-      team = where(name: name, parent_id: id).first
+      team = find_by(name: name, parent_id: id)
       team_exists = !team.nil?
       name = handle_duplicate(team, name, id, options[:handle_dups], teamtype)
       index = 1

app/views/popup/view_review_scores_popup.html.erb

@@ -38,7 +38,7 @@
               <!--Reviewee-->
               <td>
                 <p style="color: <%= col_color%>"><%=team.name%> <br>
-                <%team.get_author_names.each do |name|%>
+                <%team.author_names.each do |name|%>
                   <%= name %> <br>
                 <%end%></p>
               </td>

app/views/submitted_content/_hyperlink.html.erb

@@ -1,7 +1,7 @@
 <%= form_for :hyperlinks, url: '/submitted_content/remove_hyperlink' do |f| %>
   <% if stage != "Finished" %>
     <% team_id = TeamsUser.team_id(participant.parent_id, participant.user_id) %>
-    <% display_to_author = Team.exists?(team_id) && Team.find(team_id).has_user(@current_user)%>
+    <% display_to_author = Team.exists?(team_id) && Team.find(team_id).user?(@current_user)%>
 
     <% if display_to_author %>
       <b>Hyperlink Actions:</b><BR/>