Skip to content

Commit df40bb2

Browse files
uturunku1uturunku1
committed
feedback: to include new steps to compare state and confirm replacement of resource
1 parent 0dd5a36 commit df40bb2

File tree

2 files changed

+66
-23
lines changed

2 files changed

+66
-23
lines changed

‎internal/provider/resource_tfe_notification_configuration.go‎

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package provider
55

66
import (
77
"context"
8+
"errors"
89
"fmt"
910

1011
tfe "github.com/hashicorp/go-tfe"
@@ -328,28 +329,29 @@ func (r *resourceTFENotificationConfiguration) Create(ctx context.Context, req r
328329

329330
// Add email_addresses set to the options struct
330331
emailAddresses := make([]types.String, len(plan.EmailAddresses.Elements()))
331-
if diags := plan.EmailAddresses.ElementsAs(ctx, &emailAddresses, true); diags != nil && diags.HasError() {
332-
resp.Diagnostics.Append(diags...)
332+
resp.Diagnostics.Append(plan.EmailAddresses.ElementsAs(ctx, &emailAddresses, true)...)
333+
if resp.Diagnostics.HasError() {
333334
return
334335
}
335-
336336
options.EmailAddresses = []string{}
337337
for _, emailAddress := range emailAddresses {
338338
options.EmailAddresses = append(options.EmailAddresses, emailAddress.ValueString())
339339
}
340340

341341
// Add email_user_ids set to the options struct
342342
emailUserIDs := make([]types.String, len(plan.EmailUserIDs.Elements()))
343-
if diags := plan.EmailUserIDs.ElementsAs(ctx, &emailUserIDs, true); diags != nil && diags.HasError() {
344-
resp.Diagnostics.Append(diags...)
343+
resp.Diagnostics.Append(plan.EmailUserIDs.ElementsAs(ctx, &emailUserIDs, true)...)
344+
if resp.Diagnostics.HasError() {
345345
return
346346
}
347+
347348
options.EmailUsers = []*tfe.User{}
348349
for _, emailUserID := range emailUserIDs {
349350
options.EmailUsers = append(options.EmailUsers, &tfe.User{ID: emailUserID.ValueString()})
350351
}
351352

352353
tflog.Debug(ctx, "Creating notification configuration")
354+
353355
nc, err := r.config.Client.NotificationConfigurations.Create(ctx, workspaceID, options)
354356
if err != nil {
355357
resp.Diagnostics.AddError("Unable to create notification configuration", err.Error())
@@ -366,8 +368,8 @@ func (r *resourceTFENotificationConfiguration) Create(ctx context.Context, req r
366368

367369
// We got a notification, so set state to new values
368370
result, diags := modelFromTFENotificationConfiguration(nc, !config.TokenWO.IsNull())
369-
if diags != nil && diags.HasError() {
370-
resp.Diagnostics.Append((diags)...)
371+
resp.Diagnostics.Append(diags...)
372+
if resp.Diagnostics.HasError() {
371373
return
372374
}
373375

@@ -395,7 +397,12 @@ func (r *resourceTFENotificationConfiguration) Read(ctx context.Context, req res
395397
tflog.Debug(ctx, fmt.Sprintf("Reading notification configuration %q", state.ID.ValueString()))
396398
nc, err := r.config.Client.NotificationConfigurations.Read(ctx, state.ID.ValueString())
397399
if err != nil {
398-
resp.Diagnostics.AddError("Unable to read notification configuration", err.Error())
400+
if errors.Is(err, tfe.ErrResourceNotFound) {
401+
tflog.Debug(ctx, fmt.Sprintf("`Notification configuration %s no longer exists", state.ID))
402+
resp.State.RemoveResource(ctx)
403+
} else {
404+
resp.Diagnostics.AddError("Error reading notification configuration", "Could not read notification configuration, unexpected error: "+err.Error())
405+
}
399406
return
400407
}
401408

@@ -406,13 +413,13 @@ func (r *resourceTFENotificationConfiguration) Read(ctx context.Context, req res
406413

407414
isWriteOnly, diags := r.writeOnlyValueStore(resp.Private).PriorValueExists(ctx)
408415
resp.Diagnostics.Append(diags...)
409-
if diags.HasError() {
416+
if resp.Diagnostics.HasError() {
410417
return
411418
}
412419

413420
result, diags := modelFromTFENotificationConfiguration(nc, isWriteOnly)
414-
if diags != nil && diags.HasError() {
415-
resp.Diagnostics.Append((diags)...)
421+
resp.Diagnostics.Append(diags...)
422+
if resp.Diagnostics.HasError() {
416423
return
417424
}
418425

@@ -449,32 +456,35 @@ func (r *resourceTFENotificationConfiguration) Update(ctx context.Context, req r
449456

450457
// Add triggers set to the options struct
451458
triggers := make([]types.String, len(plan.Triggers.Elements()))
452-
if diags := plan.Triggers.ElementsAs(ctx, &triggers, true); diags != nil && diags.HasError() {
453-
resp.Diagnostics.Append(diags...)
459+
resp.Diagnostics.Append(plan.Triggers.ElementsAs(ctx, &triggers, true)...)
460+
if resp.Diagnostics.HasError() {
454461
return
455462
}
463+
456464
options.Triggers = []tfe.NotificationTriggerType{}
457465
for _, trigger := range triggers {
458466
options.Triggers = append(options.Triggers, tfe.NotificationTriggerType(trigger.ValueString()))
459467
}
460468

461469
// Add email_addresses set to the options struct
462470
emailAddresses := make([]types.String, len(plan.EmailAddresses.Elements()))
463-
if diags := plan.EmailAddresses.ElementsAs(ctx, &emailAddresses, true); diags != nil && diags.HasError() {
464-
resp.Diagnostics.Append(diags...)
471+
resp.Diagnostics.Append(plan.EmailAddresses.ElementsAs(ctx, &emailAddresses, true)...)
472+
if resp.Diagnostics.HasError() {
465473
return
466474
}
475+
467476
options.EmailAddresses = []string{}
468477
for _, emailAddress := range emailAddresses {
469478
options.EmailAddresses = append(options.EmailAddresses, emailAddress.ValueString())
470479
}
471480

472481
// Add email_user_ids set to the options struct
473482
emailUserIDs := make([]types.String, len(plan.EmailUserIDs.Elements()))
474-
if diags := plan.EmailUserIDs.ElementsAs(ctx, &emailUserIDs, true); diags != nil && diags.HasError() {
475-
resp.Diagnostics.Append(diags...)
483+
resp.Diagnostics.Append(plan.EmailUserIDs.ElementsAs(ctx, &emailUserIDs, true)...)
484+
if resp.Diagnostics.HasError() {
476485
return
477486
}
487+
478488
options.EmailUsers = []*tfe.User{}
479489
for _, emailUserID := range emailUserIDs {
480490
options.EmailUsers = append(options.EmailUsers, &tfe.User{ID: emailUserID.ValueString()})
@@ -503,8 +513,8 @@ func (r *resourceTFENotificationConfiguration) Update(ctx context.Context, req r
503513
}
504514

505515
result, diags := modelFromTFENotificationConfiguration(nc, !config.TokenWO.IsNull())
506-
if diags != nil && diags.HasError() {
507-
resp.Diagnostics.Append((diags)...)
516+
resp.Diagnostics.Append(diags...)
517+
if resp.Diagnostics.HasError() {
508518
return
509519
}
510520

‎internal/provider/resource_tfe_notification_configuration_test.go‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,11 @@ import (
1212
"time"
1313

1414
"github.com/hashicorp/go-tfe"
15+
"github.com/hashicorp/terraform-plugin-testing/compare"
1516
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
17+
"github.com/hashicorp/terraform-plugin-testing/statecheck"
1618
"github.com/hashicorp/terraform-plugin-testing/terraform"
19+
"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
1720
)
1821

1922
func TestAccTFENotificationConfiguration_basic(t *testing.T) {
@@ -51,6 +54,8 @@ func TestAccTFENotificationConfiguration_WriteOnly(t *testing.T) {
5154
notificationConfiguration := &tfe.NotificationConfiguration{}
5255
rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int()
5356

57+
compareValuesDiffer := statecheck.CompareValue(compare.ValuesDiffer())
58+
5459
resource.Test(t, resource.TestCase{
5560
PreCheck: func() { preCheckTFENotificationConfiguration(t) },
5661
ProtoV5ProviderFactories: testAccMuxedProviders,
@@ -61,7 +66,7 @@ func TestAccTFENotificationConfiguration_WriteOnly(t *testing.T) {
6166
ExpectError: regexp.MustCompile(`Attribute "token_wo" cannot be specified when "token" is specified`),
6267
},
6368
{
64-
Config: testAccTFENotificationConfiguration_tokenWriteOnly(rInt),
69+
Config: testAccTFENotificationConfiguration_tokenWriteOnly(rInt, "1"),
6570
Check: resource.ComposeTestCheckFunc(
6671
testAccCheckTFENotificationConfigurationExists(
6772
"tfe_notification_configuration.foobar", notificationConfiguration),
@@ -76,6 +81,34 @@ func TestAccTFENotificationConfiguration_WriteOnly(t *testing.T) {
7681
"tfe_notification_configuration.foobar", "url", runTasksURL()),
7782
resource.TestCheckNoResourceAttr("tfe_notification_configuration.foobar", "token_wo"),
7883
),
84+
ConfigStateChecks: []statecheck.StateCheck{
85+
compareValuesDiffer.AddStateValue(
86+
"tfe_notification_configuration.foobar", tfjsonpath.New("id"),
87+
),
88+
},
89+
},
90+
{
91+
Config: testAccTFENotificationConfiguration_tokenWriteOnly(rInt, "2"),
92+
Check: resource.ComposeTestCheckFunc(
93+
resource.TestCheckNoResourceAttr("tfe_notification_configuration.foobar", "token_wo"),
94+
),
95+
ConfigStateChecks: []statecheck.StateCheck{
96+
compareValuesDiffer.AddStateValue(
97+
"tfe_notification_configuration.foobar", tfjsonpath.New("id"),
98+
),
99+
},
100+
},
101+
{
102+
Config: testAccTFENotificationConfiguration_basic(rInt),
103+
Check: resource.ComposeTestCheckFunc(
104+
resource.TestCheckNoResourceAttr("tfe_notification_configuration.foobar", "token_wo"),
105+
resource.TestCheckNoResourceAttr("tfe_notification_configuration.foobar", "token"),
106+
),
107+
ConfigStateChecks: []statecheck.StateCheck{
108+
compareValuesDiffer.AddStateValue(
109+
"tfe_notification_configuration.foobar", tfjsonpath.New("id"),
110+
),
111+
},
79112
},
80113
},
81114
})
@@ -850,7 +883,7 @@ resource "tfe_notification_configuration" "foobar" {
850883
}`, rInt, runTasksURL())
851884
}
852885

853-
func testAccTFENotificationConfiguration_tokenWriteOnly(rInt int) string {
886+
func testAccTFENotificationConfiguration_tokenWriteOnly(rInt int, wo string) string {
854887
return fmt.Sprintf(`
855888
resource "tfe_organization" "foobar" {
856889
name = "tst-terraform-%d"
@@ -865,10 +898,10 @@ resource "tfe_workspace" "foobar" {
865898
resource "tfe_notification_configuration" "foobar" {
866899
name = "notification_basic"
867900
destination_type = "generic"
868-
token = "some-token"
901+
token_wo = "%s"
869902
url = "%s"
870903
workspace_id = tfe_workspace.foobar.id
871-
}`, rInt, runTasksURL())
904+
}`, rInt, wo, runTasksURL())
872905
}
873906

874907
func testAccTFENotificationConfiguration_tokenAndTokenWriteOnly(rInt int) string {

0 commit comments

Comments
 (0)