Skip to content
This repository has been archived by the owner on Mar 8, 2022. It is now read-only.

Support hook dependencies #312

Merged
merged 10 commits into from
Mar 24, 2021

Conversation

woz5999
Copy link
Contributor

@woz5999 woz5999 commented Jan 11, 2021

Proposed Changes

  • Add support for hook "dependencies"

Fixes #296

Acceptance Test Output

$make testacc TESTS=TestAccHook
==> Checking that code complies with gofmt requirements...
?       github.com/alexkappa/terraform-provider-auth0   [no test files]
=== RUN   TestAccHook
--- PASS: TestAccHook (5.46s)
PASS
coverage: 7.9% of statements
ok      github.com/alexkappa/terraform-provider-auth0/auth0     5.793s  coverage: 7.9% of statements
?       github.com/alexkappa/terraform-provider-auth0/auth0/internal/debug      [no test files]
testing: warning: no tests to run
PASS
coverage: 0.0% of statements
ok      github.com/alexkappa/terraform-provider-auth0/auth0/internal/random     0.277s  coverage: 0.0% of statements [no tests to run]
testing: warning: no tests to run
PASS
coverage: 0.0% of statements
ok      github.com/alexkappa/terraform-provider-auth0/auth0/internal/validation 0.266s  coverage: 0.0% of statements [no tests to run]
?       github.com/alexkappa/terraform-provider-auth0/version   [no test files]

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

@woz5999
Copy link
Contributor Author

woz5999 commented Jan 25, 2021

@alexkappa are you able to review/merge this?

@woz5999
Copy link
Contributor Author

woz5999 commented Feb 16, 2021

@alexkappa would love to get this reviewed and merged

@@ -53,6 +54,9 @@ resource "auth0_hook" "my_hook" {
trigger_id = "pre-user-registration"
script = "function (user, context, callback) { console.log(user); callback(null, { user }); }"
enabled = false
dependencies = {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the formatting is off? (tabs v space?)

@yvovandoorn
Copy link
Contributor

Hi @woz5999! Could you resolve the merge conflict that was introduced when #262 was merged in?

Both are making updates to the hook test and #262 added quite a few new lines of test code.

I can merge following the updates.

@yvovandoorn yvovandoorn self-assigned this Mar 21, 2021
@woz5999
Copy link
Contributor Author

woz5999 commented Mar 23, 2021

@yvovandoorn this should be good now.

@yvovandoorn
Copy link
Contributor

Hey @woz5999. This isn't passing my local testing setup:

make testacc TESTS=TestAccHookSecrets                          
==> Checking that code complies with gofmt requirements...
?   	github.com/alexkappa/terraform-provider-auth0	[no test files]
=== RUN   TestAccHookSecrets
    testing.go:705: Step 0 error: Check failed: Check 2/7 error: auth0_hook.my_hook: Attribute 'dependencies.auth0' not found
--- FAIL: TestAccHookSecrets (1.64s)
FAIL
coverage: 8.1% of statements
FAIL	github.com/alexkappa/terraform-provider-auth0/auth0	1.667s
?   	github.com/alexkappa/terraform-provider-auth0/auth0/internal/debug	[no test files]
testing: warning: no tests to run
PASS
coverage: 0.0% of statements
ok  	github.com/alexkappa/terraform-provider-auth0/auth0/internal/random	0.023s	coverage: 0.0% of statements [no tests to run]
testing: warning: no tests to run
PASS
coverage: 0.0% of statements
ok  	github.com/alexkappa/terraform-provider-auth0/auth0/internal/validation	0.018s	coverage: 0.0% of statements [no tests to run]
?   	github.com/alexkappa/terraform-provider-auth0/version	[no test files]
FAIL
make: *** [testacc] Error 1

It looks like

resource.TestCheckResourceAttr("auth0_hook.my_hook", "dependencies.auth0", "2.30.0"),

should be added to L64 in auth0/resource_auth0_hook_test.go

and

  dependencies = {
    auth0 = "2.30.0"
  }

to L93-95 in auth0/resource_auth0_hook_test.go

This way it passes the initial and diff check.

@woz5999
Copy link
Contributor Author

woz5999 commented Mar 24, 2021

@yvovandoorn sorry about that. got those added.

Fix gofmt on L64
@yvovandoorn
Copy link
Contributor

👍 LGTM

@yvovandoorn yvovandoorn merged commit fc08ef1 into alexkappa:master Mar 24, 2021
@galkin
Copy link

galkin commented Mar 24, 2021

@yvovandoorn, do you have new release estimate?

@yvovandoorn
Copy link
Contributor

@galkin probably in the coming days. I hope to incorporate #321 but its dependent on go-auth0/auth0#187. That said, if its taking too long, I'll talk with @alexkappa to push out a release earlier.

Thanks for your patience on this!!

@yvovandoorn
Copy link
Contributor

@galkin released! https://github.com/alexkappa/terraform-provider-auth0/releases/tag/v0.20.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please add the ability to define hook dependencies
3 participants