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

Allow disabling private messages. Fixes #3640 #4094

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

dessalines
Copy link
Member

This adds a local user setting: enable_private_messages default true.

It then checks when sending to a local user, that both you and the recipient have that setting enabled.

It also checks on an apub private message receive, that you have it enabled, before creating it.

@Nutomic
Copy link
Member

Nutomic commented Oct 25, 2023

This is not enough, you also need to federate the setting as part of Person. That way pm buttons can properly be disabled for remote users too. I didnt find any existing property for this purpose, so you can simply call it enable_private_messages and add it to the context.

@dessalines dessalines marked this pull request as draft October 25, 2023 21:00
@richardARPANET
Copy link

This is greatly needed 👍 Can there also be a global setting?

@dessalines
Copy link
Member Author

dessalines commented Jan 22, 2024

Global setting should probably be in a different PR / issue. This is specifically to prevent harassment on a user-level.

@Nutomic
Copy link
Member

Nutomic commented May 17, 2024

Outdated, feel free to reopen when you work on it again.

@Nutomic Nutomic closed this May 17, 2024
@dessalines dessalines reopened this Nov 2, 2024
@dessalines
Copy link
Member Author

This is not enough, you also need to federate the setting as part of Person. That way pm buttons can properly be disabled for remote users too. I didnt find any existing property for this purpose, so you can simply call it enable_private_messages and add it to the context.

I'm not sure how to handle this, because its a local user setting, not something that gets federated with person.

For example the into_json fields here don't select local user settings, only fields from person.

@dessalines dessalines marked this pull request as ready for review November 2, 2024 16:02
LocalUserView::read_person(&mut context.pool(), recipient.id).await
{
check_private_messages_enabled(&recipient_local_user)?;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

The check for apub is done here, so it is working, its just not a publicized setting, so like blocks, senders don't know.

@@ -130,9 +136,18 @@ impl Object for ApubPrivateMessage {
let recipient = note.to[0].dereference(context).await?;
PersonBlock::read(&mut context.pool(), recipient.id, creator.id).await?;

// If its a local user, check that they can receive private messages
if recipient.local {
Copy link
Member

Choose a reason for hiding this comment

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

This check is not really needed because the recipient of a federated private message is always a local user.

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.

@Nutomic
Copy link
Member

Nutomic commented Nov 5, 2024

Ah youre also getting the same random image test failure as in private community PR.

@dessalines
Copy link
Member Author

Weird, we haven't touched that at all.

Comment on lines +361 to +365
if !local_user_view.local_user.enable_private_messages {
Err(LemmyErrorType::CouldntCreatePrivateMessage)?
} else {
Ok(())
}
Copy link
Member

Choose a reason for hiding this comment

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

This can be expressed in a different way that you may or may not prefer:

  local_user_view
    .local_user
    .enable_private_messages
    .then_some(())
    .ok_or_else(|| LemmyErrorType::CouldntCreatePrivateMessage.into())

Copy link
Member Author

Choose a reason for hiding this comment

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

Not a huge fan of that one at least in this case, because it unnecessarily coerces it into an option, when it's already a bool.

@Nutomic Nutomic enabled auto-merge (squash) November 6, 2024 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants