Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DXCDT-323: Allow updating an email template when it does not exist #611

Merged
merged 5 commits into from
Jan 18, 2023

Conversation

sergiught
Copy link
Contributor

@sergiught sergiught commented Jan 18, 2023

🔧 Changes

Previously when trying to update an email template with auth0 email templates update, if we were to select a template that does not yet exist, we would get back a 404 error. In this PR we're changing the behavior of the command to instead create the template if it does not yet exist improving the DX.

📚 References

🔬 Testing

📝 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)

@sergiught sergiught marked this pull request as ready for review January 18, 2023 11:18
@sergiught sergiught requested a review from a team as a code owner January 18, 2023 11:18
@sergiught sergiught requested a review from a team January 18, 2023 13:07
@sergiught sergiught requested review from a team and Widcket January 18, 2023 16:46
Comment on lines +207 to +211
oldTemplate = &management.EmailTemplate{
From: auth0.String(""),
Subject: auth0.String(""),
Enabled: auth0.Bool(false),
Syntax: auth0.String("liquid"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Is it necessary to instantiate with empty values here if the emailTemplate gets fully constructed below?

Copy link
Contributor Author

@sergiught sergiught Jan 18, 2023

Choose a reason for hiding this comment

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

Sadly yes 😞 as otherwise this line https://github.com/auth0/auth0-cli/pull/611/files#diff-7c49d78e329a015986302ea95a143d68ca7d933a41514ca964b46bddcd22f31aR215 will panic when accessing oldTemplate.From. We can't replace it with the safe access getter GetFrom due to all the prompts using pointers for default values. That should be refactored in a separate dedicated PR IMO.

Copy link
Contributor

@willvedd willvedd left a comment

Choose a reason for hiding this comment

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

Only one small question about instantiating the email template. If possible, would be nice to eliminate. Otherwise lgtm 👍

@sergiught sergiught merged commit 7184197 into v1 Jan 18, 2023
@sergiught sergiught deleted the patch/DXCDT-323-email-templates-update-fix branch January 18, 2023 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants