From d74214a91012c177a9792da39c2e3e712c496c31 Mon Sep 17 00:00:00 2001 From: dal Date: Wed, 15 Jan 2025 10:14:15 -0700 Subject: [PATCH 1/4] Refactor user routes to include new endpoint for retrieving user by ID - Removed the public modifier from `get_user` and `update_user` modules to encapsulate them within the module. - Added a new route to the user router for fetching a user by their ID, enhancing the API's functionality. - This change improves the user management capabilities by allowing retrieval of specific user details based on their unique identifier. --- .../routes/rest/routes/organizations/mod.rs | 8 ++ .../routes/rest/routes/organizations/users.rs | 74 +++++++++++++++++++ .../rest/routes/users/get_user_by_id.rs | 74 +++++++++++++++++++ api/src/routes/rest/routes/users/mod.rs | 6 +- 4 files changed, 160 insertions(+), 2 deletions(-) create mode 100644 api/src/routes/rest/routes/organizations/mod.rs create mode 100644 api/src/routes/rest/routes/organizations/users.rs create mode 100644 api/src/routes/rest/routes/users/get_user_by_id.rs diff --git a/api/src/routes/rest/routes/organizations/mod.rs b/api/src/routes/rest/routes/organizations/mod.rs new file mode 100644 index 000000000..c21b36ff9 --- /dev/null +++ b/api/src/routes/rest/routes/organizations/mod.rs @@ -0,0 +1,8 @@ +use axum::{routing::get, Router}; + +mod users; + +pub fn router() -> Router { + Router::new() + .route("/:id/users", get(users::list_organization_users)) +} diff --git a/api/src/routes/rest/routes/organizations/users.rs b/api/src/routes/rest/routes/organizations/users.rs new file mode 100644 index 000000000..e74033ef5 --- /dev/null +++ b/api/src/routes/rest/routes/organizations/users.rs @@ -0,0 +1,74 @@ +use anyhow::Result; +use axum::{extract::Path, Extension, http::StatusCode}; +use diesel::{ExpressionMethods, QueryDsl, JoinOnDsl}; +use diesel_async::RunQueryDsl; +use serde::{Deserialize, Serialize}; +use uuid::Uuid; + +use crate::{ + database::{ + enums::{UserOrganizationRole, UserOrganizationStatus}, + lib::get_pg_pool, + models::User, + schema::{users, users_to_organizations}, + }, + routes::rest::ApiResponse, + utils::clients::sentry_utils::send_sentry_error, +}; + +#[derive(Serialize, Deserialize, Clone)] +pub struct UserResponse { + pub id: Uuid, + pub name: Option, + pub email: String, + pub role: UserOrganizationRole, + pub status: UserOrganizationStatus, +} + +pub async fn list_organization_users( + Extension(user): Extension, + Path(organization_id): Path, +) -> Result>, (StatusCode, &'static str)> { + let users = match list_organization_users_handler(organization_id).await { + Ok(users) => users, + Err(e) => { + tracing::error!("Error listing organization users: {:?}", e); + send_sentry_error(&e.to_string(), Some(&user.id)); + return Err(( + StatusCode::INTERNAL_SERVER_ERROR, + "Error listing organization users", + )); + } + }; + + Ok(ApiResponse::JsonData(users)) +} + +async fn list_organization_users_handler(organization_id: Uuid) -> Result> { + let mut conn = get_pg_pool().get().await?; + + let users = users::table + .inner_join(users_to_organizations::table.on(users::id.eq(users_to_organizations::user_id))) + .select(( + users::id, + users::email, + users::name.nullable(), + users_to_organizations::role, + users_to_organizations::status, + )) + .filter(users_to_organizations::organization_id.eq(organization_id)) + .filter(users_to_organizations::deleted_at.is_null()) + .load::<(Uuid, String, Option, UserOrganizationRole, UserOrganizationStatus)>(&mut conn) + .await?; + + Ok(users + .into_iter() + .map(|(id, email, name, role, status)| UserResponse { + id, + name, + email, + role, + status, + }) + .collect()) +} \ No newline at end of file diff --git a/api/src/routes/rest/routes/users/get_user_by_id.rs b/api/src/routes/rest/routes/users/get_user_by_id.rs new file mode 100644 index 000000000..02091a1b0 --- /dev/null +++ b/api/src/routes/rest/routes/users/get_user_by_id.rs @@ -0,0 +1,74 @@ +use anyhow::Result; +use axum::{extract::Path, Extension}; +use diesel_async::RunQueryDsl; +use serde::{Deserialize, Serialize}; +use uuid::Uuid; + +use crate::{ + database::{ + enums::{UserOrganizationRole, UserOrganizationStatus}, + lib::get_pg_pool, + models::User, + schema::{users, users_to_organizations}, + }, + routes::rest::ApiResponse, + utils::clients::sentry_utils::send_sentry_error, +}; +use axum::http::StatusCode; +use diesel::{ExpressionMethods, JoinOnDsl, NullableExpressionMethods, QueryDsl}; + +#[derive(Serialize, Deserialize, Clone)] +pub struct UserResponse { + pub id: Uuid, + pub name: Option, + pub email: String, + pub role: UserOrganizationRole, + pub status: UserOrganizationStatus, +} + +pub async fn get_user_by_id( + Extension(user): Extension, + Path(id): Path, +) -> Result, (StatusCode, &'static str)> { + let user_info = match get_user_information(&id).await { + Ok(user_info) => user_info, + Err(e) => { + tracing::error!("Error getting user information: {:?}", e); + send_sentry_error(&e.to_string(), Some(&user.id)); + return Err(( + StatusCode::INTERNAL_SERVER_ERROR, + "Error getting user information", + )); + } + }; + + Ok(ApiResponse::JsonData(user_info)) +} + +pub async fn get_user_information(user_id: &Uuid) -> Result { + let pg_pool = get_pg_pool(); + let mut conn = pg_pool.get().await?; + + let (user, (role, status)) = users::table + .inner_join(users_to_organizations::table.on(users::id.eq(users_to_organizations::user_id))) + .select(( + (users::id, users::email, users::name.nullable()), + (users_to_organizations::role, users_to_organizations::status), + )) + .filter(users::id.eq(user_id)) + .first::<( + (Uuid, String, Option), + (UserOrganizationRole, UserOrganizationStatus), + )>(&mut conn) + .await?; + + let (id, email, name) = user; + + Ok(UserResponse { + id, + name, + email, + role, + status, + }) +} diff --git a/api/src/routes/rest/routes/users/mod.rs b/api/src/routes/rest/routes/users/mod.rs index f3ac8ce9d..e3eaeb3af 100644 --- a/api/src/routes/rest/routes/users/mod.rs +++ b/api/src/routes/rest/routes/users/mod.rs @@ -3,11 +3,13 @@ use axum::{ Router, }; -pub mod get_user; -pub mod update_user; +mod get_user; +mod get_user_by_id; +mod update_user; pub fn router() -> Router { Router::new() .route("/", get(get_user::get_user)) .route("/", put(update_user::update_user)) + .route("/:id", get(get_user_by_id::get_user_by_id)) } From f628b38e85ea4fa0fa7bd482315527e57f0a19e1 Mon Sep 17 00:00:00 2001 From: dal Date: Wed, 15 Jan 2025 10:20:04 -0700 Subject: [PATCH 2/4] Add organizations module and integrate with user routes --- api/src/routes/rest/routes/mod.rs | 2 ++ api/src/routes/rest/routes/organizations/users.rs | 14 ++++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/api/src/routes/rest/routes/mod.rs b/api/src/routes/rest/routes/mod.rs index 9b4fcc017..491ddbb4c 100644 --- a/api/src/routes/rest/routes/mod.rs +++ b/api/src/routes/rest/routes/mod.rs @@ -3,6 +3,7 @@ mod assets; mod data_sources; mod dataset_groups; mod datasets; +mod organizations; mod permission_groups; mod sql; mod users; @@ -21,6 +22,7 @@ pub fn router() -> Router { .nest("/permission_groups", permission_groups::router()) .nest("/dataset_groups", dataset_groups::router()) .nest("/sql", sql::router()) + .nest("/organizations", organizations::router()) .route_layer(middleware::from_fn(auth)), ) } diff --git a/api/src/routes/rest/routes/organizations/users.rs b/api/src/routes/rest/routes/organizations/users.rs index e74033ef5..4277f081d 100644 --- a/api/src/routes/rest/routes/organizations/users.rs +++ b/api/src/routes/rest/routes/organizations/users.rs @@ -1,6 +1,6 @@ use anyhow::Result; -use axum::{extract::Path, Extension, http::StatusCode}; -use diesel::{ExpressionMethods, QueryDsl, JoinOnDsl}; +use axum::{extract::Path, http::StatusCode, Extension}; +use diesel::{ExpressionMethods, JoinOnDsl, NullableExpressionMethods, QueryDsl}; use diesel_async::RunQueryDsl; use serde::{Deserialize, Serialize}; use uuid::Uuid; @@ -58,7 +58,13 @@ async fn list_organization_users_handler(organization_id: Uuid) -> Result, UserOrganizationRole, UserOrganizationStatus)>(&mut conn) + .load::<( + Uuid, + String, + Option, + UserOrganizationRole, + UserOrganizationStatus, + )>(&mut conn) .await?; Ok(users @@ -71,4 +77,4 @@ async fn list_organization_users_handler(organization_id: Uuid) -> Result Date: Wed, 15 Jan 2025 11:10:26 -0700 Subject: [PATCH 3/4] Refactor user update functionality to support role changes - Enhanced the `update_user` endpoint to accept and process user role updates alongside name changes. - Introduced a new `UserResponse` struct for improved response handling. - Updated the `update_user_handler` to handle changes in both user name and organization role, improving the flexibility of user management. - Adjusted response type to return no content upon successful updates, aligning with RESTful practices. These changes enhance the user management capabilities by allowing for more comprehensive updates to user information. --- .../routes/rest/routes/users/update_user.rs | 69 +++++++++++++------ 1 file changed, 49 insertions(+), 20 deletions(-) diff --git a/api/src/routes/rest/routes/users/update_user.rs b/api/src/routes/rest/routes/users/update_user.rs index f7ee97a0c..abbeaabd4 100644 --- a/api/src/routes/rest/routes/users/update_user.rs +++ b/api/src/routes/rest/routes/users/update_user.rs @@ -1,28 +1,39 @@ use anyhow::Result; use axum::{Extension, Json}; -use diesel_async::RunQueryDsl; -use crate::database::lib::get_pg_pool; +use crate::database::enums::UserOrganizationStatus; use crate::database::models::User; -use crate::database::schema::users; +use crate::database::schema::{users, users_to_organizations}; +use crate::database::{enums::UserOrganizationRole, lib::get_pg_pool}; use crate::routes::rest::ApiResponse; use crate::utils::clients::sentry_utils::send_sentry_error; use axum::http::StatusCode; use diesel::{update, ExpressionMethods}; -use serde::Deserialize; +use diesel_async::RunQueryDsl; +use serde::{Deserialize, Serialize}; use uuid::Uuid; +#[derive(Serialize, Deserialize, Clone)] +pub struct UserResponse { + pub id: Uuid, + pub name: Option, + pub email: String, + pub role: UserOrganizationRole, + pub status: UserOrganizationStatus, +} + #[derive(Deserialize)] pub struct UpdateUserRequest { pub name: Option, + pub role: Option, } pub async fn update_user( Extension(user): Extension, Json(body): Json, -) -> Result, (StatusCode, &'static str)> { - let user_info_object = match update_user_handler(&user.id, body.name).await { - Ok(user_info_object) => user_info_object, +) -> Result, (StatusCode, &'static str)> { + match update_user_handler(&user.id, body).await { + Ok(_) => (), Err(e) => { tracing::error!("Error getting user information: {:?}", e); send_sentry_error(&e.to_string(), Some(&user.id)); @@ -33,10 +44,10 @@ pub async fn update_user( } }; - Ok(ApiResponse::JsonData(user_info_object)) + Ok(ApiResponse::NoContent) } -pub async fn update_user_handler(user_id: &Uuid, name: Option) -> Result { +pub async fn update_user_handler(user_id: &Uuid, change: UpdateUserRequest) -> Result<()> { let pg_pool = get_pg_pool(); let mut conn = match pg_pool.get().await { @@ -46,16 +57,34 @@ pub async fn update_user_handler(user_id: &Uuid, name: Option) -> Result } }; - let user = match update(users::table) - .filter(users::id.eq(user_id)) - .set(users::name.eq(name)) - .returning(users::all_columns) - .get_result::(&mut conn) - .await - { - Ok(user) => user, - Err(e) => return Err(anyhow::anyhow!("Error updating user: {:?}", e)), - }; + if let Some(name) = change.name { + match update(users::table) + .filter(users::id.eq(user_id)) + .set(users::name.eq(name)) + .execute(&mut conn) + .await + { + Ok(user) => user, + Err(e) => return Err(anyhow::anyhow!("Error updating user: {:?}", e)), + }; + } - Ok(user) + if let Some(role) = change.role { + match update(users_to_organizations::table) + .filter(users_to_organizations::user_id.eq(user_id)) + .set(users_to_organizations::role.eq(role)) + .execute(&mut conn) + .await + { + Ok(user_organization_role_update) => user_organization_role_update, + Err(e) => { + return Err(anyhow::anyhow!( + "Error updating user organization role: {:?}", + e + )) + } + }; + } + + Ok(()) } From 6aea4349a39f00dac7e17d0c49c3219c781c27b9 Mon Sep 17 00:00:00 2001 From: dal Date: Wed, 15 Jan 2025 11:17:24 -0700 Subject: [PATCH 4/4] Update user route to use ID parameter for updates - Changed the user update route to require a user ID in the URL, enhancing RESTful practices. - Updated the `update_user` function to extract the user ID from the path, ensuring the correct user is updated based on the provided ID. These changes improve the clarity and functionality of the user update endpoint, aligning it with standard REST conventions. --- api/src/routes/rest/routes/users/mod.rs | 2 +- api/src/routes/rest/routes/users/update_user.rs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/api/src/routes/rest/routes/users/mod.rs b/api/src/routes/rest/routes/users/mod.rs index e3eaeb3af..4de5bba16 100644 --- a/api/src/routes/rest/routes/users/mod.rs +++ b/api/src/routes/rest/routes/users/mod.rs @@ -10,6 +10,6 @@ mod update_user; pub fn router() -> Router { Router::new() .route("/", get(get_user::get_user)) - .route("/", put(update_user::update_user)) + .route("/:id", put(update_user::update_user)) .route("/:id", get(get_user_by_id::get_user_by_id)) } diff --git a/api/src/routes/rest/routes/users/update_user.rs b/api/src/routes/rest/routes/users/update_user.rs index abbeaabd4..1c998c666 100644 --- a/api/src/routes/rest/routes/users/update_user.rs +++ b/api/src/routes/rest/routes/users/update_user.rs @@ -1,4 +1,5 @@ use anyhow::Result; +use axum::extract::Path; use axum::{Extension, Json}; use crate::database::enums::UserOrganizationStatus; @@ -30,9 +31,10 @@ pub struct UpdateUserRequest { pub async fn update_user( Extension(user): Extension, + Path(id): Path, Json(body): Json, ) -> Result, (StatusCode, &'static str)> { - match update_user_handler(&user.id, body).await { + match update_user_handler(&id, body).await { Ok(_) => (), Err(e) => { tracing::error!("Error getting user information: {:?}", e);