-
-
Notifications
You must be signed in to change notification settings - Fork 888
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
Adding local site settings to reject federated upvotes or downvotes. #5038
Changes from 5 commits
f0adbaa
a3b7a46
4ad1fff
f583041
c4e0c24
20d0d8f
35b6e05
6e9b546
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,7 @@ use activitypub_federation::{ | |
traits::{ActivityHandler, Actor}, | ||
}; | ||
use lemmy_api_common::{context::LemmyContext, utils::check_bot_account}; | ||
use lemmy_db_schema::source::local_site::LocalSite; | ||
use lemmy_db_schema::{source::local_site::LocalSite, FederationMode}; | ||
use lemmy_utils::error::{LemmyError, LemmyResult}; | ||
use url::Url; | ||
|
||
|
@@ -68,12 +68,22 @@ impl ActivityHandler for Vote { | |
|
||
check_bot_account(&actor.0)?; | ||
|
||
let enable_downvotes = LocalSite::read(&mut context.pool()) | ||
// Check for enabled federation votes | ||
let local_site = LocalSite::read(&mut context.pool()) | ||
.await | ||
.map(|l| l.enable_downvotes) | ||
.unwrap_or(true); | ||
if self.kind == VoteType::Dislike && !enable_downvotes { | ||
// If this is a downvote but downvotes are ignored, only undo any existing vote | ||
.unwrap_or_default(); | ||
|
||
let (downvote_setting, upvote_setting) = match object { | ||
PostOrComment::Post(_) => (local_site.post_downvotes, local_site.post_upvotes), | ||
PostOrComment::Comment(_) => (local_site.comment_downvotes, local_site.comment_upvotes), | ||
}; | ||
|
||
// Don't allow dislikes for either disabled, or local only votes | ||
let downvote_fail = self.kind == VoteType::Dislike && downvote_setting != FederationMode::All; | ||
let upvote_fail = self.kind == VoteType::Like && upvote_setting != FederationMode::All; | ||
|
||
if downvote_fail || upvote_fail { | ||
// If this is a rejection, undo the vote | ||
match object { | ||
PostOrComment::Post(p) => undo_vote_post(actor, &p, context).await, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Nutomic One reason that people gave for rejecting votes, is that they'd like to decrease the DB load, and avoid federated vote inserts entirely. But L89 seems like it inserts, then removes these votes (which would fire plenty of DB triggers also). Wouldn't it be better to just not insert them in the first place? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No this logic is used when a user first upvotes and then downvotes a post. So we cant apply the downvote, but leaving it as upvoted doesnt make sense either. Instead we remove the vote entirely. You can clarify the comments. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does that mean I need to add this check elsewhere also? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most of the logic is in mod.rs so it may be easier to make these changes there. For federation this should be all, of course you still need to add api checks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Regarding db writes, my assumption is that trying to delete nonexistent row doesnt write anything. |
||
PostOrComment::Comment(c) => undo_vote_comment(actor, &c, context).await, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -251,6 +251,27 @@ pub enum CommunityVisibility { | |
LocalOnly, | ||
} | ||
|
||
#[derive( | ||
EnumString, Display, Debug, Serialize, Deserialize, Clone, Copy, PartialEq, Eq, Default, Hash, | ||
)] | ||
#[cfg_attr(feature = "full", derive(DbEnum, TS))] | ||
#[cfg_attr( | ||
feature = "full", | ||
ExistingTypePath = "crate::schema::sql_types::FederationModeEnum" | ||
)] | ||
#[cfg_attr(feature = "full", DbValueStyle = "verbatim")] | ||
#[cfg_attr(feature = "full", ts(export))] | ||
/// The federation mode for an item | ||
pub enum FederationMode { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I figure this could be used in the future to block things like federated comments, federated posts, etc. |
||
#[default] | ||
/// Allows all | ||
All, | ||
/// Allows only local | ||
Local, | ||
/// Disables | ||
Disable, | ||
} | ||
|
||
/// Wrapper for assert_eq! macro. Checks that vec matches the given length, and prints the | ||
/// vec on failure. | ||
#[macro_export] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought I left a comment here, about using the same logic as in federation mode to undo previous vote if an invalid mode is made. Eg comment upvoted then downvoted but downvotes are disabled, so it should delete the previous upvote.
Btw federation tests are failing, otherwise looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, added. The tests passed okay on my local machine, I think it was just that intermittent federation test error in CI that you just fixed.