LemmyNet
/
lemmy
3
2
Fork 3
Code Pull Requests 1 Releases Wiki Activity

Separate logic for user and community inbox #123

Merged
dessalines merged 7 commits from rewrite-inbox into main 2020-11-10 13:14:43 +00:00
Conversation 4 Commits 7 Files Changed 25 +1292 -847
nutomic commented 2020-10-28 16:15:16 +00:00
Owner
Copy Link

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

Just pushing what I have so far. Still very much WIP.
dessalines commented 2020-11-02 15:11:57 +00:00
Owner
Copy Link

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 2020-11-02 17:29:45 +00:00
lemmy_apub/src/activities/receive/private_message.rs Outdated
@ -0,0 +215,4 @@
get_or_fetch_and_upsert_user(&user_id, &context, request_counter).await?;
Ok(())
}
dessalines commented 2020-11-02 17:29:45 +00:00
Owner
Copy Link

Seems good to move this here.

Seems good to move this here.
dessalines commented 2020-11-02 17:39:35 +00:00
Owner
Copy Link

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 2020-11-02 19:43:33 +00:00
lemmy_apub/src/inbox/receive_in_community.rs Outdated
@ -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 2020-11-02 19:43:33 +00:00
Owner
Copy Link

Its fine to me.

Its fine to me.
dessalines reviewed 2020-11-02 19:46:51 +00:00
lemmy_apub/src/inbox/shared_inbox.rs Outdated
@ -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 2020-11-02 19:46:51 +00:00
Owner
Copy Link

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 2020-11-03 13:02:26 +00:00
Author
Owner
Copy Link

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 2020-11-02 19:50:06 +00:00
lemmy_apub/src/inbox/community_inbox.rs Outdated
@ -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 2020-11-02 19:50:06 +00:00
Owner
Copy Link

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 2020-11-03 13:05:15 +00:00
Author
Owner
Copy Link

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 2020-11-03 16:09:04 +00:00
lemmy_apub/src/inbox/user_inbox.rs Outdated
@ -116,3 +167,1 @@
)
.await?;
res
// TODO: would be logical to move websocket notification code here
nutomic commented 2020-11-03 16:09:04 +00:00
Author
Owner
Copy Link

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 2020-11-03 16:49:30 +00:00
Author
Owner
Copy Link

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 2020-11-09 12:56:40 +00:00
Author
Owner
Copy Link

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 2020-11-09 13:12:04 +00:00
dessalines merged commit 437809d337 into main 2020-11-10 13:14:42 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: LemmyNet/lemmy#123
No description provided.
Delete Branch "rewrite-inbox"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?