From fdfeeed6d9fa9dec92862366a8b646eb27d17422 Mon Sep 17 00:00:00 2001 From: fdb-hiroshima <35889323+fdb-hiroshima@users.noreply.github.com> Date: Mon, 24 Dec 2018 11:23:04 +0100 Subject: [PATCH] Comment visibility (#364) Add some support for comment visibility, fix #217 This add a new column to comment, denoting if they are public or not, and a new table linking private comments to those allowed to read them. There is currently no way to write a private comment from Plume. Git is having a hard time what happened in Comment::from_activity, but most of it is just re-indentation because a new block was needed to please the borrow checker. I've marked with comments where things actually changed. At this point only mentioned users can see private comments, even when posted as "follower only" or equivalent. What should we do when someone isn't allowed to see a comment? Hide the whole thread, or just the comment? If hiding just the comment, should we mark there is a comment one can't see, but answers they can, or put other comments like if they answered to the same comment the hidden one do? --- .../down.sql | 4 + .../up.sql | 9 + .../down.sql | 28 +++ .../up.sql | 9 + plume-models/src/comment_seers.rs | 32 +++ plume-models/src/comments.rs | 189 +++++++++++++----- plume-models/src/lib.rs | 1 + plume-models/src/schema.rs | 12 ++ plume-models/src/users.rs | 9 +- src/routes/comments.rs | 7 +- src/routes/posts.rs | 6 +- templates/partials/comment.rs.html | 12 +- templates/posts/details.rs.html | 6 +- 13 files changed, 259 insertions(+), 65 deletions(-) create mode 100644 migrations/postgres/2018-12-17-221135_comment_visibility/down.sql create mode 100644 migrations/postgres/2018-12-17-221135_comment_visibility/up.sql create mode 100644 migrations/sqlite/2018-12-17-221135_comment_visibility/down.sql create mode 100644 migrations/sqlite/2018-12-17-221135_comment_visibility/up.sql create mode 100644 plume-models/src/comment_seers.rs diff --git a/migrations/postgres/2018-12-17-221135_comment_visibility/down.sql b/migrations/postgres/2018-12-17-221135_comment_visibility/down.sql new file mode 100644 index 0000000..1c26b21 --- /dev/null +++ b/migrations/postgres/2018-12-17-221135_comment_visibility/down.sql @@ -0,0 +1,4 @@ +-- This file should undo anything in `up.sql` +ALTER TABLE comments DROP COLUMN public_visibility; + +DROP TABLE comment_seers; diff --git a/migrations/postgres/2018-12-17-221135_comment_visibility/up.sql b/migrations/postgres/2018-12-17-221135_comment_visibility/up.sql new file mode 100644 index 0000000..3507d62 --- /dev/null +++ b/migrations/postgres/2018-12-17-221135_comment_visibility/up.sql @@ -0,0 +1,9 @@ +-- Your SQL goes here +ALTER TABLE comments ADD public_visibility BOOLEAN NOT NULL DEFAULT 't'; + +CREATE TABLE comment_seers ( + id SERIAL PRIMARY KEY, + comment_id INTEGER REFERENCES comments(id) ON DELETE CASCADE NOT NULL, + user_id INTEGER REFERENCES users(id) ON DELETE CASCADE NOT NULL, + UNIQUE (comment_id, user_id) +) diff --git a/migrations/sqlite/2018-12-17-221135_comment_visibility/down.sql b/migrations/sqlite/2018-12-17-221135_comment_visibility/down.sql new file mode 100644 index 0000000..ea00ebd --- /dev/null +++ b/migrations/sqlite/2018-12-17-221135_comment_visibility/down.sql @@ -0,0 +1,28 @@ +-- This file should undo anything in `up.sql` +CREATE TABLE comments2 ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + content TEXT NOT NULL DEFAULT '', + in_response_to_id INTEGER REFERENCES comments(id), + post_id INTEGER REFERENCES posts(id) ON DELETE CASCADE NOT NULL, + author_id INTEGER REFERENCES users(id) ON DELETE CASCADE NOT NULL, + creation_date DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + ap_url VARCHAR, + sensitive BOOLEAN NOT NULL DEFAULT 'f', + spoiler_text TEXT NOT NULL DEFAULT '' +); + +INSERT INTO comments2 SELECT + id, + content, + in_response_to_id, + post_id, + author_id, + creation_date, + ap_url, + sensitive, + spoiler_text + FROM comments; +DROP TABLE comments; +ALTER TABLE comments2 RENAME TO comments; + +DROP TABLE comment_seers; diff --git a/migrations/sqlite/2018-12-17-221135_comment_visibility/up.sql b/migrations/sqlite/2018-12-17-221135_comment_visibility/up.sql new file mode 100644 index 0000000..88a046e --- /dev/null +++ b/migrations/sqlite/2018-12-17-221135_comment_visibility/up.sql @@ -0,0 +1,9 @@ +-- Your SQL goes here +ALTER TABLE comments ADD public_visibility BOOLEAN NOT NULL DEFAULT 't'; + +CREATE TABLE comment_seers ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + comment_id INTEGER REFERENCES comments(id) ON DELETE CASCADE NOT NULL, + user_id INTEGER REFERENCES users(id) ON DELETE CASCADE NOT NULL, + UNIQUE (comment_id, user_id) +) diff --git a/plume-models/src/comment_seers.rs b/plume-models/src/comment_seers.rs new file mode 100644 index 0000000..9083eec --- /dev/null +++ b/plume-models/src/comment_seers.rs @@ -0,0 +1,32 @@ +use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl}; + +use comments::Comment; +use schema::comment_seers; +use users::User; +use Connection; + +#[derive(Queryable, Serialize, Clone)] +pub struct CommentSeers { + pub id: i32, + pub comment_id: i32, + pub user_id: i32, +} + +#[derive(Insertable, Default)] +#[table_name = "comment_seers"] +pub struct NewCommentSeers { + pub comment_id: i32, + pub user_id: i32, +} + +impl CommentSeers { + insert!(comment_seers, NewCommentSeers); + + pub fn can_see(conn: &Connection, c: &Comment, u: &User) -> bool { + !comment_seers::table.filter(comment_seers::comment_id.eq(c.id)) + .filter(comment_seers::user_id.eq(u.id)) + .load::(conn) + .expect("Comment::get_responses: loading error") + .is_empty() + } +} diff --git a/plume-models/src/comments.rs b/plume-models/src/comments.rs index 50e323f..abcf961 100644 --- a/plume-models/src/comments.rs +++ b/plume-models/src/comments.rs @@ -3,6 +3,8 @@ use chrono::{self, NaiveDateTime}; use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl}; use serde_json; +use std::collections::HashSet; + use instance::Instance; use mentions::Mention; use notifications::*; @@ -11,6 +13,7 @@ use plume_common::activity_pub::{ Id, IntoId, PUBLIC_VISIBILTY, }; use plume_common::utils; +use comment_seers::{CommentSeers, NewCommentSeers}; use posts::Post; use safe_string::SafeString; use schema::comments; @@ -28,6 +31,7 @@ pub struct Comment { pub ap_url: Option, pub sensitive: bool, pub spoiler_text: String, + pub public_visibility: bool, } #[derive(Insertable, Default)] @@ -40,6 +44,7 @@ pub struct NewComment { pub ap_url: Option, pub sensitive: bool, pub spoiler_text: String, + pub public_visibility: bool, } impl Comment { @@ -90,6 +95,11 @@ impl Comment { format!("{}comment/{}", self.get_post(conn).ap_url, self.id) } + pub fn can_see(&self, conn: &Connection, user: Option<&User>) -> bool { + self.public_visibility || + user.as_ref().map(|u| CommentSeers::can_see(conn, self, u)).unwrap_or(false) + } + pub fn to_activity(&self, conn: &Connection) -> Note { let (html, mentions, _hashtags) = utils::md_to_html(self.content.get().as_ref(), &Instance::get_local(conn) @@ -179,59 +189,113 @@ impl Comment { impl FromActivity for Comment { fn from_activity(conn: &Connection, note: Note, actor: Id) -> Comment { - let previous_url = note - .object_props - .in_reply_to - .clone() - .expect("Comment::from_activity: not an answer error"); - let previous_url = previous_url - .as_str() - .expect("Comment::from_activity: in_reply_to parsing error"); - let previous_comment = Comment::find_by_ap_url(conn, previous_url); + let comm = { + let previous_url = note + .object_props + .in_reply_to + .as_ref() + .expect("Comment::from_activity: not an answer error") + .as_str() + .expect("Comment::from_activity: in_reply_to parsing error"); + let previous_comment = Comment::find_by_ap_url(conn, previous_url); - let comm = Comment::insert( - conn, - NewComment { - content: SafeString::new( - ¬e + let is_public = |v: &Option| match v.as_ref().unwrap_or(&serde_json::Value::Null) { + serde_json::Value::Array(v) => v.iter().filter_map(serde_json::Value::as_str).any(|s| s==PUBLIC_VISIBILTY), + serde_json::Value::String(s) => s == PUBLIC_VISIBILTY, + _ => false, + }; + + let public_visibility = is_public(¬e.object_props.to) || + is_public(¬e.object_props.bto) || + is_public(¬e.object_props.cc) || + is_public(¬e.object_props.bcc); + + let comm = Comment::insert( + conn, + NewComment { + content: SafeString::new( + ¬e + .object_props + .content_string() + .expect("Comment::from_activity: content deserialization error"), + ), + spoiler_text: note .object_props - .content_string() - .expect("Comment::from_activity: content deserialization error"), - ), - spoiler_text: note - .object_props - .summary_string() - .unwrap_or_default(), - ap_url: note.object_props.id_string().ok(), - in_response_to_id: previous_comment.clone().map(|c| c.id), - post_id: previous_comment.map(|c| c.post_id).unwrap_or_else(|| { - Post::find_by_ap_url(conn, previous_url) - .expect("Comment::from_activity: post error") - .id - }), - author_id: User::from_url(conn, actor.as_ref()) - .expect("Comment::from_activity: author error") - .id, - sensitive: false, // "sensitive" is not a standard property, we need to think about how to support it with the activitypub crate - }, - ); + .summary_string() + .unwrap_or_default(), + ap_url: note.object_props.id_string().ok(), + in_response_to_id: previous_comment.clone().map(|c| c.id), + post_id: previous_comment.map(|c| c.post_id).unwrap_or_else(|| { + Post::find_by_ap_url(conn, previous_url) + .expect("Comment::from_activity: post error") + .id + }), + author_id: User::from_url(conn, actor.as_ref()) + .expect("Comment::from_activity: author error") + .id, + sensitive: false, // "sensitive" is not a standard property, we need to think about how to support it with the activitypub crate + public_visibility + }, + ); - // save mentions - if let Some(serde_json::Value::Array(tags)) = note.object_props.tag.clone() { - for tag in tags { - serde_json::from_value::(tag) - .map(|m| { - let author = &Post::get(conn, comm.post_id) - .expect("Comment::from_activity: error") - .get_authors(conn)[0]; - let not_author = m - .link_props - .href_string() - .expect("Comment::from_activity: no href error") - != author.ap_url.clone(); - Mention::from_activity(conn, &m, comm.id, false, not_author) - }) - .ok(); + // save mentions + if let Some(serde_json::Value::Array(tags)) = note.object_props.tag.clone() { + for tag in tags { + serde_json::from_value::(tag) + .map(|m| { + let author = &Post::get(conn, comm.post_id) + .expect("Comment::from_activity: error") + .get_authors(conn)[0]; + let not_author = m + .link_props + .href_string() + .expect("Comment::from_activity: no href error") + != author.ap_url.clone(); + Mention::from_activity(conn, &m, comm.id, false, not_author) + }) + .ok(); + } + } + comm + }; + + + if !comm.public_visibility { + let receivers_ap_url = |v: Option| { + let filter = |e: serde_json::Value| if let serde_json::Value::String(s) = e { Some(s) } else { None }; + match v.unwrap_or(serde_json::Value::Null) { + serde_json::Value::Array(v) => v, + v => vec![v], + }.into_iter().filter_map(filter) + }; + + let mut note = note; + + let to = receivers_ap_url(note.object_props.to.take()); + let cc = receivers_ap_url(note.object_props.cc.take()); + let bto = receivers_ap_url(note.object_props.bto.take()); + let bcc = receivers_ap_url(note.object_props.bcc.take()); + + let receivers_ap_url = to.chain(cc).chain(bto).chain(bcc) + .collect::>()//remove duplicates (don't do a query more than once) + .into_iter() + .map(|v| if let Some(user) = User::from_url(conn,&v) { + vec![user] + } else { + vec![]// TODO try to fetch collection + }) + .flatten() + .filter(|u| u.get_instance(conn).local) + .collect::>();//remove duplicates (prevent db error) + + for user in &receivers_ap_url { + CommentSeers::insert( + conn, + NewCommentSeers { + comment_id: comm.id, + user_id: user.id + } + ); } } @@ -255,6 +319,31 @@ impl Notify for Comment { } } +pub struct CommentTree { + pub comment: Comment, + pub responses: Vec, +} + +impl CommentTree { + pub fn from_post(conn: &Connection, p: &Post, user: Option<&User>) -> Vec { + Comment::list_by_post(conn, p.id).into_iter() + .filter(|c| c.in_response_to_id.is_none()) + .filter(|c| c.can_see(conn, user)) + .map(|c| Self::from_comment(conn, c, user)) + .collect() + } + + pub fn from_comment(conn: &Connection, comment: Comment, user: Option<&User>) -> Self { + let responses = comment.get_responses(conn).into_iter() + .filter(|c| c.can_see(conn, user)) + .map(|c| Self::from_comment(conn, c, user)) + .collect(); + CommentTree { + comment, + responses, + } + } +} impl<'a> Deletable for Comment { fn delete(&self, conn: &Connection) -> Delete { diff --git a/plume-models/src/lib.rs b/plume-models/src/lib.rs index b8b49ea..0af2ebd 100644 --- a/plume-models/src/lib.rs +++ b/plume-models/src/lib.rs @@ -250,6 +250,7 @@ pub mod apps; pub mod blog_authors; pub mod blogs; pub mod comments; +pub mod comment_seers; pub mod db_conn; pub mod follows; pub mod headers; diff --git a/plume-models/src/schema.rs b/plume-models/src/schema.rs index bd36365..5cce86c 100644 --- a/plume-models/src/schema.rs +++ b/plume-models/src/schema.rs @@ -57,6 +57,15 @@ table! { ap_url -> Nullable, sensitive -> Bool, spoiler_text -> Text, + public_visibility -> Bool, + } +} + +table! { + comment_seers (id) { + id -> Int4, + comment_id -> Int4, + user_id -> Int4, } } @@ -200,6 +209,8 @@ joinable!(api_tokens -> users (user_id)); joinable!(blog_authors -> blogs (blog_id)); joinable!(blog_authors -> users (author_id)); joinable!(blogs -> instances (instance_id)); +joinable!(comment_seers -> comments (comment_id)); +joinable!(comment_seers -> users (user_id)); joinable!(comments -> posts (post_id)); joinable!(comments -> users (author_id)); joinable!(likes -> posts (post_id)); @@ -223,6 +234,7 @@ allow_tables_to_appear_in_same_query!( blog_authors, blogs, comments, + comment_seers, follows, instances, likes, diff --git a/plume-models/src/users.rs b/plume-models/src/users.rs index 242af35..ababc53 100644 --- a/plume-models/src/users.rs +++ b/plume-models/src/users.rs @@ -26,7 +26,7 @@ use rocket::{ request::{self, FromRequest, Request}, }; use serde_json; -use std::cmp::PartialEq; +use std::{cmp::PartialEq, hash::{Hash, Hasher}}; use url::Url; use webfinger::*; @@ -900,6 +900,7 @@ impl IntoId for User { } } +impl Eq for User {} impl Object for User {} impl Actor for User {} @@ -956,6 +957,12 @@ impl PartialEq for User { } } +impl Hash for User { + fn hash(&self, state: &mut H) { + self.id.hash(state); + } +} + impl NewUser { /// Creates a new local user pub fn new_local( diff --git a/src/routes/comments.rs b/src/routes/comments.rs index 65f7d6e..356cf72 100644 --- a/src/routes/comments.rs +++ b/src/routes/comments.rs @@ -45,7 +45,8 @@ pub fn create(blog_name: String, slug: String, form: LenientForm author_id: user.id, ap_url: None, sensitive: !form.warning.is_empty(), - spoiler_text: form.warning.clone() + spoiler_text: form.warning.clone(), + public_visibility: true }).update_ap_url(&*conn); let new_comment = comm.create_activity(&*conn); @@ -63,7 +64,7 @@ pub fn create(blog_name: String, slug: String, form: LenientForm }) .map_err(|errors| { // TODO: de-duplicate this code - let comments = Comment::list_by_post(&*conn, post.id); + let comments = CommentTree::from_post(&*conn, &post, Some(&user)); let previous = form.responding_to.map(|r| Comment::get(&*conn, r) .expect("comments::create: Error retrieving previous comment")); @@ -75,7 +76,7 @@ pub fn create(blog_name: String, slug: String, form: LenientForm &*form, errors, Tag::for_post(&*conn, post.id), - comments.into_iter().filter(|c| c.in_response_to_id.is_none()).collect::>(), + comments, previous, post.count_likes(&*conn), post.count_reshares(&*conn), diff --git a/src/routes/posts.rs b/src/routes/posts.rs index 00451d5..0bbb071 100644 --- a/src/routes/posts.rs +++ b/src/routes/posts.rs @@ -11,7 +11,7 @@ use plume_common::utils; use plume_models::{ blogs::*, db_conn::DbConn, - comments::Comment, + comments::{Comment, CommentTree}, instance::Instance, medias::Media, mentions::Mention, @@ -31,7 +31,7 @@ pub fn details(blog: String, slug: String, conn: DbConn, user: Option, res let blog = Blog::find_by_fqn(&*conn, &blog).ok_or_else(|| render!(errors::not_found(&(&*conn, &intl.catalog, user.clone()))))?; let post = Post::find_by_slug(&*conn, &slug, blog.id).ok_or_else(|| render!(errors::not_found(&(&*conn, &intl.catalog, user.clone()))))?; if post.published || post.get_authors(&*conn).into_iter().any(|a| a.id == user.clone().map(|u| u.id).unwrap_or(0)) { - let comments = Comment::list_by_post(&*conn, post.id); + let comments = CommentTree::from_post(&*conn, &post, user.as_ref()); let previous = responding_to.map(|r| Comment::get(&*conn, r) .expect("posts::details_reponse: Error retrieving previous comment")); @@ -64,7 +64,7 @@ pub fn details(blog: String, slug: String, conn: DbConn, user: Option, res }, ValidationErrors::default(), Tag::for_post(&*conn, post.id), - comments.into_iter().filter(|c| c.in_response_to_id.is_none()).collect::>(), + comments, previous, post.count_likes(&*conn), post.count_reshares(&*conn), diff --git a/templates/partials/comment.rs.html b/templates/partials/comment.rs.html index 284d125..57d205d 100644 --- a/templates/partials/comment.rs.html +++ b/templates/partials/comment.rs.html @@ -1,10 +1,11 @@ @use template_utils::*; -@use plume_models::comments::Comment; -@use plume_models::users::User; +@use plume_models::comments::CommentTree; @use routes::*; -@(ctx: BaseContext, comm: &Comment, author: User, in_reply_to: Option<&str>, blog: &str, slug: &str) +@(ctx: BaseContext, comment_tree: &CommentTree, in_reply_to: Option<&str>, blog: &str, slug: &str) +@if let Some(ref comm) = Some(&comment_tree.comment) { +@if let Some(author) = Some(comm.get_author(ctx.0)) { +}} diff --git a/templates/posts/details.rs.html b/templates/posts/details.rs.html index 571f60b..64ab757 100644 --- a/templates/posts/details.rs.html +++ b/templates/posts/details.rs.html @@ -1,7 +1,7 @@ @use templates::{base, partials::comment}; @use template_utils::*; @use plume_models::blogs::Blog; -@use plume_models::comments::Comment; +@use plume_models::comments::{Comment, CommentTree}; @use plume_models::posts::Post; @use plume_models::tags::Tag; @use plume_models::users::User; @@ -9,7 +9,7 @@ @use routes::comments::NewCommentForm; @use routes::*; -@(ctx: BaseContext, article: Post, blog: Blog, comment_form: &NewCommentForm, comment_errors: ValidationErrors, tags: Vec, comments: Vec, previous_comment: Option, n_likes: i64, n_reshares: i64, has_liked: bool, has_reshared: bool, is_following: bool, author: User) +@(ctx: BaseContext, article: Post, blog: Blog, comment_form: &NewCommentForm, comment_errors: ValidationErrors, tags: Vec, comments: Vec, previous_comment: Option, n_likes: i64, n_reshares: i64, has_liked: bool, has_reshared: bool, is_following: bool, author: User) @:base(ctx, &article.title.clone(), { @@ -146,7 +146,7 @@ @if !comments.is_empty() {
@for comm in comments { - @:comment(ctx, &comm, comm.get_author(ctx.0), Some(&article.ap_url), &blog.get_fqn(ctx.0), &article.slug) + @:comment(ctx, &comm, Some(&article.ap_url), &blog.get_fqn(ctx.0), &article.slug) }
} else {