neatnik / omg.lol

Cool stuff for omg.lol
MIT License
365 stars 49 forks source link

[Bug] Consistent resource identification in API deisgn #534

Open wayneyaoo opened 1 year ago

wayneyaoo commented 1 year ago

Bug Description

When patching/updating some resources, some endpoints require id to be in the url while some require to be in the body.

e.g., update an existing a DNS record:

https://api.omg.lol/address/foobar/dns/2857074

update an existing status log:

https://api.omg.lol/address/adam/statuses/

with Id in the body.

It'd be much more consistent and predictable if we use the same way to identify a resource (independent of the operation). I personally prefer to put it in the url but other options are fine, as long as it's consistent across the system.

Steps to Reproduce

No response

newbold commented 1 year ago

Agreed. Will get this updated soon!

ejstreet commented 1 year ago

A bit more consistency with types would be appreciated also, for example GETing DNS records produces a list of records as:

"dns": [
    {
        "id": "2857074",
        "type": "A",
        "name": "foobar",
        "data": "192.167.0.1",
        "priority": null,
        "ttl": "3600",
        "created_at": "2022-11-26T04:30:13Z",
        "updated_at": "2022-11-26T04:31:33Z"
    }
]

Where the ID and TTL are returned as strings with a data value, but the response_received.data when POSTing returns ID and TTL as numbers with a content value:

"response_received": {
    "data": {
        "id": 2857074,
        "name": "foobar",
        "content": "192.167.0.1",
        "ttl": 3600,
        "priority": null,
        "type": "TXT",
        "created_at": "2022-11-26T04:30:13Z",
        "updated_at": "2022-11-26T04:31:33Z"
    }
}
wayneyaoo commented 1 year ago

@ejstreet agree. Also later on when doing the overhaul, wondering if we can simplify the DNS model a little bit. The data_sent and response_received feels like the server is sending stuff to an upstream service and directly replicate the data to the client lol. Just a bit redundant.

And yeah the data and content naming thing would look better if unified. Cant recall if I mentioned this in another issue.

edit: oh yes I did #543. Sorry for mixing things up. Let's track it there.