Skip to content

Commit

Permalink
feat!: [torrust#289] do not allow empty tag names
Browse files Browse the repository at this point in the history
      It was allowed to use an empty strings like "" or " " for a tag name.

      From now on, it's not allowed.

      If there are some empty names in the database, they are not renamed.

      A migration to rename empty names was not added because there can be more than one tag, for example:

      - ""
      - " "
      - "  "
      - Etcetera

      We could have generated names like "no tag 1", "no tag 2", but it's not likely that admins have created empty tags.
  • Loading branch information
josecelano committed Sep 14, 2023
1 parent 15ac77d commit 01a7d2e
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 7 deletions.
4 changes: 4 additions & 0 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ pub enum ServiceError {
#[display(fmt = "Tag already exists.")]
TagAlreadyExists,

#[display(fmt = "Tag name cannot be empty.")]
TagNameEmpty,

#[display(fmt = "Category not found.")]
CategoryNotFound,

Expand Down Expand Up @@ -240,6 +243,7 @@ pub fn http_status_code_for_service_error(error: &ServiceError) -> StatusCode {
ServiceError::TrackerOffline => StatusCode::INTERNAL_SERVER_ERROR,
ServiceError::CategoryNameEmpty => StatusCode::BAD_REQUEST,
ServiceError::CategoryAlreadyExists => StatusCode::BAD_REQUEST,
ServiceError::TagNameEmpty => StatusCode::BAD_REQUEST,
ServiceError::TagAlreadyExists => StatusCode::BAD_REQUEST,
ServiceError::InternalServerError => StatusCode::INTERNAL_SERVER_ERROR,
ServiceError::EmailMissing => StatusCode::NOT_FOUND,
Expand Down
8 changes: 7 additions & 1 deletion src/services/tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,13 @@ impl Service {
return Err(ServiceError::Unauthorized);
}

match self.tag_repository.add(tag_name).await {
let trimmed_name = tag_name.trim();

if trimmed_name.is_empty() {
return Err(ServiceError::TagNameEmpty);
}

match self.tag_repository.add(trimmed_name).await {
Ok(()) => Ok(()),
Err(e) => match e {
DatabaseError::TagAlreadyExists => Err(ServiceError::TagAlreadyExists),
Expand Down
14 changes: 8 additions & 6 deletions tests/e2e/web/api/v1/contexts/tag/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,17 @@ async fn it_should_allow_adding_duplicated_tags() {
}

#[tokio::test]
async fn it_should_allow_adding_a_tag_with_an_empty_name() {
// code-review: is this an intended behavior?

async fn it_should_not_allow_adding_a_tag_with_an_empty_name() {
let mut env = TestEnv::new();
env.start(api::Version::V1).await;

let empty_tag_name = String::new();
let response = add_tag(&empty_tag_name, &env).await;
assert_eq!(response.status, 200);
let invalid_tag_names = vec![String::new(), " ".to_string()];

for invalid_name in invalid_tag_names {
let response = add_tag(&invalid_name, &env).await;

assert_eq!(response.status, 400);
}
}

#[tokio::test]
Expand Down

0 comments on commit 01a7d2e

Please sign in to comment.