GH-853: Allow deleting branding resource even on free tenants #875
Conversation
| if err := checkForCustomDomains(ctx, api); err != nil { | ||
| if err == errNoCustomDomain { | ||
| return nil | ||
| if err := checkForCustomDomains(ctx, api); err == nil { |
There was a problem hiding this comment.
This is now aligned with the read logic found here
where we skip reading the universal login template body if no custom domains are set on the tenant or if the tenant is a free tier tenant.
| } | ||
|
|
||
| if err := api.Branding.DeleteUniversalLogin(ctx); err != nil { | ||
| return diag.FromErr(err) | ||
| } | ||
|
|
||
| return nil |
There was a problem hiding this comment.
Since you're inverting the if/else, shouldn't you be returning err?
There was a problem hiding this comment.
If we get no error it means we're not a free tier tenant and we have the ability to delete the universal login template data, otherwise we can't, so we shouldn't return the error in that case but allow for a successful deletion of the resource.
Please also check above: #875 (comment)
There was a problem hiding this comment.
Update: Ok with ignoring the errors for now because they would be unlikely and it is too difficult to identify the specific errors that we would want to keep or ignore. Especially provided that this is a deletion operation.
🔧 Changes
This PR changes the logic within the delete func of the branding resource to allow free tier tenants to delete the resource as well. Free tier tenants won't have custom domains functionality enabled and at the moment we check if any are set on the tenant to be able to remove the universal login page template.
📚 References
🔬 Testing
📝 Checklist