Skip to content
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

Fix some federation issues #357

Merged
merged 7 commits into from
Dec 23, 2018
Merged

Fix some federation issues #357

merged 7 commits into from
Dec 23, 2018

Conversation

trinity-1686a
Copy link
Contributor

@trinity-1686a trinity-1686a commented Dec 15, 2018

There are some bugs in federation, the goal of this will be to fix most of them

(Maybe incomplete) list of things to do :

  • no notification on follow
  • can't unfollow from Mastodon/Pleroma, but can from Plume
  • Pleroma followers (maybe Mastodon too) are getting notifications from blog post (should only appear in the home timeline)
  • fix Following mastodon users won't work #172
  • fix Can't follow plume user from mastodon #334
  • mention links are a bit broken, they are relative to this instance so displaying them on another instance/ap enabled software result in 404
  • when undoing activity, verify if original activity owner is the undo activity emitter

Fix not receiving notifications when followed by remote users
Fix imposibility to be unfollowed by Mastodon/Pleroma users (tested only against Pleroma)
@trinity-1686a trinity-1686a added A: Federation Stuff related to Federation S: Incomplete This PR is not complete yet labels Dec 15, 2018
@codecov
Copy link

codecov bot commented Dec 15, 2018

Codecov Report

Merging #357 into master will decrease coverage by 0.09%.
The diff coverage is 10.14%.

@@            Coverage Diff            @@
##           master     #357     +/-   ##
=========================================
- Coverage   28.75%   28.65%   -0.1%     
=========================================
  Files          62       62             
  Lines        5651     5670     +19     
=========================================
  Hits         1625     1625             
- Misses       4026     4045     +19

@trinity-1686a
Copy link
Contributor Author

Concerning mention links, I think the only easy solution is to add a domain argument to the parser, and prefix each link with it. Any other solution I'm thinking of either imply post processing or database access in the parser

@elegaanz
Copy link
Member

Maybe we can always use the FQN for these links so that they are correct on remote instances, and add a redirection when requesting a FQN that is on the current instance?

@trinity-1686a
Copy link
Contributor Author

trinity-1686a commented Dec 15, 2018

FQN will do the trick when talking to other Plume, but Mastodon use a different model for user links (https://mastodon.local/@<name>, notice no slash between @ and <name>), and Pleroma uses yet another (https://pleroma.local/users/<name>). So either we use users ap-url (need db access), or we use an absolute link to the current instance (not the best solution, but definitely the easiest)

@trinity-1686a trinity-1686a changed the title Fix some follow issues Fix some federation issues Dec 15, 2018
@marek-lach
Copy link
Member

marek-lach commented Dec 16, 2018

FQN will do the trick when talking to other Plume, but Mastodon use a different model for user links (https://mastodon.local/@<name>, notice no slash between @ and <name>), and Pleroma uses yet another (https://pleroma.local/users/<name>). So either we use users ap-url (need db access), or we use an absolute link to the current instance (not the best solution, but definitely the easiest)

Mastodon also recognises the profile URL format of https://mastodon.local/users/<name>, to redirect to the correct user profile, I have tried it. Thus it may be possible if you were to implement the URL format of https://pleroma.local/users/<name> that it should work just fine for Mastodon too, I think?

@trinity-1686a
Copy link
Contributor Author

We could change our scheme, but it wouldn't fix how we treat remote users. /@/<user@domain> is valid with Plume, but the equivalent is not with Mastodon/Pleroma. I really think using absolute urls is the easiest way to go

Send Link instead of Object when emiting Follow request
Receive both Link and Object for Follow request
Don't panic when fetching user with no followers or posts from Pleroma
Reorder follower routes so Activity Pub one is reachable
@trinity-1686a trinity-1686a added S: Ready for review This PR is ready to be reviewed and removed S: Incomplete This PR is not complete yet S: Ready for review This PR is ready to be reviewed labels Dec 17, 2018
@trinity-1686a
Copy link
Contributor Author

One can undo activities made by others. I should fix that too before merging

@trinity-1686a trinity-1686a added the S: Ready for review This PR is ready to be reviewed label Dec 17, 2018
@elegaanz elegaanz self-requested a review December 22, 2018 10:58
Copy link
Member

@elegaanz elegaanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All your changes seems to work! The only issue, but I'm not sure if it one is that I'm still getting notifications in Pleroma: it appears on the notification page (/USER/mentions) but not in the notification panel on the left (but maybe that's just how Pleroma works, I don't use it regularly enough to know).

@trinity-1686a
Copy link
Contributor Author

I've no idea why it's that way, I would expect the left panel, /USER/mentions and the notification on /web to be in sync, apparently they aren't???

@elegaanz
Copy link
Member

Yes, I don't know how Pleroma works 🤷‍♀️ but I guess we can ignore this issue, it is still better than before even if not perfect…

@trinity-1686a trinity-1686a merged commit 0ea1d57 into master Dec 23, 2018
@trinity-1686a trinity-1686a deleted the fix-federation branch December 23, 2018 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: Federation Stuff related to Federation S: Ready for review This PR is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't follow plume user from mastodon Following mastodon users won't work
3 participants