Implement expires_at in Domain struct #98
Conversation
We deprecated expires_on attribute in our api and in docs in favor of expires_at. This commit introduces the new expires_at attribute. Also updated the fixtures with the new field and removing the old premium_price from Domain Transfer/Register/Renewal.
duduribeiro
added
enhancement
duduribeiro
removed
the
ready-for-review
|
We are trying to implement the same thing we implemented for the ruby one: dnsimple/dnsimple-ruby#186 (stoping using the expiresOn attribute from the json response, and makes the expiresOn to be set based on the expiresAt value) I still don't know if this is the best way to set expiresOn attribute in the struct. What I did:
|
duduribeiro
added
the
ready-for-review
|
Followed with @onlyhavecans proposed. Leave expires_on parsing the field and leave the deprecation note. |
There was a problem hiding this comment.
To clarify my position on this ticket that was mentioned in a prior comment; I believe the client should reflect the behavior of the API, not hot-patch the API's behavior.
The original version of this PR was splitting off providing a language-based abstraction of our API behavior and now writing in "magic" behavior only for consumers of this and future version. I believe that if we want our API to return the result of expires_at in the expires_on field as part of deprecation we should do this for everyone who accesses the API by modifying the return of in the API itself, not in select clients.
dnsimple/domains_test.go
Outdated
| t.Fatalf("Domains.ListDomains() returned ID expected to be `%v`, got `%v`", want, got) | ||
| } | ||
| if want, got := "example-alpha.com", domains[0].Name; want != got { | ||
| t.Fatalf("Domains.ListDomains() returned Name expected to be `%v`, got `%v`", want, got) | ||
| } | ||
|
|
||
| if want, got := "2021-06-05", domains[0].ExpiresOn; want != got { |
There was a problem hiding this comment.
@onlyhavecans @DXTimer @duduribeiro I see two different attributes referenced in the same spec (aka the message doesn't match the code). Is that on purpose?
There was a problem hiding this comment.
No, my fault!. should be expiresOn.
@onlyhavecans this is unfortunately not always possible, unless we provision a new version of the API on every change. Clients are the only way we have right now to reliably enforce/expose deprecations. |
We deprecated expires_on attribute in our api and in docs in favor of
expires_at. This commit introduces the new expires_at attribute.
Also updated the fixtures with the new field and removing the old
premium_price from Domain Transfer/Register/Renewal.