refactor: [torrust#801] refactor filtering code logic

Author: mario-nt

src/databases/mysql.rs

@@ -176,45 +176,21 @@ impl Database for Mysql {
             None => "username ASC".to_string(),
         };
 
-        let join_filters_query = if let Some(filters) = filters {
-            let mut join_filters = String::new();
+        let (join_filters, where_filters) = if let Some(filters) = filters {
+            let (mut join_filters_query, mut where_filters_query) = (String::new(), String::new());
             for filter in filters {
-                // don't take user input in the db query to smt join filter query
-                /* if let Some(sanitized_filter) = self.get_filters_from_name(filter).await { */
                 match filter {
-                    UsersFilters::TorrentUploader => join_filters.push_str(
+                    UsersFilters::TorrentUploader => join_filters_query.push_str(
                         "INNER JOIN torrust_torrents tt
-                    ON tu.user_id = tt.uploader_id",
+                    ON tu.user_id = tt.uploader_id ",
                     ),
-                    _ => break,
+                    UsersFilters::EmailNotVerified => where_filters_query.push_str(" AND email_verified = false"),
+                    UsersFilters::EmailVerified => where_filters_query.push_str(" AND email_verified = true"),
                 }
             }
-            //}
-            join_filters
+            (join_filters_query, where_filters_query)
         } else {
-            String::new()
-        };
-
-        let where_filters_query = if let Some(filters) = filters {
-            let mut where_filters = String::new();
-            for filter in filters {
-                // don't take user input in the db query
-                /* if let Some(sanitized_filter) = self.get_filters_from_name(filter).await { */
-                let mut filter_query = String::new();
-                match filter {
-                    UsersFilters::EmailNotVerified => filter_query.push_str("email_verified = false"),
-                    UsersFilters::EmailVerified => filter_query.push_str("email_verified = true"),
-                    _ => continue,
-                };
-
-                let str = format!("AND {filter_query} ");
-
-                where_filters.push_str(&str);
-            }
-            //}
-            where_filters
-        } else {
-            String::new()
+            (String::new(), String::new())
         };
 
         let mut query_string = format!(
@@ -228,9 +204,9 @@ impl Database for Mysql {
         FROM torrust_user_profiles tp 
         INNER JOIN torrust_users tu
         ON tp.user_id = tu.user_id 
-        {join_filters_query}
+        {join_filters}
         WHERE username LIKE ?
-        {where_filters_query}
+        {where_filters}
         "
         );
 

src/databases/sqlite.rs

@@ -177,64 +177,21 @@ impl Database for Sqlite {
             None => "username ASC".to_string(),
         };
 
-        /* let (join_filters, where_filters) = if let Some(filters) = filters {
-            let (join_filters_query, where_filters_query) = (String::new(), String::new());
+        let (join_filters, where_filters) = if let Some(filters) = filters {
+            let (mut join_filters_query, mut where_filters_query) = (String::new(), String::new());
             for filter in filters {
                 match filter {
                     UsersFilters::TorrentUploader => join_filters_query.push_str(
                         "INNER JOIN torrust_torrents tt
                     ON tu.user_id = tt.uploader_id ",
                     ),
-                    UsersFilters::EmailNotVerified => where_filters_query.push_str("email_verified = false"),
-                    UsersFilters::EmailVerified => where_filters_query.push_str("email_verified = true"),
-                    _ => continue,
+                    UsersFilters::EmailNotVerified => where_filters_query.push_str(" AND email_verified = false"),
+                    UsersFilters::EmailVerified => where_filters_query.push_str(" AND email_verified = true"),
                 }
             }
             (join_filters_query, where_filters_query)
         } else {
             (String::new(), String::new())
-        }; */
-
-        let join_filters_query = if let Some(filters) = filters {
-            let mut join_filters = String::new();
-            for filter in filters {
-                // don't take user input in the db query
-                /* if let Some(sanitized_filter) = self.get_filters_from_name(filter).await { */
-                //match sanitized_filter {
-                match filter {
-                    UsersFilters::TorrentUploader => join_filters.push_str(
-                        "INNER JOIN torrust_torrents tt
-                    ON tu.user_id = tt.uploader_id ",
-                    ),
-                    _ => continue,
-                }
-            }
-            //}
-            join_filters
-        } else {
-            String::new()
-        };
-
-        let where_filters_query = if let Some(filters) = filters {
-            let mut where_filters = String::new();
-            for filter in filters {
-                // don't take user input in the db query
-                // if let Some(sanitized_filter) = self.get_filters_from_name(filter).await {
-                let mut filter_query = String::new();
-                match filter {
-                    UsersFilters::EmailNotVerified => filter_query.push_str("email_verified = false"),
-                    UsersFilters::EmailVerified => filter_query.push_str("email_verified = true"),
-                    _ => continue,
-                };
-
-                let str = format!("AND {filter_query} ");
-
-                where_filters.push_str(&str);
-            }
-            //}
-            where_filters
-        } else {
-            String::new()
         };
 
         let mut query_string = format!(
@@ -248,9 +205,9 @@ impl Database for Sqlite {
         FROM torrust_user_profiles tp 
         INNER JOIN torrust_users tu
         ON tp.user_id = tu.user_id 
-        {join_filters_query}
+        {join_filters}
         WHERE username LIKE ?
-        {where_filters_query}
+        {where_filters}
         "
         );
 

src/services/user.rs

@@ -413,16 +413,22 @@ impl ListingService {
             None => None,
         };
 
-        let filter_values = request.filters.as_csv::<String>().unwrap_or(None);
+        let filter_values = request
+            .filters
+            .as_csv::<String>()
+            .map_err(|_| ServiceError::InvalidUserListing)?;
 
         let filters = if let Some(filter_values) = filter_values {
             let mut sanitized_filters: Vec<UsersFilters> = Vec::new();
             for filter in filter_values {
                 match filter.as_str() {
-                    "torrent_uploader" => sanitized_filters.push(UsersFilters::TorrentUploader),
-                    "email_not_verified" => sanitized_filters.push(UsersFilters::EmailNotVerified),
-                    "email_verified" => sanitized_filters.push(UsersFilters::EmailVerified),
-                    _ => (),
+                    "torrent_uploader" => sanitized_filters
+                        .push(UsersFilters::from_str("torrent_uploader").map_err(|_| ServiceError::InvalidUserListing)?),
+                    "email_not_verified" => sanitized_filters
+                        .push(UsersFilters::from_str("email_not_verified").map_err(|_| ServiceError::InvalidUserListing)?),
+                    "email_verified" => sanitized_filters
+                        .push(UsersFilters::from_str("email_verified").map_err(|_| ServiceError::InvalidUserListing)?),
+                    _ => return Err(ServiceError::InvalidUserListing),
                 }
             }
             Some(sanitized_filters)