Open pastewka opened 3 months ago
Case: I think we should use snake_case
in the whole code, including JS. (BokehJS is for example snake case .)
Reporting API endpoint: url
is fine, api_endpoint_url
would be better (but is longer). Let's stick to url
.
Granularity: Needs to be decided on a case by case basis. For properties, we can probably get rid of the dedicated route and represent this as a dictionary.
Query parameters: Avoid query parameters to configure what a route returns. Same route should always and predictively return the same dictionary. (My guess is that otherwise generators for client code based on Open API specifications may have issues.) Query parameters configure/restrict queries.
@IoannisNezis Any thoughts on the above?
Granularity: Needs to be decided on a case by case basis. For properties, we can probably get rid of the dedicated route and represent this as a dictionary.
After further thinking about this, properties are really completely independent of the core topobank functionality and could be refactored into their own app. In that case, topobank manager
would not even need to know they existed. This would speak for keeping the current design.
I think we should prioritize modularity over everything else, i.e. refactor things into their own apps to the extend possible.
Describe the issue The current API has grown organically based on need and naivety. There are some inconsistencies that need to be sorted out.
Additional context Items where @IoannisNezis and @pastewka started a discussion:
camelCase
vssnake_case
in API. Currentlysnake_case
because Python is, but JS iscamelCase
. This leads to strange-looking frontend code.url
vsuri
as key for reporting API endpoints. I now thinkurl
(which we use currently) is correct, asuri
is to generic (endpoints are actually URLs).Surface
API. How do we then add individual properties? A PATCH request could just add the properties provided, while a PUT request overrides all properties (and deletes the one that were removed). Note that this is how tags are handled now.Surface
allows to specifychildren=yes
as a query parameters, which then returns all topographies belonging to this surface. I wonder whether a better (and cleaner) design would not be to require an additional hit to theTopography
route with the surface as a query parameters that then would return all topographies belonging to this surface.