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

Refactor MetagovRequestHandler #176

Merged
merged 4 commits into from
Dec 27, 2021
Merged

Refactor MetagovRequestHandler #176

merged 4 commits into from
Dec 27, 2021

Conversation

shaunagm
Copy link
Collaborator

Moved this out of #175. Not thrilled with it as it stands, but at least I understand the code better?

This has not been tested yet.

Per conversation with @mashton, additional steps not addressed by this PR include:

  • eventually moving "received authorized state" logic to http wrapper, since Django drivers don't need it
  • handling client credentials grant type of auth2. currently we only do code grant, but for example Mastodon uses client credentials grants
  • we need to delete communities if installation fails after the point in the process where the community is created
  • we need to handle users revoking tokens/access

@shaunagm shaunagm requested a review from gigxz December 20, 2021 21:47
Copy link
Collaborator

@gigxz gigxz left a comment

Choose a reason for hiding this comment

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

Looks good to me

@shaunagm shaunagm force-pushed the refactor_metagovhandler branch from 3eb2283 to b86a64a Compare December 22, 2021 19:37
@shaunagm
Copy link
Collaborator Author

Okay, tested this on my dev server. I was able to install a Slack community, log in to that Slack community with my user, and set up a policy that responded to a message in Slack with another message.

@mashton, are there other things you think I should test, or should we go ahead and merge this?

@gigxz
Copy link
Collaborator

gigxz commented Dec 23, 2021

Testing Slack only tests the "platformwide handlers", so it wouldn't hit pass_to_plugin_instance. I'd try out Open Collective or Loomio too to test that part out!

@shaunagm
Copy link
Collaborator Author

Okay, I was able to verify that webhooks to OpenCollective are getting routed to pass_to_plugin_instance, although my dev server is then behaving weirdly. I think that's unrelated to this PR though, so I'm going to merge it.

@shaunagm shaunagm merged commit 71a0987 into master Dec 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants