Add new DNS analytics endpoint to OpenAPI spec #558
Conversation
content/v2/openapi.yml
Outdated
| enum: | ||
| - 'date:asc' | ||
| - 'date:desc' | ||
| - 'volume:asc' | ||
| - 'volume:desc' | ||
| - 'zone_name:asc' | ||
| - 'zone_name:desc' |
There was a problem hiding this comment.
Multiple sorting criteria can be defined. In fact, the default is "date:asc,zone_name:asc". Can we just not provide the enum here?
There was a problem hiding this comment.
I just followed the example of other endpoints in which we are providing the enum, even when the values can be combined. Not sure if I did the right thing.
There was a problem hiding this comment.
I'm not sure how to interpret enum either...
There was a problem hiding this comment.
I think that we're not using them correctly if values can be combined https://swagger.io/docs/specification/data-models/enums/
It sounds like we would need to list all permutations under all possible sorting, which feels wrong to do. Maybe we should not have enum unless there's a finite list of values our customers need to pick from.
There was a problem hiding this comment.
I was looking at the same thing and I agree: I do not think this is the correct use. Will update this momentarily.
Co-authored-by: Guillermo Gutiérrez <[email protected]>
Co-authored-by: Guillermo Gutiérrez <[email protected]>
There was a problem hiding this comment.
👍 You could run it into https://editor.swagger.io/ to validate.
I did, and apparently we have an issue with another endpoint but nothing was raised with the new changes.
Closes dnsimple/dnsimple-app#25676
This PR:
Note
This is my first time updating the OpenAPI spec so expect rough edges.
Note that I chose not to extract much to be referenced and write the spec pretty much in line: I do not think much of what we have in this endpoint is currently reusable.