#123 Separate logic for user and community inbox

Merged
dessalines merged 7 commits from rewrite-inbox into main 3 months ago
nutomic commented 4 months ago
Owner

Just pushing what I have so far. Still very much WIP.

Just pushing what I have so far. Still very much WIP.
Poster
Owner

Let me know if there's anything I can do for this.

Let me know if there's anything I can do for this.
dessalines reviewed 3 months ago
lemmy_apub/src/activities/receive/private_message.rs Outdated
get_or_fetch_and_upsert_user(&user_id, &context, request_counter).await?;
Ok(())
}
dessalines commented 3 months ago

Seems good to move this here.

Seems good to move this here.
Poster
Owner

This was pretty big and hard to follow, but I think I follow... its mostly extracting things from the shared inbox / other inboxes to the activity receive folders.

This was pretty big and hard to follow, but I think I follow... its mostly extracting things from the shared inbox / other inboxes to the activity receive folders.
dessalines reviewed 3 months ago
lemmy_apub/src/inbox/receive_in_community.rs Outdated
// TODO: this file is for post/comment activities received by the community, and for post/comment
// activities announced by the community and received by the user. the name is terrible but
// i cant think of anything better.
dessalines commented 3 months ago

Its fine to me.

Its fine to me.
dessalines reviewed 3 months ago
lemmy_apub/src/inbox/shared_inbox.rs Outdated
///
/// This doesnt handle the case where an activity is addressed to multiple communities (because
/// Lemmy doesnt generate such activities).
// TODO: this needs a better name
dessalines commented 3 months ago

is_addressed_to seems fine to me, otherwise maybe extract_local_community_from_to_and_ccs or something.

is_addressed_to seems fine to me, otherwise maybe `extract_local_community_from_to_and_ccs` or something.
nutomic commented 3 months ago

Thanks, I went with extract_local_community_from_destinations() and changed the others to return bool.

Thanks, I went with `extract_local_community_from_destinations()` and changed the others to return bool.
dessalines reviewed 3 months ago
lemmy_apub/src/inbox/community_inbox.rs Outdated
match activity_kind {
// Note, follow/unfollow are already handled earlier
CommunityValidTypes::Follow => {
// TODO: not sure what to do here, this was already handled before
dessalines commented 3 months ago

Follow should probably be separate from the other communityvalidtypes... this is why we had the shared inbox be all public activities, and the user and community inboxes only be the private activities , following / unfollowing, sending a dm, etc.

Follow should probably be separate from the other communityvalidtypes... this is why we had the shared inbox be all public activities, and the user and community inboxes only be the private activities , following / unfollowing, sending a dm, etc.
nutomic commented 3 months ago

Well the reason I split the inboxes is because it makes no sense to handle a Create/Post identical to an Announce/Create/Post, they are quite different things. For example it is much easier to implement community bans now.

Anyway I'm rewriting this in a better way.

Well the reason I split the inboxes is because it makes no sense to handle a `Create/Post` identical to an `Announce/Create/Post`, they are quite different things. For example it is much easier to implement community bans now. Anyway I'm rewriting this in a better way.
nutomic reviewed 3 months ago
lemmy_apub/src/inbox/user_inbox.rs Outdated
)
.await?;
res
// TODO: would be logical to move websocket notification code here
nutomic commented 3 months ago

What do you think about separating the websocket notifications like this? It seems more logical to me to separate it from the receive logic, but most likely its too complicated cause every type of activity would have to be handled separately anyway.

What do you think about separating the websocket notifications like this? It seems more logical to me to separate it from the receive logic, but most likely its too complicated cause every type of activity would have to be handled separately anyway.
nutomic commented 3 months ago
Poster
Owner

As I said, the test case A and G subscribe to B (center) A posts, G mentions B, it gets announced to A needs to be fixed, then it can be squashed and merged.

I also added a check to ensure that post, comment etc activities are addressed to public. This is to prevent the possibility of communities announcing private activities (eg private message) which they received by accident (or due to bugs).

By the way, we could merge the code for user_inbox, community_inbox and shared_inbox functions even more, because they are mostly identical.

As I said, the test case `A and G subscribe to B (center) A posts, G mentions B, it gets announced to A` needs to be fixed, then it can be squashed and merged. I also added a check to ensure that post, comment etc activities are addressed to public. This is to prevent the possibility of communities announcing private activities (eg private message) which they received by accident (or due to bugs). By the way, we could merge the code for user_inbox, community_inbox and shared_inbox functions even more, because they are mostly identical.
nutomic commented 3 months ago
Poster
Owner

Squashed this and fixed a bug in community removal. Still need to fix the test A and G subscribe to B (center) A posts, G mentions B, it gets announced to A (which I dont understand).

Squashed this and fixed a bug in community removal. Still need to fix the test `A and G subscribe to B (center) A posts, G mentions B, it gets announced to A` (which I dont understand).
nutomic changed title from WIP: Separate logic for user and community inbox to Separate logic for user and community inbox 3 months ago
dessalines merged commit 437809d337 into main 3 months ago
The pull request has been merged as 437809d337.
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.