-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor follow activities and test data previously #87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor follow activities and test data previously #87
Conversation
| # Create some follow relationships | ||
| for _ in range(2): # Each actor follows 2 other actors | ||
| # Create local follows | ||
| for _ in range(1): # Each actor follows 1 local actor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need a for here then? Perhaps we are planning to change this param later?
If so, maybe we can move this to a const or some parameter (if we want to set this dynamically during runtime)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The seeding in here is just trying to recreate the FollowActivities; as you said, I agree if we move it to a constant.
Maybe we could work in some other way to create the seeding. Any idea is good.
| local_follow_count += 1 | ||
|
|
||
| # Create remote follows | ||
| for _ in range(1): # Each actor follows 1 remote actor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto + in similar for loops across the file
| username = factory.SelfAttribute('user.username') | ||
| full_name = factory.Faker('name') | ||
| previously = factory.Dict({}) | ||
| previously = factory.List([]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC previously is semantically similar to the stack behaviour?
https://swicg.github.io/activitypub-data-portability/lola.html#adding-breadcrumbs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is
| created_at = models.DateTimeField(auto_now_add=True) | ||
| updated_at = models.DateTimeField(auto_now=True) | ||
| previously = models.JSONField(default=dict, null=True, blank=True) | ||
| previously = models.JSONField(default=list, null=True, blank=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we explicitly convert it to an empty list in several places (i.e. self.previously or [] on L77) why don't we set null=False here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. Makes sense.
alexeyqu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments -- overall looks good to me, however I'll defer the "does the model fit the proposal?" to someone else 😉
Refactor FollowActivity
Following what we did with the LikeActivity, we got the following refactor for FollowActivity:
target_actor_urlandtarget_actor_datafields for remote actorsclean()method to validate that either local or remote data is providedget_json_ldhandle both local and remote actorsThe first one is a Remote Follow Activity and the second one is a Local Follow Activity.
We are able to represent both scenarios, when an Actor follows another Actor within the same server and the other one in which an Actor follows another one from other server.
Closes #72
Adding test data in previously field
previouslyfield in the Actor model should contain historical identifiers or data about the account if it was moved/migrated from another server.In a real federation scenario, it might look like:
Thoughts
During seeding I thought that it doesn't make sense for every Actor to have a previously field filled. The previously field should only be populated for actors that have actually moved or migrated from another server.
So in our seed data, it would be more realistic to:
With no move:

With move history:

previouslyfield being display in Actors outboxes when referencing FollowActivities and so on?I figured that when a FollowActivity or other activities reference an Actor with a migration history, that history should be part of the Actor's JSON-LD representation in those activities.
So when we see a Follow activity in the outbox, if it's following a local actor who has migration history, we’ll see something like the following scenario:
For example, if Bob moved from mastodon.social to our server, and Alice follows Bob, the activity should show:
{ "type": "Follow", "actor": "https://example.com/users/alice", "object": { "type": "Person", "id": "https://example.com/users/bob", "preferredUsername": "bob", "previously": [ { "type": "Move", "object": "https://old-server.com/users/bob", "published": "2025-03-20T00:00:00Z" } ] } }I believe this is good because it maintains the portability information throughout the activity stream.
This is only for Actors who has moved or migrated from one server to another. I am sure we'll need to add previously data to each Activity when doing migration.
Closes #52