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

Added auth0 prompt custom text #497

Merged

Conversation

joaovieira-ca
Copy link
Contributor

@joaovieira-ca joaovieira-ca commented Jan 15, 2022

Proposed Changes

  • Introduces auth0_prompt_custom_text

Acceptance Test Output

Click to expand test output
$ make testacc TESTS=TestAccPromptCustomText
==> Checking that code complies with gofmt requirements...
?       github.com/alexkappa/terraform-provider-auth0   [no test files]
=== RUN   TestAccPromptCustomText
--- PASS: TestAccPromptCustomText (4.68s)
PASS
coverage: 5.0% of statements
ok      github.com/alexkappa/terraform-provider-auth0/auth0     7.574s  coverage: 5.0% of statements
?       github.com/alexkappa/terraform-provider-auth0/auth0/internal/debug      [no test files]
?       github.com/alexkappa/terraform-provider-auth0/auth0/internal/digitalocean       [no test files]
testing: warning: no tests to run
PASS
coverage: 0.0% of statements
ok      github.com/alexkappa/terraform-provider-auth0/auth0/internal/hash       0.390s  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/random     0.424s  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.610s  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

@joaovieira-ca joaovieira-ca marked this pull request as ready for review January 15, 2022 16:08
@joaovieira-ca
Copy link
Contributor Author

This is an attempt to add the prompt custom texts to the terraform resource. This is my first change to a terraform resource project, so any feedback is very welcome.

The challenge that we have in the custom text is that they don't seem to have an ID (at least none is returned on the API). One way to create an ID is using the prompt type and the language. I used this to create the ID, but I am not sure what is default pattern is for this kind of situation.

If you have any ideas for improvements or another way to do it, please provide feedback.

PS: I have tested the code, either using the test and also using the compiled file and it works fine.

@kiranms-newpage
Copy link

@sergiughf We have been waiting for this feature since long. Can you please review this and merge and release if all looks okay to you?

@sergiught
Copy link
Collaborator

Hey folks, I'll take a look at this today. 🖖🏻

@sergiught
Copy link
Collaborator

Quick update that I started taking a look at this but haven't yet finished running through all the manual tests I wanted to perform. I'll give another update later in the day.

@sergiught
Copy link
Collaborator

Hey @jluiz20, thanks a lot for the contribution! The PR was in a pretty good shape, I only pushed a couple of small improvements that we can go over together if you have any questions.

The challenge that we have in the custom text is that they don't seem to have an ID (at least none is returned on the API). One way to create an ID is using the prompt type and the language. I used this to create the ID, but I am not sure what is default pattern is for this kind of situation.

Your assumption is correct, the prompty_type + language forms a unique pair that we can consider as the ID for each one of them.

The only thing I feel is missing from this PR is to add functionality on the deletePromptCustomText IMO ideally this should reset the body to {} instead of not doing anything. Wdyt?

@joaovieira-ca
Copy link
Contributor Author

Hi @sergiughf. Thanks, very happy to contribute to the provider that I am using :D
Thank you for the improvements.

your suggestion about the missing reset for the body is good. I have already made the changes.

Thank you for reviewing!

@joaovieira-ca
Copy link
Contributor Author

@sergiughf, just added on more change, see if my thinking is right:

func deletePromptCustomText(d *schema.ResourceData, m interface{}) error {
	d.Set("body", "{}")

	updatePromptCustomText(d, m)

	d.SetId("")
	return nil
}

What I did was add the updatePromptCustomText(d,m) after resetting the body. As we don't have an API for deletion, this {} clear the custom texts.

What do you think?

@sergiught
Copy link
Collaborator

Yeah that makes sense! Thanks @jluiz20! ⭐ I tested locally and everything seems to work, although could I ask you to please not ignore the error on the delete when you call updatePromptCustomText(d, m)? 🙏🏻

@joaovieira-ca
Copy link
Contributor Author

joaovieira-ca commented Jan 19, 2022

of course. My bad for ignoring the error. I am pretty new to golang 😬

@sergiught
Copy link
Collaborator

For being pretty new to golang you did a great job @jluiz20 🏆

Thanks a lot for the contribution, this is going to help a lot of folks!

I'll merge this first thing in the morning.

@sergiught sergiught force-pushed the added_auth0_prompt_custom_text branch from d847966 to d550a43 Compare January 20, 2022 08:50
@sergiught
Copy link
Collaborator

Hey @jluiz20 👋🏻 I added a new commit to improve the error handling on importing the custom text f017f3e and rebased to push the changelog for the new version.

@sergiught sergiught merged commit b0b1c5f into alexkappa:master Jan 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants