Skip to content

[BUG] Enable setting review notifications without delegation #3220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
deiga wants to merge 13 commits into
base: main
Choose a base branch
from
Open

[BUG] Enable setting review notifications without delegation #3220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
72c3154
Update docs
deiga Feb 22, 2026
fd33657
Update tests and add missing coverage
deiga Feb 23, 2026
e9fd5be
Add more tests to cover `Import` and a few more `Update` cases
deiga Feb 23, 2026
f64a3fb
Refactor `Import` to be Context-aware
deiga Feb 23, 2026
d961c7c
Refactor to use Context-aware functions
deiga Feb 23, 2026
ed80f2b
Use `resource.ParallelTest` to make testing quicker
deiga Feb 23, 2026
6db1fc2
Remove unused call to `Read`
deiga Feb 23, 2026
5a70547
Uncouple `Create` and `Update`
deiga Feb 23, 2026
fdf3811
Fix resource to be able to set empty `review_request_delegation` block
deiga Feb 23, 2026
2376026
Add top-level `notify` to enable setting it without `review_request_d…
deiga Feb 23, 2026
a9e42e1
Update docs
deiga Feb 23, 2026
dc6f431
Consolidate `getTeamSlugContext` and `resolveTeamIDs` to use shared l…
deiga Feb 24, 2026
28b45a8
Remove named returns
deiga Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
282 changes: 193 additions & 89 deletions github/resource_github_team_settings.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,22 @@ package github

import (
"context"
"errors"
"strconv"

"github.com/hashicorp/terraform-plugin-log/tflog"
"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"
"github.com/shurcooL/githubv4"
)

func resourceGithubTeamSettings() *schema.Resource {
return &schema.Resource{
Create: resourceGithubTeamSettingsCreate,
Read: resourceGithubTeamSettingsRead,
Update: resourceGithubTeamSettingsUpdate,
Delete: resourceGithubTeamSettingsDelete,
CreateContext: resourceGithubTeamSettingsCreate,
ReadContext: resourceGithubTeamSettingsRead,
UpdateContext: resourceGithubTeamSettingsUpdate,
DeleteContext: resourceGithubTeamSettingsDelete,
Importer: &schema.ResourceImporter{
State: resourceGithubTeamSettingsImport,
StateContext: resourceGithubTeamSettingsImport,
},
Schema: map[string]*schema.Schema{
"team_id": {
Expand All @@ -36,6 +36,13 @@ func resourceGithubTeamSettings() *schema.Resource {
Computed: true,
Description: "The unique ID of the Team on GitHub. Corresponds to the ID of the 'github_team_settings' resource.",
},
"notify": {
Type: schema.TypeBool,
Optional: true,
Default: false,
Description: "Whether to notify the entire team when at least one member is also assigned to the pull request.",
ConflictsWith: []string{"review_request_delegation.0.notify"},
},
"review_request_delegation": {
Type: schema.TypeList,
MaxItems: 1,
Expand All @@ -51,19 +58,21 @@ func resourceGithubTeamSettings() *schema.Resource {
ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{string(githubv4.TeamReviewAssignmentAlgorithmRoundRobin), string(githubv4.TeamReviewAssignmentAlgorithmLoadBalance)}, false)),
},
"member_count": {
Type: schema.TypeInt,
Optional: true,
RequiredWith: []string{"review_request_delegation"},
Description: "The number of team members to assign to a pull request.",
Type: schema.TypeInt,
Optional: true,
Default: 1,
Description: "The number of team members to assign to a pull request.",
ValidateDiagFunc: validation.ToDiagFunc(validation.All(
validation.IntAtLeast(1),
)),
},
"notify": {
Type: schema.TypeBool,
Optional: true,
Default: false,
Description: "whether to notify the entire team when at least one member is also assigned to the pull request.",
Type: schema.TypeBool,
Optional: true,
Default: false,
Description: "whether to notify the entire team when at least one member is also assigned to the pull request.",
Deprecated: "Use the top-level notify attribute instead.",
ConflictsWith: []string{"notify"},
},
},
},
Expand All @@ -72,36 +81,96 @@ func resourceGithubTeamSettings() *schema.Resource {
}
}

func resourceGithubTeamSettingsCreate(d *schema.ResourceData, meta any) error {
err := checkOrganization(meta)
if err != nil {
return err
func resourceGithubTeamSettingsCreate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics {
meta := m.(*Owner)
if err := checkOrganization(m); err != nil {
return diag.FromErr(err)
}
graphql := meta.v4client

teamIDString := d.Get("team_id").(string)

tflog.Debug(ctx, "Resolving team_id to Team node_id and slug", map[string]any{
"team_id": teamIDString,
})
// Given a string that is either a team id or team slug, return the
// get the basic details of the team including node_id and slug
ctx := context.Background()

teamIDString, _ := d.Get("team_id").(string)

nodeId, slug, err := resolveTeamIDs(teamIDString, meta.(*Owner), ctx)
nodeId, slug, err := resolveTeamIDs(ctx, meta, teamIDString)
if err != nil {
return err
return diag.FromErr(err)
}
tflog.Trace(ctx, "Resolved team_id to Team node_id and slug", map[string]any{
"node_id": nodeId,
"slug": slug,
})
d.SetId(nodeId)
if err = d.Set("team_slug", slug); err != nil {
return err
return diag.FromErr(err)
}
if err = d.Set("team_uid", nodeId); err != nil {
return err
return diag.FromErr(err)
}

reviewRequestDelegation := d.Get("review_request_delegation").([]any)

var mutation struct {
UpdateTeamReviewAssignment struct {
ClientMutationId githubv4.ID `graphql:"clientMutationId"`
} `graphql:"updateTeamReviewAssignment(input:$input)"`
}
return resourceGithubTeamSettingsUpdate(d, meta)

tflog.Debug(ctx, "Review request delegation settings", map[string]any{
"team_id": d.Id(),
"team_slug": slug,
"review_request_delegation": reviewRequestDelegation,
"length_of_settings": len(reviewRequestDelegation),
})

notify := resolveNotify(ctx, d)

if len(reviewRequestDelegation) == 0 {
tflog.Debug(ctx, "No review request delegation settings provided, disabling review request delegation", map[string]any{
"team_id": d.Id(),
"team_slug": slug,
"notify": notify,
})

input := defaultTeamReviewAssignmentSettings(d.Id())
input.NotifyTeam = new(githubv4.Boolean(notify))

err := graphql.Mutate(ctx, &mutation, input, nil)
if err != nil {
return diag.FromErr(err)
}
} else {
tflog.Debug(ctx, "Review request delegation settings provided, setting according to provided configuration", map[string]any{
"team_id": d.Id(),
"team_slug": slug,
"review_request_delegation": reviewRequestDelegation,
"notify": notify,
})
settings := reviewRequestDelegation[0].(map[string]any)

teamReviewAlgorithm := githubv4.TeamReviewAssignmentAlgorithm(settings["algorithm"].(string))
updateTeamReviewAssignmentInput := githubv4.UpdateTeamReviewAssignmentInput{
ID: d.Id(),
Enabled: githubv4.Boolean(true),
Algorithm: &teamReviewAlgorithm,
TeamMemberCount: new(githubv4.Int(settings["member_count"].(int))),
NotifyTeam: new(githubv4.Boolean(notify)),
}

err := graphql.Mutate(ctx, &mutation, updateTeamReviewAssignmentInput, nil)
if err != nil {
return diag.FromErr(err)
}
}
return nil
}

func resourceGithubTeamSettingsRead(d *schema.ResourceData, meta any) error {
err := checkOrganization(meta)
if err != nil {
return err
func resourceGithubTeamSettingsRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
if err := checkOrganization(meta); err != nil {
return diag.FromErr(err)
}

graphql := meta.(*Owner).v4client
Expand All @@ -115,66 +184,95 @@ func resourceGithubTeamSettingsRead(d *schema.ResourceData, meta any) error {
"login": githubv4.String(orgName),
}

e := graphql.Query(meta.(*Owner).StopContext, &query, variables)
if e != nil {
return e
err := graphql.Query(ctx, &query, variables)
if err != nil {
return diag.FromErr(err)
}

notifyValue := query.Organization.Team.ReviewRequestDelegationNotifyAll

// Set notify in the location matching the user's config: top-level or
// deprecated nested field inside review_request_delegation.
_, usesDeprecatedNotify := d.GetOk("review_request_delegation.0.notify")
tflog.Debug(ctx, "Uses deprecated notify", map[string]any{
"uses_deprecated_notify": usesDeprecatedNotify,
"notify_value": notifyValue,
})

if !usesDeprecatedNotify {
if err = d.Set("notify", notifyValue); err != nil {
return diag.FromErr(err)
}
}

if query.Organization.Team.ReviewRequestDelegation {
reviewRequestDelegation := make(map[string]any)
reviewRequestDelegation["algorithm"] = query.Organization.Team.ReviewRequestDelegationAlgorithm
reviewRequestDelegation["member_count"] = query.Organization.Team.ReviewRequestDelegationCount
reviewRequestDelegation["notify"] = query.Organization.Team.ReviewRequestDelegationNotifyAll
if usesDeprecatedNotify {
reviewRequestDelegation["notify"] = notifyValue
}
if err = d.Set("review_request_delegation", []any{reviewRequestDelegation}); err != nil {
return err
return diag.FromErr(err)
}
} else {
if err = d.Set("review_request_delegation", []any{}); err != nil {
return err
return diag.FromErr(err)
}
}

return nil
}

func resourceGithubTeamSettingsUpdate(d *schema.ResourceData, meta any) error {
if d.HasChange("review_request_delegation") || d.IsNewResource() {
func resourceGithubTeamSettingsUpdate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics {
if d.HasChange("review_request_delegation") || d.HasChange("notify") {
meta := m.(*Owner)
graphql := meta.v4client
reviewRequestDelegation := d.Get("review_request_delegation").([]any)
notify := resolveNotify(ctx, d)

ctx := context.WithValue(context.Background(), ctxId, d.Id())
graphql := meta.(*Owner).v4client
if setting := d.Get("review_request_delegation").([]any); len(setting) == 0 {
var mutation struct {
UpdateTeamReviewAssignment struct {
ClientMutationId githubv4.ID `graphql:"clientMutationId"`
} `graphql:"updateTeamReviewAssignment(input:$input)"`
}
var mutation struct {
UpdateTeamReviewAssignment struct {
ClientMutationId githubv4.ID `graphql:"clientMutationId"`
} `graphql:"updateTeamReviewAssignment(input:$input)"`
}

return graphql.Mutate(ctx, &mutation, defaultTeamReviewAssignmentSettings(d.Id()), nil)
} else {
settings := d.Get("review_request_delegation").([]any)[0].(map[string]any)
if len(reviewRequestDelegation) == 0 {
tflog.Debug(ctx, "No review request delegation settings provided, disabling review request delegation", map[string]any{
"team_id": d.Id(),
"team_slug": d.Get("team_slug").(string),
"notify": notify,
})

input := defaultTeamReviewAssignmentSettings(d.Id())
input.NotifyTeam = new(githubv4.Boolean(notify))

var mutation struct {
UpdateTeamReviewAssignment struct {
ClientMutationId githubv4.ID `graphql:"clientMutationId"`
} `graphql:"updateTeamReviewAssignment(input:$input)"`
err := graphql.Mutate(ctx, &mutation, input, nil)
if err != nil {
return diag.FromErr(err)
}
} else {
settings := reviewRequestDelegation[0].(map[string]any)

teamReviewAlgorithm := githubv4.TeamReviewAssignmentAlgorithm(settings["algorithm"].(string))
return graphql.Mutate(ctx, &mutation, githubv4.UpdateTeamReviewAssignmentInput{
updateTeamReviewAssignmentInput := githubv4.UpdateTeamReviewAssignmentInput{
ID: d.Id(),
Enabled: githubv4.Boolean(true),
Algorithm: &teamReviewAlgorithm,
TeamMemberCount: new(githubv4.Int(settings["member_count"].(int))),
NotifyTeam: new(githubv4.Boolean(settings["notify"].(bool))),
}, nil)
}
err := graphql.Mutate(ctx, &mutation, updateTeamReviewAssignmentInput, nil)
if err != nil {
return diag.FromErr(err)
}
}
}

return resourceGithubTeamSettingsRead(d, meta)
return nil
}

func resourceGithubTeamSettingsDelete(d *schema.ResourceData, meta any) error {
ctx := context.WithValue(context.Background(), ctxId, d.Id())
func resourceGithubTeamSettingsDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
graphql := meta.(*Owner).v4client

var mutation struct {
Expand All @@ -183,11 +281,15 @@ func resourceGithubTeamSettingsDelete(d *schema.ResourceData, meta any) error {
} `graphql:"updateTeamReviewAssignment(input:$input)"`
}

return graphql.Mutate(ctx, &mutation, defaultTeamReviewAssignmentSettings(d.Id()), nil)
err := graphql.Mutate(ctx, &mutation, defaultTeamReviewAssignmentSettings(d.Id()), nil)
if err != nil {
return diag.FromErr(err)
}
return nil
}

func resourceGithubTeamSettingsImport(d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
nodeId, slug, err := resolveTeamIDs(d.Id(), meta.(*Owner), context.Background())
func resourceGithubTeamSettingsImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
nodeId, slug, err := resolveTeamIDs(ctx, meta.(*Owner), d.Id())
if err != nil {
return nil, err
}
Expand All @@ -201,37 +303,39 @@ func resourceGithubTeamSettingsImport(d *schema.ResourceData, meta any) ([]*sche
if err = d.Set("team_uid", nodeId); err != nil {
return nil, err
}
return []*schema.ResourceData{d}, resourceGithubTeamSettingsRead(d, meta)
return []*schema.ResourceData{d}, nil
}

func resolveTeamIDs(idOrSlug string, meta *Owner, ctx context.Context) (nodeId, slug string, err error) {
client := meta.v3client
orgName := meta.name
orgId := meta.id

teamId, parseIntErr := strconv.ParseInt(idOrSlug, 10, 64)
if parseIntErr != nil {
// The given id not an integer, assume it is a team slug
team, _, slugErr := client.Teams.GetTeamBySlug(ctx, orgName, idOrSlug)
if slugErr != nil {
return "", "", errors.New(parseIntErr.Error() + slugErr.Error())
}
return team.GetNodeID(), team.GetSlug(), nil
} else {
// The given id is an integer, assume it is a team id
team, _, teamIdErr := client.Teams.GetTeamByID(ctx, orgId, teamId)
if teamIdErr != nil {
// There isn't a team with the given ID, assume it is a teamslug
team, _, slugErr := client.Teams.GetTeamBySlug(ctx, orgName, idOrSlug)
if slugErr != nil {
return "", "", errors.New(teamIdErr.Error() + slugErr.Error())
}
func resolveTeamIDs(ctx context.Context, meta *Owner, idOrSlug string) (string, string, error) {
team, err := getTeam(ctx, meta, idOrSlug)
if err != nil {
return "", "", err
}
return team.GetNodeID(), team.GetSlug(), nil
}

return team.GetNodeID(), team.GetSlug(), nil
}
// resolveNotify returns the notify value from the top-level attribute or the
// deprecated nested attribute inside review_request_delegation. The top-level
// attribute takes precedence. Since ConflictsWith prevents both from being set,
// only one source can be active at a time.
func resolveNotify(ctx context.Context, d *schema.ResourceData) bool {
// Check if top-level notify is explicitly configured.
if v, ok := d.GetOk("notify"); ok {
Copy link
Copy Markdown
Collaborator

@stevehipwell stevehipwell Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure there is any value to using GetOk() here given the way it works and the fact that you're using an explicit schema?

Copy link
Copy Markdown
Collaborator Author

@deiga deiga Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way I understand the code is that this is exactly where GetOk() should work for TypeBool.
It should check if notify has been set in the config, right? Of course the tests don't have that scenario at all 🙃

Copy link
Copy Markdown
Collaborator Author

@deiga deiga Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or how else would I be checking which one of the fields is actually set? Should I just read both and see if either is true? That sounds simpler 🤔

tflog.Debug(ctx, "Top-level notify is explicitly configured", map[string]any{
"notify": v,
})
return v.(bool)
}

return team.GetNodeID(), team.GetSlug(), nil
// Fall back to deprecated nested field
if v, ok := d.GetOk("review_request_delegation.0.notify"); ok {
tflog.Debug(ctx, "Deprecated nested notify is explicitly configured", map[string]any{
"notify": v,
})
return v.(bool)
}

return false
}

func defaultTeamReviewAssignmentSettings(id string) githubv4.UpdateTeamReviewAssignmentInput {
Expand Down
Loading