Merge pull request #1094 from code-corps/change-github-issue-and-pull-request-relationships

Rewrite our GitHub API event syncing infrastructure

Author: joshsmith

.github/CONTRIBUTING.md

@@ -193,3 +193,5 @@ We've written some convenience helper modules and functions to help with API tes
 - `CodeCorpsWeb.ApiCase` - used to simplify testing JSON API endpoints
 - `CodeCorps.AuthenticationTestHelpers` - provides authentication helpers for authenticating a `conn`
 - `CodeCorps.Factories` - provides factories using [`ex_machina`](https://github.com/thoughtbot/ex_machina), which makes it easy to create test data and associations with Ecto
+- `CodeCorps.GitHub.TestHelpers` - provides test helpers for loading GitHub API endpoint or event JSON fixtures, as well as mocking the API
+- `CodeCorps.GitHub.SuccessAPI` - provides some automatic mocking of the GitHub API, or raises an exception if you have an unhandled request to the API

lib/code_corps/github/api/api.ex

@@ -1,8 +1,10 @@
 defmodule CodeCorps.GitHub.API do
   alias CodeCorps.{
+    GithubAppInstallation,
     GitHub,
     GitHub.APIError,
-    GitHub.HTTPClientError
+    GitHub.HTTPClientError,
+    User
   }
 
   @spec request(GitHub.method, String.t, GitHub.headers, GitHub.body, list) :: GitHub.response
@@ -12,6 +14,37 @@ defmodule CodeCorps.GitHub.API do
     |> marshall_response()
   end
 
+  @doc """
+  Get access token headers for a given `CodeCorps.User` and
+  `CodeCorps.GithubAppInstallation`.
+
+  If the user does not have a `github_auth_token` (meaning they are not
+  connected to GitHub), then we default to the installation which will post on
+  behalf of the user as a bot.
+  """
+  @spec opts_for(User.t, GithubAppInstallation.t) :: list
+  def opts_for(%User{github_auth_token: nil}, %GithubAppInstallation{} = installation) do
+    opts_for(installation)
+  end
+  def opts_for(%User{github_auth_token: token}, %GithubAppInstallation{}) do
+    [access_token: token]
+  end
+
+  @doc """
+  Get access token headers for a given `CodeCorps.GithubAppInstallation`.
+
+  This should only be used in instances where the full permissions of the
+  application are needed and there is no need for attribution to a user.
+  """
+  @spec opts_for(GithubAppInstallation.t) :: list
+  def opts_for(%GithubAppInstallation{} = installation) do
+    with {:ok, token} <- installation |> GitHub.API.Installation.get_access_token do
+      [access_token: token]
+    else
+      {:error, github_error} -> {:error, github_error}
+    end
+  end
+
   @typep http_success :: {:ok, integer, [{String.t, String.t}], String.t}
   @typep http_failure :: {:error, term}
 

lib/code_corps/github/api/comment.ex

@@ -1,6 +1,6 @@
 defmodule CodeCorps.GitHub.API.Comment do
   @moduledoc ~S"""
-  Handles GitHub API requests for actions on Comments
+  Functions for working with comments on GitHub.
   """
 
   alias CodeCorps.{
@@ -14,6 +14,9 @@ defmodule CodeCorps.GitHub.API.Comment do
     User
   }
 
+  @doc """
+  Create a comment on GitHub's API for a `CodeCorps.Comment`.
+  """
   @spec create(Comment.t) :: GitHub.response
   def create(
     %Comment{
@@ -28,13 +31,16 @@ defmodule CodeCorps.GitHub.API.Comment do
     endpoint = comment |> create_endpoint_for()
     attrs = comment |> GitHub.Adapters.Comment.to_api
 
-    with opts when is_list(opts) <- opts_for(user, installation) do
+    with opts when is_list(opts) <- GitHub.API.opts_for(user, installation) do
       GitHub.request(:post, endpoint, %{}, attrs, opts)
     else
       {:error, github_error} -> {:error, github_error}
     end
   end
 
+  @doc """
+  Update a comment on GitHub's API for a `CodeCorps.Comment`.
+  """
   @spec update(Comment.t) :: GitHub.response
   def update(
     %Comment{
@@ -49,7 +55,7 @@ defmodule CodeCorps.GitHub.API.Comment do
     endpoint = comment |> update_endpoint_for()
     attrs = comment |> GitHub.Adapters.Comment.to_api
 
-    with opts when is_list(opts) <- opts_for(user, installation) do
+    with opts when is_list(opts) <- GitHub.API.opts_for(user, installation) do
       GitHub.request(:patch, endpoint, %{}, attrs, opts)
     else
       {:error, github_error} -> {:error, github_error}
@@ -83,16 +89,4 @@ defmodule CodeCorps.GitHub.API.Comment do
     }) do
     "/repos/#{owner}/#{repo}/issues/#{number}/comments"
   end
-
-  @spec opts_for(User.t, GithubAppInstallation.t) :: list
-  defp opts_for(%User{github_auth_token: nil}, %GithubAppInstallation{} = installation) do
-    with {:ok, token} <- installation |> GitHub.API.Installation.get_access_token do
-      [access_token: token]
-    else
-      {:error, github_error} -> {:error, github_error}
-    end
-  end
-  defp opts_for(%User{github_auth_token: token}, %GithubAppInstallation{}) do
-    [access_token: token]
-  end
 end

lib/code_corps/github/api/installation.ex

@@ -1,6 +1,6 @@
 defmodule CodeCorps.GitHub.API.Installation do
   @moduledoc """
-  Used to perform installation actions on the GitHub API.
+  Functions for performing installation actions on the GitHub API.
   """
 
   alias CodeCorps.{

lib/code_corps/github/api/issue.ex

@@ -3,8 +3,32 @@ defmodule CodeCorps.GitHub.API.Issue do
   Functions for working with issues on GitHub.
   """
 
-  alias CodeCorps.{GitHub, GithubAppInstallation, GithubIssue, GithubRepo, Task, User}
+  alias CodeCorps.{
+    GitHub,
+    GithubAppInstallation,
+    GithubIssue,
+    GithubRepo,
+    Task,
+    User
+  }
 
+  @doc """
+  Fetches an issue from the GitHub API, given the API URL for the issue and the
+  `CodeCorps.GithubRepo` record that points to its GitHub repository.
+  """
+  def from_url(url, %GithubRepo{github_app_installation: %GithubAppInstallation{} = installation}) do
+    "https://api.github.com/" <> endpoint = url
+
+    with opts when is_list(opts) <- GitHub.API.opts_for(installation) do
+      GitHub.request(:get, endpoint, %{}, %{}, opts)
+    else
+      {:error, github_error} -> {:error, github_error}
+    end
+  end
+
+  @doc """
+  Create an issue on GitHub's API for a `CodeCorps.Task`.
+  """
   @spec create(Task.t) :: GitHub.response
   def create(%Task{
     github_repo: %GithubRepo{
@@ -16,13 +40,16 @@ defmodule CodeCorps.GitHub.API.Issue do
     endpoint = github_repo |> get_endpoint()
     attrs = task |> GitHub.Adapters.Issue.to_api
 
-    with opts when is_list(opts) <- opts_for(user, installation) do
+    with opts when is_list(opts) <- GitHub.API.opts_for(user, installation) do
       GitHub.request(:post, endpoint, %{}, attrs, opts)
     else
       {:error, github_error} -> {:error, github_error}
     end
   end
 
+  @doc """
+  Update an issue on GitHub's API for a `CodeCorps.Task`.
+  """
   @spec update(Task.t) :: GitHub.response
   def update(%Task{
     github_issue: %GithubIssue{number: number},
@@ -35,7 +62,7 @@ defmodule CodeCorps.GitHub.API.Issue do
     endpoint = "#{github_repo |> get_endpoint()}/#{number}"
     attrs = task |> GitHub.Adapters.Issue.to_api
 
-    with opts when is_list(opts) <- opts_for(user, installation) do
+    with opts when is_list(opts) <- GitHub.API.opts_for(user, installation) do
       GitHub.request(:patch, endpoint, %{}, attrs, opts)
     else
       {:error, github_error} -> {:error, github_error}
@@ -46,16 +73,4 @@ defmodule CodeCorps.GitHub.API.Issue do
   defp get_endpoint(%GithubRepo{github_account_login: owner, name: repo}) do
     "/repos/#{owner}/#{repo}/issues"
   end
-
-  @spec opts_for(User.t, GithubAppInstallation.t) :: list
-  defp opts_for(%User{github_auth_token: nil}, %GithubAppInstallation{} = installation) do
-    with {:ok, token} <- installation |> GitHub.API.Installation.get_access_token do
-      [access_token: token]
-    else
-      {:error, github_error} -> {:error, github_error}
-    end
-  end
-  defp opts_for(%User{github_auth_token: token}, %GithubAppInstallation{}) do
-    [access_token: token]
-  end
 end

lib/code_corps/github/api/pull_request.ex

@@ -0,0 +1,26 @@
+defmodule CodeCorps.GitHub.API.PullRequest do
+  @moduledoc ~S"""
+  Functions for working with pull requests on GitHub.
+  """
+
+  alias CodeCorps.{
+    GitHub,
+    GithubAppInstallation,
+    GithubRepo
+  }
+
+  @doc """
+  Fetches a pull request from the GitHub API, given the API URL for the pull
+  request and the `CodeCorps.GithubRepo` record that points to its GitHub
+  repository.
+  """
+  def from_url(url, %GithubRepo{github_app_installation: %GithubAppInstallation{} = installation}) do
+    "https://api.github.com/" <> endpoint = url
+
+    with opts when is_list(opts) <- GitHub.API.opts_for(installation) do
+      GitHub.request(:get, endpoint, %{}, %{}, opts)
+    else
+      {:error, github_error} -> {:error, github_error}
+    end
+  end
+end

lib/code_corps/github/event/issue_comment/comment_deleter.ex

@@ -12,7 +12,7 @@ defmodule CodeCorps.GitHub.Event.IssueComment.CommentDeleter do
   the specified `IssueComment`.
   """
   @spec delete_all(map) :: {:ok, list(Comment.t)}
-  def delete_all(%{"comment" => %{"id" => github_id}}) do
+  def delete_all(%{"id" => github_id}) do
     query =
       from c in Comment,
         join: gc in GithubComment, on: gc.id == c.github_comment_id, where: gc.github_id == ^github_id

lib/code_corps/github/event/issue_comment/issue_comment.ex

@@ -10,9 +10,9 @@ defmodule CodeCorps.GitHub.Event.IssueComment do
     GitHub,
     GitHub.Event.IssueComment.Validator
   }
-  alias GitHub.Sync.Comment, as: CommentSyncer
+  alias GitHub.Sync
 
-  @type outcome :: CommentSyncer.outcome
+  @type outcome :: Sync.outcome
                  | {:error, :unexpected_action}
                  | {:error, :unexpected_payload}
 
@@ -29,7 +29,7 @@ defmodule CodeCorps.GitHub.Event.IssueComment do
   def handle(payload) do
     with {:ok, :valid} <- validate_payload(payload),
          {:ok, :implemented} <- validate_action(payload) do
-      CommentSyncer.sync(payload)
+      Sync.issue_comment_event(payload)
     else
       {:error, error} -> {:error, error}
     end

lib/code_corps/github/event/issues/issues.ex

@@ -11,9 +11,9 @@ defmodule CodeCorps.GitHub.Event.Issues do
     GitHub,
     GitHub.Event.Issues.Validator
   }
-  alias GitHub.Sync.Issue, as: IssueSyncer
+  alias GitHub.Sync
 
-  @type outcome :: IssueSyncer.outcome
+  @type outcome :: Sync.outcome
                  | {:error, :unexpected_action}
                  | {:error, :not_fully_implemented}
                  | {:error, :unexpected_payload}
@@ -31,7 +31,7 @@ defmodule CodeCorps.GitHub.Event.Issues do
   def handle(payload) do
     with {:ok, :valid} <- validate_payload(payload),
          {:ok, :implemented} <- validate_action(payload) do
-      IssueSyncer.sync(payload)
+      Sync.issue_event(payload)
     else
       {:error, error} -> {:error, error}
     end

lib/code_corps/github/event/pull_request/pull_request.ex

@@ -11,9 +11,9 @@ defmodule CodeCorps.GitHub.Event.PullRequest do
     GitHub,
     GitHub.Event.PullRequest.Validator
   }
-  alias GitHub.Sync.PullRequest, as: PullRequestSyncer
+  alias GitHub.Sync
 
-  @type outcome :: PullRequestSyncer.outcome
+  @type outcome :: Sync.outcome
                  | {:error, :unexpected_action}
                  | {:error, :not_fully_implemented}
                  | {:error, :unexpected_payload}
@@ -31,7 +31,7 @@ defmodule CodeCorps.GitHub.Event.PullRequest do
   def handle(payload) do
     with {:ok, :valid} <- validate_payload(payload),
          {:ok, :implemented} <- validate_action(payload) do
-      PullRequestSyncer.sync(payload)
+      Sync.pull_request_event(payload)
     else
       {:error, error} -> {:error, error}
     end