fix: use 409 when deleting destination/templates already in use (#4294)

Fixes #4291 

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit


- **New Features**
- Introduced a new method for handling 409 Conflict responses in the
HTTP responses, improving error messaging clarity.

- **Bug Fixes**
- Updated error handling to return conflict responses instead of
forbidden responses when certain delete operations fail due to resource
conflicts.
- Enhanced clarity in user membership responses by changing the error
message when a user is already part of an organization.

These changes improve user experience by providing clearer and more
accurate feedback on error conditions related to resource management.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Subhra264
2024-08-26 11:57:10 +05:30
committed by GitHub
parent 5532b9a570
commit 13cbabd528
7 changed files with 17 additions and 10 deletions
+7
View File
@@ -102,6 +102,13 @@ impl HttpResponse {
.json(Self::error(StatusCode::FORBIDDEN.into(), error.to_string()))
}
/// Send a Forbidden response in json format and associate the
/// provided error as `error` field.
pub fn conflict(error: impl ToString) -> ActixHttpResponse {
ActixHttpResponse::Conflict()
.json(Self::error(StatusCode::CONFLICT.into(), error.to_string()))
}
/// Send a NotFound response in json format and associate the
/// provided error as `error` field.
pub fn not_found(error: impl ToString) -> ActixHttpResponse {
@@ -183,7 +183,7 @@ async fn list_destinations(
),
responses(
(status = 200, description = "Success", content_type = "application/json", body = HttpResponse),
(status = 403, description = "Forbidden", content_type = "application/json", body = HttpResponse),
(status = 409, description = "Conflict", content_type = "application/json", body = HttpResponse),
(status = 404, description = "NotFound", content_type = "application/json", body = HttpResponse),
(status = 500, description = "Failure", content_type = "application/json", body = HttpResponse),
)
@@ -194,7 +194,7 @@ async fn delete_destination(path: web::Path<(String, String)>) -> Result<HttpRes
match destinations::delete(&org_id, &name).await {
Ok(_) => Ok(MetaHttpResponse::ok("Alert destination deleted")),
Err(e) => match e {
(http::StatusCode::FORBIDDEN, e) => Ok(MetaHttpResponse::forbidden(e)),
(http::StatusCode::CONFLICT, e) => Ok(MetaHttpResponse::conflict(e)),
(http::StatusCode::NOT_FOUND, e) => Ok(MetaHttpResponse::not_found(e)),
(_, e) => Ok(MetaHttpResponse::internal_error(e)),
},
+2 -2
View File
@@ -174,7 +174,7 @@ async fn list_templates(path: web::Path<String>, _req: HttpRequest) -> Result<Ht
),
responses(
(status = 200, description = "Success", content_type = "application/json", body = HttpResponse),
(status = 403, description = "Forbidden", content_type = "application/json", body = HttpResponse),
(status = 409, description = "Conflict", content_type = "application/json", body = HttpResponse),
(status = 404, description = "NotFound", content_type = "application/json", body = HttpResponse),
(status = 500, description = "Failure", content_type = "application/json", body = HttpResponse),
)
@@ -185,7 +185,7 @@ async fn delete_template(path: web::Path<(String, String)>) -> Result<HttpRespon
match templates::delete(&org_id, &name).await {
Ok(_) => Ok(MetaHttpResponse::ok("Alert template deleted")),
Err(e) => match e {
(http::StatusCode::FORBIDDEN, e) => Ok(MetaHttpResponse::forbidden(e)),
(http::StatusCode::CONFLICT, e) => Ok(MetaHttpResponse::conflict(e)),
(http::StatusCode::NOT_FOUND, e) => Ok(MetaHttpResponse::not_found(e)),
(_, e) => Ok(MetaHttpResponse::internal_error(e)),
},
@@ -202,7 +202,7 @@ async fn delete_report(path: web::Path<(String, String)>) -> Result<HttpResponse
match reports::delete(&org_id, &name).await {
Ok(_) => Ok(MetaHttpResponse::ok("Report deleted")),
Err(e) => match e {
(http::StatusCode::FORBIDDEN, e) => Ok(MetaHttpResponse::forbidden(e)),
(http::StatusCode::CONFLICT, e) => Ok(MetaHttpResponse::conflict(e)),
(http::StatusCode::NOT_FOUND, e) => Ok(MetaHttpResponse::not_found(e)),
(_, e) => Ok(MetaHttpResponse::internal_error(e)),
},
+1 -1
View File
@@ -173,7 +173,7 @@ pub async fn delete(org_id: &str, name: &str) -> Result<(), (http::StatusCode, a
for alert in alerts.iter() {
if stream_key.starts_with(org_id) && alert.destinations.contains(&name.to_string()) {
return Err((
http::StatusCode::FORBIDDEN,
http::StatusCode::CONFLICT,
anyhow::anyhow!("Alert destination is in use for alert {}", alert.name),
));
}
+1 -1
View File
@@ -111,7 +111,7 @@ pub async fn delete(org_id: &str, name: &str) -> Result<(), (http::StatusCode, a
for dest in ALERTS_DESTINATIONS.iter() {
if dest.key().starts_with(org_id) && dest.value().template.eq(&name) {
return Err((
http::StatusCode::FORBIDDEN,
http::StatusCode::CONFLICT,
anyhow::anyhow!(
"Alert template is in use for destination {}",
&dest.value().name.clone()
+3 -3
View File
@@ -397,9 +397,9 @@ pub async fn add_user_to_org(
for org in orgs.iter() {
if org.name.eq(org_id) {
// External user is already part of this org
return Ok(HttpResponse::Forbidden().json(MetaHttpResponse::error(
http::StatusCode::FORBIDDEN.into(),
"Not Allowed".to_string(),
return Ok(HttpResponse::Conflict().json(MetaHttpResponse::error(
http::StatusCode::CONFLICT.into(),
"User is already part of the org".to_string(),
)));
}
}