DXCDT-462: Adding optional `display_name` field to `auth0_trigger_action` resource #621

willvedd · 2023-06-07T12:28:44Z

🔧 Changes

The initial iteration of the auth0_trigger_action resource (#612) did not include a display name field. This field is used to assign a distinguishable name to a specific instance of an action within a flow. This can be useful in cases where there are multiple instances of the same action with a flow, albeit configured differently. The auth0_trigger_actions resource does include this property so for API consistency, this PR adds the display_name property too.

📚 References

First iteration of auth0_trigger_action DXCDT0-425: auth0_trigger_action #612

🔬 Testing

Adding new test cases.

📝 Checklist

All new/changed/fixed functionality is covered by tests (or N/A)
I have added documentation for all new/changed functionality (or N/A)

willvedd · 2023-06-07T12:29:19Z

internal/auth0/action/resource_trigger_action.go

+	if d.GetRawConfig().GetAttr("display_name").IsNull() {
+		action, err := api.Action.Read(actionID)
+		if err != nil {
+			return diag.FromErr(err)
 		}
-		return diag.FromErr(err)
+		displayName = action.GetName()
 	}


Not sure if this is the most effective way of detecting an undefined display name. Open to suggestions on how to improve.

willvedd · 2023-06-07T12:29:54Z

internal/auth0/action/resource_trigger_action_test.go

@@ -55,6 +65,15 @@ func TestAccTriggerAction(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr("auth0_trigger_action.my_action_post_login", "trigger", "post-login"),
 					resource.TestCheckResourceAttrSet("auth0_trigger_action.my_action_post_login", "action_id"),
+					resource.TestCheckNoResourceAttr("auth0_trigger_action.my_action_post_login", "display_name"),


Wondering if there's a way to check the computed value here?

Well we know that it will take the name of the action by default, so we can check for that one: Test Action {{.testName}}.

That's in direct conflict with TestCheckNoResourceAttr.

…/github.com/auth0/terraform-provider-auth0 into DXCDT-462-auth0-action-trigger-display-name

codecov-commenter · 2023-06-07T12:38:34Z

Codecov Report

Merging #621 (1711ad4) into main (0ac7731) will decrease coverage by 0.05%.
The diff coverage is 70.31%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #621      +/-   ##
==========================================
- Coverage   86.38%   86.33%   -0.05%     
==========================================
  Files          79       79              
  Lines       12207    12259      +52     
==========================================
+ Hits        10545    10584      +39     
- Misses       1278     1287       +9     
- Partials      384      388       +4

Impacted Files	Coverage Δ
internal/auth0/action/resource_trigger_action.go	`73.19% <70.31%> (+0.66%)`	⬆️

internal/auth0/action/resource_trigger_action.go

sergiught · 2023-06-07T13:36:38Z

internal/auth0/action/resource_trigger_action.go

+		if mErr, ok := err.(management.Error); ok && mErr.Status() == http.StatusNotFound {
+			d.SetId("")
+			return nil
+		}


We shouldn't ignore the 404 here.

Can you be more specific? This is consistent across the entire codebase.

It's consistent across the entire codebase for resources that get created and get deleted. The set of triggers is static and is not something a customer can create or delete.

internal/auth0/action/resource_trigger_action.go

sergiught · 2023-06-07T13:41:21Z

internal/auth0/action/resource_trigger_action.go

+			err = d.Set("display_name", binding.GetDisplayName())
+			if err != nil {
+				return diag.FromErr(err)
+			}


Suggested change

err = d.Set("display_name", binding.GetDisplayName())

if err != nil {

return diag.FromErr(err)

}

return diag.FromErr(d.Set("display_name", binding.GetDisplayName()))

sergiught · 2023-06-07T13:43:32Z

internal/auth0/action/resource_trigger_action_test.go

@@ -55,6 +65,15 @@ func TestAccTriggerAction(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr("auth0_trigger_action.my_action_post_login", "trigger", "post-login"),
 					resource.TestCheckResourceAttrSet("auth0_trigger_action.my_action_post_login", "action_id"),
+					resource.TestCheckNoResourceAttr("auth0_trigger_action.my_action_post_login", "display_name"),


Well we know that it will take the name of the action by default, so we can check for that one: Test Action {{.testName}}.

Co-authored-by: Sergiu Ghitea <[email protected]>

…/github.com/auth0/terraform-provider-auth0 into DXCDT-462-auth0-action-trigger-display-name

sergiught · 2023-06-08T12:44:45Z

internal/auth0/action/resource_trigger_action.go

+				Type:        schema.TypeString,
+				Optional:    true,
+				Computed:    true,
+				Description: "The name for this action within the trigger. This can be useful for distinguishing between multiple instances of the same action bound to a trigger.",


Would be great to specify that this defaults to the action name if not set.

sergiught · 2023-06-08T12:45:00Z

internal/auth0/action/resource_trigger_action.go

-			return nil
+	if displayName == "" {
+		action, err := api.Action.Read(actionID)
+


Suggested change

…/github.com/auth0/terraform-provider-auth0 into DXCDT-462-auth0-action-trigger-display-name

willvedd added 5 commits June 2, 2023 08:47

Merge branch 'main' of https://github.com/auth0/terraform-provider-auth0

0dbe019

Getting working as required and forcenew

ec2fa19

Getting working as required without forcenew

ae4453a

Making display_name optional and computed

abcfb6f

Checking for undefined display_name

e4b517e

willvedd requested a review from a team as a code owner June 7, 2023 12:28

willvedd commented Jun 7, 2023

View reviewed changes

willvedd and others added 3 commits June 7, 2023 08:37

Merge branch 'main' into DXCDT-462-auth0-action-trigger-display-name

939ec0e

Regenerating docs

1b56e3e

Merge branch 'DXCDT-462-auth0-action-trigger-display-name' of https:/…

e50094f

…/github.com/auth0/terraform-provider-auth0 into DXCDT-462-auth0-action-trigger-display-name

sergiught reviewed Jun 7, 2023

View reviewed changes

willvedd and others added 7 commits June 7, 2023 10:20

Update internal/auth0/action/resource_trigger_action.go

d7a028c

Co-authored-by: Sergiu Ghitea <[email protected]>

Merge branch 'DXCDT-462-auth0-action-trigger-display-name' of https:/…

eebfcde

…/github.com/auth0/terraform-provider-auth0 into DXCDT-462-auth0-action-trigger-display-name

Applying feedback

c2062eb

Handling 404

1fc9cb9

Removing unnecessary id set

9e75a20

Ignoring specific 404

2309188

Removing computed

f930578

willvedd requested a review from sergiught June 7, 2023 22:37

willvedd and others added 4 commits June 7, 2023 19:36

Merge branch 'main' into DXCDT-462-auth0-action-trigger-display-name

db8d924

trying again

0231206

Merge branch 'DXCDT-462-auth0-action-trigger-display-name' of https:/…

7dd6b8e

…/github.com/auth0/terraform-provider-auth0 into DXCDT-462-auth0-action-trigger-display-name

Merge branch 'main' into DXCDT-462-auth0-action-trigger-display-name

c8607d0

sergiught approved these changes Jun 8, 2023

View reviewed changes

willvedd added 2 commits June 8, 2023 11:59

Updating docs

17c16ea

Merge branch 'DXCDT-462-auth0-action-trigger-display-name' of https:/…

1711ad4

…/github.com/auth0/terraform-provider-auth0 into DXCDT-462-auth0-action-trigger-display-name

willvedd merged commit e8b26b6 into main Jun 8, 2023

willvedd deleted the DXCDT-462-auth0-action-trigger-display-name branch June 8, 2023 16:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DXCDT-462: Adding optional `display_name` field to `auth0_trigger_action` resource #621

DXCDT-462: Adding optional `display_name` field to `auth0_trigger_action` resource #621

willvedd commented Jun 7, 2023 •

edited

Loading

willvedd Jun 7, 2023

willvedd Jun 7, 2023

sergiught Jun 7, 2023

willvedd Jun 7, 2023

codecov-commenter commented Jun 7, 2023 •

edited

Loading

sergiught Jun 7, 2023

willvedd Jun 7, 2023

sergiught Jun 7, 2023 •

edited

Loading

sergiught Jun 7, 2023

sergiught Jun 7, 2023

sergiught Jun 8, 2023

sergiught Jun 8, 2023

DXCDT-462: Adding optional display_name field to auth0_trigger_action resource #621

DXCDT-462: Adding optional display_name field to auth0_trigger_action resource #621

Conversation

willvedd commented Jun 7, 2023 • edited Loading

🔧 Changes

📚 References

🔬 Testing

📝 Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 7, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergiught Jun 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DXCDT-462: Adding optional `display_name` field to `auth0_trigger_action` resource #621

DXCDT-462: Adding optional `display_name` field to `auth0_trigger_action` resource #621

willvedd commented Jun 7, 2023 •

edited

Loading

codecov-commenter commented Jun 7, 2023 •

edited

Loading

sergiught Jun 7, 2023 •

edited

Loading