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

Make users provider agnostic #58

Merged
merged 16 commits into from
Jul 22, 2023
Merged

Conversation

RichDom2185
Copy link
Member

@RichDom2185 RichDom2185 commented Jul 21, 2023

Part of #18.

We include a schema migration, but not a data migration, as it is not necessary at this stage of the codebase (not deployed).

@RichDom2185 RichDom2185 self-assigned this Jul 21, 2023
@RichDom2185
Copy link
Member Author

@YaleChen299 I chose for it to be done this way so that we can keep a separate notion of which login providers are supported from which login providers are enabled for a particular deployment (can easily be checked using a config flag in the validation method).

What do you think?

@RichDom2185 RichDom2185 marked this pull request as draft July 21, 2023 15:45
@RichDom2185
Copy link
Member Author

Waiting to fix error: User creation in DB somehow calls the .String() method of the enum, thus violating the integer type.

Prevents GORM for automatically generating the string representation.
@github-actions
Copy link

github-actions bot commented Jul 21, 2023

Coverage Status

coverage: 83.0% (+0.6%) from 82.353% when pulling 7cdfa56 on make-users-provider-agnostic into 4e87ba6 on main.

@RichDom2185 RichDom2185 marked this pull request as ready for review July 21, 2023 15:53
@RichDom2185 RichDom2185 enabled auto-merge (squash) July 21, 2023 16:03
@RichDom2185 RichDom2185 merged commit 07b900b into main Jul 22, 2023
@RichDom2185 RichDom2185 deleted the make-users-provider-agnostic branch July 22, 2023 02:09
Copy link
Contributor

@YaleChen299 YaleChen299 left a comment

Choose a reason for hiding this comment

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

LGTM

@YaleChen299
Copy link
Contributor

@YaleChen299 I chose for it to be done this way so that we can keep a separate notion of which login providers are supported from which login providers are enabled for a particular deployment (can easily be checked using a config flag in the validation method).

What do you think?

Okay! I think this looks good.

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.

3 participants