feat: get config-versions api #248
Conversation
|
Can you please add the query param info as well in the description |
| async fn get( | ||
| db_conn: DbConnection, | ||
| filters: Query<QueryFilters>, | ||
| ) -> superposition::Result<HttpResponse> { |
There was a problem hiding this comment.
Instead of HttpResponse , Can you change it to a type as SnapshotResponse or something?
There was a problem hiding this comment.
Yeah can be done but the response now is
"total_pages": total_pages, "total_items": n_version, "data": config_versions
but as pagination is/will be there for so many APIs
Maybe something like this can be done
`enum DataType<A, B, C> {
TypeA(Vec),
TypeB(Vec),
TypeC(Vec),
}
struct PaginatedResponse<A, B, C> {
total_pages: i64,
total_items: i64,
data: DataType<A, B, C>,
}`
Opinions?
There was a problem hiding this comment.
Yeah , that looks a better generic solution to me , we can do that.
There was a problem hiding this comment.
@mahatoankitkumar @sauraww you don't need an enum for this
the PaginationResponse should be a generic type capable of returning any paginated response for any data type, an enum should be needed for it, as in case of Enum it would lead to unnecessary boilerplate code as well
simply doing this should be enough:
pub struct PaginatedResponse<T> {
pub total_pages: i64,
pub total_items: i64,
data: T,
}
There was a problem hiding this comment.
One more addition to this, T should be deserializable, which can be done by struct PaginatedResponse<T: DeserializeOwned>.
| async fn get( | ||
| db_conn: DbConnection, | ||
| filters: Query<QueryFilters>, | ||
| ) -> superposition::Result<HttpResponse> { |
There was a problem hiding this comment.
Yeah , that looks a better generic solution to me , we can do that.
There was a problem hiding this comment.
Can we sync one single name for this, frontend we are referring it as Snapshot here Config Version?
There was a problem hiding this comment.
Yeah @JamesGeorg we have decided to go with config-versions, as this is not a complete representation of what snapshot is.
mahatoankitkumar
force-pushed
the
feat/get-snapshots
branch
from
6594ee1
to
25eb4c6
Compare
| } | ||
|
|
||
| #[derive(Serialize, Debug, Clone, Deserialize)] | ||
| pub struct PaginatedResponse<T> { |
There was a problem hiding this comment.
should we move this to superposition_types in some new file for such common utility types as it might be useful for experimentation as well for list experiments
Problem
get config-versions api
API changes
{ count: Option<i64>, page: Option<i64> }{"total_pages": total_pages, "total_items": n_version, "data": config_versions }