Skip to content

Commit a3f7d22

Browse files
cjgajardcjgajard
committed
Fix PR comments
1 parent b8955e7 commit a3f7d22

11 files changed

+67
-45
lines changed

‎pagerdutyplugin/data_source_pagerduty_extension_schema.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,8 @@ func (d *dataSourceExtensionSchema) Read(ctx context.Context, req datasource.Rea
4848
}
4949

5050
var found *pagerduty.ExtensionSchema
51-
// TODO delete and comment in PR: changed to 2min because 5min/30s is 10 attempts
5251
err := retry.RetryContext(ctx, 2*time.Minute, func() *retry.RetryError {
53-
list, err := d.client.ListExtensionSchemasWithContext(ctx, pagerduty.ListExtensionSchemaOptions{})
52+
list, err := d.client.ListExtensionSchemasWithContext(ctx, pagerduty.ListExtensionSchemaOptions{Limit: 100})
5453
if err != nil {
5554
if util.IsBadRequestError(err) {
5655
return retry.NonRetryableError(err)

‎pagerdutyplugin/data_source_pagerduty_extension_schema_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,16 @@ func testAccDataSourcePagerDutyExtensionSchema(n string) resource.TestCheckFunc
3535
return fmt.Errorf("Expected to get an Extension Schema ID from PagerDuty")
3636
}
3737

38-
if a["id"] != "PD8SURB" {
39-
return fmt.Errorf("Expected the Slack Extension Schema ID to be: PD8SURB, but got: %s", a["id"])
38+
if a["id"] != "PAKM60Z" {
39+
return fmt.Errorf("Expected Schema ID to be: PAKM60Z, but got: %s", a["id"])
4040
}
4141

42-
if a["name"] != "Slack" {
43-
return fmt.Errorf("Expected the Slack Extension Schema Name to be: Slack, but got: %s", a["name"])
42+
if a["name"] != "ServiceNow (v7)" {
43+
return fmt.Errorf("Expected Schema Name to be: ServiceNow (v7), but got: %s", a["name"])
4444
}
4545

4646
if a["type"] != "extension_schema" {
47-
return fmt.Errorf("Expected the Slack Extension Schema Type to be: extension_schema, but got: %s", a["type"])
47+
return fmt.Errorf("Expected the Schema Type to be: extension_schema, but got: %s", a["type"])
4848
}
4949

5050
return nil
@@ -53,7 +53,7 @@ func testAccDataSourcePagerDutyExtensionSchema(n string) resource.TestCheckFunc
5353

5454
const testAccDataSourcePagerDutyExtensionSchemaConfig = `
5555
data "pagerduty_extension_schema" "foo" {
56-
name = "slack"
56+
name = "ServiceNow (v7)"
5757
}
5858
`
5959

‎pagerdutyplugin/import_pagerduty_extension_servicenow_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ func TestAccPagerDutyExtensionServiceNow_import(t *testing.T) {
2121
{
2222
Config: testAccCheckPagerDutyExtensionServiceNowConfig(name, extension_name, url, "false", "any"),
2323
},
24-
2524
{
26-
ResourceName: "pagerduty_extension_servicenow.foo",
27-
ImportState: true,
28-
ImportStateVerify: true,
25+
ResourceName: "pagerduty_extension_servicenow.foo",
26+
ImportState: true,
27+
ImportStateVerify: true,
28+
ImportStateVerifyIgnore: []string{"config"},
2929
},
3030
},
3131
})

‎pagerdutyplugin/import_pagerduty_extension_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ func TestAccPagerDutyExtension_import(t *testing.T) {
2121
{
2222
Config: testAccCheckPagerDutyExtensionConfig(name, extension_name, url, "false", "any"),
2323
},
24-
2524
{
26-
ResourceName: "pagerduty_extension.foo",
27-
ImportState: true,
28-
ImportStateVerify: true,
25+
ResourceName: "pagerduty_extension.foo",
26+
ImportState: true,
27+
ImportStateVerify: true,
28+
ImportStateVerifyIgnore: []string{"config"},
2929
},
3030
},
3131
})

‎pagerdutyplugin/provider_test.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,12 @@ func testAccProtoV5ProviderFactories() map[string]func() (tfprotov5.ProviderServ
7171
}
7272
}
7373

74-
// testAccTimeNow returns the current time in the given location.
75-
// The location defaults to Europe/Dublin but can be controlled by the
76-
// PAGERDUTY_TIME_ZONE environment variable. The location must match the
77-
// PagerDuty account time zone or diff issues might bubble up in tests.
74+
// testAccTimeNow returns the current time in the given location. The location
75+
// defaults to Europe/Dublin but can be controlled by the PAGERDUTY_TIME_ZONE
76+
// environment variable. The location must match the PagerDuty account time
77+
// zone or diff issues might bubble up in tests. Here is the list of allowed
78+
// Time Zone Identifier for PagerDuty accounts
79+
// https://developer.pagerduty.com/docs/1afe25e9c94cb-types#time-zone
7880
func testAccTimeNow() time.Time {
7981
name := "Europe/Dublin"
8082
if v := os.Getenv("PAGERDUTY_TIME_ZONE"); v != "" {

‎pagerdutyplugin/resource_pagerduty_business_service.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func (r *resourceBusinessService) Create(ctx context.Context, req resource.Creat
6969
if resp.Diagnostics.HasError() {
7070
return
7171
}
72-
businessServicePlan := buildPagerdutyBusinessService(ctx, &plan)
72+
businessServicePlan := buildPagerdutyBusinessService(&plan)
7373
log.Printf("[INFO] Creating PagerDuty business service %s", plan.Name)
7474

7575
err := helperResource.RetryContext(ctx, 5*time.Minute, func() *helperResource.RetryError {
@@ -120,7 +120,7 @@ func (r *resourceBusinessService) Update(ctx context.Context, req resource.Updat
120120
return
121121
}
122122

123-
businessServicePlan := buildPagerdutyBusinessService(ctx, &plan)
123+
businessServicePlan := buildPagerdutyBusinessService(&plan)
124124
if businessServicePlan.ID == "" {
125125
var id string
126126
req.State.GetAttribute(ctx, path.Root("id"), &id)
@@ -202,7 +202,7 @@ func requestGetBusinessService(ctx context.Context, client *pagerduty.Client, id
202202
return model
203203
}
204204

205-
func buildPagerdutyBusinessService(_ context.Context, model *resourceBusinessServiceModel) *pagerduty.BusinessService {
205+
func buildPagerdutyBusinessService(model *resourceBusinessServiceModel) *pagerduty.BusinessService {
206206
businessService := pagerduty.BusinessService{
207207
ID: model.ID.ValueString(),
208208
Description: model.Description.ValueString(),

‎pagerdutyplugin/resource_pagerduty_extension_servicenow.go‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,8 @@ func (r *resourceExtensionServiceNow) Update(ctx context.Context, req resource.U
160160
model, err = r.requestGetExtensionServiceNow(ctx, requestGetExtensionServiceNowOptions{
161161
ID: plan.ID,
162162
RetryNotFound: true,
163-
SnowPassword: extractString(ctx, req.State, "snow_password", &resp.Diagnostics),
164-
EndpointURL: extractString(ctx, req.State, "endpoint_url", &resp.Diagnostics),
163+
SnowPassword: extractString(ctx, req.Plan, "snow_password", &resp.Diagnostics),
164+
EndpointURL: extractString(ctx, req.Plan, "endpoint_url", &resp.Diagnostics),
165165
Diagnostics: &resp.Diagnostics,
166166
})
167167
if err != nil {
@@ -199,8 +199,6 @@ func (r *resourceExtensionServiceNow) Configure(ctx context.Context, req resourc
199199
}
200200

201201
func (r *resourceExtensionServiceNow) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) {
202-
// resource.ImportStatePassthroughID(ctx, path.Root("id"), req, resp)
203-
204202
model, err := r.requestGetExtensionServiceNow(ctx, requestGetExtensionServiceNowOptions{
205203
ID: req.ID,
206204
RetryNotFound: false,
@@ -212,10 +210,6 @@ func (r *resourceExtensionServiceNow) ImportState(ctx context.Context, req resou
212210
}
213211
return
214212
}
215-
216-
// model.EndpointURL
217-
// model.ExtensionObjects = []string{extension.ExtensionObjects[0].ID}
218-
// model.ExtensionSchema
219213
resp.Diagnostics.Append(resp.State.Set(ctx, &model)...)
220214
}
221215

@@ -313,22 +307,25 @@ func flattenExtensionServiceNow(src *pagerduty.Extension, snowPassword *string,
313307
model := resourceExtensionServiceNowModel{
314308
ID: types.StringValue(src.ID),
315309
Name: types.StringValue(src.Name),
316-
EndpointURL: types.StringValue(src.EndpointURL),
317310
HTMLURL: types.StringValue(src.HTMLURL),
318311
ExtensionSchema: types.StringValue(src.ExtensionSchema.ID),
319312
ExtensionObjects: flattenExtensionServiceNowObjects(src.ExtensionObjects),
320313
}
321314

322315
b, _ := json.Marshal(src.Config)
323316
var config PagerDutyExtensionServiceNowConfig
324-
json.Unmarshal(b, &config)
317+
_ = json.Unmarshal(b, &config)
325318

326319
model.SnowUser = types.StringValue(config.User)
327320
if snowPassword != nil {
328321
model.SnowPassword = types.StringValue(*snowPassword)
322+
} else if config.Password != "" {
323+
model.SnowPassword = types.StringValue(config.Password)
329324
}
330325
if endpointURL != nil {
331-
model.SnowPassword = types.StringValue(*endpointURL)
326+
model.EndpointURL = types.StringValue(*endpointURL)
327+
} else if src.EndpointURL != "" {
328+
model.EndpointURL = types.StringValue(src.EndpointURL)
332329
}
333330
model.SyncOptions = types.StringValue(config.SyncOptions)
334331
model.Target = types.StringValue(config.Target)

‎pagerdutyplugin/resource_pagerduty_extension_servicenow_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func TestAccPagerDutyExtensionServiceNow_Basic(t *testing.T) {
4444
extension_name := id.PrefixedUniqueId("tf-")
4545
extension_name_updated := id.PrefixedUniqueId("tf-")
4646
name := id.PrefixedUniqueId("tf-")
47-
url := "https://example.com/recieve_a_pagerduty_webhook"
47+
url := "https://example.com/receive_a_pagerduty_webhook"
4848
url_updated := "https://example.com/webhook_foo"
4949

5050
resource.Test(t, resource.TestCase{

‎pagerdutyplugin/resource_pagerduty_extension_test.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"testing"
99

1010
"github.com/PagerDuty/go-pagerduty"
11+
"github.com/PagerDuty/terraform-provider-pagerduty/util"
1112
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/id"
1213
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
1314
"github.com/hashicorp/terraform-plugin-testing/terraform"
@@ -44,7 +45,7 @@ func TestAccPagerDutyExtension_Basic(t *testing.T) {
4445
extension_name := id.PrefixedUniqueId("tf-")
4546
extension_name_updated := id.PrefixedUniqueId("tf-")
4647
name := id.PrefixedUniqueId("tf-")
47-
url := "https://example.com/recieve_a_pagerduty_webhook"
48+
url := "https://example.com/receive_a_pagerduty_webhook"
4849
url_updated := "https://example.com/webhook_foo"
4950

5051
resource.Test(t, resource.TestCase{
@@ -62,8 +63,8 @@ func TestAccPagerDutyExtension_Basic(t *testing.T) {
6263
"pagerduty_extension.foo", "extension_schema", "PJFWPEP"),
6364
resource.TestCheckResourceAttr(
6465
"pagerduty_extension.foo", "endpoint_url", url),
65-
resource.TestCheckResourceAttr(
66-
"pagerduty_extension.foo", "config", "{\"notify_types\":{\"acknowledge\":false,\"assignments\":false,\"resolve\":false},\"restrict\":\"any\"}"),
66+
resource.TestCheckResourceAttrWith(
67+
"pagerduty_extension.foo", "config", util.CheckJSONEqual("{\"notify_types\":{\"acknowledge\":false,\"assignments\":false,\"resolve\":false},\"restrict\":\"any\"}")),
6768
resource.TestCheckResourceAttr(
6869
"pagerduty_extension.foo", "html_url", ""),
6970
),
@@ -78,8 +79,8 @@ func TestAccPagerDutyExtension_Basic(t *testing.T) {
7879
"pagerduty_extension.foo", "extension_schema", "PJFWPEP"),
7980
resource.TestCheckResourceAttr(
8081
"pagerduty_extension.foo", "endpoint_url", url_updated),
81-
resource.TestCheckResourceAttr(
82-
"pagerduty_extension.foo", "config", "{\"notify_types\":{\"acknowledge\":true,\"assignments\":true,\"resolve\":true},\"restrict\":\"pd-users\"}"),
82+
resource.TestCheckResourceAttrWith(
83+
"pagerduty_extension.foo", "config", util.CheckJSONEqual("{\"notify_types\":{\"acknowledge\":true,\"assignments\":true,\"resolve\":true},\"restrict\":\"pd-users\"}")),
8384
),
8485
},
8586
},
@@ -178,9 +179,9 @@ resource "pagerduty_extension" "foo"{
178179
{
179180
"restrict": "%[4]v",
180181
"notify_types": {
181-
"resolve": %[5]v,
182-
"acknowledge": %[5]v,
183-
"assignments": %[5]v
182+
"resolve": %[5]v,
183+
"acknowledge": %[5]v,
184+
"assignments": %[5]v
184185
}
185186
}
186187
EOF

‎pagerdutyplugin/resource_pagerduty_tag_assignment.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,6 @@ func (r *resourceTagAssignment) Configure(ctx context.Context, req resource.Conf
245245
}
246246

247247
func (r *resourceTagAssignment) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) {
248-
// resource.ImportStatePassthroughID(ctx, path.Root("id"), req, resp)
249248
ids := strings.Split(req.ID, ".")
250249
if len(ids) != 3 {
251250
resp.Diagnostics.AddError(

0 commit comments

Comments
 (0)