Feature filter keywords 3710 #5263

leoseg · 2024-12-14T14:41:27Z

Option to block keyswords so no post containing this keywords in their name, body or url are returned, keywords are stored in seperate table related to person

aeharding · 2024-12-16T03:10:33Z

What happens if you block "X"? Will it match "Xylophone"? What about "x.com"?

Unlike person and community blocks this seems a bit more ambiguous.

leoseg · 2024-12-16T08:39:35Z

In the moment yes, because the ilike searches for case insensitive substrings.
A possibility could be to block only posts and descriptions which containing the blocked keyword as one word of his own (without being part of another word) , or urls having the keyword as parameter, or identifier without prefixes and suffices. For example for www.mcdonalds.com and the 'mcdonalds' part should be equal the blocked keyword if it should be filtered.

Nutomic · 2024-12-17T09:50:09Z

You could also consider a minimum length of eg 3 characters for each blocked string.

And you need to make sure that the checks are passing (click the details link at the bottom of this page). See .woodpecker.yml in the repo for the commands it runs, so you can check and fix them locally.

leoseg · 2024-12-19T18:48:18Z

yes this sounds more simple and in the Connect Lenny app they do it like this. I will add the restriction, fix checks add unit tests when I have time at the beginning of the next year.

…nding functions in diesel

dessalines · 2025-02-11T20:33:39Z

I apologize for being slow to review this, ping me or request review when you'd like me to take a look.

# Conflicts: # crates/db_views/src/post/post_view.rs # src/api_routes_v3.rs

Nutomic · 2025-03-05T13:40:11Z

src/api_routes_v3.rs

-            .route("/report/resolve", put().to(resolve_post_report)),
+            .route("/report/resolve", put().to(resolve_post_report))
+            .route("/site_metadata", get().to(get_link_metadata))
+            .route("/block", post().to(user_block_keyword_for_posts)),


Dont change api v3, this is only kept for backwards compatibility.

I removed the changes

Nutomic · 2025-03-05T13:40:24Z

src/api_routes_v4.rs

@@ -355,7 +356,8 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) {
            scope("/block")
              .route("/person", post().to(user_block_person))
              .route("/community", post().to(user_block_community))
-              .route("/instance", post().to(user_block_instance)),
+              .route("/instance", post().to(user_block_instance))
+              .route("/post", post().to(user_block_keyword_for_posts)),


How about /account/block/post_keywords?

Nutomic · 2025-03-05T13:41:41Z

migrations/2025-02-06-220512_update_keyword_length/up.sql

@@ -0,0 +1,3 @@
+ALTER TABLE post_keyword_block
+    ALTER COLUMN keyword TYPE varchar(50);
+


There should only be a single migration per pull request. For the table name user_post_keyword_block is clearer.

I deleted the new migration and altered the first migration.

Nutomic · 2025-03-05T13:42:32Z

migrations/2024-12-29-203717_add_post_keyword_block_table/up.sql

+    id serial PRIMARY KEY,
+    keyword varchar(255) NOT NULL,
+    person_id int REFERENCES person (id) ON UPDATE CASCADE ON DELETE CASCADE NOT NULL
+);


Needs a unique constraint for (keyword, person_id). You also might be able to use that as primary key and get rid of id.

Nutomic · 2025-03-05T13:46:18Z

crates/db_schema/src/schema.rs

+diesel::table! {
+    post_keyword_block (id) {
+        id -> Int4,
+        #[max_length = 20]


This length doesnt match the migration.

Nutomic · 2025-03-05T13:46:45Z

crates/db_schema/src/newtypes.rs

+#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq, Serialize, Deserialize, Default)]
+#[cfg_attr(feature = "full", derive(DieselNewType, TS))]
+#[cfg_attr(feature = "full", ts(export))]
+/// The comment reply id.


Wrong copypaste

Is removed because id is replaced by compose primary key

Nutomic · 2025-03-05T13:52:48Z

crates/api/src/post/block.rs

+    PostKeywordBlock::unblock_keyword(&mut context.pool(), &post_keyword_block_form).await?;
+  }
+  Ok(Json(SuccessResponse::default()))
+}


Instead of blocking or unblocking keywords one by one, you could also update the whole blocklist at once (just like we do for user languages). Not sure which approach is preferable.

Sounds good, should be more easy for the Frontend to implement, updated that this way.

Nutomic · 2025-03-06T08:57:47Z

crates/db_views/src/post/post_view.rs

+                  sql::<Bool>("NOT (post.body LIKE ANY(")
+                    .bind::<Array<Text>, _>(transformed_strings.clone())
+                    .sql("))"),
+                ),


Instead of this raw sql you can build a regex like (keyword1|keyword2|...) and filter with that.

I thought this would be less performant, as the regex string could be a bit long when there are for example 15 keywords? If i would do that would this be with like and than the regex (keyword1|keyword2|...) ?

Not sure, cc @dessalines @dullbananas

You should be able to use not similar to:

https://docs.rs/diesel/latest/diesel/expression_methods/trait.PgTextExpressionMethods.html#method.not_similar_to

https://stackoverflow.com/a/4928155/1655478

Regex would def be slower than this. But yeah none of this custom binding is necessary. Also extract the building of this string to its own function.

Use this loop:

for keyword in blocked_keywords { let pattern = format!("%{}%", s); query = query.filter(post::name.not_like(pattern)); query = query.filter(post::url.not_like(pattern)); query = query.filter(post::body.not_like(pattern)); }

Also, it should probably be ilike instead of like

…nged of api path in v4

…block and replaced id as primary key with the composed key person_id and keyword. Also now use update function which updates the blocked keywords for an user with a new list instead keyword by keyword.

Nutomic · 2025-03-10T10:01:15Z

crates/db_schema/src/impls/post_keyword_block.rs

+      .values(post_keyword_block_form)
+      .get_result::<Self>(conn)
+      .await
+  }


This is only used in a test, you can replace it with update(). And the unblock method below is completely unused.

dessalines · 2025-03-10T13:46:20Z

crates/api_common/src/post.rs

+#[cfg_attr(feature = "full", derive(TS))]
+#[cfg_attr(feature = "full", ts(export))]
+pub struct BlockKeywordForPost {
+  pub keywords_to_block: Vec<String>,


This shouldn't be here, but it should be part of the SaveUserSettings struct in crates/api_common/src/person.rs .

Look at its discussion_languages field for an example of a vector update.

dessalines · 2025-03-10T13:48:44Z

crates/api/src/post/block.rs

Most of this code should be in crates/api/src/local_user/save_settings.rs , because its a user setting.

Try to extract most of the checking to a function also, to keep that function readable.

dessalines · 2025-03-10T13:49:49Z

crates/db_schema/src/impls/post_keyword_block.rs

@@ -0,0 +1,87 @@
+use crate::{
+  newtypes::PersonId,
+  schema::user_post_keyword_block::dsl::{keyword, person_id, user_post_keyword_block},


Don't import the dsl use keyword_block::table and keyword_block::{column_name}. Check the other db schema impl files for examples.

dessalines · 2025-03-10T13:51:27Z

crates/db_schema/src/impls/post_keyword_block.rs

+    conn
+      .build_transaction()
+      .run(|conn| {
+        Box::pin(async move {


Remove this transaction, as we realized that using transactions within DB code is a no no, and the transactions should be done at the highest level (the API level). Otherwise we can get untraceable errors from embedded transactions.

No this should be fine, see #5480 (comment)

Clarifying @Nutomic's comment: .build_transaction().run(...) must be changed to .transaction(...) to make it fine

dessalines · 2025-03-10T13:54:52Z

crates/db_schema/src/impls/post_keyword_block.rs

+    let current = UserPostKeywordBlock::for_person(&mut conn.into(), for_person_id).await?;
+    if current
+      .iter()
+      .map(|obj| obj.keyword.clone())
+      .collect::<Vec<_>>()
+      == keywords_to_block_posts
+    {
+      return Ok(());
+    }


Remove this block, its pointless. Just delete then update no matter what. The on_conflict_do_nothing below should handle everything fine.

If they passed Some(keywords_to_block) we can assume they want to update them.

dessalines · 2025-03-10T14:03:52Z

crates/utils/src/error.rs

@@ -10,6 +10,9 @@ use strum::{Display, EnumIter};
 #[non_exhaustive]
 // TODO: order these based on the crate they belong to (utils, federation, db, api)
 pub enum LemmyErrorType {
+  BlockKeywordLimitReached,


I don't think we need a limit, right?

dessalines · 2025-03-10T14:04:07Z

crates/utils/src/error.rs

@@ -10,6 +10,9 @@ use strum::{Display, EnumIter};
 #[non_exhaustive]
 // TODO: order these based on the crate they belong to (utils, federation, db, api)
 pub enum LemmyErrorType {
+  BlockKeywordLimitReached,
+  BlockKeywordToShort,


BlockKeywordTooShort

dessalines · 2025-03-10T14:04:12Z

crates/utils/src/error.rs

@@ -10,6 +10,9 @@ use strum::{Display, EnumIter};
 #[non_exhaustive]
 // TODO: order these based on the crate they belong to (utils, federation, db, api)
 pub enum LemmyErrorType {
+  BlockKeywordLimitReached,
+  BlockKeywordToShort,
+  BlockKeywordToLong,


dessalines · 2025-03-10T14:12:46Z

migrations/2024-12-29-203717_add_post_keyword_block_table/up.sql

@@ -0,0 +1,6 @@
+CREATE TABLE user_post_keyword_block (


This table name doesn't look like any other one.

Look at local_user_language for an example.

It should be named local_user_keyword_block , as its a local user setting, not a person field that we need to worry about federation.

Its columns should be local_user_id, keyword (in that order)

I don't think we need to reference that this applies only to posts : keyword_block is enough. In the future we might want this to apply to comments and other things anyway.

The performance of this scares me, and I don't think there's really a way we can make lower(post.title) not similar to '%(keyword_1|keyword_2|...)%' performant. But I'm still willing to merge it and hack on that later.

Also, "REFERENCES person" becomes "REFERENCES local_user"

dessalines · 2025-03-10T14:13:38Z

src/api_routes_v4.rs

@@ -355,7 +356,8 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) {
            scope("/block")
              .route("/person", post().to(user_block_person))
              .route("/community", post().to(user_block_community))
-              .route("/instance", post().to(user_block_instance)),
+              .route("/instance", post().to(user_block_instance))
+              .route("/post_keywords", post().to(user_block_keyword_for_posts)),


This can be removed, as the keywords to block will be included as part of save_user_settings

dullbananas · 2025-03-11T00:41:21Z

.gitignore

@@ -34,3 +34,4 @@ dev_pgdata/

 # database dumps
 *.sqldump
+tmp.schema


This is probably unnecessary. If you're locally running the commands from the "check_diesel_schema" CI step, then skip the cp and diff commands.

dullbananas · 2025-03-11T00:58:33Z

crates/api/src/post/block.rs

+  local_user_view: LocalUserView,
+) -> LemmyResult<Json<SuccessResponse>> {
+  for keyword in &data.keywords_to_block {
+    let trimmed = keyword.trim();


Create a Vec of trimmed keywords, and use it in both the length check and the call to UserPostKeywordBlock::update.

dullbananas · 2025-03-11T01:05:53Z

crates/db_schema/src/impls/post_keyword_block.rs

+    conn
+      .build_transaction()
+      .run(|conn| {
+        Box::pin(async move {


Clarifying @Nutomic's comment: .build_transaction().run(...) must be changed to .transaction(...) to make it fine

dullbananas · 2025-03-11T01:19:09Z

crates/api/src/post/block.rs

+  if data.keywords_to_block.len() >= 15 {
+    Err(LemmyErrorType::BlockKeywordLimitReached)?;
+  }


This is how length is checked in other places, and this will also change the limit to 1000 which is good. The newly created error type must also be removed.

Suggested change

if data.keywords_to_block.len() >= 15 {

Err(LemmyErrorType::BlockKeywordLimitReached)?;

}

if data.keywords_to_block.len() > MAX_API_PARAM_ELEMENTS {

Err(LemmyErrorType::TooManyItems)?;

}

dullbananas · 2025-03-11T01:24:53Z

migrations/2024-12-29-203717_add_post_keyword_block_table/up.sql

@@ -0,0 +1,6 @@
+CREATE TABLE user_post_keyword_block (


Also, "REFERENCES person" becomes "REFERENCES local_user"

leoseg · 2025-03-14T16:41:58Z

Thanks for all the feedback! I'll look into it soon and update the PR.

leoseg mentioned this pull request Dec 14, 2024

[Feature Request] Allow a user to filter on keywords #3710

Open

4 tasks

leoseg added 15 commits January 31, 2025 17:27

created new table post_keyword_block related to a person and correspo…

2b2c026

…nding functions in diesel

created corresponding api functions

a4a771c

converted like to ilike in filter

5a55f02

Fixxed errors for adding keywords for blocking posts

b751617

Fixxed errors in diesel migrations

211d65b

Removed debug print

1f3a546

LemmyNet#3710: Fixxed errors caused by cargo check

35284eb

LemmyNet#3710: added up and down scripts for diesel

d134d67

added tmp.schema to gitignore

7ad853b

updated tests

f13c33f

fixxed test

c92b824

fixxed error from rebasing

3e38887

Added max number of block tags and modified max length of block tags

b79711d

formatted code

0f13d43

added migration to restrict block keyword length

3d1e7d5

leoseg force-pushed the Feature-Filter-Keywords-3710 branch from 0441a7d to 3d1e7d5 Compare February 6, 2025 22:51

leoseg added 6 commits March 2, 2025 17:48

LemmyNet#3710: Added check if Blocked keyword is already existing

b4f75b4

LemmyNet#3710: Removed deprecated any

ba2c73a

LemmyNet#3710: Updated keyword length

1acb42a

Merge branch 'main' into Feature-Filter-Keywords-3710

e385c15

# Conflicts: # crates/db_views/src/post/post_view.rs # src/api_routes_v3.rs

LemmyNet#3710: Fixxed errors from master merge

60d7bdd

LemmyNet#3710: Formatting

26f909f

leoseg marked this pull request as ready for review March 5, 2025 10:14

leoseg requested a review from dessalines as a code owner March 5, 2025 10:14

leoseg requested review from phiresky, dullbananas, Nutomic and SleeplessOne1917 as code owners March 5, 2025 10:14

Nutomic reviewed Mar 5, 2025

View reviewed changes

Nutomic reviewed Mar 6, 2025

View reviewed changes

leoseg added 3 commits March 9, 2025 13:10

LemmyNet#3710: Removed api to block keywords for post from v3 and cha…

0cf1ed6

…nged of api path in v4

LemmyNet#3710: Renamed post_keyword_block table to user_post_keyword_…

24af7e3

…block and replaced id as primary key with the composed key person_id and keyword. Also now use update function which updates the blocked keywords for an user with a new list instead keyword by keyword.

LemmyNet#3710: Formatted code

fc49f01

leoseg force-pushed the Feature-Filter-Keywords-3710 branch from 3524070 to fc49f01 Compare March 10, 2025 09:39

Merge branch 'main' into Feature-Filter-Keywords-3710

51d5c96

Nutomic reviewed Mar 10, 2025

View reviewed changes

dessalines reviewed Mar 10, 2025

View reviewed changes

dullbananas requested changes Mar 11, 2025

View reviewed changes

		@@ -0,0 +1,3 @@
		ALTER TABLE post_keyword_block
		ALTER COLUMN keyword TYPE varchar(50);

Feature filter keywords 3710 #5263

Are you sure you want to change the base?

Feature filter keywords 3710 #5263

Conversation

leoseg commented Dec 14, 2024

aeharding commented Dec 16, 2024

leoseg commented Dec 16, 2024

Nutomic commented Dec 17, 2024 • edited Loading

leoseg commented Dec 19, 2024

dessalines commented Feb 11, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nutomic Mar 5, 2025 • edited Loading

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

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

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

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

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

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

leoseg commented Mar 14, 2025

Nutomic commented Dec 17, 2024 •

edited

Loading

Nutomic Mar 5, 2025 •

edited

Loading