diff --git a/src/errors.rs b/src/errors.rs index 6706cc57..eb7b6dab 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -130,6 +130,9 @@ pub enum ServiceError { #[display(fmt = "Category already exists.")] CategoryAlreadyExists, + #[display(fmt = "Category name cannot be empty.")] + CategoryNameEmpty, + #[display(fmt = "Tag already exists.")] TagAlreadyExists, @@ -235,6 +238,7 @@ pub fn http_status_code_for_service_error(error: &ServiceError) -> StatusCode { ServiceError::CanonicalInfoHashAlreadyExists => StatusCode::BAD_REQUEST, ServiceError::TorrentTitleAlreadyExists => StatusCode::BAD_REQUEST, ServiceError::TrackerOffline => StatusCode::INTERNAL_SERVER_ERROR, + ServiceError::CategoryNameEmpty => StatusCode::BAD_REQUEST, ServiceError::CategoryAlreadyExists => StatusCode::BAD_REQUEST, ServiceError::TagAlreadyExists => StatusCode::BAD_REQUEST, ServiceError::InternalServerError => StatusCode::INTERNAL_SERVER_ERROR, diff --git a/src/services/category.rs b/src/services/category.rs index 548a2374..5abe8aa6 100644 --- a/src/services/category.rs +++ b/src/services/category.rs @@ -38,7 +38,13 @@ impl Service { return Err(ServiceError::Unauthorized); } - match self.category_repository.add(category_name).await { + let trimmed_name = category_name.trim(); + + if trimmed_name.is_empty() { + return Err(ServiceError::CategoryNameEmpty); + } + + match self.category_repository.add(trimmed_name).await { Ok(id) => Ok(id), Err(e) => match e { DatabaseError::CategoryAlreadyExists => Err(ServiceError::CategoryAlreadyExists), diff --git a/tests/e2e/web/api/v1/contexts/category/contract.rs b/tests/e2e/web/api/v1/contexts/category/contract.rs index 0ec559c9..84494ee9 100644 --- a/tests/e2e/web/api/v1/contexts/category/contract.rs +++ b/tests/e2e/web/api/v1/contexts/category/contract.rs @@ -104,32 +104,25 @@ async fn it_should_allow_admins_to_add_new_categories() { } #[tokio::test] -async fn it_should_allow_adding_empty_categories() { - // code-review: this is a bit weird, is it a intended behavior? - +async fn it_should_not_allow_adding_empty_categories() { let mut env = TestEnv::new(); env.start(api::Version::V1).await; - if env.is_shared() { - // This test cannot be run in a shared test env because it will fail - // when the empty category already exits - println!("Skipped"); - return; - } - let logged_in_admin = new_logged_in_admin(&env).await; let client = Client::authenticated(&env.server_socket_addr().unwrap(), &logged_in_admin.token); - let category_name = String::new(); + let invalid_category_names = vec![String::new(), " ".to_string()]; - let response = client - .add_category(AddCategoryForm { - name: category_name.to_string(), - icon: None, - }) - .await; + for invalid_name in invalid_category_names { + let response = client + .add_category(AddCategoryForm { + name: invalid_name, + icon: None, + }) + .await; - assert_added_category_response(&response, &category_name); + assert_eq!(response.status, 400); + } } #[tokio::test]