Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add user setting to auto-mark fetched posts as read. #5160

Merged
merged 17 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions .woodpecker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -138,17 +138,6 @@ steps:
- diff tmp.schema crates/db_schema/src/schema.rs
when: *slow_check_paths

check_db_perf_tool:
image: *rust_image
environment:
LEMMY_DATABASE_URL: postgres://lemmy:password@database:5432/lemmy
RUST_BACKTRACE: "1"
CARGO_HOME: .cargo_home
commands:
# same as scripts/db_perf.sh but without creating a new database server
- cargo run --package lemmy_db_perf -- --posts 10 --read-post-pages 1
when: *slow_check_paths

cargo_clippy:
image: *rust_image
environment:
Expand Down Expand Up @@ -221,6 +210,17 @@ steps:
- diff before.sqldump after.sqldump
when: *slow_check_paths

check_db_perf_tool:
image: *rust_image
environment:
LEMMY_DATABASE_URL: postgres://lemmy:password@database:5432/lemmy
RUST_BACKTRACE: "1"
CARGO_HOME: .cargo_home
commands:
# same as scripts/db_perf.sh but without creating a new database server
- cargo run --package lemmy_db_perf -- --posts 10 --read-post-pages 1
when: *slow_check_paths

run_federation_tests:
image: node:22-bookworm-slim
environment:
Expand Down
1 change: 1 addition & 0 deletions crates/api/src/local_user/save_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ pub async fn save_user_settings(
enable_keyboard_navigation: data.enable_keyboard_navigation,
enable_animated_images: data.enable_animated_images,
collapse_bot_comments: data.collapse_bot_comments,
auto_mark_fetched_posts_as_read: data.auto_mark_fetched_posts_as_read,
..Default::default()
};

Expand Down
13 changes: 4 additions & 9 deletions crates/api/src/post/like.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,13 @@ use lemmy_api_common::{
context::LemmyContext,
post::{CreatePostLike, PostResponse},
send_activity::{ActivityChannel, SendActivityData},
utils::{
check_bot_account,
check_community_user_action,
check_local_vote_mode,
mark_post_as_read,
VoteItem,
},
utils::{check_bot_account, check_community_user_action, check_local_vote_mode, VoteItem},
};
use lemmy_db_schema::{
source::{
community::Community,
local_site::LocalSite,
post::{Post, PostLike, PostLikeForm},
post::{Post, PostLike, PostLikeForm, PostRead},
},
traits::{Crud, Likeable},
};
Expand Down Expand Up @@ -73,7 +67,8 @@ pub async fn like_post(
.with_lemmy_type(LemmyErrorType::CouldntLikePost)?;
}

mark_post_as_read(person_id, post_id, &mut context.pool()).await?;
// Mark Post Read
PostRead::mark_as_read(&mut context.pool(), &[post_id], person_id).await?;

let community = Community::read(&mut context.pool(), post.community_id).await?;

Expand Down
13 changes: 4 additions & 9 deletions crates/api/src/post/mark_read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,15 @@ use actix_web::web::{Data, Json};
use lemmy_api_common::{context::LemmyContext, post::MarkPostAsRead, SuccessResponse};
use lemmy_db_schema::source::post::PostRead;
use lemmy_db_views::structs::LocalUserView;
use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult, MAX_API_PARAM_ELEMENTS};
use std::collections::HashSet;
use lemmy_utils::error::{LemmyErrorType, LemmyResult, MAX_API_PARAM_ELEMENTS};

#[tracing::instrument(skip(context))]
pub async fn mark_post_as_read(
data: Json<MarkPostAsRead>,
context: Data<LemmyContext>,
local_user_view: LocalUserView,
) -> LemmyResult<Json<SuccessResponse>> {
let post_ids = HashSet::from_iter(data.post_ids.clone());
let post_ids = &data.post_ids;

Copy link
Member Author

@dessalines dessalines Nov 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got rid of all these fairly pointless HashSets (a way to enforce uniqueness of post ids to be marked as read).

Since we're no longer allowing vectors of marked read post ids via the API, and only through code, these seemed redundant. Also postgres can handle in queries with the same ID with no problem anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this function is unused now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed that now in other PRs.

if post_ids.len() > MAX_API_PARAM_ELEMENTS {
Err(LemmyErrorType::TooManyItems)?;
Expand All @@ -21,13 +20,9 @@ pub async fn mark_post_as_read(

// Mark the post as read / unread
if data.read {
PostRead::mark_as_read(&mut context.pool(), post_ids, person_id)
.await
.with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead)?;
PostRead::mark_as_read(&mut context.pool(), post_ids, person_id).await?;
} else {
PostRead::mark_as_unread(&mut context.pool(), post_ids, person_id)
.await
.with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead)?;
PostRead::mark_as_unread(&mut context.pool(), post_ids, person_id).await?;
}

Ok(Json(SuccessResponse::default()))
Expand Down
5 changes: 2 additions & 3 deletions crates/api/src/post/save.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@ use actix_web::web::{Data, Json};
use lemmy_api_common::{
context::LemmyContext,
post::{PostResponse, SavePost},
utils::mark_post_as_read,
};
use lemmy_db_schema::{
source::post::{PostSaved, PostSavedForm},
source::post::{PostRead, PostSaved, PostSavedForm},
traits::Saveable,
};
use lemmy_db_views::structs::{LocalUserView, PostView};
Expand Down Expand Up @@ -42,7 +41,7 @@ pub async fn save_post(
)
.await?;

mark_post_as_read(person_id, post_id, &mut context.pool()).await?;
PostRead::mark_as_read(&mut context.pool(), &[post_id], person_id).await?;

Ok(Json(PostResponse { post_view }))
}
2 changes: 2 additions & 0 deletions crates/api_common/src/person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ pub struct SaveUserSettings {
pub show_downvotes: Option<bool>,
#[cfg_attr(feature = "full", ts(optional))]
pub show_upvote_percentage: Option<bool>,
/// Whether to automatically mark fetched posts as read.
pub auto_mark_fetched_posts_as_read: Option<bool>,
}

#[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq, Hash)]
Expand Down
3 changes: 3 additions & 0 deletions crates/api_common/src/post.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ pub struct GetPosts {
/// If true, then show the nsfw posts (even if your user setting is to hide them)
#[cfg_attr(feature = "full", ts(optional))]
pub show_nsfw: Option<bool>,
/// Whether to automatically mark fetched posts as read.
#[cfg_attr(feature = "full", ts(optional))]
pub auto_mark_fetched_posts_as_read: Option<bool>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be changed to mark_as_read

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

K fixed.

#[cfg_attr(feature = "full", ts(optional))]
/// If true, then only show posts with no comments
pub no_comments_only: Option<bool>,
Expand Down
17 changes: 2 additions & 15 deletions crates/api_common/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use lemmy_db_schema::{
password_reset_request::PasswordResetRequest,
person::{Person, PersonUpdateForm},
person_block::PersonBlock,
post::{Post, PostLike, PostRead},
post::{Post, PostLike},
registration_application::RegistrationApplication,
site::Site,
},
Expand Down Expand Up @@ -64,7 +64,7 @@ use lemmy_utils::{
use moka::future::Cache;
use regex::{escape, Regex, RegexSet};
use rosetta_i18n::{Language, LanguageId};
use std::{collections::HashSet, sync::LazyLock};
use std::sync::LazyLock;
use tracing::warn;
use url::{ParseError, Url};
use urlencoding::encode;
Expand Down Expand Up @@ -140,19 +140,6 @@ pub fn is_top_mod(
}
}

/// Marks a post as read for a given person.
#[tracing::instrument(skip_all)]
pub async fn mark_post_as_read(
person_id: PersonId,
post_id: PostId,
pool: &mut DbPool<'_>,
) -> LemmyResult<()> {
PostRead::mark_as_read(pool, HashSet::from([post_id]), person_id)
.await
.with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead)?;
Ok(())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of these changes are boilerplate, because I've gotten rid of this pointless wrapper function, by having PostRead::mark_as_read return the correct LemmyError.

}

/// Updates the read comment count for a post. Usually done when reading or creating a new comment.
#[tracing::instrument(skip_all)]
pub async fn update_read_comments(
Expand Down
5 changes: 2 additions & 3 deletions crates/api_crud/src/post/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use lemmy_api_common::{
get_url_blocklist,
honeypot_check,
local_site_to_slur_regex,
mark_post_as_read,
process_markdown_opt,
},
};
Expand All @@ -22,7 +21,7 @@ use lemmy_db_schema::{
actor_language::CommunityLanguage,
community::Community,
local_site::LocalSite,
post::{Post, PostInsertForm, PostLike, PostLikeForm},
post::{Post, PostInsertForm, PostLike, PostLikeForm, PostRead},
},
traits::{Crud, Likeable},
utils::diesel_url_create,
Expand Down Expand Up @@ -169,7 +168,7 @@ pub async fn create_post(
.await
.with_lemmy_type(LemmyErrorType::CouldntLikePost)?;

mark_post_as_read(person_id, post_id, &mut context.pool()).await?;
PostRead::mark_as_read(&mut context.pool(), &[post_id], person_id).await?;

build_post_response(&context, community_id, local_user_view, post_id).await
}
Expand Down
9 changes: 6 additions & 3 deletions crates/api_crud/src/post/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ use actix_web::web::{Data, Json, Query};
use lemmy_api_common::{
context::LemmyContext,
post::{GetPost, GetPostResponse},
utils::{check_private_instance, is_mod_or_admin_opt, mark_post_as_read, update_read_comments},
utils::{check_private_instance, is_mod_or_admin_opt, update_read_comments},
};
use lemmy_db_schema::{
source::{comment::Comment, post::Post},
source::{
comment::Comment,
post::{Post, PostRead},
},
traits::Crud,
};
use lemmy_db_views::{
Expand Down Expand Up @@ -62,7 +65,7 @@ pub async fn get_post(

let post_id = post_view.post.id;
if let Some(person_id) = person_id {
mark_post_as_read(person_id, post_id, &mut context.pool()).await?;
PostRead::mark_as_read(&mut context.pool(), &[post_id], person_id).await?;

update_read_comments(
person_id,
Expand Down
16 changes: 15 additions & 1 deletion crates/apub/src/api/list_posts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ use lemmy_api_common::{
post::{GetPosts, GetPostsResponse},
utils::{check_conflicting_like_filters, check_private_instance},
};
use lemmy_db_schema::source::community::Community;
use lemmy_db_schema::{
newtypes::PostId,
source::{community::Community, post::PostRead},
};
use lemmy_db_views::{
post_view::PostQuery,
structs::{LocalUserView, PaginationCursor, SiteView},
Expand Down Expand Up @@ -90,6 +93,17 @@ pub async fn list_posts(
.await
.with_lemmy_type(LemmyErrorType::CouldntGetPosts)?;

// If in their user settings (or as part of the API request), auto-mark fetched posts as read
if let Some(local_user) = local_user {
if data
.auto_mark_fetched_posts_as_read
.unwrap_or(local_user.auto_mark_fetched_posts_as_read)
{
let post_ids = posts.iter().map(|p| p.post.id).collect::<Vec<PostId>>();
PostRead::mark_as_read(&mut context.pool(), &post_ids, local_user.person_id).await?;
}
}
dessalines marked this conversation as resolved.
Show resolved Hide resolved

// if this page wasn't empty, then there is a next page after the last post on this page
let next_page = posts.last().map(PaginationCursor::after_post);
Ok(Json(GetPostsResponse { posts, next_page }))
Expand Down
25 changes: 15 additions & 10 deletions crates/db_schema/src/impls/post.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use diesel::{
TextExpressionMethods,
};
use diesel_async::RunQueryDsl;
use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult};
use std::collections::HashSet;

#[async_trait]
Expand Down Expand Up @@ -322,36 +323,41 @@ impl Saveable for PostSaved {
impl PostRead {
pub async fn mark_as_read(
pool: &mut DbPool<'_>,
post_ids: HashSet<PostId>,
post_ids: &[PostId],
person_id: PersonId,
) -> Result<usize, Error> {
) -> LemmyResult<usize> {
let conn = &mut get_conn(pool).await?;

let forms = post_ids
.into_iter()
.map(|post_id| PostReadForm { post_id, person_id })
.iter()
.map(|post_id| PostReadForm {
post_id: *post_id,
person_id,
})
.collect::<Vec<PostReadForm>>();
insert_into(post_read::table)
.values(forms)
.on_conflict_do_nothing()
.execute(conn)
.await
.with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead)
}

pub async fn mark_as_unread(
pool: &mut DbPool<'_>,
post_id_: HashSet<PostId>,
post_ids: &[PostId],
person_id_: PersonId,
) -> Result<usize, Error> {
) -> LemmyResult<usize> {
let conn = &mut get_conn(pool).await?;

diesel::delete(
post_read::table
.filter(post_read::post_id.eq_any(post_id_))
.filter(post_read::post_id.eq_any(post_ids))
.filter(post_read::person_id.eq(person_id_)),
)
.execute(conn)
.await
.with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead)
}
}

Expand Down Expand Up @@ -417,7 +423,6 @@ mod tests {
use lemmy_utils::error::LemmyResult;
use pretty_assertions::assert_eq;
use serial_test::serial;
use std::collections::HashSet;
use url::Url;

#[tokio::test]
Expand Down Expand Up @@ -521,7 +526,7 @@ mod tests {
// Post Read
let marked_as_read = PostRead::mark_as_read(
pool,
HashSet::from([inserted_post.id, inserted_post2.id]),
&[inserted_post.id, inserted_post2.id],
inserted_person.id,
)
.await?;
Expand All @@ -545,7 +550,7 @@ mod tests {
assert_eq!(1, saved_removed);
let read_removed = PostRead::mark_as_unread(
pool,
HashSet::from([inserted_post.id, inserted_post2.id]),
&[inserted_post.id, inserted_post2.id],
inserted_person.id,
)
.await?;
Expand Down
1 change: 1 addition & 0 deletions crates/db_schema/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,7 @@ diesel::table! {
enable_animated_images -> Bool,
collapse_bot_comments -> Bool,
default_comment_sort_type -> CommentSortTypeEnum,
auto_mark_fetched_posts_as_read -> Bool,
}
}

Expand Down
5 changes: 5 additions & 0 deletions crates/db_schema/src/source/local_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ pub struct LocalUser {
/// Whether to auto-collapse bot comments.
pub collapse_bot_comments: bool,
pub default_comment_sort_type: CommentSortType,
/// Whether to automatically mark fetched posts as read.
pub auto_mark_fetched_posts_as_read: bool,
}

#[derive(Clone, derive_new::new)]
Expand Down Expand Up @@ -120,6 +122,8 @@ pub struct LocalUserInsertForm {
pub collapse_bot_comments: Option<bool>,
#[new(default)]
pub default_comment_sort_type: Option<CommentSortType>,
#[new(default)]
pub auto_mark_fetched_posts_as_read: Option<bool>,
}

#[derive(Clone, Default)]
Expand Down Expand Up @@ -150,4 +154,5 @@ pub struct LocalUserUpdateForm {
pub enable_animated_images: Option<bool>,
pub collapse_bot_comments: Option<bool>,
pub default_comment_sort_type: Option<CommentSortType>,
pub auto_mark_fetched_posts_as_read: Option<bool>,
}
2 changes: 1 addition & 1 deletion crates/db_views/src/post_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1654,7 +1654,7 @@ mod tests {
// Mark a post as read
PostRead::mark_as_read(
pool,
HashSet::from([data.inserted_bot_post.id]),
&[data.inserted_bot_post.id],
data.local_user_view.person.id,
)
.await?;
Expand Down
Loading