feat!: [torrust#289] do not allow duplicate tags

Duplicate tag names were allowed. This introduce unique constrains.
If there were duyplicate tags they are updated appeding the id as a suffix.
For example, having two tags:

```json
[
  {
    tag_id: 1,
    name: "fractal"
  },
  {
    tag_id: 2,
    name: "fractal"
  }
]
```

They would be renamed to:

```json
[
  {
    tag_id: 1,
    name: "fractal_1"
  },
  {
    tag_id: 2,
    name: "fractal_2"
  }
]
```

From now on, duplicate tags are not allowed.

migrations/mysql/20230914155441_torrust_no_duplicate_tags.sql

@@ -0,0 +1,12 @@
+-- Step 1 & 2: Identify and update the duplicate names
+UPDATE torrust_torrent_tags
+JOIN (
+    SELECT name
+    FROM torrust_torrent_tags
+    GROUP BY name
+    HAVING COUNT(*) > 1
+) AS DuplicateNames ON torrust_torrent_tags.name = DuplicateNames.name
+SET torrust_torrent_tags.name = CONCAT(torrust_torrent_tags.name, '_', torrust_torrent_tags.tag_id);
+
+-- Step 3: Add the UNIQUE constraint to the name column
+ALTER TABLE torrust_torrent_tags ADD UNIQUE (name);

migrations/sqlite3/20230914155441_torrust_no_duplicate_tags.sql

@@ -0,0 +1,13 @@
+-- Step 1: Identify and update the duplicate names
+WITH DuplicateNames AS (
+    SELECT name
+    FROM torrust_torrent_tags
+    GROUP BY name
+    HAVING COUNT(*) > 1
+)
+UPDATE torrust_torrent_tags
+SET name = name || '_' || tag_id
+WHERE name IN (SELECT name FROM DuplicateNames);
+
+-- Step 2: Create a UNIQUE index on the name column
+CREATE UNIQUE INDEX idx_unique_name ON torrust_torrent_tags(name);

src/databases/database.rs

@@ -287,7 +287,7 @@ pub trait Database: Sync + Send {
     async fn update_torrent_category(&self, torrent_id: i64, category_id: CategoryId) -> Result<(), Error>;
 
     /// Add a new tag.
-    async fn add_tag(&self, name: &str) -> Result<(), Error>;
+    async fn insert_tag_and_get_id(&self, name: &str) -> Result<i64, Error>;
 
     /// Delete a tag.
     async fn delete_tag(&self, tag_id: TagId) -> Result<(), Error>;

src/databases/mysql.rs

@@ -804,13 +804,23 @@ impl Database for Mysql {
             })
     }
 
-    async fn add_tag(&self, name: &str) -> Result<(), database::Error> {
+    async fn insert_tag_and_get_id(&self, name: &str) -> Result<i64, database::Error> {
         query("INSERT INTO torrust_torrent_tags (name) VALUES (?)")
             .bind(name)
             .execute(&self.pool)
             .await
-            .map(|_| ())
-            .map_err(|err| database::Error::ErrorWithText(err.to_string()))
+            .map(|v| i64::try_from(v.last_insert_id()).expect("last ID is larger than i64"))
+            .map_err(|e| match e {
+                sqlx::Error::Database(err) => {
+                    log::error!("DB error: {:?}", err);
+                    if err.message().contains("Duplicate entry") && err.message().contains("name") {
+                        database::Error::TagAlreadyExists
+                    } else {
+                        database::Error::Error
+                    }
+                }
+                _ => database::Error::Error,
+            })
     }
 
     async fn delete_tag(&self, tag_id: TagId) -> Result<(), database::Error> {

src/databases/sqlite.rs

@@ -794,13 +794,23 @@ impl Database for Sqlite {
             })
     }
 
-    async fn add_tag(&self, name: &str) -> Result<(), database::Error> {
+    async fn insert_tag_and_get_id(&self, tag_name: &str) -> Result<i64, database::Error> {
         query("INSERT INTO torrust_torrent_tags (name) VALUES (?)")
-            .bind(name)
+            .bind(tag_name)
             .execute(&self.pool)
             .await
-            .map(|_| ())
-            .map_err(|err| database::Error::ErrorWithText(err.to_string()))
+            .map(|v| v.last_insert_rowid())
+            .map_err(|e| match e {
+                sqlx::Error::Database(err) => {
+                    log::error!("DB error: {:?}", err);
+                    if err.message().contains("UNIQUE") && err.message().contains("name") {
+                        database::Error::TagAlreadyExists
+                    } else {
+                        database::Error::Error
+                    }
+                }
+                _ => database::Error::Error,
+            })
     }
 
     async fn delete_tag(&self, tag_id: TagId) -> Result<(), database::Error> {

src/services/tag.rs

@@ -29,7 +29,7 @@ impl Service {
     ///
     /// * The user does not have the required permissions.
     /// * There is a database error.
-    pub async fn add_tag(&self, tag_name: &str, user_id: &UserId) -> Result<(), ServiceError> {
+    pub async fn add_tag(&self, tag_name: &str, user_id: &UserId) -> Result<TagId, ServiceError> {
         let user = self.user_repository.get_compact(user_id).await?;
 
         // Check if user is administrator
@@ -45,7 +45,7 @@ impl Service {
         }
 
         match self.tag_repository.add(trimmed_name).await {
-            Ok(()) => Ok(()),
+            Ok(id) => Ok(id),
             Err(e) => match e {
                 DatabaseError::TagAlreadyExists => Err(ServiceError::TagAlreadyExists),
                 _ => Err(ServiceError::DatabaseError),
@@ -95,8 +95,8 @@ impl DbTagRepository {
     /// # Errors
     ///
     /// It returns an error if there is a database error.
-    pub async fn add(&self, tag_name: &str) -> Result<(), Error> {
-        self.database.add_tag(tag_name).await
+    pub async fn add(&self, tag_name: &str) -> Result<TagId, Error> {
+        self.database.insert_tag_and_get_id(tag_name).await
     }
 
     /// It returns all the tags.

src/upgrades/from_v1_0_0_to_v2_0_0/databases/sqlite_v2_0_0.rs

@@ -280,7 +280,7 @@ impl SqliteDatabaseV2_0_0 {
             query(&format!("DELETE FROM {table};"))
                 .execute(&self.pool)
                 .await
-                .expect("table {table} should be deleted");
+                .unwrap_or_else(|_| panic!("table {table} should be deleted"));
         }
 
         Ok(())

src/web/api/v1/contexts/tag/handlers.rs

@@ -52,7 +52,7 @@ pub async fn add_handler(
     };
 
     match app_data.tag_service.add_tag(&add_tag_form.name, &user_id).await {
-        Ok(()) => added_tag(&add_tag_form.name).into_response(),
+        Ok(_) => added_tag(&add_tag_form.name).into_response(),
         Err(error) => error.into_response(),
     }
 }

tests/e2e/web/api/v1/contexts/tag/contract.rs

@@ -104,9 +104,7 @@ async fn it_should_allow_admins_to_add_new_tags() {
 }
 
 #[tokio::test]
-async fn it_should_allow_adding_duplicated_tags() {
-    // code-review: is this an intended behavior?
-
+async fn it_should_not_allow_adding_duplicated_tags() {
     let mut env = TestEnv::new();
     env.start(api::Version::V1).await;
 
@@ -117,7 +115,8 @@ async fn it_should_allow_adding_duplicated_tags() {
 
     // Try to add the same tag again
     let response = add_tag(&random_tag_name, &env).await;
-    assert_eq!(response.status, 200);
+
+    assert_eq!(response.status, 400);
 }
 
 #[tokio::test]

tests/e2e/web/api/v1/contexts/user/steps.rs

@@ -20,7 +20,7 @@ pub async fn new_logged_in_admin(env: &TestEnv) -> LoggedInUserData {
     let user_profile = database
         .get_user_profile_from_username(&user.username)
         .await
-        .unwrap_or_else(|_| panic!("user {user:#?} should  have a profile."));
+        .unwrap_or_else(|_| panic!("no user profile for the user: {user:#?}."));
 
     database.grant_admin_role(user_profile.user_id).await.unwrap();