Merge remote-tracking branch 'origin/main' into vote_display_mode

api_tests/src/post.spec.ts

@@ -270,8 +270,10 @@ test("Lock a post", async () => {
     post => !!post && post.post.locked,
   );
 
-  // Try to make a new comment there, on alpha
+  // Try to make a new comment there, on alpha. For this we need to create a normal
-  await expect(createComment(alpha, alphaPost1.post.id)).rejects.toStrictEqual(
+  // user account because admins/mods can comment in locked posts.
+  let user = await registerUser(alpha, alphaUrl);
+  await expect(createComment(user, alphaPost1.post.id)).rejects.toStrictEqual(
     Error("locked"),
   );
 
@@ -290,7 +292,7 @@ test("Lock a post", async () => {
   expect(alphaPost2.post.locked).toBe(false);
 
   // Try to create a new comment, on alpha
-  let commentAlpha = await createComment(alpha, alphaPost1.post.id);
+  let commentAlpha = await createComment(user, alphaPost1.post.id);
   expect(commentAlpha).toBeDefined();
 });
 

crates/api_crud/src/comment/create.rs

@@ -10,6 +10,7 @@ use lemmy_api_common::{
     check_post_deleted_or_removed,
     generate_local_apub_endpoint,
     get_post,
+    is_mod_or_admin,
     local_site_to_slur_regex,
     process_markdown,
     EndpointType,
@@ -55,7 +56,10 @@ pub async fn create_comment(
   check_post_deleted_or_removed(&post)?;
 
   // Check if post is locked, no new comments
-  if post.locked {
+  let is_mod_or_admin = is_mod_or_admin(&mut context.pool(), &local_user_view.person, community_id)
+    .await
+    .is_ok();
+  if post.locked && !is_mod_or_admin {
     Err(LemmyErrorType::Locked)?
   }
 

crates/apub/src/objects/comment.rs

@@ -18,7 +18,7 @@ use activitypub_federation::{
 use chrono::{DateTime, Utc};
 use lemmy_api_common::{
   context::LemmyContext,
-  utils::{local_site_opt_to_slur_regex, process_markdown},
+  utils::{is_mod_or_admin, local_site_opt_to_slur_regex, process_markdown},
 };
 use lemmy_db_schema::{
   source::{
@@ -142,7 +142,11 @@ impl Object for ApubComment {
     verify_is_remote_object(note.id.inner(), context.settings())?;
     verify_person_in_community(&note.attributed_to, &community, context).await?;
     let (post, _) = note.get_parents(context).await?;
-    if post.locked {
+    let creator = note.attributed_to.dereference(context).await?;
+    let is_mod_or_admin = is_mod_or_admin(&mut context.pool(), &creator, community.id)
+      .await
+      .is_ok();
+    if post.locked && !is_mod_or_admin {
       Err(LemmyErrorType::PostIsLocked)?
     } else {
       Ok(())

crates/db_views/Cargo.toml

@@ -39,10 +39,10 @@ tracing = { workspace = true, optional = true }
 ts-rs = { workspace = true, optional = true }
 actix-web = { workspace = true, optional = true }
 i-love-jesus = { workspace = true, optional = true }
+chrono = { workspace = true }
 
 [dev-dependencies]
 serial_test = { workspace = true }
 tokio = { workspace = true }
-chrono = { workspace = true }
 pretty_assertions = { workspace = true }
 url = { workspace = true }

crates/db_views/src/comment_view.rs

@@ -1,4 +1,5 @@
 use crate::structs::{CommentView, LocalUserView};
+use chrono::{DateTime, Utc};
 use diesel::{
   dsl::{exists, not},
   pg::Pg,
@@ -53,13 +54,14 @@ fn queries<'a>() -> Queries<
   );
 
   let is_saved = |person_id| {
-    exists(
+    comment_saved::table
-      comment_saved::table.filter(
+      .filter(
         comment::id
           .eq(comment_saved::comment_id)
           .and(comment_saved::person_id.eq(person_id)),
-      ),
+      )
-    )
+      .select(comment_saved::published.nullable())
+      .single_value()
   };
 
   let is_community_followed = |person_id| {
@@ -110,9 +112,7 @@ fn queries<'a>() -> Queries<
     ),
   );
 
-  let all_joins = move |query: comment::BoxedQuery<'a, Pg>,
+  let all_joins = move |query: comment::BoxedQuery<'a, Pg>, my_person_id: Option<PersonId>| {
-                        my_person_id: Option<PersonId>,
-                        saved_only: bool| {
     let score_selection: Box<
       dyn BoxableExpression<_, Pg, SqlType = sql_types::Nullable<sql_types::SmallInt>>,
     > = if let Some(person_id) = my_person_id {
@@ -129,14 +129,13 @@ fn queries<'a>() -> Queries<
       Box::new(None::<bool>.into_sql::<sql_types::Nullable<sql_types::Bool>>())
     };
 
-    let is_saved_selection: Box<dyn BoxableExpression<_, Pg, SqlType = sql_types::Bool>> =
+    let is_saved_selection: Box<
-      if saved_only {
+      dyn BoxableExpression<_, Pg, SqlType = sql_types::Nullable<sql_types::Timestamptz>>,
-        Box::new(true.into_sql::<sql_types::Bool>())
+    > = if let Some(person_id) = my_person_id {
-      } else if let Some(person_id) = my_person_id {
+      Box::new(is_saved(person_id))
-        Box::new(is_saved(person_id))
+    } else {
-      } else {
+      Box::new(None::<DateTime<Utc>>.into_sql::<sql_types::Nullable<sql_types::Timestamptz>>())
-        Box::new(false.into_sql::<sql_types::Bool>())
+    };
-      };
 
     let is_creator_blocked_selection: Box<dyn BoxableExpression<_, Pg, SqlType = sql_types::Bool>> =
       if let Some(person_id) = my_person_id {
@@ -160,7 +159,7 @@ fn queries<'a>() -> Queries<
         creator_is_moderator,
         creator_is_admin,
         subscribed_type_selection,
-        is_saved_selection,
+        is_saved_selection.is_not_null(),
         is_creator_blocked_selection,
         score_selection,
       ))
@@ -168,11 +167,7 @@ fn queries<'a>() -> Queries<
 
   let read = move |mut conn: DbConn<'a>,
                    (comment_id, my_person_id): (CommentId, Option<PersonId>)| async move {
-    let mut query = all_joins(
+    let mut query = all_joins(comment::table.find(comment_id).into_boxed(), my_person_id);
-      comment::table.find(comment_id).into_boxed(),
-      my_person_id,
-      false,
-    );
     // Hide local only communities from unauthenticated users
     if my_person_id.is_none() {
       query = query.filter(community::visibility.eq(CommunityVisibility::Public));
@@ -188,11 +183,7 @@ fn queries<'a>() -> Queries<
     let person_id_join = my_person_id.unwrap_or(PersonId(-1));
     let local_user_id_join = my_local_user_id.unwrap_or(LocalUserId(-1));
 
-    let mut query = all_joins(
+    let mut query = all_joins(comment::table.into_boxed(), my_person_id);
-      comment::table.into_boxed(),
-      my_person_id,
-      options.saved_only,
-    );
 
     if let Some(creator_id) = options.creator_id {
       query = query.filter(comment::creator_id.eq(creator_id));
@@ -243,8 +234,11 @@ fn queries<'a>() -> Queries<
       }
     }
 
+    // If its saved only, then filter, and order by the saved time, not the comment creation time.
     if options.saved_only {
-      query = query.filter(is_saved(person_id_join));
+      query = query
+        .filter(is_saved(person_id_join).is_not_null())
+        .then_order_by(is_saved(person_id_join).desc());
     }
 
     if options.liked_only {
@@ -413,7 +407,15 @@ mod tests {
     newtypes::LanguageId,
     source::{
       actor_language::LocalUserLanguage,
-      comment::{Comment, CommentInsertForm, CommentLike, CommentLikeForm, CommentUpdateForm},
+      comment::{
+        Comment,
+        CommentInsertForm,
+        CommentLike,
+        CommentLikeForm,
+        CommentSaved,
+        CommentSavedForm,
+        CommentUpdateForm,
+      },
       community::{
         Community,
         CommunityInsertForm,
@@ -429,7 +431,7 @@ mod tests {
       person_block::{PersonBlock, PersonBlockForm},
       post::{Post, PostInsertForm},
     },
-    traits::{Blockable, Crud, Joinable, Likeable},
+    traits::{Blockable, Crud, Joinable, Likeable, Saveable},
     utils::{build_db_pool_for_tests, RANK_DEFAULT},
     CommunityVisibility,
     SubscribedType,
@@ -929,6 +931,52 @@ mod tests {
     cleanup(data, pool).await;
   }
 
+  #[tokio::test]
+  #[serial]
+  async fn test_saved_order() {
+    let pool = &build_db_pool_for_tests().await;
+    let pool = &mut pool.into();
+    let data = init_data(pool).await;
+
+    // Save two comments
+    let save_comment_0_form = CommentSavedForm {
+      person_id: data.timmy_local_user_view.person.id,
+      comment_id: data.inserted_comment_0.id,
+    };
+    CommentSaved::save(pool, &save_comment_0_form)
+      .await
+      .unwrap();
+
+    let save_comment_2_form = CommentSavedForm {
+      person_id: data.timmy_local_user_view.person.id,
+      comment_id: data.inserted_comment_2.id,
+    };
+    CommentSaved::save(pool, &save_comment_2_form)
+      .await
+      .unwrap();
+
+    // Fetch the saved comments
+    let comments = CommentQuery {
+      local_user: Some(&data.timmy_local_user_view),
+      saved_only: true,
+      ..Default::default()
+    }
+    .list(pool)
+    .await
+    .unwrap();
+
+    // There should only be two comments
+    assert_eq!(2, comments.len());
+
+    // The first comment, should be the last one saved (descending order)
+    assert_eq!(comments[0].comment.id, data.inserted_comment_2.id);
+
+    // The second comment, should be the first one saved
+    assert_eq!(comments[1].comment.id, data.inserted_comment_0.id);
+
+    cleanup(data, pool).await;
+  }
+
   async fn cleanup(data: Data, pool: &mut DbPool<'_>) {
     CommentLike::remove(
       pool,

crates/db_views/src/post_view.rs

@@ -1,4 +1,5 @@
 use crate::structs::{LocalUserView, PaginationCursor, PostView};
+use chrono::{DateTime, Utc};
 use diesel::{
   debug_query,
   dsl::{exists, not, IntervalDsl},
@@ -89,13 +90,14 @@ fn queries<'a>() -> Queries<
   );
 
   let is_saved = |person_id| {
-    exists(
+    post_saved::table
-      post_saved::table.filter(
+      .filter(
         post_aggregates::post_id
           .eq(post_saved::post_id)
           .and(post_saved::person_id.eq(person_id)),
-      ),
+      )
-    )
+      .select(post_saved::published.nullable())
+      .single_value()
   };
 
   let is_read = |person_id| {
@@ -140,16 +142,14 @@ fn queries<'a>() -> Queries<
   };
 
   let all_joins = move |query: post_aggregates::BoxedQuery<'a, Pg>,
-                        my_person_id: Option<PersonId>,
+                        my_person_id: Option<PersonId>| {
-                        saved_only: bool| {
+    let is_saved_selection: Box<
-    let is_saved_selection: Box<dyn BoxableExpression<_, Pg, SqlType = sql_types::Bool>> =
+      dyn BoxableExpression<_, Pg, SqlType = sql_types::Nullable<sql_types::Timestamptz>>,
-      if saved_only {
+    > = if let Some(person_id) = my_person_id {
-        Box::new(true.into_sql::<sql_types::Bool>())
+      Box::new(is_saved(person_id))
-      } else if let Some(person_id) = my_person_id {
+    } else {
-        Box::new(is_saved(person_id))
+      Box::new(None::<DateTime<Utc>>.into_sql::<sql_types::Nullable<sql_types::Timestamptz>>())
-      } else {
+    };
-        Box::new(false.into_sql::<sql_types::Bool>())
-      };
 
     let is_read_selection: Box<dyn BoxableExpression<_, Pg, SqlType = sql_types::Bool>> =
       if let Some(person_id) = my_person_id {
@@ -227,7 +227,7 @@ fn queries<'a>() -> Queries<
         creator_is_admin,
         post_aggregates::all_columns,
         subscribed_type_selection,
-        is_saved_selection,
+        is_saved_selection.is_not_null(),
         is_read_selection,
         is_hidden_selection,
         is_creator_blocked_selection,
@@ -250,7 +250,6 @@ fn queries<'a>() -> Queries<
           .filter(post_aggregates::post_id.eq(post_id))
           .into_boxed(),
         my_person_id,
-        false,
       );
 
       // Hide deleted and removed for non-admins or mods
@@ -298,11 +297,7 @@ fn queries<'a>() -> Queries<
     let person_id_join = my_person_id.unwrap_or(PersonId(-1));
     let local_user_id_join = my_local_user_id.unwrap_or(LocalUserId(-1));
 
-    let mut query = all_joins(
+    let mut query = all_joins(post_aggregates::table.into_boxed(), my_person_id);
-      post_aggregates::table.into_boxed(),
-      my_person_id,
-      options.saved_only,
-    );
 
     // hide posts from deleted communities
     query = query.filter(community::deleted.eq(false));
@@ -408,8 +403,11 @@ fn queries<'a>() -> Queries<
       query = query.filter(person::bot_account.eq(false));
     };
 
+    // If its saved only, then filter, and order by the saved time, not the comment creation time.
     if let (true, Some(person_id)) = (options.saved_only, my_person_id) {
-      query = query.filter(is_saved(person_id));
+      query = query
+        .filter(is_saved(person_id).is_not_null())
+        .then_order_by(is_saved(person_id).desc());
     }
     // Only hide the read posts, if the saved_only is false. Otherwise ppl with the hide_read
     // setting wont be able to see saved posts.