Zone Record Regions #47
Conversation
jodosha
added
the
ready-for-review
| @@ -90,28 +93,37 @@ func TestZonesService_CreateRecord(t *testing.T) { | |||
| testMethod(t, r, "POST") | |||
| testHeaders(t, r) | |||
|
|
|||
| want := map[string]interface{}{"name": "foo", "content": "192.168.0.10", "type": "A"} | |||
| testRequestJSON(t, r, want) | |||
| // --- FAIL: TestZonesService_CreateRecord (0.00s) | |||
There was a problem hiding this comment.
Let me download and try it locally.
There was a problem hiding this comment.
@jodosha it's a type issue. I was able to reproduce it
➜ dnsimple-go git:(zone-record-regions) go test ./...
--- FAIL: TestZonesService_CreateRecord (0.00s)
dnsimple_test.go:68: Request parameters = map[type:MX name: content:mxa.example.com regions:[SV1 IAD]], want map[content:mxa.example.com type:MX regions:[SV1 IAD]]
FAIL
FAIL github.com/dnsimple/dnsimple-go/dnsimple 0.072s
ok github.com/dnsimple/dnsimple-go/dnsimple/webhook 0.020s
changing the format of the response from %v to %#v revealed the issue
--- FAIL: TestZonesService_CreateRecord (0.00s)
dnsimple_test.go:68: Request parameters = map[string]interface {}{"type":"MX", "name":"", "content":"mxa.example.com", "regions":[]interface {}{"SV1", "IAD"}}, want map[string]interface {}{"content":"mxa.example.com", "type":"MX", "regions":[]string{"SV1", "IAD"}}
FAIL
FAIL github.com/dnsimple/dnsimple-go/dnsimple 0.064s
ok github.com/dnsimple/dnsimple-go/dnsimple/webhook 0.015s
| ZoneID string `json:"zone_id,omitempty"` | ||
| ParentID int `json:"parent_id,omitempty"` | ||
| Type string `json:"type,omitempty"` | ||
| Name string `json:"name",omitempty` |
There was a problem hiding this comment.
@jodosha why did you change name to add omitempty?
There was a problem hiding this comment.
Also note the sintax is incorrect, it should be
`json:"name,omitempty"`
not
`json:"name",omitempty`
However, that should not be changed.
There was a problem hiding this comment.
Because it is mandatory for record creation, but not for its update.
There was a problem hiding this comment.
Sadly, we have no way to specify this different today. It's an open bug #33
Note that if you set omitempty, it will cause more issues as "" is the zero string value in Go, but it's also a valid value for a root domain.
There is no easy solution today, if not switching to pointers. But it will require a larger refactoring.
I also split the region tests from the main test, and copied the same specs to both create and update. I prefer to have duplicates tests, than testing different cases in different methods (e.g. "global" in update and array in created, because it is very fragile. if we refactor a single endpoint we risk to lose half the tests).
|
@weppos cool, thank you. |
@weppos @aeden Please review. Thanks!