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

Added "github" context to users created during github sync #955

Merged
merged 1 commit into from
Sep 20, 2017

Conversation

begedin
Copy link
Contributor

@begedin begedin commented Sep 19, 2017

Closes #948

Issues.UserLinker and IssueComment.UserLinker ended up needing the exact same change and are quite similar to begin with. Instead of trying to force this into a Github.Event.Common module, I figured this is a good opportunity to start a CodeCorps.Account bounded context, with its first function, Account.create_from_github/1

  • added Account bounded context
  • added Account.create_from_github/1
  • added AccountTest
  • added fixtures/github/endpoint/user.json

The context is simply "github" since the whole idea is that the user being created originates from github.

@joshsmith
Copy link
Contributor

The Phoenix docs on contexts seem to indicate that a pluralized form – Accounts – might make more sense here. How do you feel about this?

%User{}
|> Changeset.change(attrs |> Adapters.User.from_github_user())
|> Changeset.put_change(:context, "github")
|> Changeset.unique_constraint(:email)
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if the email is nil? GitHub is not guaranteed to return one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We already merged in a PR where a user can be inserted without an email, so we should be fine. Really, what's important for us here is the github id, so we can associate this "stub" with an actual user later, when they connect with github.

@begedin
Copy link
Contributor Author

begedin commented Sep 20, 2017

The Phoenix docs on contexts seem to indicate that a pluralized form – Accounts – might make more sense here. How do you feel about this?

I don't feel strongly about it one way or another. I can make the quick change

@begedin begedin force-pushed the 948-add-signup-context-to-github-user branch from f410ef9 to 5e6d0ea Compare September 20, 2017 05:41
joshsmith
joshsmith previously approved these changes Sep 20, 2017
- added Accounts bounded context
- added Accounts.create_from_github
- added AccountsTest
- added user.json github fixture
@begedin begedin merged commit 758f326 into develop Sep 20, 2017
@begedin begedin deleted the 948-add-signup-context-to-github-user branch September 20, 2017 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When creating a user from GitHub, set their signup context
2 participants