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

support pubgate #111

Merged
merged 6 commits into from Jun 3, 2019
Merged

support pubgate #111

merged 6 commits into from Jun 3, 2019

Conversation

ghost
Copy link

@ghost ghost commented May 20, 2019

this moves the unmarshaling of a remote actor out into a new helper which
accounts for the possibility of a context being a list or a single entity.
i.e. a string or an object.

basics tests are provided for both situations

also go fmt'd the file activitypub.go

during the course of working on this I found a missing string format variable in exports.go, in fixing that I also go fmt'd the file.

Fixes #100
This also fixes #116 by using the followers Inbox if SharedInbox is not set.

  • I have signed the CLA

@ghost
Copy link
Author

ghost commented May 20, 2019

sorry for the force pushes 😄 I haven't rebased in a while

@ghost ghost changed the title fix for #100 - can't follow from pubgate fixes #100 - can't follow from pubgate May 20, 2019
@ghost
Copy link
Author

ghost commented May 21, 2019

Oops, I left an extra field in the test table. Let me fix that

this moves the unmarshaling of a remote actor out into a new helper which
accounts for the possibility of a context being a list or a single entity.
i.e. a string or an object.

basics tests are provided for both situations

also go fmt'd the file activitypub.go
@thebaer
Copy link
Member

thebaer commented May 21, 2019

Thanks! I think the code looks good overall, just need to test. Was this route better than changing the type of Context to an interface{}, do you think?

Also, @mrvdb could you take a look at this?

@ghost
Copy link
Author

ghost commented May 21, 2019

I though so at first, after going with this instead I feel they would both be similar.

Either way we need to check for a string or slice and convert any string contexts into a slice. Due to the append behaviour when adding a private key in the core library.

So this could easily be moved there instead if preferred.

@mrvdb
Copy link
Collaborator

mrvdb commented May 22, 2019

Has this been this tested by someone already or is this a 'theoretical fix' at this time? (perhaps a good time to discuss at some point what is expected in terms of integration tests from contributors?)

@thebaer I will have a look indeed.

@ghost
Copy link
Author

ghost commented May 22, 2019

@mrvdb I have not had a chance to manually test the integration.

But I agree this is a good time to start talking about integration tests in general. There would ideally be a suite that runs on pull requests to test automatically, as well as being able to run locally while developing.

@joyeusenoelle
Copy link
Contributor

joyeusenoelle commented May 22, 2019

TravisCI does run on each commit on a pull request in these repos, to make sure the code's not broken. But that's not necessarily the same thing as "the code does what I want it to". ;)

.vscode/settings.json Outdated Show resolved Hide resolved
this moves the unmarshaling of a remote actor out into a new helper which
accounts for the possibility of a context being a list or a single entity.
i.e. a string or an object.

basics tests are provided for both situations

also go fmt'd the file activitypub.go
@thebaer
Copy link
Member

thebaer commented May 25, 2019

@robjloranger By the way, the best way to remove files that get accidentally committed like that would be with a new commit, instead of a force-push. Force-pushing is okay for single-user repos, but can mess things up when other people have pulled your branch.

Another helpful hint, if you didn't know about it: using git add -p to stage changes will let you walk through each chunk of code and decide whether or not to include it. This is the way I do all my commits, and has helped a lot over the years.

@ghost
Copy link
Author

ghost commented May 25, 2019

@mrvdb could you test by trying to follow @log@rob.writefreely.dev from your pubgate instance? this branch is live there now.

I will try to set one up today and test as well.

@mrvdb
Copy link
Collaborator

mrvdb commented May 25, 2019

@robjloranger Thanks, I will certainly do so, but I'll only have the ability to do it in about a week or so. (I'm hiking next week)

@ghost
Copy link
Author

ghost commented May 25, 2019

ok no trouble, I'll set up a local instance and try it out

@ghost
Copy link
Author

ghost commented May 26, 2019

if anyone familiar with pubgate and writefreely could help, I can't figure out how to follow a writefreely user from pubgate.

I assume it is an outbox follow on the pubgate side, but pointed at what endpoint on the writefreely side for a given user I do not know.

@mrvdb
Copy link
Collaborator

mrvdb commented Jun 3, 2019

Following https://rob.writefreely.dev from pubgate does not error out anymore, so that's a good sign.

The follow action works and the endpoint is correctly entered into the following list on my end. If you can start writing some posts on that endpoint I will monitor if the follow does actually work.

( @robjloranger I find it easiest to use https://www.getpostman.com/ and use the pubgate API that way)

@ghost
Copy link
Author

ghost commented Jun 3, 2019

@mrvdb OK wrote two test posts. I see both from mast still. And can unfollow and follow from there as well still.

@mrvdb
Copy link
Collaborator

mrvdb commented Jun 3, 2019

I don't see any connection coming in, can you post again? (I've upped logging so I see more)

@ghost
Copy link
Author

ghost commented Jun 3, 2019

All set

@mrvdb
Copy link
Collaborator

mrvdb commented Jun 3, 2019

Nothing on the pubgate side, my follow on mastodon gets the post. I think the error is on my side.

@mrvdb
Copy link
Collaborator

mrvdb commented Jun 3, 2019

Not sure what is going on. I get objects from my other subscriptions, so pretty sure the setup is ok now.

I've unfollowed and re-followed, could you post again?

@ghost ghost mentioned this pull request Jun 3, 2019
@ghost
Copy link
Author

ghost commented Jun 3, 2019

This issue appears fixed, but we have a new interoperability issue: #116

@ghost ghost changed the title fixes #100 - can't follow from pubgate support pubgate Jun 3, 2019
@ghost
Copy link
Author

ghost commented Jun 3, 2019

all is working well now with pubgate @thebaer

@thebaer thebaer self-requested a review June 3, 2019 20:33
Copy link
Member

@thebaer thebaer left a comment

Choose a reason for hiding this comment

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

Awesome, everything looks good to me. Thanks, @robjloranger and @mrvdb! Merging this now.

@thebaer thebaer merged commit c87b7ab into develop Jun 3, 2019
@ghost ghost deleted the gh100 branch June 8, 2019 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Pubgate: unsupported protocol scheme Can't Follow from pubgate
3 participants