Conversation
Gargron
added
activitypub
Gargron
force-pushed
the
feature-activitypub-delivery
branch
2 times, most recently
from
9408a6b to
798f967
Compare
Incidentally fix #889
Gargron
force-pushed
the
feature-activitypub-delivery
branch
from
e35c6d1 to
3ce4a0b
Compare
| current_account.update!(account_params) | ||
| @account = current_account | ||
| @account.update!(account_params) | ||
| ActivityPub::UpdateDistributionWorker.perform_async(@account.id) |
There was a problem hiding this comment.
this feels like it should either be in a model callback or a service
There was a problem hiding this comment.
- Can't be in a callback, callbacks execute in-transaction (queued job might get old data)
- Can't be a service because while this controller uses
update!, the preferences controller usesif update(different usage)
With only two usages, it's easier to just have this one extra line in both places.
app/models/account.rb
Outdated
| end | ||
|
|
||
| def inboxes | ||
| reorder(nil).where(protocol: :activitypub).pluck('distinct coalesce(nullif(accounts.shared_inbox_url, \'\'), accounts.inbox_url)') |
There was a problem hiding this comment.
minor, but seems like using double quotes here would make it so you don't have to escape the sql single quotes?
app/models/account.rb
Outdated
| end | ||
|
|
||
| def inboxes | ||
| reorder(nil).where(protocol: :activitypub).pluck('distinct coalesce(nullif(accounts.shared_inbox_url, \'\'), accounts.inbox_url)') |
| end | ||
|
|
||
| def batch_activity_json(account, statuses) | ||
| account.followers.inboxes.each do |inbox_url| |
There was a problem hiding this comment.
Delete isn't an addressed activity, and publicInbox is only for activities that are addressed to the Public collection. this feels like it should maybe be addressed by w3c/activitypub#242 although that hasn't been spec'd yet.
There was a problem hiding this comment.
sharedInbox does not have any limitations.
| def remove_from_mentioned(stream_entry) | ||
| salmon_xml = stream_entry_to_xml(stream_entry) | ||
| target_accounts = @mentions.map(&:account).reject(&:local?).uniq(&:domain) | ||
| def remove_from_remote_mentioned |
There was a problem hiding this comment.
remote_mentioned should either be renamed to remote_reblogged_or_mentioned or this should be split into two methods.
| # send it to them | ||
|
|
||
| target_accounts = (@mentions.map(&:account).reject(&:local?) + @reblogs.map(&:account).reject(&:local?)).uniq(&:id) | ||
|
|
There was a problem hiding this comment.
If I have a status because someone I follow reblogged it, but I didn't reblog it myself, I wouldn't receive a delete.
ActivityPub does not specify how to federate a Delete activity. tagging @cwebber because that feels like something the group should address.
|
|
||
| sidekiq_options queue: 'push', retry: 5, dead: false | ||
|
|
||
| HEADERS = { 'Content-Type' => 'application/activity+json' }.freeze |
There was a problem hiding this comment.
ActivityPub does not specify the content type for Server-to-Server interactions, but https://w3c.github.io/activitypub/#client-to-server-interactions implies that the primary content type of ActivityPub is application/ld+json; profile="https://www.w3.org/ns/activitystreams", and that all clients need to use that content type and that's the only one that's a must in the spec (activity+json is a should)
|
Looks good! some specific stuff i'm not super sure about, and I didn't read the tests super throughly but the rest of it looks great |
2 participants
TODO: