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 note wrapper (fixes #2657) #5221

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

Add note wrapper (fixes #2657) #5221

wants to merge 8 commits into from

Conversation

Nutomic
Copy link
Member

@Nutomic Nutomic commented Nov 22, 2024

Private messages and public comments are quite awkward in Activitypub, because the json format looks identical. They only way to differentiate them is to check for the presence or absence of https://www.w3.org/ns/activitystreams#Public in to or cc. This PR adds a new wrapper for federated comments and private messages, which allows both to have type Note and handles them appropriately. This means that private messages will be compatible with other Fediverse platforms like Mastodon.

It is also backwards compatible with private messages on older Lemmy versions. For receiving it simply accepts the old ChatMessage type. For sending, it checks the software and version of the target instance. If it is a Lemmy instance prior to 0.20, a ChatMessage is sent, and otherwise a Note.

@Nutomic Nutomic marked this pull request as ready for review November 22, 2024 11:25
crates/apub/src/objects/note_wrapper.rs Outdated Show resolved Hide resolved
crates/apub/src/objects/note_wrapper.rs Show resolved Hide resolved
crates/apub/src/objects/private_message.rs Outdated Show resolved Hide resolved
@@ -32,12 +32,12 @@ impl Object for ApubNote {
_object_id: Url,
_context: &Data<Self::DataType>,
) -> LemmyResult<Option<Self>> {
unimplemented!()
Err(LemmyErrorType::Unknown("not implemented".to_string()).into())
Copy link
Member

Choose a reason for hiding this comment

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

I looked at some of the others, and these all have specific FederationErrors for actions that can't be done. But its up to you there.

Also worth looking into why the unimplemented and todo passed the CI.

Copy link
Member Author

Choose a reason for hiding this comment

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

Those other errors can actually occur in production. But the ones here are never be called, they are only relevant when fetching remote objects or sending objects out, neither of which is done with NoteWrapper (sending is done via PrivateMessage/Comment).

Copy link
Member Author

Choose a reason for hiding this comment

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

As for the lints not working it seems to be related to async_trait, outside of that it works fine.

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.

2 participants