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

Add community outbox #74

Merged
nutomic merged 2 commits from community-outbox into main 2020-07-29 20:51:55 +00:00
Conversation 8 Commits 2 Files Changed 6 +127 -64
nutomic commented 2020-07-24 15:09:03 +00:00
Owner
Copy Link

This is trickier than I thought, so I'd like you to have a look before I end up rewriting everything.

This is trickier than I thought, so I'd like you to have a look before I end up rewriting everything.
nutomic reviewed 2020-07-24 15:10:08 +00:00
server/src/apub/fetcher.rs Outdated
@ -310,0 +317,4 @@
};
// TODO: this violates unique constraint (on update presumably)
//CommunityModerator::join(conn, &community_moderator_form)?;
nutomic commented 2020-07-24 15:10:08 +00:00
Author
Owner
Copy Link

This violates a DB constraint, and anyway it wont handle removed moderators.

This violates a DB constraint, and anyway it wont handle removed moderators.
dessalines commented 2020-07-24 15:35:01 +00:00
Owner
Copy Link

2 options:

  • Create a quick DB helper method to remove all community moderators, then re-add them. (This won't handle community transfers tho, which actually change the community.creator_id).
  • Or better, in api/community.rs, in the AddModToCommunity, do an apub send, similar to this one in api/comment.rs:
    // Send the apub message
    if deleted {
      updated_comment
        .send_delete(&user, &self.client, pool)
        .await?;
    } else {
      updated_comment
        .send_undo_delete(&user, &self.client, pool)
        .await?;
    }

And create the activity.

2 options: - Create a quick DB helper method to remove all community moderators, then re-add them. (This won't handle community transfers tho, which actually change the `community.creator_id`). - Or better, in `api/community.rs`, in the `AddModToCommunity`, do an apub send, similar to this one in `api/comment.rs`: ``` // Send the apub message if deleted { updated_comment .send_delete(&user, &self.client, pool) .await?; } else { updated_comment .send_undo_delete(&user, &self.client, pool) .await?; } ``` And create the activity.
nutomic reviewed 2020-07-24 15:11:39 +00:00
server/src/apub/fetcher.rs Outdated
@ -312,0 +329,4 @@
for o in outbox_items.many().unwrap() {
let page = PageExt::from_any_base(o)?.unwrap();
// TODO: this should not fetch the community, as it causes infinite recursion
let post = PostForm::from_apub(&page, client, pool, &community.actor_id()?).await?;
nutomic commented 2020-07-24 15:11:39 +00:00
Author
Owner
Copy Link

This causes infinite recursion, because from_apub() fetches the community (which means it also fetches the outbox again, and so on). I could pass the community in, but thats annoying because I'd have to change all implementations of FromApub.

This causes infinite recursion, because `from_apub()` fetches the community (which means it also fetches the outbox again, and so on). I could pass the community in, but thats annoying because I'd have to change all implementations of `FromApub`.
dessalines commented 2020-07-24 15:40:28 +00:00
Owner
Copy Link

hrmm.... shouldn't the post::from_apub() only fetch the community if it doesn't exist yet / hits that refetch timer?

hrmm.... shouldn't the post::from_apub() only fetch the community if it doesn't exist yet / hits that refetch timer?
nutomic reviewed 2020-07-29 18:31:26 +00:00
server/lemmy_db/src/post.rs Outdated
@ -77,2 +77,4 @@
post
.filter(community_id.eq(the_community_id))
.then_order_by(stickied.desc())
.limit(20)
nutomic commented 2020-07-29 18:31:26 +00:00
Author
Owner
Copy Link

This function was unused before so this should be fine (but I can rename it if you want).

This function was unused before so this should be fine (but I can rename it if you want).
nutomic changed title from WIP: Add community outbox to Add community outbox 2020-07-29 19:13:59 +00:00
nutomic commented 2020-07-29 19:16:02 +00:00
Author
Owner
Copy Link

Ready to review. Regarding mod updates, I just disabled that for now, so behaviour is the same as before (mods are only fetched the first time, but never updated). We should note that somewhere on the issue tracker, but not quite sure where.

Its worth emphasizing that this will not fetch any comments or votes, only posts. In the future we might add those to the outbox as well, but that will make parsing more difficult.

Ready to review. Regarding mod updates, I just disabled that for now, so behaviour is the same as before (mods are only fetched the first time, but never updated). We should note that somewhere on the issue tracker, but not quite sure where. Its worth emphasizing that this will not fetch any comments or votes, only posts. In the future we might add those to the outbox as well, but that will make parsing more difficult.
dessalines reviewed 2020-07-29 19:37:33 +00:00
server/lemmy_db/src/post.rs Outdated
dessalines commented 2020-07-29 19:37:33 +00:00
Owner
Copy Link

I spose we only really care about the stickies right? Maybe we should just filter by stickied, order by published.desc(). And rename this stickied_for_community

I spose we only really care about the stickies right? Maybe we should just filter by stickied, order by `published.desc()`. And rename this `stickied_for_community`
dessalines commented 2020-07-29 19:41:18 +00:00
Owner
Copy Link

Giteas giving me a really weird issue when I try to click on the files tab here: template: repo/diff/box:152:65: executing "repo/diff/box" at <0>: nil pointer evaluating *models.Review.Type

I'll just leave comments below.

  pub fn list_for_community(
    conn: &PgConnection,
    the_community_id: i32,
  ) -> Result<Vec<Self>, Error> {
    use crate::schema::post::dsl::*;
    post
      .filter(community_id.eq(the_community_id))
      .then_order_by(stickied.desc())
      .limit(20)
      .load::<Self>(conn)
  }

This is only really caring about the stickieds right? And I spose some recent posts. I'd also add an .then_order_by(published.desc() (the hot rank isn't available on the post table, just the view and fast_view).

Giteas giving me a really weird issue when I try to click on the files tab here: `template: repo/diff/box:152:65: executing "repo/diff/box" at <0>: nil pointer evaluating *models.Review.Type` I'll just leave comments below. ``` pub fn list_for_community( conn: &PgConnection, the_community_id: i32, ) -> Result<Vec<Self>, Error> { use crate::schema::post::dsl::*; post .filter(community_id.eq(the_community_id)) .then_order_by(stickied.desc()) .limit(20) .load::<Self>(conn) } ``` This is only really caring about the stickieds right? And I spose some recent posts. I'd also add an `.then_order_by(published.desc()` (the hot rank isn't available on the post table, just the view and fast_view).
dessalines commented 2020-07-29 19:42:47 +00:00
Owner
Copy Link
  let mut pages: Vec<AnyBase> = vec![];
  for p in posts {
    pages.push(p.to_apub(&db).await?.into_any_base()?);
  }

  let len = pages.len();
  let mut collection = OrderedCollection::new(pages);
  collection
    .set_context(context())
    .set_id(community.get_outbox_url()?)
    .set_total_items(len as u64);
  Ok(create_apub_response(&collection))

Nice, that's not too bad actually.

``` let mut pages: Vec<AnyBase> = vec![]; for p in posts { pages.push(p.to_apub(&db).await?.into_any_base()?); } let len = pages.len(); let mut collection = OrderedCollection::new(pages); collection .set_context(context()) .set_id(community.get_outbox_url()?) .set_total_items(len as u64); Ok(create_apub_response(&collection)) ``` Nice, that's not too bad actually.
dessalines commented 2020-07-29 19:44:47 +00:00
Owner
Copy Link
fn should_refetch_actor(last_refreshed: NaiveDateTime) -> bool {
  if cfg!(debug_assertions) {
    // avoid infinite loop when fetching community outbox
    let update_interval = chrono::Duration::seconds(ACTOR_REFETCH_INTERVAL_SECONDS_DEBUG);
    last_refreshed.lt(&(naive_now() - update_interval))
  } else {
    let update_interval = chrono::Duration::seconds(ACTOR_REFETCH_INTERVAL_SECONDS);
    last_refreshed.lt(&(naive_now() - update_interval))
  }
}

You can move the check after, this can be

fn should_refetch_actor(last_refreshed: NaiveDateTime) -> bool {
  let update_interval = if cfg!(debug_assertions) {
    // avoid infinite loop when fetching community outbox
    chrono::Duration::seconds(ACTOR_REFETCH_INTERVAL_SECONDS_DEBUG)
  } else {
    chrono::Duration::seconds(ACTOR_REFETCH_INTERVAL_SECONDS)
  };
  
  last_refreshed.lt(&(naive_now() - update_interval))
}
``` fn should_refetch_actor(last_refreshed: NaiveDateTime) -> bool { if cfg!(debug_assertions) { // avoid infinite loop when fetching community outbox let update_interval = chrono::Duration::seconds(ACTOR_REFETCH_INTERVAL_SECONDS_DEBUG); last_refreshed.lt(&(naive_now() - update_interval)) } else { let update_interval = chrono::Duration::seconds(ACTOR_REFETCH_INTERVAL_SECONDS); last_refreshed.lt(&(naive_now() - update_interval)) } } ``` You can move the check after, this can be ``` fn should_refetch_actor(last_refreshed: NaiveDateTime) -> bool { let update_interval = if cfg!(debug_assertions) { // avoid infinite loop when fetching community outbox chrono::Duration::seconds(ACTOR_REFETCH_INTERVAL_SECONDS_DEBUG) } else { chrono::Duration::seconds(ACTOR_REFETCH_INTERVAL_SECONDS) }; last_refreshed.lt(&(naive_now() - update_interval)) } ```
dessalines commented 2020-07-29 19:49:50 +00:00
Owner
Copy Link

Some of the stuff in fetcher.rs scares me cause we need to test the outbox fetching, and there isn't a test in here for that. But we can do that later.

Some of the stuff in fetcher.rs scares me cause we need to test the outbox fetching, and there isn't a test in here for that. But we can do that later.
dessalines commented 2020-07-29 19:58:38 +00:00
Owner
Copy Link

Looks good tho, tests all passed.

Looks good tho, tests all passed.
nutomic commented 2020-07-29 20:50:26 +00:00
Author
Owner
Copy Link

Good suggestions, thanks!

Good suggestions, thanks!
nutomic merged commit 6d8e1a7ab5 into main 2020-07-29 20:51:55 +00:00
nutomic commented 2020-07-29 20:52:10 +00:00
Author
Owner
Copy Link

Merged, I just hope I got the order of those filters right.

Merged, I just hope I got the order of those filters right.
nutomic deleted branch community-outbox 2020-07-29 20:52:31 +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#74
No description provided.
Delete Branch "community-outbox"

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?