update tests and add a bit more logging

Author: GrantBirki

lib/entitlements/backend/github_team/service.rb

@@ -32,7 +32,7 @@ class TeamNotFound < RuntimeError; end
           ou: String,
           ignore_not_found: C::Maybe[C::Bool],
         ] => C::Any
-        def initialize(addr: nil, org:, token:, ou:, ignore_not_found: false)
+        def initialize(org:, token:, ou:, addr: nil, ignore_not_found: false)
           super
           Entitlements.cache[:github_team_members] ||= {}
           Entitlements.cache[:github_team_members][org_signature] ||= {}
@@ -107,8 +107,8 @@ def read_team(entitlement_group)
                 end
 
                 maintainers = teamdata[:members].select { |u| teamdata[:roles][u] == "maintainer" }
-                team_metadata = team_metadata || {}
-                team_metadata = team_metadata.merge({"team_maintainers" => maintainers.any? ? maintainers.join(",") : nil})
+                team_metadata ||= {}
+                team_metadata = team_metadata.merge({ "team_maintainers" => maintainers.any? ? maintainers.join(",") : nil })
 
                 team = Entitlements::Backend::GitHubTeam::Models::Team.new(
                   team_id: teamdata[:team_id],
@@ -139,7 +139,7 @@ def read_team(entitlement_group)
         def from_predictive_cache?(entitlement_group)
           team_identifier = entitlement_group.cn.downcase
           read_team(entitlement_group) unless @team_cache[team_identifier]
-          (@team_cache[team_identifier] && @team_cache[team_identifier][:cache]) ? true : false
+          @team_cache[team_identifier] && @team_cache[team_identifier][:cache] ? true : false
         end
 
         # Declare the entry to be invalid for a specific team, and if the prior knowledge
@@ -192,7 +192,7 @@ def sync_team(desired_state, current_state)
             if desired_metadata["parent_team_name"].nil?
               Entitlements.logger.debug "sync_team(team=#{current_state.team_name}): IGNORING GitHub Parent Team DELETE"
             else
-            # :nocov:
+              # :nocov:
               Entitlements.logger.debug "sync_team(#{current_state.team_name}=#{current_state.team_id}): Parent team change found - From #{current_metadata["parent_team_name"] || "No Parent Team"} to #{desired_metadata["parent_team_name"]}"
               desired_parent_team_id = team_by_name(org_name: org, team_name: desired_metadata["parent_team_name"])[:id]
               unless desired_parent_team_id.nil?
@@ -240,17 +240,20 @@ def sync_team(desired_state, current_state)
                 Entitlements.logger.debug "sync_team(#{current_state.team_name}=#{current_state.team_id}): Textual change but no semantic change in maintainers. It is remains: #{current_maintainers.to_a}."
               else
                 Entitlements.logger.debug "sync_team(#{current_state.team_name}=#{current_state.team_id}): Maintainer members change found - From #{current_maintainers.to_a} to #{desired_maintainers.to_a}"
-                added_maintainers.select! { |username| add_user_to_team(user: username, team: current_state, role: "maintainer") }
+                added_maintainers.select! do |username|
+                  add_user_to_team(user: username, team: current_state, role: "maintainer")
+                end
 
                 ## We only touch previous maintainers who are actually still going to be members of the team
                 removed_maintainers = removed_maintainers.intersection(desired_team_members)
                 ## Downgrade membership to default (role: "member")
-                removed_maintainers.select! { |username| add_user_to_team(user: username, team: current_state, role: "member") }
+                removed_maintainers.select! do |username|
+                  add_user_to_team(user: username, team: current_state, role: "member")
+                end
               end
             end
           end
 
-
           Entitlements.logger.debug "sync_team(#{current_state.team_name}=#{current_state.team_id}): Added #{added_members.count}, removed #{removed_members.count}"
           added_members.any? || removed_members.any? || added_maintainers.any? || removed_maintainers.any? || changed_parent_team
         end
@@ -264,39 +267,41 @@ def sync_team(desired_state, current_state)
                    entitlement_group: Entitlements::Models::Group,
                  ] => C::Bool
         def create_team(entitlement_group:)
+          team_name = entitlement_group.cn.downcase
+          team_options = { name: team_name, repo_names: [], privacy: "closed" }
+
           begin
-            team_name = entitlement_group.cn.downcase
-            team_options = { name: team_name, repo_names: [], privacy: "closed" }
+            entitlement_metadata = entitlement_group.metadata
+            unless entitlement_metadata["parent_team_name"].nil?
 
-            begin
-              entitlement_metadata = entitlement_group.metadata
-              unless entitlement_metadata["parent_team_name"].nil?
-
-                begin
-                  parent_team_data = graphql_team_data(entitlement_metadata["parent_team_name"])
-                  team_options[:parent_team_id] = parent_team_data[:team_id]
-                rescue TeamNotFound
-                  # if the parent team does not exist, create it (think `mkdir -p` logic here)
-                  result = octokit.create_team(
-                    org,
-                    { name: entitlement_metadata["parent_team_name"], repo_names: [], privacy: "closed" }
-                  )
-                  team_options[:parent_team_id] = result[:id]
-                end
+              begin
+                parent_team_data = graphql_team_data(entitlement_metadata["parent_team_name"])
+                team_options[:parent_team_id] = parent_team_data[:team_id]
+              rescue TeamNotFound
+                # if the parent team does not exist, create it (think `mkdir -p` logic here)
+                result = octokit.create_team(
+                  org,
+                  { name: entitlement_metadata["parent_team_name"], repo_names: [], privacy: "closed" }
+                )
+
+                Entitlements.logger.debug "created parent team #{entitlement_metadata["parent_team_name"]} with id #{result[:id]}"
 
-                Entitlements.logger.debug "create_team(team=#{team_name}) Parent team #{entitlement_metadata["parent_team_name"]} with id #{parent_team_data[:team_id]} found"
+                team_options[:parent_team_id] = result[:id]
               end
-            rescue Entitlements::Models::Group::NoMetadata
-              Entitlements.logger.debug "create_team(team=#{team_name}) No metadata found"
-            end
 
-            Entitlements.logger.debug "create_team(team=#{team_name})"
-            octokit.create_team(org, team_options)
-            true
-          rescue Octokit::UnprocessableEntity => e
-            Entitlements.logger.debug "create_team(team=#{team_name}) ERROR - #{e.message}"
-            false
+              Entitlements.logger.debug "create_team(team=#{team_name}) Parent team #{entitlement_metadata["parent_team_name"]} with id #{team_options[:parent_team_id]} found"
+            end
+          rescue Entitlements::Models::Group::NoMetadata
+            Entitlements.logger.debug "create_team(team=#{team_name}) No metadata found"
           end
+
+          Entitlements.logger.debug "create_team(team=#{team_name})"
+          result = octokit.create_team(org, team_options)
+          Entitlements.logger.debug "created team #{team_name} with id #{result[:id]}"
+          true
+        rescue Octokit::UnprocessableEntity => e
+          Entitlements.logger.debug "create_team(team=#{team_name}) ERROR - #{e.message}"
+          false
         end
 
         # Update a team
@@ -309,15 +314,14 @@ def create_team(entitlement_group:)
                    metadata: C::Or[Hash, nil]
                  ] => C::Bool
         def update_team(team:, metadata: {})
-          begin
-            Entitlements.logger.debug "update_team(team=#{team.team_name})"
-            options = { name: team.team_name, repo_names: [], privacy: "closed", parent_team_id: metadata[:parent_team_id] }
-            octokit.update_team(team.team_id, options)
-            true
-          rescue Octokit::UnprocessableEntity => e
-            Entitlements.logger.debug "update_team(team=#{team.team_name}) ERROR - #{e.message}"
-            false
-          end
+          Entitlements.logger.debug "update_team(team=#{team.team_name})"
+          options = { name: team.team_name, repo_names: [], privacy: "closed",
+                      parent_team_id: metadata[:parent_team_id] }
+          octokit.update_team(team.team_id, options)
+          true
+        rescue Octokit::UnprocessableEntity => e
+          Entitlements.logger.debug "update_team(team=#{team.team_name}) ERROR - #{e.message}"
+          false
         end
 
         # Gets a team by name
@@ -343,7 +347,8 @@ def team_by_name(org_name:, team_name:)
         # team_slug - Identifier of the team to retrieve.
         #
         # Returns a data structure with team data.
-        Contract String => { members: C::ArrayOf[String], team_id: Integer, parent_team_name: C::Or[String, nil], roles: C::HashOf[String => String] }
+        Contract String => { members: C::ArrayOf[String], team_id: Integer, parent_team_name: C::Or[String, nil],
+                             roles: C::HashOf[String => String] }
         def graphql_team_data(team_slug)
           cursor = nil
           team_id = nil
@@ -381,9 +386,7 @@ def graphql_team_data(team_slug)
             end
 
             team = response[:data].fetch("data").fetch("organization").fetch("team")
-            if team.nil?
-              raise TeamNotFound, "Requested team #{team_slug} does not exist in #{org}!"
-            end
+            raise TeamNotFound, "Requested team #{team_slug} does not exist in #{org}!" if team.nil?
 
             team_id = team.fetch("databaseId")
             parent_team_name = team.dig("parentTeam", "slug")
@@ -401,6 +404,7 @@ def graphql_team_data(team_slug)
 
             cursor = edges.last.fetch("cursor")
             next if cursor && buffer.size == max_graphql_results
+
             break
           end
 
@@ -426,6 +430,7 @@ def validate_team_id_and_slug!(team_id, team_slug)
             team_data[:slug]
           end
           return if @validation_cache[team_id] == team_slug
+
           raise "validate_team_id_and_slug! mismatch: team_id=#{team_id} expected=#{team_slug.inspect} got=#{@validation_cache[team_id].inspect}"
         end
 
@@ -443,10 +448,11 @@ def validate_team_id_and_slug!(team_id, team_slug)
         ] => C::Bool
         def add_user_to_team(user:, team:, role: "member")
           return false unless org_members.include?(user.downcase)
-          unless role == "member" || role == "maintainer"
+          unless ["member", "maintainer"].include?(role)
             # :nocov:
             raise "add_user_to_team role mismatch: team_id=#{team.team_id} user=#{user} expected role=maintainer/member got=#{role}"
           end
+
           Entitlements.logger.debug "#{identifier} add_user_to_team(user=#{user}, org=#{org}, team_id=#{team.team_id}, role=#{role})"
           validate_team_id_and_slug!(team.team_id, team.team_name)
 
@@ -473,6 +479,7 @@ def add_user_to_team(user:, team:, role: "member")
         ] => C::Bool
         def remove_user_from_team(user:, team:)
           return false unless org_members.include?(user.downcase)
+
           Entitlements.logger.debug "#{identifier} remove_user_from_team(user=#{user}, org=#{org}, team_id=#{team.team_id})"
           validate_team_id_and_slug!(team.team_id, team.team_name)
           octokit.remove_team_membership(team.team_id, user)