#123 Separate logic for user and community inbox

Merged
dessalines merged 7 commits from rewrite-inbox into main 2 weeks ago
nutomic commented 4 weeks ago

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

Just pushing what I have so far. Still very much WIP.
dessalines commented 3 weeks ago
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 weeks ago
@@ -0,0 +215,4 @@
get_or_fetch_and_upsert_user(&user_id, &context, request_counter).await?;

Ok(())
}
dessalines commented 3 weeks ago

Seems good to move this here.

Seems good to move this here.
dessalines commented 3 weeks ago
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 weeks ago
lemmy_apub/src/inbox/receive_in_community.rs
@@ -0,0 +45,4 @@

// 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 weeks ago

Its fine to me.

Its fine to me.
dessalines reviewed 3 weeks ago
lemmy_apub/src/inbox/shared_inbox.rs
@@ -268,0 +119,4 @@
///
/// 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 weeks 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 weeks 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 weeks ago
lemmy_apub/src/inbox/community_inbox.rs
@@ -95,0 +141,4 @@
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 weeks 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 weeks 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 weeks ago
lemmy_apub/src/inbox/user_inbox.rs
@@ -116,3 +167,1 @@
)
.await?;
res
// TODO: would be logical to move websocket notification code here
nutomic commented 3 weeks 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 weeks ago
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 2 weeks ago
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 2 weeks ago
dessalines merged commit 437809d337 into main 2 weeks 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.