From 6f9c1a227da4d418dd887395a69fb05dd5507448 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 12 Jun 2023 15:52:58 +0100 Subject: [PATCH 1/4] refactor: API category context tetests --- tests/e2e/contexts/category/contract.rs | 75 ++++++++++++++----------- tests/e2e/contexts/category/steps.rs | 10 ++++ 2 files changed, 51 insertions(+), 34 deletions(-) diff --git a/tests/e2e/contexts/category/contract.rs b/tests/e2e/contexts/category/contract.rs index b5682327..43d854c9 100644 --- a/tests/e2e/contexts/category/contract.rs +++ b/tests/e2e/contexts/category/contract.rs @@ -6,17 +6,10 @@ use crate::common::client::Client; use crate::common::contexts::category::fixtures::random_category_name; use crate::common::contexts::category::forms::{AddCategoryForm, DeleteCategoryForm}; use crate::common::contexts::category::responses::{AddedCategoryResponse, ListResponse}; -use crate::e2e::contexts::category::steps::add_category; +use crate::e2e::contexts::category::steps::{add_category, add_random_category}; use crate::e2e::contexts::user::steps::{new_logged_in_admin, new_logged_in_user}; use crate::e2e::environment::TestEnv; -/* todo: - - it should allow adding a new category to authenticated clients - - it should not allow adding a new category with an empty name - - it should allow adding a new category with an optional icon - - ... -*/ - #[tokio::test] async fn it_should_return_an_empty_category_list_when_there_are_no_categories() { let mut env = TestEnv::new(); @@ -34,17 +27,15 @@ async fn it_should_return_a_category_list() { env.start(api::Implementation::ActixWeb).await; let client = Client::unauthenticated(&env.server_socket_addr().unwrap()); - // Add a category - let category_name = random_category_name(); - let response = add_category(&category_name, &env).await; - assert_eq!(response.status, 200); + add_random_category(&env).await; let response = client.get_categories().await; let res: ListResponse = serde_json::from_str(&response.body).unwrap(); // There should be at least the category we added. - // Since this is an E2E test, there might be more categories. + // Since this is an E2E test and it could be run in a shared test env, + // there might be more categories. assert!(res.data.len() > 1); if let Some(content_type) = &response.content_type { assert_eq!(content_type, "application/json"); @@ -114,17 +105,42 @@ async fn it_should_allow_admins_to_add_new_categories() { } #[tokio::test] -async fn it_should_not_allow_adding_duplicated_categories() { +async fn it_should_allow_adding_empty_categories() { + // code-review: this is a bit weird, is it a intended behavior? + let mut env = TestEnv::new(); env.start(api::Implementation::ActixWeb).await; - // Add a category - let random_category_name = random_category_name(); - let response = add_category(&random_category_name, &env).await; + 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 response = client + .add_category(AddCategoryForm { + name: category_name.to_string(), + icon: None, + }) + .await; + + let res: AddedCategoryResponse = serde_json::from_str(&response.body).unwrap(); + + assert_eq!(res.data, category_name); + if let Some(content_type) = &response.content_type { + assert_eq!(content_type, "application/json"); + } assert_eq!(response.status, 200); +} + +#[tokio::test] +async fn it_should_not_allow_adding_duplicated_categories() { + let mut env = TestEnv::new(); + env.start(api::Implementation::ActixWeb).await; + + let added_category_name = add_random_category(&env).await; // Try to add the same category again - let response = add_category(&random_category_name, &env).await; + let response = add_category(&added_category_name, &env).await; assert_eq!(response.status, 400); } @@ -136,21 +152,18 @@ async fn it_should_allow_admins_to_delete_categories() { let logged_in_admin = new_logged_in_admin(&env).await; let client = Client::authenticated(&env.server_socket_addr().unwrap(), &logged_in_admin.token); - // Add a category - let category_name = random_category_name(); - let response = add_category(&category_name, &env).await; - assert_eq!(response.status, 200); + let added_category_name = add_random_category(&env).await; let response = client .delete_category(DeleteCategoryForm { - name: category_name.to_string(), + name: added_category_name.to_string(), icon: None, }) .await; let res: AddedCategoryResponse = serde_json::from_str(&response.body).unwrap(); - assert_eq!(res.data, category_name); + assert_eq!(res.data, added_category_name); if let Some(content_type) = &response.content_type { assert_eq!(content_type, "application/json"); } @@ -162,17 +175,14 @@ async fn it_should_not_allow_non_admins_to_delete_categories() { let mut env = TestEnv::new(); env.start(api::Implementation::ActixWeb).await; - // Add a category - let category_name = random_category_name(); - let response = add_category(&category_name, &env).await; - assert_eq!(response.status, 200); + let added_category_name = add_random_category(&env).await; let logged_in_non_admin = new_logged_in_user(&env).await; let client = Client::authenticated(&env.server_socket_addr().unwrap(), &logged_in_non_admin.token); let response = client .delete_category(DeleteCategoryForm { - name: category_name.to_string(), + name: added_category_name.to_string(), icon: None, }) .await; @@ -186,14 +196,11 @@ async fn it_should_not_allow_guests_to_delete_categories() { env.start(api::Implementation::ActixWeb).await; let client = Client::unauthenticated(&env.server_socket_addr().unwrap()); - // Add a category - let category_name = random_category_name(); - let response = add_category(&category_name, &env).await; - assert_eq!(response.status, 200); + let added_category_name = add_random_category(&env).await; let response = client .delete_category(DeleteCategoryForm { - name: category_name.to_string(), + name: added_category_name.to_string(), icon: None, }) .await; diff --git a/tests/e2e/contexts/category/steps.rs b/tests/e2e/contexts/category/steps.rs index 2150a7a8..321cdddc 100644 --- a/tests/e2e/contexts/category/steps.rs +++ b/tests/e2e/contexts/category/steps.rs @@ -1,9 +1,19 @@ use crate::common::client::Client; +use crate::common::contexts::category::fixtures::random_category_name; use crate::common::contexts::category::forms::AddCategoryForm; +use crate::common::contexts::category::responses::AddedCategoryResponse; use crate::common::responses::TextResponse; use crate::e2e::contexts::user::steps::new_logged_in_admin; use crate::e2e::environment::TestEnv; +/// Add a random category and return its name. +pub async fn add_random_category(env: &TestEnv) -> String { + let category_name = random_category_name(); + let response = add_category(&category_name, env).await; + let res: AddedCategoryResponse = serde_json::from_str(&response.body).unwrap(); + res.data +} + pub async fn add_category(category_name: &str, env: &TestEnv) -> TextResponse { let logged_in_admin = new_logged_in_admin(env).await; let client = Client::authenticated(&env.server_socket_addr().unwrap(), &logged_in_admin.token); From bb6d9bf72697d1d25d53759f145a30d8eb9dbdac Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 12 Jun 2023 16:59:02 +0100 Subject: [PATCH 2/4] refactor(api): [#179] Axum API, category context, get all categories --- src/web/api/v1/contexts/category/handlers.rs | 33 ++++++++++++++++++++ src/web/api/v1/contexts/category/mod.rs | 2 ++ src/web/api/v1/contexts/category/routes.rs | 15 +++++++++ src/web/api/v1/routes.rs | 13 ++++---- tests/e2e/contexts/category/contract.rs | 20 ++++++++++++ 5 files changed, 77 insertions(+), 6 deletions(-) create mode 100644 src/web/api/v1/contexts/category/handlers.rs create mode 100644 src/web/api/v1/contexts/category/routes.rs diff --git a/src/web/api/v1/contexts/category/handlers.rs b/src/web/api/v1/contexts/category/handlers.rs new file mode 100644 index 00000000..4cc34dc3 --- /dev/null +++ b/src/web/api/v1/contexts/category/handlers.rs @@ -0,0 +1,33 @@ +//! API handlers for the the [`category`](crate::web::api::v1::contexts::category) API +//! context. +use std::sync::Arc; + +use axum::extract::State; +use axum::response::Json; + +use crate::common::AppData; +use crate::databases::database::{self, Category}; +use crate::web::api::v1::responses; + +/// It handles the request to get all the categories. +/// +/// It returns: +/// +/// - `200` response with a json containing the category list [`Vec`](crate::databases::database::Category). +/// - Other error status codes if there is a database error. +/// +/// Refer to the [API endpoint documentation](crate::web::api::v1::contexts::category) +/// for more information about this endpoint. +/// +/// # Errors +/// +/// It returns an error if there is a database error. +#[allow(clippy::unused_async)] +pub async fn get_all_handler( + State(app_data): State>, +) -> Result>>, database::Error> { + match app_data.category_repository.get_all().await { + Ok(categories) => Ok(Json(responses::OkResponse { data: categories })), + Err(error) => Err(error), + } +} diff --git a/src/web/api/v1/contexts/category/mod.rs b/src/web/api/v1/contexts/category/mod.rs index 68cf07e3..65d74e49 100644 --- a/src/web/api/v1/contexts/category/mod.rs +++ b/src/web/api/v1/contexts/category/mod.rs @@ -138,3 +138,5 @@ //! Refer to [`OkResponse`](crate::models::response::OkResponse) for more //! information about the response attributes. The response contains only the //! name of the deleted category. +pub mod handlers; +pub mod routes; diff --git a/src/web/api/v1/contexts/category/routes.rs b/src/web/api/v1/contexts/category/routes.rs new file mode 100644 index 00000000..c405714a --- /dev/null +++ b/src/web/api/v1/contexts/category/routes.rs @@ -0,0 +1,15 @@ +//! API routes for the [`category`](crate::web::api::v1::contexts::category) API context. +//! +//! Refer to the [API endpoint documentation](crate::web::api::v1::contexts::category). +use std::sync::Arc; + +use axum::routing::get; +use axum::Router; + +use super::handlers::get_all_handler; +use crate::common::AppData; + +/// Routes for the [`category`](crate::web::api::v1::contexts::category) API context. +pub fn router(app_data: Arc) -> Router { + Router::new().route("/", get(get_all_handler).with_state(app_data)) +} diff --git a/src/web/api/v1/routes.rs b/src/web/api/v1/routes.rs index 79185bf6..f7cbf5e2 100644 --- a/src/web/api/v1/routes.rs +++ b/src/web/api/v1/routes.rs @@ -4,22 +4,23 @@ use std::sync::Arc; use axum::Router; //use tower_http::cors::CorsLayer; -use super::contexts::{about, user}; +use super::contexts::about; +use super::contexts::{category, user}; use crate::common::AppData; /// Add all API routes to the router. #[allow(clippy::needless_pass_by_value)] pub fn router(app_data: Arc) -> Router { - let user_routes = user::routes::router(app_data.clone()); - let about_routes = about::routes::router(app_data); + let api_routes = Router::new() + .nest("/user", user::routes::router(app_data.clone())) + .nest("/about", about::routes::router(app_data.clone())) + .nest("/category", category::routes::router(app_data)); - let api_routes = Router::new().nest("/user", user_routes).nest("/about", about_routes); + Router::new().nest("/v1", api_routes) // For development purposes only. // It allows calling the API on a different port. For example // API: http://localhost:3000/v1 // Webapp: http://localhost:8080 //Router::new().nest("/v1", api_routes).layer(CorsLayer::permissive()) - - Router::new().nest("/v1", api_routes) } diff --git a/tests/e2e/contexts/category/contract.rs b/tests/e2e/contexts/category/contract.rs index 43d854c9..a26df3b5 100644 --- a/tests/e2e/contexts/category/contract.rs +++ b/tests/e2e/contexts/category/contract.rs @@ -207,3 +207,23 @@ async fn it_should_not_allow_guests_to_delete_categories() { assert_eq!(response.status, 401); } + +mod with_axum_implementation { + use torrust_index_backend::web::api; + + use crate::common::asserts::assert_json_ok; + use crate::common::client::Client; + use crate::e2e::environment::TestEnv; + + #[tokio::test] + async fn it_should_return_an_empty_category_list_when_there_are_no_categories() { + let mut env = TestEnv::new(); + env.start(api::Implementation::Axum).await; + + let client = Client::unauthenticated(&env.server_socket_addr().unwrap()); + + let response = client.get_categories().await; + + assert_json_ok(&response); + } +} From f63bf050cec0f3806f326a8c38a6aae197656e21 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 15 Jun 2023 12:00:37 +0100 Subject: [PATCH 3/4] refactor(api): [#179] Axum API, category context, add category --- src/web/api/v1/contexts/category/forms.rs | 7 + src/web/api/v1/contexts/category/handlers.rs | 30 +++- src/web/api/v1/contexts/category/mod.rs | 2 + src/web/api/v1/contexts/category/responses.rs | 12 ++ src/web/api/v1/contexts/category/routes.rs | 8 +- tests/common/contexts/category/asserts.rs | 12 ++ tests/common/contexts/category/mod.rs | 1 + tests/e2e/contexts/category/contract.rs | 153 ++++++++++++++++++ tests/e2e/contexts/category/steps.rs | 7 +- tests/e2e/contexts/user/steps.rs | 5 +- 10 files changed, 230 insertions(+), 7 deletions(-) create mode 100644 src/web/api/v1/contexts/category/forms.rs create mode 100644 src/web/api/v1/contexts/category/responses.rs create mode 100644 tests/common/contexts/category/asserts.rs diff --git a/src/web/api/v1/contexts/category/forms.rs b/src/web/api/v1/contexts/category/forms.rs new file mode 100644 index 00000000..ecddcadb --- /dev/null +++ b/src/web/api/v1/contexts/category/forms.rs @@ -0,0 +1,7 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Serialize, Deserialize, Debug)] +pub struct CategoryForm { + pub name: String, + pub icon: Option, +} diff --git a/src/web/api/v1/contexts/category/handlers.rs b/src/web/api/v1/contexts/category/handlers.rs index 4cc34dc3..12e29532 100644 --- a/src/web/api/v1/contexts/category/handlers.rs +++ b/src/web/api/v1/contexts/category/handlers.rs @@ -2,12 +2,16 @@ //! context. use std::sync::Arc; -use axum::extract::State; +use axum::extract::{self, State}; use axum::response::Json; +use super::forms::CategoryForm; +use super::responses::added_category; use crate::common::AppData; use crate::databases::database::{self, Category}; -use crate::web::api::v1::responses; +use crate::errors::ServiceError; +use crate::web::api::v1::extractors::bearer_token::Extract; +use crate::web::api::v1::responses::{self, OkResponse}; /// It handles the request to get all the categories. /// @@ -31,3 +35,25 @@ pub async fn get_all_handler( Err(error) => Err(error), } } + +/// It adds a new category. +/// +/// # Errors +/// +/// It returns an error if: +/// +/// - The user does not have permissions to create a new category. +/// - There is a database error. +#[allow(clippy::unused_async)] +pub async fn add_handler( + State(app_data): State>, + Extract(maybe_bearer_token): Extract, + extract::Json(category_form): extract::Json, +) -> Result>, ServiceError> { + let user_id = app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await?; + + match app_data.category_service.add_category(&category_form.name, &user_id).await { + Ok(_) => Ok(added_category(&category_form.name)), + Err(error) => Err(error), + } +} diff --git a/src/web/api/v1/contexts/category/mod.rs b/src/web/api/v1/contexts/category/mod.rs index 65d74e49..804faf27 100644 --- a/src/web/api/v1/contexts/category/mod.rs +++ b/src/web/api/v1/contexts/category/mod.rs @@ -138,5 +138,7 @@ //! Refer to [`OkResponse`](crate::models::response::OkResponse) for more //! information about the response attributes. The response contains only the //! name of the deleted category. +pub mod forms; pub mod handlers; +pub mod responses; pub mod routes; diff --git a/src/web/api/v1/contexts/category/responses.rs b/src/web/api/v1/contexts/category/responses.rs new file mode 100644 index 00000000..97b0ebb7 --- /dev/null +++ b/src/web/api/v1/contexts/category/responses.rs @@ -0,0 +1,12 @@ +//! API responses for the the [`category`](crate::web::api::v1::contexts::category) API +//! context. +use axum::Json; + +use crate::web::api::v1::responses::OkResponse; + +/// Response after successfully creating a new category. +pub fn added_category(category_name: &str) -> Json> { + Json(OkResponse { + data: category_name.to_string(), + }) +} diff --git a/src/web/api/v1/contexts/category/routes.rs b/src/web/api/v1/contexts/category/routes.rs index c405714a..e34d2ef4 100644 --- a/src/web/api/v1/contexts/category/routes.rs +++ b/src/web/api/v1/contexts/category/routes.rs @@ -3,13 +3,15 @@ //! Refer to the [API endpoint documentation](crate::web::api::v1::contexts::category). use std::sync::Arc; -use axum::routing::get; +use axum::routing::{get, post}; use axum::Router; -use super::handlers::get_all_handler; +use super::handlers::{add_handler, get_all_handler}; use crate::common::AppData; /// Routes for the [`category`](crate::web::api::v1::contexts::category) API context. pub fn router(app_data: Arc) -> Router { - Router::new().route("/", get(get_all_handler).with_state(app_data)) + Router::new() + .route("/", get(get_all_handler).with_state(app_data.clone())) + .route("/", post(add_handler).with_state(app_data)) } diff --git a/tests/common/contexts/category/asserts.rs b/tests/common/contexts/category/asserts.rs new file mode 100644 index 00000000..2e39d9fd --- /dev/null +++ b/tests/common/contexts/category/asserts.rs @@ -0,0 +1,12 @@ +use crate::common::asserts::assert_json_ok; +use crate::common::contexts::category::responses::AddedCategoryResponse; +use crate::common::responses::TextResponse; + +pub fn assert_added_category_response(response: &TextResponse, category_name: &str) { + let added_category_response: AddedCategoryResponse = serde_json::from_str(&response.body) + .unwrap_or_else(|_| panic!("response {:#?} should be a AddedCategoryResponse", response.body)); + + assert_eq!(added_category_response.data, category_name); + + assert_json_ok(response); +} diff --git a/tests/common/contexts/category/mod.rs b/tests/common/contexts/category/mod.rs index 6f27f51d..cfe5dd24 100644 --- a/tests/common/contexts/category/mod.rs +++ b/tests/common/contexts/category/mod.rs @@ -1,3 +1,4 @@ +pub mod asserts; pub mod fixtures; pub mod forms; pub mod responses; diff --git a/tests/e2e/contexts/category/contract.rs b/tests/e2e/contexts/category/contract.rs index a26df3b5..336a35c8 100644 --- a/tests/e2e/contexts/category/contract.rs +++ b/tests/e2e/contexts/category/contract.rs @@ -209,10 +209,19 @@ async fn it_should_not_allow_guests_to_delete_categories() { } mod with_axum_implementation { + use std::env; + use torrust_index_backend::web::api; use crate::common::asserts::assert_json_ok; use crate::common::client::Client; + use crate::common::contexts::category::asserts::assert_added_category_response; + use crate::common::contexts::category::fixtures::random_category_name; + use crate::common::contexts::category::forms::AddCategoryForm; + use crate::common::contexts::category::responses::ListResponse; + use crate::e2e::config::ENV_VAR_E2E_EXCLUDE_AXUM_IMPL; + use crate::e2e::contexts::category::steps::{add_category, add_random_category}; + use crate::e2e::contexts::user::steps::{new_logged_in_admin, new_logged_in_user}; use crate::e2e::environment::TestEnv; #[tokio::test] @@ -226,4 +235,148 @@ mod with_axum_implementation { assert_json_ok(&response); } + + #[tokio::test] + async fn it_should_return_a_category_list() { + let mut env = TestEnv::new(); + env.start(api::Implementation::Axum).await; + + if env::var(ENV_VAR_E2E_EXCLUDE_AXUM_IMPL).is_ok() { + println!("Skipped"); + return; + } + + let client = Client::unauthenticated(&env.server_socket_addr().unwrap()); + + add_random_category(&env).await; + + let response = client.get_categories().await; + + let res: ListResponse = serde_json::from_str(&response.body).unwrap(); + + // There should be at least the category we added. + // Since this is an E2E test and it could be run in a shared test env, + // there might be more categories. + assert!(res.data.len() > 1); + if let Some(content_type) = &response.content_type { + assert_eq!(content_type, "application/json"); + } + assert_eq!(response.status, 200); + } + + #[tokio::test] + async fn it_should_not_allow_adding_a_new_category_to_unauthenticated_users() { + let mut env = TestEnv::new(); + env.start(api::Implementation::Axum).await; + + if env::var(ENV_VAR_E2E_EXCLUDE_AXUM_IMPL).is_ok() { + println!("Skipped"); + return; + } + + let client = Client::unauthenticated(&env.server_socket_addr().unwrap()); + + let response = client + .add_category(AddCategoryForm { + name: "CATEGORY NAME".to_string(), + icon: None, + }) + .await; + + assert_eq!(response.status, 401); + } + + #[tokio::test] + async fn it_should_not_allow_adding_a_new_category_to_non_admins() { + let mut env = TestEnv::new(); + env.start(api::Implementation::Axum).await; + + if env::var(ENV_VAR_E2E_EXCLUDE_AXUM_IMPL).is_ok() { + println!("Skipped"); + return; + } + + let logged_non_admin = new_logged_in_user(&env).await; + + let client = Client::authenticated(&env.server_socket_addr().unwrap(), &logged_non_admin.token); + + let response = client + .add_category(AddCategoryForm { + name: "CATEGORY NAME".to_string(), + icon: None, + }) + .await; + + assert_eq!(response.status, 403); + } + + #[tokio::test] + async fn it_should_allow_admins_to_add_new_categories() { + let mut env = TestEnv::new(); + env.start(api::Implementation::Axum).await; + + if env::var(ENV_VAR_E2E_EXCLUDE_AXUM_IMPL).is_ok() { + 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 = random_category_name(); + + let response = client + .add_category(AddCategoryForm { + name: category_name.to_string(), + icon: None, + }) + .await; + + assert_added_category_response(&response, &category_name); + } + + #[tokio::test] + async fn it_should_allow_adding_empty_categories() { + // code-review: this is a bit weird, is it a intended behavior? + + let mut env = TestEnv::new(); + env.start(api::Implementation::Axum).await; + + if env::var(ENV_VAR_E2E_EXCLUDE_AXUM_IMPL).is_ok() { + 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 response = client + .add_category(AddCategoryForm { + name: category_name.to_string(), + icon: None, + }) + .await; + + assert_added_category_response(&response, &category_name); + } + + #[tokio::test] + async fn it_should_not_allow_adding_duplicated_categories() { + let mut env = TestEnv::new(); + env.start(api::Implementation::Axum).await; + + if env::var(ENV_VAR_E2E_EXCLUDE_AXUM_IMPL).is_ok() { + println!("Skipped"); + return; + } + + let added_category_name = add_random_category(&env).await; + + // Try to add the same category again + let response = add_category(&added_category_name, &env).await; + + assert_eq!(response.status, 400); + } } diff --git a/tests/e2e/contexts/category/steps.rs b/tests/e2e/contexts/category/steps.rs index 321cdddc..ab000dab 100644 --- a/tests/e2e/contexts/category/steps.rs +++ b/tests/e2e/contexts/category/steps.rs @@ -9,13 +9,18 @@ use crate::e2e::environment::TestEnv; /// Add a random category and return its name. pub async fn add_random_category(env: &TestEnv) -> String { let category_name = random_category_name(); + let response = add_category(&category_name, env).await; - let res: AddedCategoryResponse = serde_json::from_str(&response.body).unwrap(); + + let res: AddedCategoryResponse = serde_json::from_str(&response.body) + .unwrap_or_else(|_| panic!("response {:#?} should be a AddedCategoryResponse", response.body)); + res.data } pub async fn add_category(category_name: &str, env: &TestEnv) -> TextResponse { let logged_in_admin = new_logged_in_admin(env).await; + let client = Client::authenticated(&env.server_socket_addr().unwrap(), &logged_in_admin.token); client diff --git a/tests/e2e/contexts/user/steps.rs b/tests/e2e/contexts/user/steps.rs index f4892325..f0c8a531 100644 --- a/tests/e2e/contexts/user/steps.rs +++ b/tests/e2e/contexts/user/steps.rs @@ -17,7 +17,10 @@ pub async fn new_logged_in_admin(env: &TestEnv) -> LoggedInUserData { .expect("Database error."), ); - let user_profile = database.get_user_profile_from_username(&user.username).await.unwrap(); + let user_profile = database + .get_user_profile_from_username(&user.username) + .await + .unwrap_or_else(|_| panic!("user {user:#?} should have a profile.")); database.grant_admin_role(user_profile.user_id).await.unwrap(); From b4a7ea6e923a6a00594c137b51666b12061b092a Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 15 Jun 2023 12:23:06 +0100 Subject: [PATCH 4/4] refactor(api): [#179] Axum API, category context, delete category --- src/web/api/v1/contexts/category/forms.rs | 4 +- src/web/api/v1/contexts/category/handlers.rs | 32 +++++++- src/web/api/v1/contexts/category/responses.rs | 7 ++ src/web/api/v1/contexts/category/routes.rs | 7 +- tests/common/contexts/category/asserts.rs | 11 ++- tests/common/contexts/category/responses.rs | 5 ++ tests/e2e/contexts/category/contract.rs | 78 ++++++++++++++++++- 7 files changed, 134 insertions(+), 10 deletions(-) diff --git a/src/web/api/v1/contexts/category/forms.rs b/src/web/api/v1/contexts/category/forms.rs index ecddcadb..1ad7767a 100644 --- a/src/web/api/v1/contexts/category/forms.rs +++ b/src/web/api/v1/contexts/category/forms.rs @@ -1,7 +1,9 @@ use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug)] -pub struct CategoryForm { +pub struct AddCategoryForm { pub name: String, pub icon: Option, } + +pub type DeleteCategoryForm = AddCategoryForm; diff --git a/src/web/api/v1/contexts/category/handlers.rs b/src/web/api/v1/contexts/category/handlers.rs index 12e29532..3d09008a 100644 --- a/src/web/api/v1/contexts/category/handlers.rs +++ b/src/web/api/v1/contexts/category/handlers.rs @@ -5,8 +5,8 @@ use std::sync::Arc; use axum::extract::{self, State}; use axum::response::Json; -use super::forms::CategoryForm; -use super::responses::added_category; +use super::forms::{AddCategoryForm, DeleteCategoryForm}; +use super::responses::{added_category, deleted_category}; use crate::common::AppData; use crate::databases::database::{self, Category}; use crate::errors::ServiceError; @@ -48,7 +48,7 @@ pub async fn get_all_handler( pub async fn add_handler( State(app_data): State>, Extract(maybe_bearer_token): Extract, - extract::Json(category_form): extract::Json, + extract::Json(category_form): extract::Json, ) -> Result>, ServiceError> { let user_id = app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await?; @@ -57,3 +57,29 @@ pub async fn add_handler( Err(error) => Err(error), } } + +/// It deletes a category. +/// +/// # Errors +/// +/// It returns an error if: +/// +/// - The user does not have permissions to delete category. +/// - There is a database error. +#[allow(clippy::unused_async)] +pub async fn delete_handler( + State(app_data): State>, + Extract(maybe_bearer_token): Extract, + extract::Json(category_form): extract::Json, +) -> Result>, ServiceError> { + // code-review: why do we need to send the whole category object to delete it? + // And we should use the ID instead of the name, because the name could change + // or we could add support for multiple languages. + + let user_id = app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await?; + + match app_data.category_service.delete_category(&category_form.name, &user_id).await { + Ok(_) => Ok(deleted_category(&category_form.name)), + Err(error) => Err(error), + } +} diff --git a/src/web/api/v1/contexts/category/responses.rs b/src/web/api/v1/contexts/category/responses.rs index 97b0ebb7..cb372801 100644 --- a/src/web/api/v1/contexts/category/responses.rs +++ b/src/web/api/v1/contexts/category/responses.rs @@ -10,3 +10,10 @@ pub fn added_category(category_name: &str) -> Json> { data: category_name.to_string(), }) } + +/// Response after successfully deleting a new category. +pub fn deleted_category(category_name: &str) -> Json> { + Json(OkResponse { + data: category_name.to_string(), + }) +} diff --git a/src/web/api/v1/contexts/category/routes.rs b/src/web/api/v1/contexts/category/routes.rs index e34d2ef4..2d762c47 100644 --- a/src/web/api/v1/contexts/category/routes.rs +++ b/src/web/api/v1/contexts/category/routes.rs @@ -3,15 +3,16 @@ //! Refer to the [API endpoint documentation](crate::web::api::v1::contexts::category). use std::sync::Arc; -use axum::routing::{get, post}; +use axum::routing::{delete, get, post}; use axum::Router; -use super::handlers::{add_handler, get_all_handler}; +use super::handlers::{add_handler, delete_handler, get_all_handler}; use crate::common::AppData; /// Routes for the [`category`](crate::web::api::v1::contexts::category) API context. pub fn router(app_data: Arc) -> Router { Router::new() .route("/", get(get_all_handler).with_state(app_data.clone())) - .route("/", post(add_handler).with_state(app_data)) + .route("/", post(add_handler).with_state(app_data.clone())) + .route("/", delete(delete_handler).with_state(app_data)) } diff --git a/tests/common/contexts/category/asserts.rs b/tests/common/contexts/category/asserts.rs index 2e39d9fd..ae47bdca 100644 --- a/tests/common/contexts/category/asserts.rs +++ b/tests/common/contexts/category/asserts.rs @@ -1,5 +1,5 @@ use crate::common::asserts::assert_json_ok; -use crate::common::contexts::category::responses::AddedCategoryResponse; +use crate::common::contexts::category::responses::{AddedCategoryResponse, DeletedCategoryResponse}; use crate::common::responses::TextResponse; pub fn assert_added_category_response(response: &TextResponse, category_name: &str) { @@ -10,3 +10,12 @@ pub fn assert_added_category_response(response: &TextResponse, category_name: &s assert_json_ok(response); } + +pub fn assert_deleted_category_response(response: &TextResponse, category_name: &str) { + let deleted_category_response: DeletedCategoryResponse = serde_json::from_str(&response.body) + .unwrap_or_else(|_| panic!("response {:#?} should be a DeletedCategoryResponse", response.body)); + + assert_eq!(deleted_category_response.data, category_name); + + assert_json_ok(response); +} diff --git a/tests/common/contexts/category/responses.rs b/tests/common/contexts/category/responses.rs index a345d523..cbadb631 100644 --- a/tests/common/contexts/category/responses.rs +++ b/tests/common/contexts/category/responses.rs @@ -5,6 +5,11 @@ pub struct AddedCategoryResponse { pub data: String, } +#[derive(Deserialize)] +pub struct DeletedCategoryResponse { + pub data: String, +} + #[derive(Deserialize, Debug)] pub struct ListResponse { pub data: Vec, diff --git a/tests/e2e/contexts/category/contract.rs b/tests/e2e/contexts/category/contract.rs index 336a35c8..d1970063 100644 --- a/tests/e2e/contexts/category/contract.rs +++ b/tests/e2e/contexts/category/contract.rs @@ -215,9 +215,9 @@ mod with_axum_implementation { use crate::common::asserts::assert_json_ok; use crate::common::client::Client; - use crate::common::contexts::category::asserts::assert_added_category_response; + use crate::common::contexts::category::asserts::{assert_added_category_response, assert_deleted_category_response}; use crate::common::contexts::category::fixtures::random_category_name; - use crate::common::contexts::category::forms::AddCategoryForm; + use crate::common::contexts::category::forms::{AddCategoryForm, DeleteCategoryForm}; use crate::common::contexts::category::responses::ListResponse; use crate::e2e::config::ENV_VAR_E2E_EXCLUDE_AXUM_IMPL; use crate::e2e::contexts::category::steps::{add_category, add_random_category}; @@ -379,4 +379,78 @@ mod with_axum_implementation { assert_eq!(response.status, 400); } + + #[tokio::test] + async fn it_should_allow_admins_to_delete_categories() { + let mut env = TestEnv::new(); + env.start(api::Implementation::Axum).await; + + if env::var(ENV_VAR_E2E_EXCLUDE_AXUM_IMPL).is_ok() { + 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 added_category_name = add_random_category(&env).await; + + let response = client + .delete_category(DeleteCategoryForm { + name: added_category_name.to_string(), + icon: None, + }) + .await; + + assert_deleted_category_response(&response, &added_category_name); + } + + #[tokio::test] + async fn it_should_not_allow_non_admins_to_delete_categories() { + let mut env = TestEnv::new(); + env.start(api::Implementation::Axum).await; + + if env::var(ENV_VAR_E2E_EXCLUDE_AXUM_IMPL).is_ok() { + println!("Skipped"); + return; + } + + let added_category_name = add_random_category(&env).await; + + let logged_in_non_admin = new_logged_in_user(&env).await; + let client = Client::authenticated(&env.server_socket_addr().unwrap(), &logged_in_non_admin.token); + + let response = client + .delete_category(DeleteCategoryForm { + name: added_category_name.to_string(), + icon: None, + }) + .await; + + assert_eq!(response.status, 403); + } + + #[tokio::test] + async fn it_should_not_allow_guests_to_delete_categories() { + let mut env = TestEnv::new(); + env.start(api::Implementation::Axum).await; + + if env::var(ENV_VAR_E2E_EXCLUDE_AXUM_IMPL).is_ok() { + println!("Skipped"); + return; + } + + let client = Client::unauthenticated(&env.server_socket_addr().unwrap()); + + let added_category_name = add_random_category(&env).await; + + let response = client + .delete_category(DeleteCategoryForm { + name: added_category_name.to_string(), + icon: None, + }) + .await; + + assert_eq!(response.status, 401); + } }