Skip to content

Commit 4ea56b8

Browse files
deigadeiga
authored
[BUG] Enable setting review notifications without delegation (#3220)
* Update docs Signed-off-by: Timo Sand <[email protected]> * Update tests and add missing coverage Signed-off-by: Timo Sand <[email protected]> * Add more tests to cover `Import` and a few more `Update` cases Signed-off-by: Timo Sand <[email protected]> * Refactor `Import` to be Context-aware Signed-off-by: Timo Sand <[email protected]> * Refactor to use Context-aware functions Signed-off-by: Timo Sand <[email protected]> * Use `resource.ParallelTest` to make testing quicker Signed-off-by: Timo Sand <[email protected]> * Remove unused call to `Read` Signed-off-by: Timo Sand <[email protected]> * Uncouple `Create` and `Update` Signed-off-by: Timo Sand <[email protected]> * Fix resource to be able to set empty `review_request_delegation` block Signed-off-by: Timo Sand <[email protected]> * Add top-level `notify` to enable setting it without `review_request_delegation` Mark nested `notify` as deprecated Signed-off-by: Timo Sand <[email protected]> * Update docs Signed-off-by: Timo Sand <[email protected]> * Consolidate `getTeamSlugContext` and `resolveTeamIDs` to use shared logic with `getTeam` Signed-off-by: Timo Sand <[email protected]> * Remove named returns * Fix missing reference Signed-off-by: Timo Sand <[email protected]> * Simplify `resolveNotify` Signed-off-by: Timo Sand <[email protected]> --------- Signed-off-by: Timo Sand <[email protected]>
1 parent e72c809 commit 4ea56b8

4 files changed

Lines changed: 865 additions & 126 deletions

File tree

‎github/resource_github_team_settings.go‎

Lines changed: 194 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,22 @@ package github
22

33
import (
44
"context"
5-
"errors"
6-
"strconv"
75

6+
"github.com/hashicorp/terraform-plugin-log/tflog"
7+
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
88
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
99
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
1010
"github.com/shurcooL/githubv4"
1111
)
1212

1313
func resourceGithubTeamSettings() *schema.Resource {
1414
return &schema.Resource{
15-
Create: resourceGithubTeamSettingsCreate,
16-
Read: resourceGithubTeamSettingsRead,
17-
Update: resourceGithubTeamSettingsUpdate,
18-
Delete: resourceGithubTeamSettingsDelete,
15+
CreateContext: resourceGithubTeamSettingsCreate,
16+
ReadContext: resourceGithubTeamSettingsRead,
17+
UpdateContext: resourceGithubTeamSettingsUpdate,
18+
DeleteContext: resourceGithubTeamSettingsDelete,
1919
Importer: &schema.ResourceImporter{
20-
State: resourceGithubTeamSettingsImport,
20+
StateContext: resourceGithubTeamSettingsImport,
2121
},
2222
Schema: map[string]*schema.Schema{
2323
"team_id": {
@@ -36,6 +36,13 @@ func resourceGithubTeamSettings() *schema.Resource {
3636
Computed: true,
3737
Description: "The unique ID of the Team on GitHub. Corresponds to the ID of the 'github_team_settings' resource.",
3838
},
39+
"notify": {
40+
Type: schema.TypeBool,
41+
Optional: true,
42+
Default: false,
43+
Description: "Whether to notify the entire team when at least one member is also assigned to the pull request.",
44+
ConflictsWith: []string{"review_request_delegation.0.notify"},
45+
},
3946
"review_request_delegation": {
4047
Type: schema.TypeList,
4148
MaxItems: 1,
@@ -51,19 +58,21 @@ func resourceGithubTeamSettings() *schema.Resource {
5158
ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{string(githubv4.TeamReviewAssignmentAlgorithmRoundRobin), string(githubv4.TeamReviewAssignmentAlgorithmLoadBalance)}, false)),
5259
},
5360
"member_count": {
54-
Type: schema.TypeInt,
55-
Optional: true,
56-
RequiredWith: []string{"review_request_delegation"},
57-
Description: "The number of team members to assign to a pull request.",
61+
Type: schema.TypeInt,
62+
Optional: true,
63+
Default: 1,
64+
Description: "The number of team members to assign to a pull request.",
5865
ValidateDiagFunc: validation.ToDiagFunc(validation.All(
5966
validation.IntAtLeast(1),
6067
)),
6168
},
6269
"notify": {
63-
Type: schema.TypeBool,
64-
Optional: true,
65-
Default: false,
66-
Description: "whether to notify the entire team when at least one member is also assigned to the pull request.",
70+
Type: schema.TypeBool,
71+
Optional: true,
72+
Default: false,
73+
Description: "whether to notify the entire team when at least one member is also assigned to the pull request.",
74+
Deprecated: "Use the top-level notify attribute instead.",
75+
ConflictsWith: []string{"notify"},
6776
},
6877
},
6978
},
@@ -72,36 +81,96 @@ func resourceGithubTeamSettings() *schema.Resource {
7281
}
7382
}
7483

75-
func resourceGithubTeamSettingsCreate(d *schema.ResourceData, meta any) error {
76-
err := checkOrganization(meta)
77-
if err != nil {
78-
return err
84+
func resourceGithubTeamSettingsCreate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics {
85+
meta := m.(*Owner)
86+
if err := checkOrganization(m); err != nil {
87+
return diag.FromErr(err)
7988
}
89+
graphql := meta.v4client
8090

91+
teamIDString := d.Get("team_id").(string)
92+
93+
tflog.Debug(ctx, "Resolving team_id to Team node_id and slug", map[string]any{
94+
"team_id": teamIDString,
95+
})
8196
// Given a string that is either a team id or team slug, return the
8297
// get the basic details of the team including node_id and slug
83-
ctx := context.Background()
84-
85-
teamIDString, _ := d.Get("team_id").(string)
86-
87-
nodeId, slug, err := resolveTeamIDs(teamIDString, meta.(*Owner), ctx)
98+
nodeId, slug, err := resolveTeamIDs(ctx, meta, teamIDString)
8899
if err != nil {
89-
return err
100+
return diag.FromErr(err)
90101
}
102+
tflog.Trace(ctx, "Resolved team_id to Team node_id and slug", map[string]any{
103+
"node_id": nodeId,
104+
"slug": slug,
105+
})
91106
d.SetId(nodeId)
92107
if err = d.Set("team_slug", slug); err != nil {
93-
return err
108+
return diag.FromErr(err)
94109
}
95110
if err = d.Set("team_uid", nodeId); err != nil {
96-
return err
111+
return diag.FromErr(err)
112+
}
113+
114+
reviewRequestDelegation := d.Get("review_request_delegation").([]any)
115+
116+
var mutation struct {
117+
UpdateTeamReviewAssignment struct {
118+
ClientMutationId githubv4.ID `graphql:"clientMutationId"`
119+
} `graphql:"updateTeamReviewAssignment(input:$input)"`
97120
}
98-
return resourceGithubTeamSettingsUpdate(d, meta)
121+
122+
tflog.Debug(ctx, "Review request delegation settings", map[string]any{
123+
"team_id": d.Id(),
124+
"team_slug": slug,
125+
"review_request_delegation": reviewRequestDelegation,
126+
"length_of_settings": len(reviewRequestDelegation),
127+
})
128+
129+
notify := resolveNotify(ctx, d)
130+
131+
if len(reviewRequestDelegation) == 0 {
132+
tflog.Debug(ctx, "No review request delegation settings provided, disabling review request delegation", map[string]any{
133+
"team_id": d.Id(),
134+
"team_slug": slug,
135+
"notify": notify,
136+
})
137+
138+
input := defaultTeamReviewAssignmentSettings(d.Id())
139+
input.NotifyTeam = new(githubv4.Boolean(notify))
140+
141+
err := graphql.Mutate(ctx, &mutation, input, nil)
142+
if err != nil {
143+
return diag.FromErr(err)
144+
}
145+
} else {
146+
tflog.Debug(ctx, "Review request delegation settings provided, setting according to provided configuration", map[string]any{
147+
"team_id": d.Id(),
148+
"team_slug": slug,
149+
"review_request_delegation": reviewRequestDelegation,
150+
"notify": notify,
151+
})
152+
settings := reviewRequestDelegation[0].(map[string]any)
153+
154+
teamReviewAlgorithm := githubv4.TeamReviewAssignmentAlgorithm(settings["algorithm"].(string))
155+
updateTeamReviewAssignmentInput := githubv4.UpdateTeamReviewAssignmentInput{
156+
ID: d.Id(),
157+
Enabled: githubv4.Boolean(true),
158+
Algorithm: &teamReviewAlgorithm,
159+
TeamMemberCount: new(githubv4.Int(settings["member_count"].(int))),
160+
NotifyTeam: new(githubv4.Boolean(notify)),
161+
}
162+
163+
err := graphql.Mutate(ctx, &mutation, updateTeamReviewAssignmentInput, nil)
164+
if err != nil {
165+
return diag.FromErr(err)
166+
}
167+
}
168+
return nil
99169
}
100170

101-
func resourceGithubTeamSettingsRead(d *schema.ResourceData, meta any) error {
102-
err := checkOrganization(meta)
103-
if err != nil {
104-
return err
171+
func resourceGithubTeamSettingsRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
172+
if err := checkOrganization(meta); err != nil {
173+
return diag.FromErr(err)
105174
}
106175

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

118-
e := graphql.Query(meta.(*Owner).StopContext, &query, variables)
119-
if e != nil {
120-
return e
187+
err := graphql.Query(ctx, &query, variables)
188+
if err != nil {
189+
return diag.FromErr(err)
190+
}
191+
192+
notifyValue := query.Organization.Team.ReviewRequestDelegationNotifyAll
193+
194+
// Set notify in the location matching the user's config: top-level or
195+
// deprecated nested field inside review_request_delegation.
196+
_, usesDeprecatedNotify := d.GetOk("review_request_delegation.0.notify")
197+
tflog.Debug(ctx, "Uses deprecated notify", map[string]any{
198+
"uses_deprecated_notify": usesDeprecatedNotify,
199+
"notify_value": notifyValue,
200+
})
201+
202+
if !usesDeprecatedNotify {
203+
if err = d.Set("notify", notifyValue); err != nil {
204+
return diag.FromErr(err)
205+
}
121206
}
122207

123208
if query.Organization.Team.ReviewRequestDelegation {
124209
reviewRequestDelegation := make(map[string]any)
125210
reviewRequestDelegation["algorithm"] = query.Organization.Team.ReviewRequestDelegationAlgorithm
126211
reviewRequestDelegation["member_count"] = query.Organization.Team.ReviewRequestDelegationCount
127-
reviewRequestDelegation["notify"] = query.Organization.Team.ReviewRequestDelegationNotifyAll
212+
if usesDeprecatedNotify {
213+
reviewRequestDelegation["notify"] = notifyValue
214+
}
128215
if err = d.Set("review_request_delegation", []any{reviewRequestDelegation}); err != nil {
129-
return err
216+
return diag.FromErr(err)
130217
}
131218
} else {
132219
if err = d.Set("review_request_delegation", []any{}); err != nil {
133-
return err
220+
return diag.FromErr(err)
134221
}
135222
}
136223

137224
return nil
138225
}
139226

140-
func resourceGithubTeamSettingsUpdate(d *schema.ResourceData, meta any) error {
141-
if d.HasChange("review_request_delegation") || d.IsNewResource() {
227+
func resourceGithubTeamSettingsUpdate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics {
228+
if d.HasChange("review_request_delegation") || d.HasChange("notify") {
229+
meta := m.(*Owner)
230+
graphql := meta.v4client
231+
reviewRequestDelegation := d.Get("review_request_delegation").([]any)
232+
notify := resolveNotify(ctx, d)
142233

143-
ctx := context.WithValue(context.Background(), ctxId, d.Id())
144-
graphql := meta.(*Owner).v4client
145-
if setting := d.Get("review_request_delegation").([]any); len(setting) == 0 {
146-
var mutation struct {
147-
UpdateTeamReviewAssignment struct {
148-
ClientMutationId githubv4.ID `graphql:"clientMutationId"`
149-
} `graphql:"updateTeamReviewAssignment(input:$input)"`
150-
}
234+
var mutation struct {
235+
UpdateTeamReviewAssignment struct {
236+
ClientMutationId githubv4.ID `graphql:"clientMutationId"`
237+
} `graphql:"updateTeamReviewAssignment(input:$input)"`
238+
}
151239

152-
return graphql.Mutate(ctx, &mutation, defaultTeamReviewAssignmentSettings(d.Id()), nil)
153-
} else {
154-
settings := d.Get("review_request_delegation").([]any)[0].(map[string]any)
240+
if len(reviewRequestDelegation) == 0 {
241+
tflog.Debug(ctx, "No review request delegation settings provided, disabling review request delegation", map[string]any{
242+
"team_id": d.Id(),
243+
"team_slug": d.Get("team_slug").(string),
244+
"notify": notify,
245+
})
246+
247+
input := defaultTeamReviewAssignmentSettings(d.Id())
248+
input.NotifyTeam = new(githubv4.Boolean(notify))
155249

156-
var mutation struct {
157-
UpdateTeamReviewAssignment struct {
158-
ClientMutationId githubv4.ID `graphql:"clientMutationId"`
159-
} `graphql:"updateTeamReviewAssignment(input:$input)"`
250+
err := graphql.Mutate(ctx, &mutation, input, nil)
251+
if err != nil {
252+
return diag.FromErr(err)
160253
}
254+
} else {
255+
settings := reviewRequestDelegation[0].(map[string]any)
161256

162257
teamReviewAlgorithm := githubv4.TeamReviewAssignmentAlgorithm(settings["algorithm"].(string))
163-
return graphql.Mutate(ctx, &mutation, githubv4.UpdateTeamReviewAssignmentInput{
258+
updateTeamReviewAssignmentInput := githubv4.UpdateTeamReviewAssignmentInput{
164259
ID: d.Id(),
165260
Enabled: githubv4.Boolean(true),
166261
Algorithm: &teamReviewAlgorithm,
167262
TeamMemberCount: new(githubv4.Int(settings["member_count"].(int))),
168-
NotifyTeam: new(githubv4.Boolean(settings["notify"].(bool))),
169-
}, nil)
263+
NotifyTeam: new(githubv4.Boolean(notify)),
264+
}
265+
err := graphql.Mutate(ctx, &mutation, updateTeamReviewAssignmentInput, nil)
266+
if err != nil {
267+
return diag.FromErr(err)
268+
}
170269
}
171270
}
172271

173-
return resourceGithubTeamSettingsRead(d, meta)
272+
return nil
174273
}
175274

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

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

186-
return graphql.Mutate(ctx, &mutation, defaultTeamReviewAssignmentSettings(d.Id()), nil)
284+
err := graphql.Mutate(ctx, &mutation, defaultTeamReviewAssignmentSettings(d.Id()), nil)
285+
if err != nil {
286+
return diag.FromErr(err)
287+
}
288+
return nil
187289
}
188290

189-
func resourceGithubTeamSettingsImport(d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
190-
nodeId, slug, err := resolveTeamIDs(d.Id(), meta.(*Owner), context.Background())
291+
func resourceGithubTeamSettingsImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
292+
nodeId, slug, err := resolveTeamIDs(ctx, meta.(*Owner), d.Id())
191293
if err != nil {
192294
return nil, err
193295
}
@@ -201,37 +303,39 @@ func resourceGithubTeamSettingsImport(d *schema.ResourceData, meta any) ([]*sche
201303
if err = d.Set("team_uid", nodeId); err != nil {
202304
return nil, err
203305
}
204-
return []*schema.ResourceData{d}, resourceGithubTeamSettingsRead(d, meta)
306+
return []*schema.ResourceData{d}, nil
205307
}
206308

207-
func resolveTeamIDs(idOrSlug string, meta *Owner, ctx context.Context) (nodeId, slug string, err error) {
208-
client := meta.v3client
209-
orgName := meta.name
210-
orgId := meta.id
211-
212-
teamId, parseIntErr := strconv.ParseInt(idOrSlug, 10, 64)
213-
if parseIntErr != nil {
214-
// The given id not an integer, assume it is a team slug
215-
team, _, slugErr := client.Teams.GetTeamBySlug(ctx, orgName, idOrSlug)
216-
if slugErr != nil {
217-
return "", "", errors.New(parseIntErr.Error() + slugErr.Error())
218-
}
219-
return team.GetNodeID(), team.GetSlug(), nil
220-
} else {
221-
// The given id is an integer, assume it is a team id
222-
team, _, teamIdErr := client.Teams.GetTeamByID(ctx, orgId, teamId)
223-
if teamIdErr != nil {
224-
// There isn't a team with the given ID, assume it is a teamslug
225-
team, _, slugErr := client.Teams.GetTeamBySlug(ctx, orgName, idOrSlug)
226-
if slugErr != nil {
227-
return "", "", errors.New(teamIdErr.Error() + slugErr.Error())
228-
}
309+
func resolveTeamIDs(ctx context.Context, meta *Owner, idOrSlug string) (string, string, error) {
310+
team, err := getTeam(ctx, meta, idOrSlug)
311+
if err != nil {
312+
return "", "", err
313+
}
314+
return team.GetNodeID(), team.GetSlug(), nil
315+
}
229316

230-
return team.GetNodeID(), team.GetSlug(), nil
231-
}
317+
// resolveNotify returns the notify value from the top-level attribute or the
318+
// deprecated nested attribute inside review_request_delegation. The top-level
319+
// attribute takes precedence. Since ConflictsWith prevents both from being set,
320+
// only one source can be active at a time.
321+
func resolveNotify(ctx context.Context, d *schema.ResourceData) bool {
322+
// Check if top-level notify is explicitly configured.
323+
if v := d.Get("notify").(bool); v {
324+
tflog.Debug(ctx, "Top-level notify is explicitly configured", map[string]any{
325+
"notify": v,
326+
})
327+
return v
328+
}
232329

233-
return team.GetNodeID(), team.GetSlug(), nil
330+
// Fall back to deprecated nested field
331+
if v := d.Get("review_request_delegation.0.notify").(bool); v {
332+
tflog.Debug(ctx, "Deprecated nested notify is explicitly configured", map[string]any{
333+
"notify": v,
334+
})
335+
return v
234336
}
337+
338+
return false
235339
}
236340

237341
func defaultTeamReviewAssignmentSettings(id string) githubv4.UpdateTeamReviewAssignmentInput {

0 commit comments

Comments
 (0)