implement language tags for site/community in db and api #2434

Nutomic · 2022-09-08T11:38:23Z

Check all the commit messages for a list of changes.

Nutomic · 2022-09-12T10:16:05Z

I think instead of Vec<LanguageId>, it would make sense to use some kind of Set<LanguageId>, as items dont need to be ordered, and set directly provides an intersect() method which we need. Problem is that a normal HashSet or BTreeSet needs more allocations and cpu work, so it would be necessary to pull in something like tinyset. Also, diesel only returns vec for db queries, it seems there is no way to get another collection without manual conversion. So probably not worth the effort.

Nutomic · 2022-09-12T14:14:26Z

This should be ready to merge. Still needs api tests (which require updated lemmy-js-client), and a lot of additional testing (during and after lemmy-ui implementation of this feature). Also need to test that db upgrades are working fine.

Nutomic · 2022-09-14T19:52:29Z

Last commit is broken, adding the list of languages to CommunityView seems tricky.

migrations/2022-09-08-102358_site-and-community-languages/down.sql

scripts/test.sh

crates/db_schema/src/impls/actor_language.rs

crates/db_schema/src/aggregates/site_aggregates.rs

crates/db_schema/src/impls/actor_language.rs

crates/api_crud/src/post/create.rs

crates/api_crud/src/comment/update.rs

crates/db_schema/src/impls/actor_language.rs

dessalines · 2022-09-19T20:51:20Z

crates/api_crud/src/community/update.rs

+        return Err(LemmyError::from_message("language_not_allowed"));
+      }
+      blocking(context.pool(), move |conn| {
+        CommunityLanguage::update(conn, languages, community_id)


You can delete the subset check above, and just make sure CommunityLanguage::update is doing the subset in SQL by reading from SiteLanguages.

You mean as part of the insert statement? How would that work in sql?

No, I mean inside the CommunityLanguage::update( fn, where it looks like you are already reading the site languages anyway. Looks like since users are providing any list of languages, you do have to read the site languages from sql, but can do a .retain type thing in that function.

I see, but i cant move the check in here because of LemmyError in return value. Then i would also have to return LemmyError from Community::create(), which is incompatible with the trait.

crates/api_crud/src/post/update.rs

crates/db_schema/src/impls/actor_language.rs

less

migrations/2022-09-08-102358_site-and-community-languages/down.sql

dessalines · 2022-09-30T15:11:31Z

crates/api_crud/src/post/create.rs

+    Ok::<(Vec<LanguageId>, Vec<LanguageId>, LanguageId), LemmyError>((
+      LocalUserLanguage::read(conn, local_user_id)?,
+      CommunityLanguage::read(conn, community_id)?,
+      Language::undetermined(),


Still an issue, this intersect logic should be done in SQL.

I didnt finish addressing all the review comments yet, thats why they arent marked as resolved.

crates/db_schema/src/impls/actor_language.rs

crates/db_views_actor/src/structs.rs

dessalines · 2022-09-30T15:24:03Z

crates/api_crud/src/community/update.rs

+        return Err(LemmyError::from_message("language_not_allowed"));
+      }
+      blocking(context.pool(), move |conn| {
+        CommunityLanguage::update(conn, languages, community_id)


No, I mean inside the CommunityLanguage::update( fn, where it looks like you are already reading the site languages anyway. Looks like since users are providing any list of languages, you do have to read the site languages from sql, but can do a .retain type thing in that function.

crates/db_schema/src/impls/actor_language.rs

Nutomic · 2022-10-03T13:28:21Z

Okay made all the requested changes, except for the one in EditCommunity which is not possible due to error types.

dessalines

Nice job, the main thing I was concerned about was the intersection logic being in SQL. Feel free to merge once you add the TODO.

dessalines · 2022-10-05T16:39:51Z

crates/db_schema/src/impls/actor_language.rs

+      .limit(1)
+      .into_boxed();
+    site_language
+      .filter(site_id.eq_any(subquery))


Put a TODO here because this subquery is bad. This should go away once we add a site:local column.

dessalines · 2022-10-05T16:44:57Z

crates/db_schema/src/impls/actor_language.rs

+      .filter(c::local)
+      .filter(sl::language_id.is_null())
+      .select(cl::language_id)
+      .get_results(conn)?;


Looks great, nice job.

dessalines · 2022-10-05T16:46:47Z

crates/db_schema/src/impls/actor_language.rs

+    .filter(ul::local_user_id.eq(local_user_id))
+    .filter(cl::community_id.eq(community_id))
+    .select(cl::language_id)
+    .get_results::<LanguageId>(conn)?;


Perfect. I think I need some way to get this info to the front end tho.

Added this to GetCommunityView.

dessalines · 2022-10-05T16:50:03Z

Once this is merged, I'll start on the front end for it.

…e all languages enabled

also, when making a new post and subset of user lang, community lang contains only one item, use that as post lang

Nutomic force-pushed the site-and-community-languages branch 3 times, most recently from c703d13 to de90e5b Compare September 9, 2022 20:05

Nutomic force-pushed the site-and-community-languages branch from 28c4b01 to f1388e0 Compare September 12, 2022 13:48

Nutomic marked this pull request as ready for review September 12, 2022 14:13

Nutomic requested a review from dessalines as a code owner September 12, 2022 14:13

Nutomic mentioned this pull request Sep 12, 2022

Add post and comment language tags LemmyNet/lemmy-ui#771

Closed

Nutomic marked this pull request as draft September 14, 2022 19:46

dessalines reviewed Sep 19, 2022

View reviewed changes

Nutomic force-pushed the site-and-community-languages branch from 171d714 to ddbc7b6 Compare September 30, 2022 13:41

Nutomic commented Sep 30, 2022

View reviewed changes

crates/db_schema/src/impls/actor_language.rs Show resolved Hide resolved

Nutomic force-pushed the site-and-community-languages branch 2 times, most recently from 8ae41fc to 5aad293 Compare September 30, 2022 14:55

dessalines reviewed Sep 30, 2022

View reviewed changes

Nutomic force-pushed the site-and-community-languages branch 4 times, most recently from 670d201 to aad0f16 Compare October 3, 2022 13:13

Nutomic marked this pull request as ready for review October 3, 2022 13:14

Nutomic force-pushed the site-and-community-languages branch 2 times, most recently from e163855 to b33ab7d Compare October 3, 2022 13:33

dessalines approved these changes Oct 5, 2022

View reviewed changes

Nutomic added 3 commits October 6, 2022 10:26

implement language tags for site/community in db and api

0aff90a

add api checks for valid languages

5ccb0cf

during db migration, update existing users, sites, communities to hav…

3e48c5b

…e all languages enabled

Nutomic added 10 commits October 6, 2022 10:26

init new users/communities with site languages (not all languages)

32fedad

federate site/community languages

6aa54d1

fix tests

94acc8c

when updating site languages, limit community languages to this subset

c67c0c2

also, when making a new post and subset of user lang, community lang contains only one item, use that as post lang

add tests for actor_language db functions

e44e447

include language list in siteview/communityview

3dadc6b

Fix some of the review comments

11ba8d6

Some more review changes

bacae61

Add todo about boxed query

c16a31a

Add default_post_language to GetCommunityResponse

a4e427e

Nutomic force-pushed the site-and-community-languages branch from 540b062 to a4e427e Compare October 6, 2022 08:26

dessalines merged commit 2ef0f8f into main Oct 6, 2022

Nutomic mentioned this pull request Dec 2, 2022

Limit language for instances and communities #1596

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement language tags for site/community in db and api #2434

implement language tags for site/community in db and api #2434

Nutomic commented Sep 8, 2022 •

edited

Loading

Nutomic commented Sep 12, 2022

Nutomic commented Sep 12, 2022

Nutomic commented Sep 14, 2022

dessalines Sep 19, 2022

Nutomic Sep 30, 2022

dessalines Sep 30, 2022

Nutomic Sep 30, 2022

dessalines Sep 30, 2022

Nutomic Sep 30, 2022 •

edited

Loading

dessalines Sep 30, 2022

Nutomic commented Oct 3, 2022 •

edited

Loading

dessalines left a comment

dessalines Oct 5, 2022

Nutomic Oct 6, 2022

dessalines Oct 5, 2022

dessalines Oct 5, 2022

Nutomic Oct 6, 2022

dessalines commented Oct 5, 2022

implement language tags for site/community in db and api #2434

implement language tags for site/community in db and api #2434

Conversation

Nutomic commented Sep 8, 2022 • edited Loading

Nutomic commented Sep 12, 2022

Nutomic commented Sep 12, 2022

Nutomic commented Sep 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nutomic Sep 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nutomic commented Oct 3, 2022 • edited Loading

dessalines left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dessalines commented Oct 5, 2022

Nutomic commented Sep 8, 2022 •

edited

Loading

Nutomic Sep 30, 2022 •

edited

Loading

Nutomic commented Oct 3, 2022 •

edited

Loading