Some cleanups related to groups (#9377)

* Some cleanups related to groups

* Review changes

app/controllers/api/v0/api_controller.rb

@@ -156,13 +156,13 @@ def omni_search
   end
 
   def delegates
-    paginate json: UserGroup.delegate_region_groups.flat_map(&:active_users)
+    paginate json: UserGroup.delegate_regions.flat_map(&:active_users)
   end
 
   def delegates_search_index
     # TODO: There is a `uniq` call at the end which I feel shouldn't be necessary?!
     #   Postponing investigation until the Roles system migration is complete.
-    all_delegates = UserGroup.includes(roles: [:user]).delegate_region_groups.flat_map(&:active_users).uniq
+    all_delegates = UserGroup.includes(roles: [:user]).delegate_regions.flat_map(&:active_users).uniq
 
     search_index = all_delegates.map do |delegate|
       delegate.slice(:id, :name, :wca_id).merge({ thumb_url: delegate.avatar.url(:thumb) })

app/controllers/translations_controller.rb

@@ -57,7 +57,7 @@ def update
     info = ["WCA Account ID: *#{user.id}*"]
     info.unshift("WCA ID: *[#{user.wca_id}](#{person_url(user.wca_id)})*") if user.wca_id
     is_verified_translator = false
-    UserGroup.translator_groups.each do |translators_group|
+    UserGroup.translators.each do |translators_group|
       if translators_group.roles.any? { |role| role.user_id == user.id && role.group.metadata.locale == locale }
         is_verified_translator = true
         break

app/jobs/sync_mailing_lists_job.rb

@@ -9,7 +9,7 @@ class SyncMailingListsJob < WcaCronjob
   def perform
     GsuiteMailingLists.sync_group("[email protected]", UserGroup.teams_committees.map(&:lead_user).compact.map(&:email))
     GsuiteMailingLists.sync_group(GroupsMetadataBoard.email, UserGroup.board_group.active_users.map(&:email))
-    translator_users = UserGroup.translator_groups.flat_map(&:users)
+    translator_users = UserGroup.translators.flat_map(&:users)
     GsuiteMailingLists.sync_group("[email protected]", translator_users.map(&:email))
     User.clear_receive_delegate_reports_if_not_eligible
     GsuiteMailingLists.sync_group("[email protected]", User.delegate_reports_receivers_emails)
@@ -23,7 +23,7 @@ def perform
     delegate_emails = []
     trainee_emails = []
     senior_emails = []
-    active_root_delegate_regions = UserGroup.delegate_region_groups.where(parent_group_id: nil, is_active: true)
+    active_root_delegate_regions = UserGroup.delegate_regions.where(parent_group_id: nil, is_active: true)
     active_root_delegate_regions.each do |region|
       region_emails = []
       (region.active_roles + region.active_roles_of_all_child_groups).each do |role|

app/models/roles_metadata_delegate_regions.rb

@@ -11,5 +11,6 @@ class RolesMetadataDelegateRegions < ApplicationRecord
 
   has_one :user_role, as: :metadata
   has_one :group, through: :user_role
+  has_one :user, through: :user_role
   has_one :delegate_region, through: :group, source: :metadata, source_type: "GroupsMetadataDelegateRegions"
 end

app/models/roles_metadata_teams_committees.rb

@@ -7,10 +7,13 @@ class RolesMetadataTeamsCommittees < ApplicationRecord
     member: "member",
   }
 
+  has_one :user_role, as: :metadata
+  has_one :user, through: :user_role
+
   def at_least_senior_member?
     [
-      RolesMetadataTeamsCommittees.statuses[:senior_member],
-      RolesMetadataTeamsCommittees.statuses[:leader],
+      statuses[:senior_member],
+      statuses[:leader],
     ].include?(status)
   end
 end

app/models/user.rb

@@ -68,8 +68,8 @@ def self.eligible_voters
   end
 
   def self.leader_senior_voters
-    team_leaders = UserGroup.teams_committees.map(&:lead_user)
-    senior_delegates = UserGroup.delegate_region_groups_senior_delegates
+    team_leaders = RolesMetadataTeamsCommittees.leader.includes(:user, :user_role).select { |role_metadata| role_metadata.user_role.is_active? }.map(&:user)
+    senior_delegates = RolesMetadataDelegateRegions.senior_delegate.includes(:user, :user_role).select { |role_metadata| role_metadata.user_role.is_active? }.map(&:user)
     (team_leaders + senior_delegates).uniq.compact
   end
 
@@ -1253,7 +1253,7 @@ def accepted_competitions
   end
 
   def is_delegate_in_probation
-    UserGroup.delegate_probation_groups.flat_map(&:active_users).include?(self)
+    UserGroup.delegate_probation.flat_map(&:active_users).include?(self)
   end
 
   private def can_manage_delegate_probation?

app/models/user_group.rb

@@ -17,28 +17,21 @@ class UserGroup < ApplicationRecord
   }
 
   has_many :direct_child_groups, class_name: "UserGroup", inverse_of: :parent_group, foreign_key: "parent_group_id"
+  has_many :roles, foreign_key: "group_id", class_name: "UserRole"
+  has_many :active_roles, -> { active }, foreign_key: "group_id", class_name: "UserRole"
+  has_many :users, through: :roles
+  has_many :active_users, through: :active_roles, source: :user, class_name: "User"
+
   belongs_to :metadata, polymorphic: true, optional: true
   belongs_to :parent_group, class_name: "UserGroup", optional: true
 
-  has_many :roles, foreign_key: "group_id", class_name: "UserRole"
-
   scope :root_groups, -> { where(parent_group: nil) }
   scope :active_groups, -> { where(is_active: true) }
 
   def all_child_groups
     [direct_child_groups, direct_child_groups.map(&:all_child_groups)].flatten
   end
 
-  # For teams which have groups migrated but not roles, this method will help to get the
-  # corresponding team to fetch the team_members.
-  def team
-    Team.find_by(friendly_id: self.metadata.friendly_id)
-  end
-
-  def active_roles
-    self.roles.select { |role| role.is_active? }
-  end
-
   def roles_of_direct_child_groups
     self.direct_child_groups.map(&:roles).flatten
   end
@@ -55,14 +48,6 @@ def active_roles_of_all_child_groups
     self.all_child_groups.map(&:active_roles).flatten
   end
 
-  def users
-    self.roles.map { |role| role.user }
-  end
-
-  def active_users
-    self.active_roles.map { |role| role.user }
-  end
-
   def users_of_direct_child_groups
     self.roles_of_direct_child_groups.map { |role| role.user }
   end
@@ -101,22 +86,6 @@ def self.group_type_name
     }
   end
 
-  def self.delegate_region_groups
-    UserGroup.where(group_type: UserGroup.group_types[:delegate_regions])
-  end
-
-  def self.delegate_region_groups_senior_delegates
-    UserGroup.delegate_region_groups.root_groups.map(&:lead_user).compact
-  end
-
-  def self.delegate_probation_groups
-    UserGroup.where(group_type: UserGroup.group_types[:delegate_probation])
-  end
-
-  def self.translator_groups
-    UserGroup.where(group_type: UserGroup.group_types[:translators])
-  end
-
   def self.board_group
     UserGroup.board.first
   end
@@ -198,7 +167,7 @@ def senior_delegate
   end
 
   def lead_role
-    self.active_roles.find { |role| role.is_lead? }
+    active_roles.includes(:group, :metadata).find { |role| role.is_lead? }
   end
 
   # TODO: Once the roles migration is done, add a validation to make sure there is only one lead_user per group.

app/webpacker/lib/requests/routes.js.erb

@@ -211,7 +211,6 @@ export const apiV0Urls = {
     registrationData: `<%= CGI.unescape(Rails.application.routes.url_helpers.api_v0_registration_data_path) %>`
   },
   delegates: {
-    list: `<%= CGI.unescape(Rails.application.routes.url_helpers.api_v0_delegates_path)%>`,
     searchIndex: `<%= CGI.unescape(Rails.application.routes.url_helpers.api_v0_delegates_search_index_path)%>`,
   },
 }