Add unimplemented API methods #35
Conversation
Patched a seaorm autogenerated file
Patch seaorm autogenerated file
| pub visible: Option<bool>, | ||
| } | ||
|
|
||
| pub struct APIResponse<R>(pub Status, pub Option<R>); |
There was a problem hiding this comment.
Why implement APIResponse twice?
| Err(_) => Err(Status::BadRequest), | ||
| } | ||
| } | ||
|
|
||
| #[get("/server/<id_type>/<id>")] |
There was a problem hiding this comment.
move this function to server.rs and delete get.res so its consistent with the other route files
| #[get("/builders")] | ||
| pub async fn builders_get_all( | ||
| conn: Connection<'_, Db>, | ||
| ) -> Result<Json<Vec<plotsystem_builders::Model>>, APIResponse<String>> { |
There was a problem hiding this comment.
Why not return an APIResponse in both cases (like in function builders_get) for more code consistency
| #[get("/city_projects")] | ||
| pub async fn city_get_all( | ||
| conn: Connection<'_, Db>, | ||
| ) -> Result<Json<Vec<plotsystem_city_projects::Model>>, Status> { |
There was a problem hiding this comment.
Consider returning APIResponses here aswell
| if (builder.is_none()) { | ||
| Ok(APIResponse(Status::NotFound, None)) | ||
| } else { | ||
| Ok(APIResponse(Status::Ok, Some(Json(builder.unwrap())))) | ||
| } |
There was a problem hiding this comment.
Use match syntax here instead.
| if (builder.is_none()) { | |
| Ok(APIResponse(Status::NotFound, None)) | |
| } else { | |
| Ok(APIResponse(Status::Ok, Some(Json(builder.unwrap())))) | |
| } | |
| match builder { | |
| Some(b) => { | |
| Ok(APIResponse(Status::Ok, Some(Json(b.unwrap())))) | |
| }, | |
| None => { | |
| Ok(APIResponse(Status::NotFound, None)) | |
| } | |
| } |
| match db_get::builder::by_uuid(db, parsed_uuid.unwrap()).await { | ||
| Ok(builder) => { | ||
| if (builder.is_none()) { | ||
| Ok(APIResponse(Status::NotFound, None)) |
There was a problem hiding this comment.
An unsucessful API request shouldnt return the Ok() Variant. Return an error instead.
| Ok(APIResponse(Status::NotFound, None)) | |
| Err(APIResponse(Status::NotFound, None)) |
| // all modify request values are optional; if one is omitted, don't change it | ||
| #[derive(Deserialize)] | ||
| pub struct EditCityJson { | ||
| pub country_id: Option<i32>, | ||
| pub name: Option<String>, | ||
| pub description: Option<String>, | ||
| pub visible: Option<bool>, | ||
| } |
There was a problem hiding this comment.
Maybe this can be merged with the other struct somehow .
| if !country { | ||
| return Err(APIResponse( | ||
| Status::Unauthorized, | ||
| Some("You don't have access to this country.".to_owned()), | ||
| )); | ||
| } |
There was a problem hiding this comment.
not neccesarily but maybe replace this with the match syntax
| if !city_json.country_id.is_none() { | ||
| city.country_id = Set(city_json.country_id.unwrap().to_owned()); | ||
| modified = true; | ||
| } | ||
|
|
||
| if !city_json.name.is_none() { | ||
| city.name = Set(city_json.name.as_ref().unwrap().to_owned()); | ||
| modified = true; | ||
| } | ||
|
|
||
| if !city_json.description.is_none() { | ||
| city.description = Set(city_json.description.as_ref().unwrap().to_owned()); | ||
| modified = true; | ||
| } | ||
|
|
||
| if !city_json.visible.is_none() { | ||
| city.visible = Set(city_json.visible.unwrap().to_owned()); | ||
| modified = true; | ||
| } |
There was a problem hiding this comment.
Use match syntax for Option<T> and Result<T, Err> types
| if !city_json.country_id.is_none() { | ||
| match db_get::api_keys::country_related_to_api_key( | ||
| db, | ||
| &api_key, | ||
| city_json.country_id.unwrap(), | ||
| ) | ||
| .await | ||
| { | ||
| Ok(country) => { | ||
| if !country { | ||
| return Err(APIResponse( | ||
| Status::Unauthorized, | ||
| Some("You don't have access to this country.".to_owned()), | ||
| )); | ||
| } | ||
| } | ||
| Err(e) => return Err(APIResponse(Status::BadRequest, Some(e.to_string()))), | ||
| }; | ||
| } |
|
@jokil123 grahhnt is currently on LOA till the end of the month so it could take a while till he has time to take a look at it again. I will try to find out if anyone else knows Rust and has interest in helping out. |
No description provided.