From d476d322008116ddba25e2dee3f9e26b08734836 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Mon, 23 Sep 2024 20:55:35 -0400 Subject: [PATCH] Removing a few more Result . (#4977) * Removing a few more Result . * Running taplo fmt. * Running fmt. * Adding email taken test. * Fixing tests. * Adding back in missing admin check. * Rename check_has_local_followers function. --- api_tests/package.json | 10 +-- api_tests/pnpm-lock.yaml | 10 +-- api_tests/src/post.spec.ts | 2 +- crates/api/src/community/add_mod.rs | 5 +- crates/api/src/local_user/save_settings.rs | 4 +- crates/api_common/src/claims.rs | 8 +- crates/api_common/src/utils.rs | 84 ++----------------- crates/api_crud/src/post/create.rs | 5 +- crates/api_crud/src/private_message/create.rs | 8 +- crates/api_crud/src/user/create.rs | 29 +++---- .../apub/src/activities/community/announce.rs | 8 +- crates/apub/src/activities/mod.rs | 18 +--- crates/apub/src/objects/post.rs | 7 +- crates/apub/src/objects/private_message.rs | 5 +- crates/db_schema/src/impls/captcha_answer.rs | 11 +-- crates/db_schema/src/impls/community.rs | 13 +-- crates/db_schema/src/impls/community_block.rs | 15 ++-- crates/db_schema/src/impls/instance_block.rs | 15 ++-- crates/db_schema/src/impls/local_user.rs | 40 +++++++-- crates/db_schema/src/impls/login_token.rs | 9 +- crates/db_schema/src/impls/person_block.rs | 15 ++-- crates/db_views_actor/Cargo.toml | 9 +- .../src/community_moderator_view.rs | 15 ++-- .../src/community_person_ban_view.rs | 20 +++-- crates/db_views_actor/src/community_view.rs | 37 ++++---- 25 files changed, 182 insertions(+), 220 deletions(-) diff --git a/api_tests/package.json b/api_tests/package.json index cacd476ea..b06b84fbf 100644 --- a/api_tests/package.json +++ b/api_tests/package.json @@ -21,16 +21,16 @@ }, "devDependencies": { "@types/jest": "^29.5.12", - "@types/node": "^22.0.2", - "@typescript-eslint/eslint-plugin": "^8.0.0", - "@typescript-eslint/parser": "^8.0.0", - "eslint": "^9.8.0", + "@types/node": "^22.3.0", + "@typescript-eslint/eslint-plugin": "^8.1.0", + "@typescript-eslint/parser": "^8.1.0", + "eslint": "^9.9.0", "eslint-plugin-prettier": "^5.1.3", "jest": "^29.5.0", "lemmy-js-client": "0.20.0-alpha.11", "prettier": "^3.2.5", "ts-jest": "^29.1.0", "typescript": "^5.5.4", - "typescript-eslint": "^8.0.0" + "typescript-eslint": "^8.1.0" } } diff --git a/api_tests/pnpm-lock.yaml b/api_tests/pnpm-lock.yaml index f72450015..54c8a9d96 100644 --- a/api_tests/pnpm-lock.yaml +++ b/api_tests/pnpm-lock.yaml @@ -12,16 +12,16 @@ importers: specifier: ^29.5.12 version: 29.5.12 '@types/node': - specifier: ^22.0.2 + specifier: ^22.3.0 version: 22.3.0 '@typescript-eslint/eslint-plugin': - specifier: ^8.0.0 + specifier: ^8.1.0 version: 8.1.0(@typescript-eslint/parser@8.1.0(eslint@9.9.0)(typescript@5.5.4))(eslint@9.9.0)(typescript@5.5.4) '@typescript-eslint/parser': - specifier: ^8.0.0 + specifier: ^8.1.0 version: 8.1.0(eslint@9.9.0)(typescript@5.5.4) eslint: - specifier: ^9.8.0 + specifier: ^9.9.0 version: 9.9.0 eslint-plugin-prettier: specifier: ^5.1.3 @@ -42,7 +42,7 @@ importers: specifier: ^5.5.4 version: 5.5.4 typescript-eslint: - specifier: ^8.0.0 + specifier: ^8.1.0 version: 8.1.0(eslint@9.9.0)(typescript@5.5.4) packages: diff --git a/api_tests/src/post.spec.ts b/api_tests/src/post.spec.ts index bb0416057..ee9cc441d 100644 --- a/api_tests/src/post.spec.ts +++ b/api_tests/src/post.spec.ts @@ -628,7 +628,7 @@ test("Enforce community ban for federated user", async () => { // Alpha tries to make post on beta, but it fails because of ban await expect( createPost(alpha, betaCommunity.community.id), - ).rejects.toStrictEqual(Error("banned_from_community")); + ).rejects.toStrictEqual(Error("person_is_banned_from_community")); // Unban alpha let unBanAlpha = await banPersonFromCommunity( diff --git a/crates/api/src/community/add_mod.rs b/crates/api/src/community/add_mod.rs index 09e2c5b61..7d04f6bb0 100644 --- a/crates/api/src/community/add_mod.rs +++ b/crates/api/src/community/add_mod.rs @@ -52,15 +52,12 @@ pub async fn add_mod_to_community( // moderator. This is necessary because otherwise the action would be rejected // by the community's home instance. if local_user_view.local_user.admin && !community.local { - let is_mod = CommunityModeratorView::is_community_moderator( + CommunityModeratorView::check_is_community_moderator( &mut context.pool(), community.id, local_user_view.person.id, ) .await?; - if !is_mod { - Err(LemmyErrorType::NotAModerator)? - } } // Update in local database diff --git a/crates/api/src/local_user/save_settings.rs b/crates/api/src/local_user/save_settings.rs index c62ab0e01..029914545 100644 --- a/crates/api/src/local_user/save_settings.rs +++ b/crates/api/src/local_user/save_settings.rs @@ -63,9 +63,7 @@ pub async fn save_user_settings( let previous_email = local_user_view.local_user.email.clone().unwrap_or_default(); // if email was changed, check that it is not taken and send verification mail if previous_email.deref() != email { - if LocalUser::is_email_taken(&mut context.pool(), email).await? { - return Err(LemmyErrorType::EmailAlreadyExists)?; - } + LocalUser::check_is_email_taken(&mut context.pool(), email).await?; send_verification_email( &local_user_view, email, diff --git a/crates/api_common/src/claims.rs b/crates/api_common/src/claims.rs index 905394785..06e62bae3 100644 --- a/crates/api_common/src/claims.rs +++ b/crates/api_common/src/claims.rs @@ -29,12 +29,8 @@ impl Claims { let claims = decode::(jwt, &key, &validation).with_lemmy_type(LemmyErrorType::NotLoggedIn)?; let user_id = LocalUserId(claims.claims.sub.parse()?); - let is_valid = LoginToken::validate(&mut context.pool(), user_id, jwt).await?; - if !is_valid { - Err(LemmyErrorType::NotLoggedIn)? - } else { - Ok(user_id) - } + LoginToken::validate(&mut context.pool(), user_id, jwt).await?; + Ok(user_id) } pub async fn generate( diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index db186247c..c34261511 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -73,13 +73,7 @@ pub async fn is_mod_or_admin( community_id: CommunityId, ) -> LemmyResult<()> { check_user_valid(person)?; - - let is_mod_or_admin = CommunityView::is_mod_or_admin(pool, person.id, community_id).await?; - if !is_mod_or_admin { - Err(LemmyErrorType::NotAModOrAdmin)? - } else { - Ok(()) - } + CommunityView::check_is_mod_or_admin(pool, person.id, community_id).await } #[tracing::instrument(skip_all)] @@ -110,13 +104,7 @@ pub async fn check_community_mod_of_any_or_admin_action( let person = &local_user_view.person; check_user_valid(person)?; - - let is_mod_of_any_or_admin = CommunityView::is_mod_of_any_or_admin(pool, person.id).await?; - if !is_mod_of_any_or_admin { - Err(LemmyErrorType::NotAModOrAdmin)? - } else { - Ok(()) - } + CommunityView::check_is_mod_of_any_or_admin(pool, person.id).await } pub fn is_admin(local_user_view: &LocalUserView) -> LemmyResult<()> { @@ -242,7 +230,7 @@ pub async fn check_community_user_action( ) -> LemmyResult<()> { check_user_valid(person)?; check_community_deleted_removed(community_id, pool).await?; - check_community_ban(person, community_id, pool).await?; + CommunityPersonBanView::check(pool, person.id, community_id).await?; Ok(()) } @@ -257,19 +245,6 @@ async fn check_community_deleted_removed( Ok(()) } -async fn check_community_ban( - person: &Person, - community_id: CommunityId, - pool: &mut DbPool<'_>, -) -> LemmyResult<()> { - // check if user was banned from site or community - let is_banned = CommunityPersonBanView::get(pool, person.id, community_id).await?; - if is_banned { - Err(LemmyErrorType::BannedFromCommunity)? - } - Ok(()) -} - /// Check that the given user can perform a mod action in the community. /// /// In particular it checks that he is an admin or mod, wasn't banned and the community isn't @@ -281,7 +256,7 @@ pub async fn check_community_mod_action( pool: &mut DbPool<'_>, ) -> LemmyResult<()> { is_mod_or_admin(pool, person, community_id).await?; - check_community_ban(person, community_id, pool).await?; + CommunityPersonBanView::check(pool, person.id, community_id).await?; // it must be possible to restore deleted community if !allow_deleted { @@ -307,51 +282,6 @@ pub fn check_comment_deleted_or_removed(comment: &Comment) -> LemmyResult<()> { } } -/// Throws an error if a recipient has blocked a person. -#[tracing::instrument(skip_all)] -pub async fn check_person_block( - my_id: PersonId, - potential_blocker_id: PersonId, - pool: &mut DbPool<'_>, -) -> LemmyResult<()> { - let is_blocked = PersonBlock::read(pool, potential_blocker_id, my_id).await?; - if is_blocked { - Err(LemmyErrorType::PersonIsBlocked)? - } else { - Ok(()) - } -} - -/// Throws an error if a recipient has blocked a community. -#[tracing::instrument(skip_all)] -async fn check_community_block( - community_id: CommunityId, - person_id: PersonId, - pool: &mut DbPool<'_>, -) -> LemmyResult<()> { - let is_blocked = CommunityBlock::read(pool, person_id, community_id).await?; - if is_blocked { - Err(LemmyErrorType::CommunityIsBlocked)? - } else { - Ok(()) - } -} - -/// Throws an error if a recipient has blocked an instance. -#[tracing::instrument(skip_all)] -async fn check_instance_block( - instance_id: InstanceId, - person_id: PersonId, - pool: &mut DbPool<'_>, -) -> LemmyResult<()> { - let is_blocked = InstanceBlock::read(pool, person_id, instance_id).await?; - if is_blocked { - Err(LemmyErrorType::InstanceIsBlocked)? - } else { - Ok(()) - } -} - #[tracing::instrument(skip_all)] pub async fn check_person_instance_community_block( my_id: PersonId, @@ -360,9 +290,9 @@ pub async fn check_person_instance_community_block( community_id: CommunityId, pool: &mut DbPool<'_>, ) -> LemmyResult<()> { - check_person_block(my_id, potential_blocker_id, pool).await?; - check_instance_block(community_instance_id, potential_blocker_id, pool).await?; - check_community_block(community_id, potential_blocker_id, pool).await?; + PersonBlock::read(pool, potential_blocker_id, my_id).await?; + InstanceBlock::read(pool, potential_blocker_id, community_instance_id).await?; + CommunityBlock::read(pool, potential_blocker_id, community_id).await?; Ok(()) } diff --git a/crates/api_crud/src/post/create.rs b/crates/api_crud/src/post/create.rs index 95b4e4722..cdeb10f44 100644 --- a/crates/api_crud/src/post/create.rs +++ b/crates/api_crud/src/post/create.rs @@ -95,15 +95,12 @@ pub async fn create_post( let community = Community::read(&mut context.pool(), community_id).await?; if community.posting_restricted_to_mods { let community_id = data.community_id; - let is_mod = CommunityModeratorView::is_community_moderator( + CommunityModeratorView::check_is_community_moderator( &mut context.pool(), community_id, local_user_view.local_user.person_id, ) .await?; - if !is_mod { - Err(LemmyErrorType::OnlyModsCanPostInCommunity)? - } } // Only need to check if language is allowed in case user set it explicitly. When using default diff --git a/crates/api_crud/src/private_message/create.rs b/crates/api_crud/src/private_message/create.rs index e79241022..2a49e4ac0 100644 --- a/crates/api_crud/src/private_message/create.rs +++ b/crates/api_crud/src/private_message/create.rs @@ -5,7 +5,6 @@ use lemmy_api_common::{ private_message::{CreatePrivateMessage, PrivateMessageResponse}, send_activity::{ActivityChannel, SendActivityData}, utils::{ - check_person_block, get_interface_language, get_url_blocklist, local_site_to_slur_regex, @@ -16,6 +15,7 @@ use lemmy_api_common::{ use lemmy_db_schema::{ source::{ local_site::LocalSite, + person_block::PersonBlock, private_message::{PrivateMessage, PrivateMessageInsertForm}, }, traits::Crud, @@ -39,10 +39,10 @@ pub async fn create_private_message( let content = process_markdown(&data.content, &slur_regex, &url_blocklist, &context).await?; is_valid_body_field(&content, false)?; - check_person_block( - local_user_view.person.id, - data.recipient_id, + PersonBlock::read( &mut context.pool(), + data.recipient_id, + local_user_view.person.id, ) .await?; diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index 174b02930..ec859cd49 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -92,22 +92,15 @@ pub async fn register( } if local_site.site_setup && local_site.captcha_enabled { - if let Some(captcha_uuid) = &data.captcha_uuid { - let uuid = uuid::Uuid::parse_str(captcha_uuid)?; - let check = CaptchaAnswer::check_captcha( - &mut context.pool(), - CheckCaptchaAnswer { - uuid, - answer: data.captcha_answer.clone().unwrap_or_default(), - }, - ) - .await?; - if !check { - Err(LemmyErrorType::CaptchaIncorrect)? - } - } else { - Err(LemmyErrorType::CaptchaIncorrect)? - } + let uuid = uuid::Uuid::parse_str(&data.captcha_uuid.clone().unwrap_or_default())?; + CaptchaAnswer::check_captcha( + &mut context.pool(), + CheckCaptchaAnswer { + uuid, + answer: data.captcha_answer.clone().unwrap_or_default(), + }, + ) + .await?; } let slur_regex = local_site_to_slur_regex(&local_site); @@ -119,9 +112,7 @@ pub async fn register( } if let Some(email) = &data.email { - if LocalUser::is_email_taken(&mut context.pool(), email).await? { - Err(LemmyErrorType::EmailAlreadyExists)? - } + LocalUser::check_is_email_taken(&mut context.pool(), email).await?; } // We have to create both a person, and local_user diff --git a/crates/apub/src/activities/community/announce.rs b/crates/apub/src/activities/community/announce.rs index 69dbbdd3d..0a506a791 100644 --- a/crates/apub/src/activities/community/announce.rs +++ b/crates/apub/src/activities/community/announce.rs @@ -213,15 +213,13 @@ async fn can_accept_activity_in_community( context: &Data, ) -> LemmyResult<()> { if let Some(community) = community { - if !community.local - && !CommunityFollower::has_local_followers(&mut context.pool(), community.id).await? - { - Err(LemmyErrorType::CommunityHasNoFollowers)? - } // Local only community can't federate if community.visibility != CommunityVisibility::Public { return Err(LemmyErrorType::NotFound.into()); } + if !community.local { + CommunityFollower::check_has_local_followers(&mut context.pool(), community.id).await? + } } Ok(()) } diff --git a/crates/apub/src/activities/mod.rs b/crates/apub/src/activities/mod.rs index 5389e5fdd..2c371f71c 100644 --- a/crates/apub/src/activities/mod.rs +++ b/crates/apub/src/activities/mod.rs @@ -87,12 +87,7 @@ pub(crate) async fn verify_person_in_community( } let person_id = person.id; let community_id = community.id; - let is_banned = CommunityPersonBanView::get(&mut context.pool(), person_id, community_id).await?; - if is_banned { - Err(LemmyErrorType::PersonIsBannedFromCommunity)? - } else { - Ok(()) - } + CommunityPersonBanView::check(&mut context.pool(), person_id, community_id).await } /// Verify that mod action in community was performed by a moderator. @@ -106,14 +101,6 @@ pub(crate) async fn verify_mod_action( community: &Community, context: &Data, ) -> LemmyResult<()> { - let mod_ = mod_id.dereference(context).await?; - - let is_mod_or_admin = - CommunityView::is_mod_or_admin(&mut context.pool(), mod_.id, community.id).await?; - if is_mod_or_admin { - return Ok(()); - } - // mod action comes from the same instance as the community, so it was presumably done // by an instance admin. // TODO: federate instance admin status and check it here @@ -121,7 +108,8 @@ pub(crate) async fn verify_mod_action( return Ok(()); } - Err(LemmyErrorType::NotAModerator)? + let mod_ = mod_id.dereference(context).await?; + CommunityView::check_is_mod_or_admin(&mut context.pool(), mod_.id, community.id).await } pub(crate) fn verify_is_public(to: &[Url], cc: &[Url]) -> LemmyResult<()> { diff --git a/crates/apub/src/objects/post.rs b/crates/apub/src/objects/post.rs index dd8b247d7..dfc9d79f9 100644 --- a/crates/apub/src/objects/post.rs +++ b/crates/apub/src/objects/post.rs @@ -39,7 +39,7 @@ use lemmy_db_schema::{ }; use lemmy_db_views_actor::structs::CommunityModeratorView; use lemmy_utils::{ - error::{LemmyError, LemmyErrorType, LemmyResult}, + error::{LemmyError, LemmyResult}, spawn_try_task, utils::{ markdown::markdown_to_html, @@ -180,15 +180,12 @@ impl Object for ApubPost { let creator = page.creator()?.dereference(context).await?; let community = page.community(context).await?; if community.posting_restricted_to_mods { - let is_mod = CommunityModeratorView::is_community_moderator( + CommunityModeratorView::check_is_community_moderator( &mut context.pool(), community.id, creator.id, ) .await?; - if !is_mod { - Err(LemmyErrorType::OnlyModsCanPostInCommunity)? - } } let mut name = page .name diff --git a/crates/apub/src/objects/private_message.rs b/crates/apub/src/objects/private_message.rs index ccebcf110..d3ca340db 100644 --- a/crates/apub/src/objects/private_message.rs +++ b/crates/apub/src/objects/private_message.rs @@ -15,12 +15,13 @@ use activitypub_federation::{ use chrono::{DateTime, Utc}; use lemmy_api_common::{ context::LemmyContext, - utils::{check_person_block, get_url_blocklist, local_site_opt_to_slur_regex, process_markdown}, + utils::{get_url_blocklist, local_site_opt_to_slur_regex, process_markdown}, }; use lemmy_db_schema::{ source::{ local_site::LocalSite, person::Person, + person_block::PersonBlock, private_message::{PrivateMessage, PrivateMessageInsertForm}, }, traits::Crud, @@ -126,7 +127,7 @@ impl Object for ApubPrivateMessage { ) -> LemmyResult { let creator = note.attributed_to.dereference(context).await?; let recipient = note.to[0].dereference(context).await?; - check_person_block(creator.id, recipient.id, &mut context.pool()).await?; + PersonBlock::read(&mut context.pool(), recipient.id, creator.id).await?; let local_site = LocalSite::read(&mut context.pool()).await.ok(); let slur_regex = &local_site_opt_to_slur_regex(&local_site); diff --git a/crates/db_schema/src/impls/captcha_answer.rs b/crates/db_schema/src/impls/captcha_answer.rs index 1d0604c0a..976d89515 100644 --- a/crates/db_schema/src/impls/captcha_answer.rs +++ b/crates/db_schema/src/impls/captcha_answer.rs @@ -13,6 +13,7 @@ use diesel::{ QueryDsl, }; use diesel_async::RunQueryDsl; +use lemmy_utils::{error::LemmyResult, LemmyErrorType}; impl CaptchaAnswer { pub async fn insert(pool: &mut DbPool<'_>, captcha: &CaptchaAnswerForm) -> Result { @@ -27,7 +28,7 @@ impl CaptchaAnswer { pub async fn check_captcha( pool: &mut DbPool<'_>, to_check: CheckCaptchaAnswer, - ) -> Result { + ) -> LemmyResult<()> { let conn = &mut get_conn(pool).await?; // fetch requested captcha @@ -43,7 +44,9 @@ impl CaptchaAnswer { .execute(conn) .await?; - Ok(captcha_exists) + captcha_exists + .then_some(()) + .ok_or(LemmyErrorType::CaptchaIncorrect.into()) } } @@ -83,7 +86,6 @@ mod tests { .await; assert!(result.is_ok()); - assert!(result.unwrap()); } #[tokio::test] @@ -119,7 +121,6 @@ mod tests { ) .await; - assert!(result_repeat.is_ok()); - assert!(!result_repeat.unwrap()); + assert!(result_repeat.is_err()); } } diff --git a/crates/db_schema/src/impls/community.rs b/crates/db_schema/src/impls/community.rs index a26fdc233..f106ca424 100644 --- a/crates/db_schema/src/impls/community.rs +++ b/crates/db_schema/src/impls/community.rs @@ -35,8 +35,7 @@ use crate::{ use chrono::{DateTime, Utc}; use diesel::{ deserialize, - dsl, - dsl::{exists, insert_into}, + dsl::{self, exists, insert_into}, pg::Pg, result::Error, select, @@ -320,16 +319,18 @@ impl CommunityFollower { /// Check if a remote instance has any followers on local instance. For this it is enough to check /// if any follow relation is stored. Dont use this for local community. - pub async fn has_local_followers( + pub async fn check_has_local_followers( pool: &mut DbPool<'_>, remote_community_id: CommunityId, - ) -> Result { + ) -> LemmyResult<()> { let conn = &mut get_conn(pool).await?; select(exists(community_follower::table.filter( community_follower::community_id.eq(remote_community_id), ))) - .get_result(conn) - .await + .get_result::(conn) + .await? + .then_some(()) + .ok_or(LemmyErrorType::CommunityHasNoFollowers.into()) } } diff --git a/crates/db_schema/src/impls/community_block.rs b/crates/db_schema/src/impls/community_block.rs index 944a53ae3..cd541cd8b 100644 --- a/crates/db_schema/src/impls/community_block.rs +++ b/crates/db_schema/src/impls/community_block.rs @@ -9,26 +9,29 @@ use crate::{ utils::{get_conn, DbPool}, }; use diesel::{ - dsl::{exists, insert_into}, + dsl::{exists, insert_into, not}, result::Error, select, ExpressionMethods, QueryDsl, }; use diesel_async::RunQueryDsl; +use lemmy_utils::{error::LemmyResult, LemmyErrorType}; impl CommunityBlock { pub async fn read( pool: &mut DbPool<'_>, for_person_id: PersonId, for_community_id: CommunityId, - ) -> Result { + ) -> LemmyResult<()> { let conn = &mut get_conn(pool).await?; - select(exists( + select(not(exists( community_block::table.find((for_person_id, for_community_id)), - )) - .get_result(conn) - .await + ))) + .get_result::(conn) + .await? + .then_some(()) + .ok_or(LemmyErrorType::CommunityIsBlocked.into()) } pub async fn for_person( diff --git a/crates/db_schema/src/impls/instance_block.rs b/crates/db_schema/src/impls/instance_block.rs index 15fcd1443..1eb6e8f04 100644 --- a/crates/db_schema/src/impls/instance_block.rs +++ b/crates/db_schema/src/impls/instance_block.rs @@ -9,26 +9,29 @@ use crate::{ utils::{get_conn, DbPool}, }; use diesel::{ - dsl::{exists, insert_into}, + dsl::{exists, insert_into, not}, result::Error, select, ExpressionMethods, QueryDsl, }; use diesel_async::RunQueryDsl; +use lemmy_utils::{error::LemmyResult, LemmyErrorType}; impl InstanceBlock { pub async fn read( pool: &mut DbPool<'_>, for_person_id: PersonId, for_instance_id: InstanceId, - ) -> Result { + ) -> LemmyResult<()> { let conn = &mut get_conn(pool).await?; - select(exists( + select(not(exists( instance_block::table.find((for_person_id, for_instance_id)), - )) - .get_result(conn) - .await + ))) + .get_result::(conn) + .await? + .then_some(()) + .ok_or(LemmyErrorType::InstanceIsBlocked.into()) } pub async fn for_person( diff --git a/crates/db_schema/src/impls/local_user.rs b/crates/db_schema/src/impls/local_user.rs index 2330a3864..a06f22e34 100644 --- a/crates/db_schema/src/impls/local_user.rs +++ b/crates/db_schema/src/impls/local_user.rs @@ -136,14 +136,16 @@ impl LocalUser { diesel::delete(persons).execute(conn).await } - pub async fn is_email_taken(pool: &mut DbPool<'_>, email: &str) -> Result { + pub async fn check_is_email_taken(pool: &mut DbPool<'_>, email: &str) -> LemmyResult<()> { use diesel::dsl::{exists, select}; let conn = &mut get_conn(pool).await?; - select(exists(local_user::table.filter( + select(not(exists(local_user::table.filter( lower(coalesce(local_user::email, "")).eq(email.to_lowercase()), - ))) - .get_result(conn) - .await + )))) + .get_result::(conn) + .await? + .then_some(()) + .ok_or(LemmyErrorType::EmailAlreadyExists.into()) } // TODO: maybe move this and pass in LocalUserView @@ -419,4 +421,32 @@ mod tests { Ok(()) } + + #[tokio::test] + #[serial] + async fn test_email_taken() -> LemmyResult<()> { + let pool = &build_db_pool_for_tests().await; + let pool = &mut pool.into(); + + let darwin_email = "charles.darwin@gmail.com"; + + let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string()).await?; + + let darwin_person = PersonInsertForm::test_form(inserted_instance.id, "darwin"); + let inserted_darwin_person = Person::create(pool, &darwin_person).await?; + + let mut darwin_local_user_form = + LocalUserInsertForm::test_form_admin(inserted_darwin_person.id); + darwin_local_user_form.email = Some(darwin_email.into()); + let _inserted_darwin_local_user = + LocalUser::create(pool, &darwin_local_user_form, vec![]).await?; + + let check = LocalUser::check_is_email_taken(pool, darwin_email).await; + assert!(check.is_err()); + + let passed_check = LocalUser::check_is_email_taken(pool, "not_charles@gmail.com").await; + assert!(passed_check.is_ok()); + + Ok(()) + } } diff --git a/crates/db_schema/src/impls/login_token.rs b/crates/db_schema/src/impls/login_token.rs index 71cac6a19..c8c44c506 100644 --- a/crates/db_schema/src/impls/login_token.rs +++ b/crates/db_schema/src/impls/login_token.rs @@ -7,6 +7,7 @@ use crate::{ }; use diesel::{delete, dsl::exists, insert_into, result::Error, select}; use diesel_async::RunQueryDsl; +use lemmy_utils::{error::LemmyResult, LemmyErrorType}; impl LoginToken { pub async fn create(pool: &mut DbPool<'_>, form: LoginTokenCreateForm) -> Result { @@ -22,13 +23,15 @@ impl LoginToken { pool: &mut DbPool<'_>, user_id_: LocalUserId, token_: &str, - ) -> Result { + ) -> LemmyResult<()> { let conn = &mut get_conn(pool).await?; select(exists( login_token.find(token_).filter(user_id.eq(user_id_)), )) - .get_result(conn) - .await + .get_result::(conn) + .await? + .then_some(()) + .ok_or(LemmyErrorType::NotLoggedIn.into()) } pub async fn list( diff --git a/crates/db_schema/src/impls/person_block.rs b/crates/db_schema/src/impls/person_block.rs index cfd41f6d6..7f2286616 100644 --- a/crates/db_schema/src/impls/person_block.rs +++ b/crates/db_schema/src/impls/person_block.rs @@ -9,7 +9,7 @@ use crate::{ utils::{get_conn, DbPool}, }; use diesel::{ - dsl::{exists, insert_into}, + dsl::{exists, insert_into, not}, result::Error, select, ExpressionMethods, @@ -17,19 +17,22 @@ use diesel::{ QueryDsl, }; use diesel_async::RunQueryDsl; +use lemmy_utils::{error::LemmyResult, LemmyErrorType}; impl PersonBlock { pub async fn read( pool: &mut DbPool<'_>, for_person_id: PersonId, for_recipient_id: PersonId, - ) -> Result { + ) -> LemmyResult<()> { let conn = &mut get_conn(pool).await?; - select(exists( + select(not(exists( person_block::table.find((for_person_id, for_recipient_id)), - )) - .get_result(conn) - .await + ))) + .get_result::(conn) + .await? + .then_some(()) + .ok_or(LemmyErrorType::PersonIsBlocked.into()) } pub async fn for_person( diff --git a/crates/db_views_actor/Cargo.toml b/crates/db_views_actor/Cargo.toml index af139b8b2..c29b7d66d 100644 --- a/crates/db_views_actor/Cargo.toml +++ b/crates/db_views_actor/Cargo.toml @@ -15,7 +15,13 @@ doctest = false workspace = true [features] -full = ["lemmy_db_schema/full", "diesel", "diesel-async", "ts-rs"] +full = [ + "lemmy_db_schema/full", + "lemmy_utils/full", + "diesel", + "diesel-async", + "ts-rs", +] [dependencies] lemmy_db_schema = { workspace = true } @@ -33,6 +39,7 @@ serde_with = { workspace = true } ts-rs = { workspace = true, optional = true } chrono.workspace = true strum = { workspace = true } +lemmy_utils = { workspace = true, optional = true } [dev-dependencies] serial_test = { workspace = true } diff --git a/crates/db_views_actor/src/community_moderator_view.rs b/crates/db_views_actor/src/community_moderator_view.rs index f2a59fd9f..ebcdcbd25 100644 --- a/crates/db_views_actor/src/community_moderator_view.rs +++ b/crates/db_views_actor/src/community_moderator_view.rs @@ -8,13 +8,14 @@ use lemmy_db_schema::{ source::local_user::LocalUser, utils::{get_conn, DbPool}, }; +use lemmy_utils::{error::LemmyResult, LemmyErrorType}; impl CommunityModeratorView { - pub async fn is_community_moderator( + pub async fn check_is_community_moderator( pool: &mut DbPool<'_>, find_community_id: CommunityId, find_person_id: PersonId, - ) -> Result { + ) -> LemmyResult<()> { use lemmy_db_schema::schema::community_moderator::dsl::{ community_id, community_moderator, @@ -27,20 +28,24 @@ impl CommunityModeratorView { .filter(person_id.eq(find_person_id)), )) .get_result::(conn) - .await + .await? + .then_some(()) + .ok_or(LemmyErrorType::NotAModerator.into()) } pub(crate) async fn is_community_moderator_of_any( pool: &mut DbPool<'_>, find_person_id: PersonId, - ) -> Result { + ) -> LemmyResult<()> { use lemmy_db_schema::schema::community_moderator::dsl::{community_moderator, person_id}; let conn = &mut get_conn(pool).await?; select(exists( community_moderator.filter(person_id.eq(find_person_id)), )) .get_result::(conn) - .await + .await? + .then_some(()) + .ok_or(LemmyErrorType::NotAModerator.into()) } pub async fn for_community( diff --git a/crates/db_views_actor/src/community_person_ban_view.rs b/crates/db_views_actor/src/community_person_ban_view.rs index 712bb2d3a..5543222f3 100644 --- a/crates/db_views_actor/src/community_person_ban_view.rs +++ b/crates/db_views_actor/src/community_person_ban_view.rs @@ -1,25 +1,33 @@ use crate::structs::CommunityPersonBanView; -use diesel::{dsl::exists, result::Error, select, ExpressionMethods, QueryDsl}; +use diesel::{ + dsl::{exists, not}, + select, + ExpressionMethods, + QueryDsl, +}; use diesel_async::RunQueryDsl; use lemmy_db_schema::{ newtypes::{CommunityId, PersonId}, schema::community_person_ban, utils::{get_conn, DbPool}, }; +use lemmy_utils::{error::LemmyResult, LemmyErrorType}; impl CommunityPersonBanView { - pub async fn get( + pub async fn check( pool: &mut DbPool<'_>, from_person_id: PersonId, from_community_id: CommunityId, - ) -> Result { + ) -> LemmyResult<()> { let conn = &mut get_conn(pool).await?; - select(exists( + select(not(exists( community_person_ban::table .filter(community_person_ban::community_id.eq(from_community_id)) .filter(community_person_ban::person_id.eq(from_person_id)), - )) + ))) .get_result::(conn) - .await + .await? + .then_some(()) + .ok_or(LemmyErrorType::PersonIsBannedFromCommunity.into()) } } diff --git a/crates/db_views_actor/src/community_view.rs b/crates/db_views_actor/src/community_view.rs index 6282bef31..a1b387bea 100644 --- a/crates/db_views_actor/src/community_view.rs +++ b/crates/db_views_actor/src/community_view.rs @@ -26,6 +26,7 @@ use lemmy_db_schema::{ ListingType, PostSortType, }; +use lemmy_utils::{error::LemmyResult, LemmyErrorType}; fn queries<'a>() -> Queries< impl ReadFn<'a, CommunityView, (CommunityId, Option<&'a LocalUser>, bool)>, @@ -185,35 +186,39 @@ impl CommunityView { .await } - pub async fn is_mod_or_admin( + pub async fn check_is_mod_or_admin( pool: &mut DbPool<'_>, person_id: PersonId, community_id: CommunityId, - ) -> Result { + ) -> LemmyResult<()> { let is_mod = - CommunityModeratorView::is_community_moderator(pool, community_id, person_id).await?; - if is_mod { - Ok(true) - } else if let Ok(person_view) = PersonView::read(pool, person_id).await { - Ok(person_view.is_admin) + CommunityModeratorView::check_is_community_moderator(pool, community_id, person_id).await; + if is_mod.is_ok() + || PersonView::read(pool, person_id) + .await + .is_ok_and(|t| t.is_admin) + { + Ok(()) } else { - Ok(false) + Err(LemmyErrorType::NotAModOrAdmin)? } } /// Checks if a person is an admin, or moderator of any community. - pub async fn is_mod_of_any_or_admin( + pub async fn check_is_mod_of_any_or_admin( pool: &mut DbPool<'_>, person_id: PersonId, - ) -> Result { + ) -> LemmyResult<()> { let is_mod_of_any = - CommunityModeratorView::is_community_moderator_of_any(pool, person_id).await?; - if is_mod_of_any { - Ok(true) - } else if let Ok(person_view) = PersonView::read(pool, person_id).await { - Ok(person_view.is_admin) + CommunityModeratorView::is_community_moderator_of_any(pool, person_id).await; + if is_mod_of_any.is_ok() + || PersonView::read(pool, person_id) + .await + .is_ok_and(|t| t.is_admin) + { + Ok(()) } else { - Ok(false) + Err(LemmyErrorType::NotAModOrAdmin)? } } }