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

First draft of identity db changes and internal api #119

Merged
merged 22 commits into from
Sep 24, 2021
Merged

Conversation

shaunagm
Copy link
Collaborator

@shaunagm shaunagm commented Aug 31, 2021

This isn't ready to go - it isn't tested, there aren't migrations made - but I thought I'd open up a PR for review. I have a few design questions tagged as #FIXME (three total).

Plus I have a general question of whether the DB design makes the internal API too complex to read or too costly to perform.

@shaunagm shaunagm requested a review from gigxz August 31, 2021 20:29
@shaunagm
Copy link
Collaborator Author

shaunagm commented Sep 13, 2021

I've updated the PR to use a newer, simplified data model as well as making some other changes for readability's sake. Going to work on tests now.

@shaunagm
Copy link
Collaborator Author

shaunagm commented Sep 14, 2021

Whenever you're ready @mashton (no rush! I cannot stress that enough) I've got two things for you to look at:

  1. the HTTP APIs are available to view in core/views.py. They're a bit janky right now, I can clean them up, but want your feedback first. I thought about using serializers here but I think if we are going to use serializers they should be within the internal API so we don't have to do everything twice. see 1c55974

  2. I've got a very rough stab at incorporating the internal API into the oauth workflow for Slack specifically (with some of the logic abstracted into the base Plugin model). see 7307079

@shaunagm shaunagm merged commit efd3cd6 into master Sep 24, 2021
@gigxz gigxz deleted the identity-system branch November 28, 2021 23:07
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