-
Notifications
You must be signed in to change notification settings - Fork 24
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
fix(recipients): dynamic block bug #444
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great and a deep dive into the Framework without a ton of examples available. Thank you! ✨
I'm generally torn on if we should have these expand/flatten/mutate functions returning (value, diag)
or passing it in: perhaps Hashi will provide some idiomatic guidance when they get their examples updated. 🤔
A few nit-ish suggestions, but all are non-blocking.
detailsObj := map[string]attr.Value{"pagerduty_severity": types.StringValue(s)} | ||
return []attr.Value{types.ObjectValueMust(models.NotificationRecipientDetailsAttrTypes, detailsObj)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
detailsObj := map[string]attr.Value{"pagerduty_severity": types.StringValue(s)} | |
return []attr.Value{types.ObjectValueMust(models.NotificationRecipientDetailsAttrTypes, detailsObj)} | |
return []attr.Value{types.ObjectValueMust(models.NotificationRecipientDetailsAttrTypes, | |
map[string]attr.Value{"pagerduty_severity": types.StringValue(s)})} |
} | ||
|
||
type NotificationRecipientDetailsModel struct { | ||
PDSeverity types.String `tfsdk:"pagerduty_severity"` | ||
} | ||
|
||
var NotificationRecipientAttrTypes = map[string]attr.Type{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit-ish, but I think this should be singular
var NotificationRecipientAttrTypes = map[string]attr.Type{ | |
var NotificationRecipientAttrType = map[string]attr.Type{ |
"notification_details": types.ListType{ElemType: types.ObjectType{AttrTypes: NotificationRecipientDetailsAttrTypes}}, | ||
} | ||
|
||
var NotificationRecipientDetailsAttrTypes = map[string]attr.Type{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
var NotificationRecipientDetailsAttrTypes = map[string]attr.Type{ | |
var NotificationRecipientDetailsAttrType = map[string]attr.Type{ |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #444 +/- ##
==========================================
+ Coverage 83.21% 83.28% +0.06%
==========================================
Files 62 62
Lines 4170 4235 +65
==========================================
+ Hits 3470 3527 +57
- Misses 547 551 +4
- Partials 153 157 +4 ☔ View full report in Codecov by Sentry. |
Which problem is this PR solving?
recipient
"Value Conversion Error" #434Short description of the changes
Migrates
NotificationRecipients
totypes.Set
andNotificationRecipientDetails
totypes.List
instead of slices.