Let community announce posts #36
Loading…
Reference in New Issue
No description provided.
Delete Branch "announce-posts"
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?
Not finished yet, and the tests are failing.
@ -291,3 +281,1 @@
"".to_string()
}
})
.map(|c| get_shared_inbox(&c.user_actor_id))
Honestly I have no clue what your code is doing, but I dont think its needed.
Let community announce poststo WIP: Let community announce postsNeed to change this so that comments, votes and undos are federated (probably by having the community announce all of them?).
Yeah, everything in the receive needs to be "forwarded" / announced to the other correct servers.
To test this correctly, we should also make sure the federation-test adds a third server.
@ -125,3 +125,3 @@
for cpost in &incorrect_posts {
Post::update_ap_id(&conn, cpost.id)?;
Post::update_ap_id(&conn, cpost.id, todo!())?;
Dont forget to implement this properly.
I'm not sure where exactly
MentionsAndAddresses::inboxes
was used before, but I'm getting warnings now that its unused. Any advice how to integrate it again?Edit: I think I got it, will add another commit.
Edit 2: Done, in the remote case comment notifications will be handled just like before, but I'm not sure if the local case is handled correctly.
WIP: Let community announce poststo Let community announce posts@ -71,1 +73,3 @@
Ok(Activity::create(&conn, &activity_form)?)
debug!("inserting activity for user {}, data {:?}", user_id, data);
// TODO: this is broken
//Activity::create(&conn, &activity_form)?;
As I wrote in the chat, this is breaking with errors like
Key (user_id)=(-1) is not present in table "user_"
. We should probably add foreign keys user_id and community_id and require one of them to be correct. Or instead a column likeactor_id
?I'll read through again, the user_id shouldn't ever be -1, but should be the local DB's id for that user.
That doesnt work if the action is taken by the community (eg announce).
One level inside announce tho, the activity will have the creator / user that did the action.
Okay I went with community.creator_id, which is the same thing we use for the accept follow activity. Still seems wrong.
Oh also, the federation tests did pass, but we really didn't test out 3 instance thing which this really requires. I can write those tests tho, probably will be able to get to it tmrw.
@ -483,0 +486,4 @@
send_activity(&activity, creator, to)?;
}
Ok(())
}
This is the same as what's in the
impl Post
, and since it doesn't useself
, you could move this up tomod.rs
Right, good catch. I moved it to activities::send_activity_to_community().
@ -393,0 +392,4 @@
where
A: Activity + Base + Serialize + Debug,
{
insert_activity(&conn, -1, &activity, is_local_activity)?;
There's a user_id that's doing the activity, so that needs to get passed through. Instead of
sender: &str
, use ``creator: &User_`, then you'll have both the actor_id, and the user_id.Also just to add, part of my thinking in making this a necessary column in the activity table, is to ensure that federated users also have local ids.
@ -393,0 +410,4 @@
// dont send to the instance where the activity originally came from, because that would result
// in a database error (same data inserted twice)
let mut to = community.get_follower_inboxes(&conn)?;
let sending_user = get_or_fetch_and_upsert_remote_user(&sender, conn)?;
Now you won't have to do this, because you already have the sending user, which already implements
ActorType
and you can get the shared_inbox that way.Right, done. There is a similar issue in shared_inbox but that is trickier to fix (I left a todo).
@ -393,0 +405,4 @@
.set_actor_xsd_any_uri(community.actor_id.to_owned())?
.set_object_base_box(BaseBox::from_concrete(activity)?)?;
insert_activity(&conn, -1, &announce, true)?;
Thinking about it, this is correct, it does make sense to insert both the activity, and the announce.
@ -56,0 +57,4 @@
return Err(format_err!(
"Received activity is addressed to remote community {}",
&community.actor_id
));
Good call, I didn't think about this.
@ -68,0 +99,4 @@
.unwrap()
.next()
.unwrap()
.to_string()
I have to think about this one. The first item of the array is
community/followers
, but the other ones might be mentions and such.I'm pretty sure in my code I just scan the comment text again anyway to handle mentions, but it would be good to make sure this returns the full array.
@ -68,0 +86,4 @@
// TODO: there is probably an easier way to do this
let oprops = match self {
SharedAcceptedObjects::Create(c) => &c.object_props,
SharedAcceptedObjects::Update(u) => &u.object_props,
There's gotta be a cleaner way to do all of these, I'll play around a bit.
I hope the new apub library will make it unnecessary.
@ -85,0 +128,4 @@
let c = get_or_fetch_and_upsert_remote_community(&sender.to_string(), &conn)?;
verify(&request, &c)
}
}?;
I can't think of a good cross-compatible way to do this either. We can't use urls because other future group-implementers will probably not use
/c/
...We could move the check into the match because announce is the only activity that comes from a community. But then we probably have to duplicate the verify a lot more times. Lets see.
@ -137,0 +220,4 @@
if !community.local {
// ignore this object
}
Community::do_announce(activity, &community, sender, conn, false)
Shouldn't this go in
if community.local {
Right, I think I forgot to finish this.
@ -137,0 +233,4 @@
.get_object_base_box()
.unwrap()
.to_owned();
// TODO: too much copy paste
I'll have to play around and see if there's a cleaner way to do it too, but I doubt there is.
I guess we can leave the refactoring until after we switch to the new library version.
Good call.