-
Notifications
You must be signed in to change notification settings - Fork 190
fix: Fixes JSON comparison in audit_filter
field in mongodbatlas_auditing
#3302
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
Conversation
APIx bot: a message has been sent to Docs Slack channel |
@@ -150,3 +152,7 @@ func resourceDelete(ctx context.Context, d *schema.ResourceData, meta any) diag. | |||
d.SetId("") | |||
return nil | |||
} | |||
|
|||
func diffSuppressJSON(k, old, newStr string, d *schema.ResourceData) bool { |
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.
can we update/add an acc test that would fail before this change?
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.
done in 1d4e78a
Also, you can see failing tests if there is no DiffSuppressFunc: diffSuppressJSON,
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.
thanks!
@@ -31,10 +40,15 @@ func TestAccGenericAuditing_basic(t *testing.T) { | |||
{ | |||
Config: configBasic(projectID, auditFilter, true, true), | |||
Check: resource.ComposeAggregateTestCheckFunc(checks(auditFilter, true, true)...), | |||
ConfigPlanChecks: resource.ConfigPlanChecks{ |
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] I guess this explicit check is necessary because it is a SDKv2 resource? Maybe add a comment why
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.
not really necessary now that I think about it. Removed in 76582ca
@@ -89,18 +103,23 @@ func importStateIDFunc(resourceName string) resource.ImportStateIdFunc { | |||
} | |||
|
|||
func configBasic(projectID, auditFilter string, auditAuth, enabled bool) string { | |||
filterValue := fmt.Sprintf("%q", auditFilter) | |||
if auditFilter != "{}" { |
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] use previous var? emptyAuditFilter
?
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.
thanks, done in 40750dc
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.
LGTM
Description
Fixes JSON comparison in
audit_filter
field inmongodbatlas_auditing
Link to any related issue(s): CLOUDP-315476 & HELP-74398
Type of change:
Required Checklist:
Further comments