From 846848c4f6a5ab0ebce9c513f62ff0074b27f50c Mon Sep 17 00:00:00 2001 From: Nutomic Date: Mon, 25 Mar 2024 21:02:12 +0100 Subject: [PATCH] On registration, automatically set content languages from `accept-language` header (#4550) * On registration, automatically set content languages from accept header * no need to set site language or default language for new user anymore * fix test * fix langs * avoid duplicate writing of new user languages --- api_tests/src/user.spec.ts | 20 ++++ .../src/local_user/generate_totp_secret.rs | 5 +- crates/api/src/local_user/update_totp.rs | 5 +- crates/api_common/src/claims.rs | 4 +- crates/api_crud/src/user/create.rs | 32 ++++-- crates/apub/src/api/user_settings_backup.rs | 2 +- crates/db_schema/src/impls/actor_language.rs | 19 ++-- crates/db_schema/src/impls/local_user.rs | 100 +++++++++--------- .../src/impls/password_reset_request.rs | 4 +- crates/db_views/src/comment_report_view.rs | 4 +- crates/db_views/src/comment_view.rs | 2 +- crates/db_views/src/post_report_view.rs | 4 +- crates/db_views/src/post_view.rs | 3 +- .../src/registration_application_view.rs | 6 +- crates/db_views_actor/src/community_view.rs | 4 +- crates/db_views_actor/src/person_view.rs | 4 +- src/code_migrations.rs | 2 +- src/session_middleware.rs | 4 +- 18 files changed, 130 insertions(+), 94 deletions(-) diff --git a/api_tests/src/user.spec.ts b/api_tests/src/user.spec.ts index 4846d60f7..73f3f3942 100644 --- a/api_tests/src/user.spec.ts +++ b/api_tests/src/user.spec.ts @@ -139,3 +139,23 @@ test("Create user with Arabic name", async () => { let alphaPerson = (await resolvePerson(alpha, apShortname)).person; expect(alphaPerson).toBeDefined(); }); + +test("Create user with accept-language", async () => { + let lemmy_http = new LemmyHttp(alphaUrl, { + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Language#syntax + headers: { "Accept-Language": "fr-CH, en;q=0.8, de;q=0.7, *;q=0.5" }, + }); + let user = await registerUser(lemmy_http, alphaUrl); + + let site = await getSite(user); + expect(site.my_user).toBeDefined(); + expect(site.my_user?.local_user_view.local_user.interface_language).toBe( + "fr", + ); + let langs = site.all_languages + .filter(a => site.my_user?.discussion_languages.includes(a.id)) + .map(l => l.code); + // should have languages from accept header, as well as "undetermined" + // which is automatically enabled by backend + expect(langs).toStrictEqual(["und", "de", "en", "fr"]); +}); diff --git a/crates/api/src/local_user/generate_totp_secret.rs b/crates/api/src/local_user/generate_totp_secret.rs index a983beaaa..342a90a78 100644 --- a/crates/api/src/local_user/generate_totp_secret.rs +++ b/crates/api/src/local_user/generate_totp_secret.rs @@ -6,10 +6,7 @@ use lemmy_api_common::{ person::GenerateTotpSecretResponse, sensitive::Sensitive, }; -use lemmy_db_schema::{ - source::local_user::{LocalUser, LocalUserUpdateForm}, - traits::Crud, -}; +use lemmy_db_schema::source::local_user::{LocalUser, LocalUserUpdateForm}; use lemmy_db_views::structs::{LocalUserView, SiteView}; use lemmy_utils::error::{LemmyError, LemmyErrorType}; diff --git a/crates/api/src/local_user/update_totp.rs b/crates/api/src/local_user/update_totp.rs index 8f37213e2..c8ca9f64e 100644 --- a/crates/api/src/local_user/update_totp.rs +++ b/crates/api/src/local_user/update_totp.rs @@ -4,10 +4,7 @@ use lemmy_api_common::{ context::LemmyContext, person::{UpdateTotp, UpdateTotpResponse}, }; -use lemmy_db_schema::{ - source::local_user::{LocalUser, LocalUserUpdateForm}, - traits::Crud, -}; +use lemmy_db_schema::source::local_user::{LocalUser, LocalUserUpdateForm}; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::error::LemmyError; diff --git a/crates/api_common/src/claims.rs b/crates/api_common/src/claims.rs index 0a96f7455..0e6968742 100644 --- a/crates/api_common/src/claims.rs +++ b/crates/api_common/src/claims.rs @@ -124,7 +124,9 @@ mod tests { .password_encrypted("123456".to_string()) .build(); - let inserted_local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); + let inserted_local_user = LocalUser::create(pool, &local_user_form, vec![]) + .await + .unwrap(); let req = TestRequest::default().to_http_request(); let jwt = Claims::generate(inserted_local_user.id, req, &context) diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index 29e16cb20..d24a287db 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -20,6 +20,7 @@ use lemmy_db_schema::{ aggregates::structs::PersonAggregates, source::{ captcha_answer::{CaptchaAnswer, CheckCaptchaAnswer}, + language::Language, local_user::{LocalUser, LocalUserInsertForm}, local_user_vote_display_mode::LocalUserVoteDisplayMode, person::{Person, PersonInsertForm}, @@ -36,6 +37,7 @@ use lemmy_utils::{ validation::is_valid_actor_name, }, }; +use std::collections::HashSet; #[tracing::instrument(skip(context))] pub async fn register( @@ -128,12 +130,15 @@ pub async fn register( let accepted_application = Some(!require_registration_application); // Get the user's preferred language using the Accept-Language header - let language_tag = req.headers().get("Accept-Language").and_then(|hdr| { - accept_language::parse(hdr.to_str().unwrap_or_default()) - .first() - // Remove the optional region code - .map(|lang_str| lang_str.split('-').next().unwrap_or_default().to_string()) - }); + let language_tags: Vec = req + .headers() + .get("Accept-Language") + .map(|hdr| accept_language::parse(hdr.to_str().unwrap_or_default())) + .iter() + .flatten() + // Remove the optional region code + .map(|lang_str| lang_str.split('-').next().unwrap_or_default().to_string()) + .collect(); // Create the local user let local_user_form = LocalUserInsertForm::builder() @@ -144,12 +149,23 @@ pub async fn register( .accepted_application(accepted_application) .default_listing_type(Some(local_site.default_post_listing_type)) .post_listing_mode(Some(local_site.default_post_listing_mode)) - .interface_language(language_tag) + .interface_language(language_tags.first().cloned()) // If its the initial site setup, they are an admin .admin(Some(!local_site.site_setup)) .build(); - let inserted_local_user = LocalUser::create(&mut context.pool(), &local_user_form).await?; + let all_languages = Language::read_all(&mut context.pool()).await?; + // use hashset to avoid duplicates + let mut language_ids = HashSet::new(); + for l in language_tags { + if let Some(found) = all_languages.iter().find(|all| all.code == l) { + language_ids.insert(found.id); + } + } + let language_ids = language_ids.into_iter().collect(); + + let inserted_local_user = + LocalUser::create(&mut context.pool(), &local_user_form, language_ids).await?; if local_site.site_setup && require_registration_application { // Create the registration application diff --git a/crates/apub/src/api/user_settings_backup.rs b/crates/apub/src/api/user_settings_backup.rs index a2e6c55ac..d46e415b0 100644 --- a/crates/apub/src/api/user_settings_backup.rs +++ b/crates/apub/src/api/user_settings_backup.rs @@ -361,7 +361,7 @@ mod tests { .person_id(person.id) .password_encrypted("pass".to_string()) .build(); - let local_user = LocalUser::create(&mut context.pool(), &user_form).await?; + let local_user = LocalUser::create(&mut context.pool(), &user_form, vec![]).await?; Ok(LocalUserView::read(&mut context.pool(), local_user.id).await?) } diff --git a/crates/db_schema/src/impls/actor_language.rs b/crates/db_schema/src/impls/actor_language.rs index 1cb5f4c77..29d99b2d8 100644 --- a/crates/db_schema/src/impls/actor_language.rs +++ b/crates/db_schema/src/impls/actor_language.rs @@ -523,10 +523,6 @@ mod tests { let pool = &mut pool.into(); let (site, instance) = create_test_site(pool).await; - let mut test_langs = test_langs1(pool).await; - SiteLanguage::update(pool, test_langs.clone(), &site) - .await - .unwrap(); let person_form = PersonInsertForm::builder() .name("my test person".to_string()) @@ -539,14 +535,13 @@ mod tests { .password_encrypted("my_pw".to_string()) .build(); - let local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); + let local_user = LocalUser::create(pool, &local_user_form, vec![]) + .await + .unwrap(); let local_user_langs1 = LocalUserLanguage::read(pool, local_user.id).await.unwrap(); - // new user should be initialized with site languages and undetermined - //test_langs.push(UNDETERMINED_ID); - //test_langs.sort(); - test_langs.insert(0, UNDETERMINED_ID); - assert_eq!(test_langs, local_user_langs1); + // new user should be initialized with all languages + assert_eq!(0, local_user_langs1.len()); // update user languages let test_langs2 = test_langs2(pool).await; @@ -655,7 +650,9 @@ mod tests { .person_id(person.id) .password_encrypted("my_pw".to_string()) .build(); - let local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); + let local_user = LocalUser::create(pool, &local_user_form, vec![]) + .await + .unwrap(); LocalUserLanguage::update(pool, test_langs2, local_user.id) .await .unwrap(); diff --git a/crates/db_schema/src/impls/local_user.rs b/crates/db_schema/src/impls/local_user.rs index 14ae65469..d253afd8d 100644 --- a/crates/db_schema/src/impls/local_user.rs +++ b/crates/db_schema/src/impls/local_user.rs @@ -1,12 +1,11 @@ use crate::{ - newtypes::{DbUrl, LocalUserId, PersonId}, + newtypes::{DbUrl, LanguageId, LocalUserId, PersonId}, schema::{local_user, person, registration_application}, source::{ - actor_language::{LocalUserLanguage, SiteLanguage}, + actor_language::LocalUserLanguage, local_user::{LocalUser, LocalUserInsertForm, LocalUserUpdateForm}, local_user_vote_display_mode::{LocalUserVoteDisplayMode, LocalUserVoteDisplayModeInsertForm}, }, - traits::Crud, utils::{ functions::{coalesce, lower}, get_conn, @@ -25,6 +24,52 @@ use diesel::{ use diesel_async::RunQueryDsl; impl LocalUser { + pub async fn create( + pool: &mut DbPool<'_>, + form: &LocalUserInsertForm, + languages: Vec, + ) -> Result { + let conn = &mut get_conn(pool).await?; + let mut form_with_encrypted_password = form.clone(); + let password_hash = + hash(&form.password_encrypted, DEFAULT_COST).expect("Couldn't hash password"); + form_with_encrypted_password.password_encrypted = password_hash; + + let local_user_ = insert_into(local_user::table) + .values(form_with_encrypted_password) + .get_result::(conn) + .await?; + + LocalUserLanguage::update(pool, languages, local_user_.id).await?; + + // Create their vote_display_modes + let vote_display_mode_form = LocalUserVoteDisplayModeInsertForm::builder() + .local_user_id(local_user_.id) + .build(); + LocalUserVoteDisplayMode::create(pool, &vote_display_mode_form).await?; + + Ok(local_user_) + } + + pub async fn update( + pool: &mut DbPool<'_>, + local_user_id: LocalUserId, + form: &LocalUserUpdateForm, + ) -> Result { + let conn = &mut get_conn(pool).await?; + diesel::update(local_user::table.find(local_user_id)) + .set(form) + .get_result::(conn) + .await + } + + pub async fn delete(pool: &mut DbPool<'_>, id: LocalUserId) -> Result { + let conn = &mut *get_conn(pool).await?; + diesel::delete(local_user::table.find(id)) + .execute(conn) + .await + } + pub async fn update_password( pool: &mut DbPool<'_>, local_user_id: LocalUserId, @@ -183,52 +228,3 @@ pub struct UserBackupLists { pub blocked_users: Vec, pub blocked_instances: Vec, } - -#[async_trait] -impl Crud for LocalUser { - type InsertForm = LocalUserInsertForm; - type UpdateForm = LocalUserUpdateForm; - type IdType = LocalUserId; - - async fn create(pool: &mut DbPool<'_>, form: &Self::InsertForm) -> Result { - let conn = &mut get_conn(pool).await?; - let mut form_with_encrypted_password = form.clone(); - let password_hash = - hash(&form.password_encrypted, DEFAULT_COST).expect("Couldn't hash password"); - form_with_encrypted_password.password_encrypted = password_hash; - - let local_user_ = insert_into(local_user::table) - .values(form_with_encrypted_password) - .get_result::(conn) - .await?; - - let site_languages = SiteLanguage::read_local_raw(pool).await; - if let Ok(langs) = site_languages { - // if site exists, init user with site languages - LocalUserLanguage::update(pool, langs, local_user_.id).await?; - } else { - // otherwise, init with all languages (this only happens during tests and - // for first admin user, which is created before site) - LocalUserLanguage::update(pool, vec![], local_user_.id).await?; - } - - // Create their vote_display_modes - let vote_display_mode_form = LocalUserVoteDisplayModeInsertForm::builder() - .local_user_id(local_user_.id) - .build(); - LocalUserVoteDisplayMode::create(pool, &vote_display_mode_form).await?; - - Ok(local_user_) - } - async fn update( - pool: &mut DbPool<'_>, - local_user_id: LocalUserId, - form: &Self::UpdateForm, - ) -> Result { - let conn = &mut get_conn(pool).await?; - diesel::update(local_user::table.find(local_user_id)) - .set(form) - .get_result::(conn) - .await - } -} diff --git a/crates/db_schema/src/impls/password_reset_request.rs b/crates/db_schema/src/impls/password_reset_request.rs index 6fcf86014..3b910b1d3 100644 --- a/crates/db_schema/src/impls/password_reset_request.rs +++ b/crates/db_schema/src/impls/password_reset_request.rs @@ -121,7 +121,9 @@ mod tests { .password_encrypted("pass".to_string()) .build(); - let inserted_local_user = LocalUser::create(pool, &new_local_user).await.unwrap(); + let inserted_local_user = LocalUser::create(pool, &new_local_user, vec![]) + .await + .unwrap(); let token = "nope"; diff --git a/crates/db_views/src/comment_report_view.rs b/crates/db_views/src/comment_report_view.rs index 1aa3bb6e7..317ebf29a 100644 --- a/crates/db_views/src/comment_report_view.rs +++ b/crates/db_views/src/comment_report_view.rs @@ -308,7 +308,9 @@ mod tests { .person_id(inserted_timmy.id) .password_encrypted("123".to_string()) .build(); - let timmy_local_user = LocalUser::create(pool, &new_local_user).await.unwrap(); + let timmy_local_user = LocalUser::create(pool, &new_local_user, vec![]) + .await + .unwrap(); let timmy_view = LocalUserView { local_user: timmy_local_user, local_user_vote_display_mode: LocalUserVoteDisplayMode::default(), diff --git a/crates/db_views/src/comment_view.rs b/crates/db_views/src/comment_view.rs index 5f811cb0d..d80cc2e58 100644 --- a/crates/db_views/src/comment_view.rs +++ b/crates/db_views/src/comment_view.rs @@ -503,7 +503,7 @@ mod tests { .admin(Some(true)) .password_encrypted(String::new()) .build(); - let inserted_timmy_local_user = LocalUser::create(pool, &timmy_local_user_form) + let inserted_timmy_local_user = LocalUser::create(pool, &timmy_local_user_form, vec![]) .await .unwrap(); diff --git a/crates/db_views/src/post_report_view.rs b/crates/db_views/src/post_report_view.rs index 0f1f3639c..20795c8c0 100644 --- a/crates/db_views/src/post_report_view.rs +++ b/crates/db_views/src/post_report_view.rs @@ -330,7 +330,9 @@ mod tests { .person_id(inserted_timmy.id) .password_encrypted("123".to_string()) .build(); - let timmy_local_user = LocalUser::create(pool, &new_local_user).await.unwrap(); + let timmy_local_user = LocalUser::create(pool, &new_local_user, vec![]) + .await + .unwrap(); let timmy_view = LocalUserView { local_user: timmy_local_user, local_user_vote_display_mode: LocalUserVoteDisplayMode::default(), diff --git a/crates/db_views/src/post_view.rs b/crates/db_views/src/post_view.rs index ae5ec383a..e1891b37a 100644 --- a/crates/db_views/src/post_view.rs +++ b/crates/db_views/src/post_view.rs @@ -829,7 +829,7 @@ mod tests { admin: Some(true), ..LocalUserInsertForm::test_form(inserted_person.id) }; - let inserted_local_user = LocalUser::create(pool, &local_user_form).await?; + let inserted_local_user = LocalUser::create(pool, &local_user_form, vec![]).await?; let new_bot = PersonInsertForm { bot_account: Some(true), @@ -855,6 +855,7 @@ mod tests { let inserted_blocked_local_user = LocalUser::create( pool, &LocalUserInsertForm::test_form(inserted_blocked_person.id), + vec![], ) .await?; diff --git a/crates/db_views/src/registration_application_view.rs b/crates/db_views/src/registration_application_view.rs index a59158318..c2f2e3120 100644 --- a/crates/db_views/src/registration_application_view.rs +++ b/crates/db_views/src/registration_application_view.rs @@ -176,7 +176,7 @@ mod tests { .admin(Some(true)) .build(); - let _inserted_timmy_local_user = LocalUser::create(pool, &timmy_local_user_form) + let _inserted_timmy_local_user = LocalUser::create(pool, &timmy_local_user_form, vec![]) .await .unwrap(); @@ -193,7 +193,7 @@ mod tests { .password_encrypted("nada".to_string()) .build(); - let inserted_sara_local_user = LocalUser::create(pool, &sara_local_user_form) + let inserted_sara_local_user = LocalUser::create(pool, &sara_local_user_form, vec![]) .await .unwrap(); @@ -224,7 +224,7 @@ mod tests { .password_encrypted("nada".to_string()) .build(); - let inserted_jess_local_user = LocalUser::create(pool, &jess_local_user_form) + let inserted_jess_local_user = LocalUser::create(pool, &jess_local_user_form, vec![]) .await .unwrap(); diff --git a/crates/db_views_actor/src/community_view.rs b/crates/db_views_actor/src/community_view.rs index c1cb6eee1..98a146c15 100644 --- a/crates/db_views_actor/src/community_view.rs +++ b/crates/db_views_actor/src/community_view.rs @@ -296,7 +296,9 @@ mod tests { .person_id(inserted_person.id) .password_encrypted(String::new()) .build(); - let local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); + let local_user = LocalUser::create(pool, &local_user_form, vec![]) + .await + .unwrap(); let new_community = CommunityInsertForm::builder() .name("test_community_3".to_string()) diff --git a/crates/db_views_actor/src/person_view.rs b/crates/db_views_actor/src/person_view.rs index aee56748e..1ee51a819 100644 --- a/crates/db_views_actor/src/person_view.rs +++ b/crates/db_views_actor/src/person_view.rs @@ -204,7 +204,7 @@ mod tests { .person_id(alice.id) .password_encrypted(String::new()) .build(); - let alice_local_user = LocalUser::create(pool, &alice_local_user_form).await?; + let alice_local_user = LocalUser::create(pool, &alice_local_user_form, vec![]).await?; let bob_form = PersonInsertForm::builder() .name("bob".to_string()) @@ -218,7 +218,7 @@ mod tests { .person_id(bob.id) .password_encrypted(String::new()) .build(); - let bob_local_user = LocalUser::create(pool, &bob_local_user_form).await?; + let bob_local_user = LocalUser::create(pool, &bob_local_user_form, vec![]).await?; Ok(Data { alice, diff --git a/src/code_migrations.rs b/src/code_migrations.rs index 8e17b8a8c..cee02075c 100644 --- a/src/code_migrations.rs +++ b/src/code_migrations.rs @@ -475,7 +475,7 @@ async fn initialize_local_site_2022_10_10( .email(setup.admin_email.clone()) .admin(Some(true)) .build(); - LocalUser::create(pool, &local_user_form).await?; + LocalUser::create(pool, &local_user_form, vec![]).await?; }; // Add an entry for the site table diff --git a/src/session_middleware.rs b/src/session_middleware.rs index 2c4e36913..474709dbe 100644 --- a/src/session_middleware.rs +++ b/src/session_middleware.rs @@ -155,7 +155,9 @@ mod tests { .password_encrypted("123456".to_string()) .build(); - let inserted_local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); + let inserted_local_user = LocalUser::create(pool, &local_user_form, vec![]) + .await + .unwrap(); let req = TestRequest::default().to_http_request(); let jwt = Claims::generate(inserted_local_user.id, req, &context)