refactor(enterprise_team): address PR review feedback

github/data_source_github_enterprise_team.go

@@ -2,7 +2,6 @@ package github
 
 import (
 	"context"
-	"fmt"
 	"strconv"
 	"strings"
 
@@ -25,16 +24,18 @@ func dataSourceGithubEnterpriseTeam() *schema.Resource {
 				ValidateDiagFunc: validation.ToDiagFunc(validation.All(validation.StringIsNotWhiteSpace, validation.StringIsNotEmpty)),
 			},
 			"slug": {
-				Type:         schema.TypeString,
-				Optional:     true,
-				Computed:     true,
-				ExactlyOneOf: []string{"team_id"},
-				Description:  "The slug of the enterprise team.",
+				Type:             schema.TypeString,
+				Optional:         true,
+				Computed:         true,
+				ExactlyOneOf:     []string{"slug", "team_id"},
+				Description:      "The slug of the enterprise team.",
+				ValidateDiagFunc: validation.ToDiagFunc(validation.All(validation.StringIsNotWhiteSpace, validation.StringIsNotEmpty)),
 			},
 			"team_id": {
 				Type:             schema.TypeInt,
 				Optional:         true,
 				Computed:         true,
+				ExactlyOneOf:     []string{"slug", "team_id"},
 				Description:      "The numeric ID of the enterprise team.",
 				ValidateDiagFunc: validation.ToDiagFunc(validation.IntAtLeast(1)),
 			},
@@ -137,22 +138,3 @@ func dataSourceGithubEnterpriseTeamRead(ctx context.Context, d *schema.ResourceD
 
 	return nil
 }
-
-// findEnterpriseTeamBySlugOrID finds a team by slug. If the slug looks like a numeric ID,
-// it will search for a team with that ID. Otherwise, it uses GetTeam to fetch directly by slug.
-func findEnterpriseTeamBySlugOrID(ctx context.Context, client *github.Client, enterpriseSlug, teamSlugOrID string) (*github.EnterpriseTeam, error) {
-	// First, try to get the team directly by slug
-	team, resp, err := client.Enterprise.GetTeam(ctx, enterpriseSlug, teamSlugOrID)
-	if err == nil {
-		return team, nil
-	}
-	// If we got a 404, try searching by ID in case it's a numeric ID
-	if resp != nil && resp.StatusCode == 404 {
-		// Try to parse as int64 and search by ID
-		var id int64
-		if _, scanErr := fmt.Sscanf(teamSlugOrID, "%d", &id); scanErr == nil {
-			return findEnterpriseTeamByID(ctx, client, enterpriseSlug, id)
-		}
-	}
-	return nil, err
-}

github/data_source_github_enterprise_team_membership.go

@@ -21,10 +21,10 @@ func dataSourceGithubEnterpriseTeamMembership() *schema.Resource {
 				Description:      "The slug of the enterprise.",
 				ValidateDiagFunc: validation.ToDiagFunc(validation.All(validation.StringIsNotWhiteSpace, validation.StringIsNotEmpty)),
 			},
-			"enterprise_team": {
+			"team_slug": {
 				Type:             schema.TypeString,
 				Required:         true,
-				Description:      "The slug or ID of the enterprise team.",
+				Description:      "The slug of the enterprise team.",
 				ValidateDiagFunc: validation.ToDiagFunc(validation.All(validation.StringIsNotWhiteSpace, validation.StringIsNotEmpty)),
 			},
 			"username": {
@@ -45,20 +45,20 @@ func dataSourceGithubEnterpriseTeamMembership() *schema.Resource {
 func dataSourceGithubEnterpriseTeamMembershipRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*Owner).v3client
 	enterpriseSlug := strings.TrimSpace(d.Get("enterprise_slug").(string))
-	enterpriseTeam := strings.TrimSpace(d.Get("enterprise_team").(string))
+	teamSlug := strings.TrimSpace(d.Get("team_slug").(string))
 	username := strings.TrimSpace(d.Get("username").(string))
 
 	// Get the membership using the SDK
-	user, _, err := client.Enterprise.GetTeamMembership(ctx, enterpriseSlug, enterpriseTeam, username)
+	user, _, err := client.Enterprise.GetTeamMembership(ctx, enterpriseSlug, teamSlug, username)
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
-	d.SetId(buildThreePartID(enterpriseSlug, enterpriseTeam, username))
+	d.SetId(buildEnterpriseTeamMembershipID(enterpriseSlug, teamSlug, username))
 	if err := d.Set("enterprise_slug", enterpriseSlug); err != nil {
 		return diag.FromErr(err)
 	}
-	if err := d.Set("enterprise_team", enterpriseTeam); err != nil {
+	if err := d.Set("team_slug", teamSlug); err != nil {
 		return diag.FromErr(err)
 	}
 	if err := d.Set("username", username); err != nil {

github/data_source_github_enterprise_team_organizations.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"strings"
 
-	"github.com/google/go-github/v81/github"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
@@ -22,10 +21,10 @@ func dataSourceGithubEnterpriseTeamOrganizations() *schema.Resource {
 				Description:      "The slug of the enterprise.",
 				ValidateDiagFunc: validation.ToDiagFunc(validation.All(validation.StringIsNotWhiteSpace, validation.StringIsNotEmpty)),
 			},
-			"enterprise_team": {
+			"team_slug": {
 				Type:             schema.TypeString,
 				Required:         true,
-				Description:      "The slug or ID of the enterprise team.",
+				Description:      "The slug of the enterprise team.",
 				ValidateDiagFunc: validation.ToDiagFunc(validation.All(validation.StringIsNotWhiteSpace, validation.StringIsNotEmpty)),
 			},
 			"organization_slugs": {
@@ -42,8 +41,8 @@ func dataSourceGithubEnterpriseTeamOrganizations() *schema.Resource {
 func dataSourceGithubEnterpriseTeamOrganizationsRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*Owner).v3client
 	enterpriseSlug := strings.TrimSpace(d.Get("enterprise_slug").(string))
-	enterpriseTeam := strings.TrimSpace(d.Get("enterprise_team").(string))
-	orgs, err := listAllEnterpriseTeamOrganizations(ctx, client, enterpriseSlug, enterpriseTeam)
+	teamSlug := strings.TrimSpace(d.Get("team_slug").(string))
+	orgs, err := listAllEnterpriseTeamOrganizations(ctx, client, enterpriseSlug, teamSlug)
 	if err != nil {
 		return diag.FromErr(err)
 	}
@@ -55,35 +54,15 @@ func dataSourceGithubEnterpriseTeamOrganizationsRead(ctx context.Context, d *sch
 		}
 	}
 
-	d.SetId(buildTwoPartID(enterpriseSlug, enterpriseTeam))
+	d.SetId(buildEnterpriseTeamOrganizationsID(enterpriseSlug, teamSlug))
 	if err := d.Set("enterprise_slug", enterpriseSlug); err != nil {
 		return diag.FromErr(err)
 	}
-	if err := d.Set("enterprise_team", enterpriseTeam); err != nil {
+	if err := d.Set("team_slug", teamSlug); err != nil {
 		return diag.FromErr(err)
 	}
 	if err := d.Set("organization_slugs", slugs); err != nil {
 		return diag.FromErr(err)
 	}
 	return nil
 }
-
-// listAllEnterpriseTeamOrganizations returns all organizations assigned to an enterprise team with pagination handled.
-func listAllEnterpriseTeamOrganizations(ctx context.Context, client *github.Client, enterpriseSlug, enterpriseTeam string) ([]*github.Organization, error) {
-	var all []*github.Organization
-	opt := &github.ListOptions{PerPage: maxPerPage}
-
-	for {
-		orgs, resp, err := client.Enterprise.ListAssignments(ctx, enterpriseSlug, enterpriseTeam, opt)
-		if err != nil {
-			return nil, err
-		}
-		all = append(all, orgs...)
-		if resp.NextPage == 0 {
-			break
-		}
-		opt.Page = resp.NextPage
-	}
-
-	return all, nil
-}

github/data_source_github_enterprise_team_test.go

@@ -33,7 +33,7 @@ func TestAccGithubEnterpriseTeamDataSource(t *testing.T) {
 	`, testAccConf.enterpriseSlug, randomID)
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:  func() { skipUnlessMode(t, enterprise) },
+		PreCheck:          func() { skipUnlessMode(t, enterprise) },
 		ProviderFactories: providerFactories,
 		Steps: []resource.TestStep{
 			{
@@ -68,19 +68,19 @@ func TestAccGithubEnterpriseTeamOrganizationsDataSource(t *testing.T) {
 
 		resource "github_enterprise_team_organizations" "assign" {
 			enterprise_slug    = data.github_enterprise.enterprise.slug
-			enterprise_team    = github_enterprise_team.test.slug
+			team_slug          = github_enterprise_team.test.slug
 			organization_slugs = ["%s"]
 		}
 
 		data "github_enterprise_team_organizations" "test" {
 			enterprise_slug = data.github_enterprise.enterprise.slug
-			enterprise_team = github_enterprise_team.test.slug
+			team_slug       = github_enterprise_team.test.slug
 			depends_on      = [github_enterprise_team_organizations.assign]
 		}
 	`, testAccConf.enterpriseSlug, testResourcePrefix, randomID, testAccConf.owner)
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:  func() { skipUnlessMode(t, enterprise) },
+		PreCheck:          func() { skipUnlessMode(t, enterprise) },
 		ProviderFactories: providerFactories,
 		Steps: []resource.TestStep{
 			{
@@ -111,20 +111,20 @@ func TestAccGithubEnterpriseTeamMembershipDataSource(t *testing.T) {
 
 		resource "github_enterprise_team_membership" "test" {
 			enterprise_slug = data.github_enterprise.enterprise.slug
-			enterprise_team = github_enterprise_team.test.slug
+			team_slug       = github_enterprise_team.test.slug
 			username        = "%s"
 		}
 
 		data "github_enterprise_team_membership" "test" {
 			enterprise_slug = data.github_enterprise.enterprise.slug
-			enterprise_team = github_enterprise_team.test.slug
+			team_slug       = github_enterprise_team.test.slug
 			username        = "%s"
 			depends_on      = [github_enterprise_team_membership.test]
 		}
 	`, testAccConf.enterpriseSlug, testResourcePrefix, randomID, username, username)
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:  func() { skipUnlessMode(t, enterprise) },
+		PreCheck:          func() { skipUnlessMode(t, enterprise) },
 		ProviderFactories: providerFactories,
 		Steps: []resource.TestStep{
 			{

github/data_source_github_enterprise_teams_test.go

@@ -28,7 +28,7 @@ func TestAccGithubEnterpriseTeamsDataSource(t *testing.T) {
 	`, testAccConf.enterpriseSlug, testResourcePrefix, randomID)
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:  func() { skipUnlessMode(t, enterprise) },
+		PreCheck:          func() { skipUnlessMode(t, enterprise) },
 		ProviderFactories: providerFactories,
 		Steps: []resource.TestStep{
 			{

github/resource_github_enterprise_team.go

@@ -92,9 +92,7 @@ func resourceGithubEnterpriseTeamCreate(ctx context.Context, d *schema.ResourceD
 		Name:                      name,
 		Description:               github.Ptr(description),
 		OrganizationSelectionType: github.Ptr(orgSelection),
-	}
-	if groupID != "" {
-		req.GroupID = github.Ptr(groupID)
+		GroupID:                   github.Ptr(groupID), // Empty string is valid for no group
 	}
 
 	ctx = context.WithValue(ctx, ctxId, d.Id())
@@ -104,7 +102,16 @@ func resourceGithubEnterpriseTeamCreate(ctx context.Context, d *schema.ResourceD
 	}
 
 	d.SetId(strconv.FormatInt(te.ID, 10))
-	return resourceGithubEnterpriseTeamRead(context.WithValue(ctx, ctxId, d.Id()), d, meta)
+
+	// Set computed fields directly from API response
+	if err := d.Set("slug", te.Slug); err != nil {
+		return diag.FromErr(err)
+	}
+	if err := d.Set("team_id", int(te.ID)); err != nil {
+		return diag.FromErr(err)
+	}
+
+	return nil
 }
 
 func resourceGithubEnterpriseTeamRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
@@ -194,24 +201,7 @@ func resourceGithubEnterpriseTeamRead(ctx context.Context, d *schema.ResourceDat
 func resourceGithubEnterpriseTeamUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*Owner).v3client
 	enterpriseSlug := d.Get("enterprise_slug").(string)
-
-	// We need a team slug for the API. If state is missing, re-discover it by ID.
-	teamSlug := strings.TrimSpace(d.Get("slug").(string))
-	if teamSlug == "" {
-		teamID, err := strconv.ParseInt(d.Id(), 10, 64)
-		if err != nil {
-			return diag.FromErr(unconvertibleIdErr(d.Id(), err))
-		}
-		ctx = context.WithValue(ctx, ctxId, d.Id())
-		te, err := findEnterpriseTeamByID(ctx, client, enterpriseSlug, teamID)
-		if err != nil {
-			return diag.FromErr(err)
-		}
-		if te == nil {
-			return diag.FromErr(fmt.Errorf("enterprise team %s no longer exists", d.Id()))
-		}
-		teamSlug = te.Slug
-	}
+	teamSlug := d.Get("slug").(string)
 
 	name := d.Get("name").(string)
 	description := d.Get("description").(string)
@@ -222,18 +212,21 @@ func resourceGithubEnterpriseTeamUpdate(ctx context.Context, d *schema.ResourceD
 		Name:                      name,
 		Description:               github.Ptr(description),
 		OrganizationSelectionType: github.Ptr(orgSelection),
-	}
-	if groupID != "" {
-		req.GroupID = github.Ptr(groupID)
+		GroupID:                   github.Ptr(groupID), // Empty string clears the group
 	}
 
 	ctx = context.WithValue(ctx, ctxId, d.Id())
-	_, _, err := client.Enterprise.UpdateTeam(ctx, enterpriseSlug, teamSlug, req)
+	te, _, err := client.Enterprise.UpdateTeam(ctx, enterpriseSlug, teamSlug, req)
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
-	return resourceGithubEnterpriseTeamRead(ctx, d, meta)
+	// Update slug in case it changed (e.g., team was renamed)
+	if err := d.Set("slug", te.Slug); err != nil {
+		return diag.FromErr(err)
+	}
+
+	return nil
 }
 
 func resourceGithubEnterpriseTeamDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
@@ -258,14 +251,13 @@ func resourceGithubEnterpriseTeamDelete(ctx context.Context, d *schema.ResourceD
 	}
 
 	log.Printf("[INFO] Deleting enterprise team: %s/%s (%s)", enterpriseSlug, teamSlug, d.Id())
-	resp, err := client.Enterprise.DeleteTeam(ctx, enterpriseSlug, teamSlug)
+	_, err := client.Enterprise.DeleteTeam(ctx, enterpriseSlug, teamSlug)
 	if err != nil {
 		// Already gone? That's fine, we wanted it deleted anyway.
 		ghErr := &github.ErrorResponse{}
 		if errors.As(err, &ghErr) && ghErr.Response.StatusCode == http.StatusNotFound {
 			return nil
 		}
-		_ = resp
 		return diag.FromErr(err)
 	}
 
@@ -281,6 +273,8 @@ func resourceGithubEnterpriseTeamImport(_ context.Context, d *schema.ResourceDat
 
 	enterpriseSlug, teamID := parts[0], parts[1]
 	d.SetId(teamID)
-	_ = d.Set("enterprise_slug", enterpriseSlug)
+	if err := d.Set("enterprise_slug", enterpriseSlug); err != nil {
+		return nil, err
+	}
 	return []*schema.ResourceData{d}, nil
 }