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

Add CodeCorps.Github module with connect/2 function #789

Closed
5 tasks
begedin opened this issue Apr 24, 2017 · 7 comments
Closed
5 tasks

Add CodeCorps.Github module with connect/2 function #789

begedin opened this issue Apr 24, 2017 · 7 comments

Comments

@begedin
Copy link
Contributor

begedin commented Apr 24, 2017

Problem

#788 defines a UserController action which calls CodeCorps.Github.connect/2.

This function should do the following:

Handle the auth token part of the web application flow for github authentication.

The first step is to make a post request to github to get the token. There is the https://github.com/edgurgel/tentacat API wrapper for elixir, which we will very likely end up using.

However, this wrapper has no "friendly" endpoint to just generate a token using a code. Instead, we'd have to use a POST request created manually, something like:

post(@token_url, %{
  client_id: @client_id,
  client_secret: @client_secret,
  code: code,
  accept: :json
})

The code would be provided as a function argument, while all the other parameters are set in the environment.

This call will return an {:ok, response_struct} or an {:error, error_struct}. We should handle the response and have the private function making the call return the access token. We should also handle a generalized error response by returning a general {:error, error_struct}. We can expand this to handle specific errors at a later time, in separate tasks.

Retrieve a GitHub user

We can use the auth_token to generate a Tentakit.Client and use this client to retrieve user information. We need a github id for the user, in order to associate it with our own user account.

Associate GitHub user with current user

This is blocked until #791 is done. To work on the remainder of this PR, use a virtual github_id field on the User model and simply have the GitHub.associate function set that field on the user struct.

We associate the github user information provided in the previous step with the github user in this step.

Return the updated current user

The previous step should give us an updated CodeCorps.User struct freshly retrieved from the database. We should return this as {:ok, user} from the service.

Testing

Since this will be making web requests to the github API, we should mock the API. We can use dependency injection to do this, or if that seems to complicated, we can fall back to using VCR. Someone will be available to help with this once a partial PR is submitted. The core team is still figuring this thing out just as everyone else, so no need to be apprehensive about asking for help here, or anywhere else.

There are examples of us already using dependency injection in how we use stripity_stripe, for example.

Recap

  • service receives current_user, code
  • posts code to github
  • retrieves github user info from github
  • associates github user info with current user
  • returns {:ok, updated_current_user}
  • if there was an error with github, returns {:error, "Something went wrong."}
  • if there was an error with associating the current user, returns {:error, changeset}

Subtasks

  • write service
  • write test for correct output on successful scenario
  • write test for correct output if github fails on posting
  • write test for correct output if github fails on retrieving user
  • write test for correct output on db update failure

References

Partially blocked by #791, but can be worked on.

@begedin begedin added this to the GitHub authentication milestone Apr 24, 2017
@begedin begedin changed the title Add CodeCorps.Github module with connect function Add CodeCorps.Github module with connect/2 function Apr 24, 2017
@snewcomer snewcomer self-assigned this Apr 24, 2017
@snewcomer
Copy link
Contributor

@begedin is this process_response/process_response_body useful or are we just extracting the access token that comes back manually?

https://hexdocs.pm/tentacat/Tentacat.html#process_response_body/1

@begedin
Copy link
Contributor Author

begedin commented Apr 29, 2017

@snewcomer It looks like they may be useful. They'll transform the response into a map with string keys, so we can access the values that way.

@begedin
Copy link
Contributor Author

begedin commented May 2, 2017

@snewcomer We have the controller action now, so if you have a pending PR for this part, you can rebase off of develop.

@begedin
Copy link
Contributor Author

begedin commented May 9, 2017

@snewcomer I'm aware you're working on this, but would you mind creating a PR of what you have right now?

It would allow us to judge direction of the implementation and possibly work on related things more easily.

I almost started work on it myself due to not seeing a PR.

@snewcomer
Copy link
Contributor

Hey @begedin. I'm spending all day Thursday and Friday on OSS (gotta get something out by Wed). Is getting it up there on Thursday ok with you?

@begedin
Copy link
Contributor Author

begedin commented May 10, 2017

Any time is ok, really.

Just as a general rule, any time you have basically any work done, feel free to create a PR, no matter how incomplete it is. There's always some use to get out of it.

@begedin
Copy link
Contributor Author

begedin commented Sep 13, 2017

Closed by #807

@begedin begedin closed this as completed Sep 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants