-
Notifications
You must be signed in to change notification settings - Fork 34
fix: Removed omitempty from tags in ServerlessUpdateRequestParams #509
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
This will allow for operators to remove all tags from Serverless Instances as tags will not be omitted when sending an empty array of tags.
TerminationProtectionEnabled *bool `json:"terminationProtectionEnabled,omitempty"` | ||
Tag []*Tag `json:"tags,omitempty"` | ||
Tag []*Tag `json:"tags"` | ||
} |
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.
Could you write a unit test for the condition you're looking to cover?
@wtrocki This approach is a convention across this client, so I think we're fine to go with this for consistency (also any other approach would likely introduce a breaking change) |
I understand that. Trick is that removing ommitempty has no impact unless (breaking) suggestion by @gssbzn is applied. |
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 after unit test covering this case and passing
} | ||
|
||
// Testing for removing tags | ||
client, mux, teardown = setup() |
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 you please group the tests using t.Run
for example
go-client-mongodb-atlas/mongodbatlas/organizations_test.go
Lines 28 to 173 in 774bf48
t.Run("default", func(t *testing.T) { | |
client, mux, teardown := setup() | |
defer teardown() | |
mux.HandleFunc("/api/atlas/v1.0/orgs", func(w http.ResponseWriter, r *http.Request) { | |
testMethod(t, r, http.MethodGet) | |
_, _ = fmt.Fprint(w, `{ | |
"links": [{ | |
"href": "https://cloud.mongodb.com/api/public/v1.0/orgs", | |
"rel": "self" | |
}], | |
"results": [{ | |
"id": "56a10a80e4b0fd3b9a9bb0c2", | |
"links": [{ | |
"href": "https://cloud.mongodb.com/api/public/v1.0/orgs/56a10a80e4b0fd3b9a9bb0c2", | |
"rel": "self" | |
}], | |
"name": "012i3091203jioawjioej" | |
}, { | |
"id": "56aa691ce4b0a0e8c4be51f7", | |
"links": [{ | |
"href": "https://cloud.mongodb.com/api/public/v1.0/orgs/56aa691ce4b0a0e8c4be51f7", | |
"rel": "self" | |
}], | |
"name": "1454008603036" | |
}], | |
"totalCount": 2 | |
}`) | |
}) | |
orgs, _, err := client.Organizations.List(ctx, nil) | |
if err != nil { | |
t.Fatalf("Organizations.List returned error: %v", err) | |
} | |
expected := &Organizations{ | |
Links: []*Link{ | |
{ | |
Href: "https://cloud.mongodb.com/api/public/v1.0/orgs", | |
Rel: "self", | |
}, | |
}, | |
Results: []*Organization{ | |
{ | |
ID: "56a10a80e4b0fd3b9a9bb0c2", | |
Links: []*Link{ | |
{ | |
Href: "https://cloud.mongodb.com/api/public/v1.0/orgs/56a10a80e4b0fd3b9a9bb0c2", | |
Rel: "self", | |
}, | |
}, | |
Name: "012i3091203jioawjioej", | |
}, | |
{ | |
ID: "56aa691ce4b0a0e8c4be51f7", | |
Links: []*Link{ | |
{ | |
Href: "https://cloud.mongodb.com/api/public/v1.0/orgs/56aa691ce4b0a0e8c4be51f7", | |
Rel: "self", | |
}, | |
}, | |
Name: "1454008603036", | |
}, | |
}, | |
TotalCount: 2, | |
} | |
if diff := deep.Equal(orgs, expected); diff != nil { | |
t.Error(diff) | |
} | |
}) | |
t.Run("by page number", func(t *testing.T) { | |
client, mux, teardown := setup() | |
defer teardown() | |
mux.HandleFunc("/api/atlas/v1.0/orgs", func(w http.ResponseWriter, r *http.Request) { | |
testMethod(t, r, http.MethodGet) | |
_, _ = fmt.Fprint(w, `{ | |
"links": [ | |
{ | |
"href": "https://cloud.mongodb.com/api/public/v1.0/orgs?pageNum=1&itemsPerPage=1", | |
"rel": "previous" | |
}, | |
{ | |
"href": "https://cloud.mongodb.com/api/public/v1.0/orgs?pageNum=2&itemsPerPage=1", | |
"rel": "self" | |
}, | |
{ | |
"href": "https://cloud.mongodb.com/api/public/v1.0/orgs?itemsPerPage=3&pageNum=2", | |
"rel": "next" | |
} | |
], | |
"results": [{ | |
"id": "56a10a80e4b0fd3b9a9bb0c2", | |
"links": [{ | |
"href": "https://cloud.mongodb.com/api/public/v1.0/orgs/56a10a80e4b0fd3b9a9bb0c2", | |
"rel": "self" | |
}], | |
"name": "FooBar" | |
}], | |
"totalCount": 3 | |
}`) | |
}) | |
opt := &OrganizationsListOptions{ListOptions: ListOptions{PageNum: 2}, Name: "FooBar"} | |
orgs, _, err := client.Organizations.List(ctx, opt) | |
if err != nil { | |
t.Fatalf("Organizations.List returned error: %v", err) | |
} | |
expected := &Organizations{ | |
Links: []*Link{ | |
{ | |
Href: "https://cloud.mongodb.com/api/public/v1.0/orgs?pageNum=1&itemsPerPage=1", | |
Rel: "previous", | |
}, | |
{ | |
Href: "https://cloud.mongodb.com/api/public/v1.0/orgs?pageNum=2&itemsPerPage=1", | |
Rel: "self", | |
}, | |
{ | |
Href: "https://cloud.mongodb.com/api/public/v1.0/orgs?itemsPerPage=3&pageNum=2", | |
Rel: "next", | |
}, | |
}, | |
Results: []*Organization{ | |
{ | |
ID: "56a10a80e4b0fd3b9a9bb0c2", | |
Links: []*Link{ | |
{ | |
Href: "https://cloud.mongodb.com/api/public/v1.0/orgs/56a10a80e4b0fd3b9a9bb0c2", | |
Rel: "self", | |
}, | |
}, | |
Name: "FooBar", | |
}, | |
}, | |
TotalCount: 3, | |
} | |
if diff := deep.Equal(orgs, expected); diff != nil { | |
t.Error(diff) | |
} | |
}) | |
} |
This will allow for operators to remove all tags from Serverless Instances as tags will not be omitted when sending an empty array of tags.
Description
Please include a summary of the fix/feature/change, including any relevant motivation and context.
Link to any related issue(s):
Type of change:
Required Checklist:
make fmt
and formatted my codeFurther comments
This is a minor change to allow AKO to remove all tags from a Serverless Instance