Implement handling of "InstallationRepositories" "added" and "removed…

…" events

lib/code_corps/github/event.ex

@@ -0,0 +1,36 @@
+defmodule CodeCorps.GitHub.Event do
+  @moduledoc ~S"""
+  In charge of marking `GithubEvent` records as "processing", "processed" or
+  "errored", based on the outcome of processing a webhook event payload.
+  """
+
+  alias CodeCorps.{GithubEvent, Repo}
+  alias Ecto.Changeset
+
+  @type error :: atom | Changeset.t
+  @type processing_result :: {:ok, any} | {:error, error}
+
+  @doc ~S"""
+  Sets record status to "processing", marking it as being processed at this
+  moment. Our webhook handling should skip processing payloads for events which
+  are already being processed.
+  """
+  @spec start_processing(GithubEvent.t) :: {:ok, GithubEvent.t}
+  def start_processing(%GithubEvent{} = event) do
+    event |> Changeset.change(%{status: "processing"}) |> Repo.update()
+  end
+
+  @doc ~S"""
+  Sets record status to "processed" or "errored" based on the first element of
+  first argument, which is the result tuple. The result tuple should always be
+  either `{:ok, data}` if the the processing of the event payload went as
+  expected, or `{:error, reason}` if something went wrong.
+  """
+  @spec stop_processing(processing_result, GithubEvent.t) :: {:ok, GithubEvent.t}
+  def stop_processing({:ok, _data}, %GithubEvent{} = event) do
+    event |> Changeset.change(%{status: "processed"}) |> Repo.update
+  end
+  def stop_processing({:error, _reason}, %GithubEvent{} = event) do
+    event |> Changeset.change(%{status: "errored"}) |> Repo.update
+  end
+end

lib/code_corps/github/events/installation.ex → lib/code_corps/github/event/installation.ex

@@ -1,4 +1,4 @@
-defmodule CodeCorps.GitHub.Events.Installation do
+defmodule CodeCorps.GitHub.Event.Installation do
   @moduledoc """
   In charge of dealing with "Installation" GitHub Webhook events
   """
@@ -8,6 +8,7 @@ defmodule CodeCorps.GitHub.Events.Installation do
     GithubAppInstallation,
     GithubEvent,
     GithubRepo,
+    GitHub.Event,
     Repo,
     User
   }
@@ -27,11 +28,12 @@ defmodule CodeCorps.GitHub.Events.Installation do
   @spec handle(GithubEvent.t, map) :: {:ok, GithubEvent.t}
   def handle(%GithubEvent{action: "created"} = event, payload) do
     event
-    |> start_event_processing()
+    |> Event.start_processing()
     |> do_handle(payload)
-    |> stop_event_processing(event)
+    |> Event.stop_processing(event)
   end
 
+  @spec do_handle({:ok, GithubEvent.t}, map) :: {:ok, GithubAppInstallation.t} | {:error, :api_error}
   defp do_handle({:ok, %GithubEvent{}}, %{"installation" => installation_attrs, "sender" => sender_attrs}) do
     case {sender_attrs |> find_user, installation_attrs |> find_installation} do
       {nil, nil} -> create_unmatched_user_installation(installation_attrs)
@@ -40,67 +42,63 @@ defmodule CodeCorps.GitHub.Events.Installation do
     end
   end
 
+  @spec create_installation_initiated_on_github(User.t, map) :: {:ok, GithubAppInstallation.t}
   defp create_installation_initiated_on_github(%User{} = user, %{"id" => github_id}) do
     %GithubAppInstallation{}
     |> Changeset.change(%{github_id: github_id, installed: true, state: "initiated_on_github"})
     |> Changeset.put_assoc(:user, user)
     |> Repo.insert()
   end
 
+  @spec create_matched_installation(GithubAppInstallation.t) :: {:ok, GithubAppInstallation.t}
   defp create_matched_installation(%GithubAppInstallation{} = installation) do
     installation
     |> start_installation_processing()
     |> process_repos()
     |> stop_installation_processing()
   end
 
+  @spec create_unmatched_user_installation(map) :: {:ok, GithubAppInstallation.t}
+  defp create_unmatched_user_installation(%{"id" => github_id}) do
+    %GithubAppInstallation{}
+    |> Changeset.change(%{github_id: github_id, installed: true, state: "unmatched_user"})
+    |> Repo.insert()
+  end
+
+  @spec create_repository(GithubAppInstallation.t, map) :: {:ok, GithubRepo.t}
   defp create_repository(%GithubAppInstallation{} = installation, repo_attributes) do
     %GithubRepo{}
     |> Changeset.change(repo_attributes |> GithubRepoAdapter.from_api)
     |> Changeset.put_assoc(:github_app_installation, installation)
     |> Repo.insert()
   end
 
-  defp create_unmatched_user_installation(%{"id" => github_id}) do
-    %GithubAppInstallation{}
-    |> Changeset.change(%{github_id: github_id, installed: true, state: "unmatched_user"})
-    |> Repo.insert()
-  end
-
+  @spec find_installation(map) :: GithubAppInstallation.t | nil
   defp find_installation(%{"id" => github_id}), do: GithubAppInstallation |> Repo.get_by(github_id: github_id)
 
+  @spec find_user(map) :: User.t | nil
   defp find_user(%{"id" => github_id}), do: User |> Repo.get_by(github_id: github_id)
 
+  @spec process_repos({:ok, GithubAppInstallation.t}) :: {:ok, GithubAppInstallation.t} | {:error, :api_error}
   defp process_repos({:ok, %GithubAppInstallation{} = installation}) do
-    # TODO: Consider moving into transaction
-    {:ok, repositories} =
-      installation
-      |> GitHub.Installation.repositories()
-      |> (fn {:ok, repositories} -> repositories end).()
-      |> Enum.map(&create_repository(installation, &1))
-      |> Enum.map(fn {:ok, repository} -> repository end)
-      |> (fn repositories -> {:ok, repositories} end).()
-
-    {:ok, installation |> Map.put(:github_repos, repositories)}
-  end
+    with {:ok, repo_payloads} <- installation |> GitHub.Installation.repositories() do
+      repositories =
+        repo_payloads
+        |> Enum.map(&create_repository(installation, &1))
+        |> Enum.map(fn {:ok, repository} -> repository end)
 
-  defp start_event_processing(%GithubEvent{} = event) do
-    event |> Changeset.change(%{status: "processing"}) |> Repo.update()
-  end
-
-  defp stop_event_processing({:ok, %GithubAppInstallation{}}, %GithubEvent{} = event) do
-    event |> Changeset.change(%{status: "processed"}) |> Repo.update
-  end
-  defp stop_event_processing(_, %GithubEvent{} = event) do
-    event |> Changeset.change(%{status: "errored"}) |> Repo.update
+        {:ok, installation |> Map.put(:github_repos, repositories)}
+    end
   end
 
+  @spec start_installation_processing({:ok, GithubAppInstallation.t}) :: {:ok, GithubAppInstallation.t}
   defp start_installation_processing(%GithubAppInstallation{} = installation) do
     installation
     |> Changeset.change(%{installed: true, state: "processing"})
     |> Repo.update()
   end
 
+  @spec stop_installation_processing({:ok, GithubAppInstallation.t}) :: {:ok, GithubAppInstallation.t}
   defp stop_installation_processing({:ok, %GithubAppInstallation{} = installation}) do
     installation
     |> Changeset.change(%{state: "processed"})

lib/code_corps/github/event/installation_repositories.ex

@@ -0,0 +1,133 @@
+defmodule CodeCorps.GitHub.Event.InstallationRepositories do
+  @moduledoc """
+  In charge of dealing with "InstallationRepositories" GitHub Webhook events
+
+  TODO: This module is not fully implemented. There are edge cases and other
+  bits to be handled:
+    - The payload contains a "sender" map, which can be used to identify the
+      sender. I'm not sure what to do with that.
+    - In case of the "added" event, if the `GithubRepo` was not found, we create
+      it. However, the payload only contains some of the data we store locally,
+      so we should fetch the rest of the data from the API and fill it out.
+    - Handling both the `added` and `removed` events can result in some edge
+      cases not handled at the moment
+      - for "added", the {:error, :no_installation} outcome means there we do
+        not have the associated `GithubAppInstallation` record locally
+      - for "removed", the {:error, :no_installation} outcome means there we do
+        not have the associated `GithubAppInstallation` record locally
+      - for "removed", `{:error, :no_repo}` means the `GithubRepo` for which we
+        are supposed to remove the `ProjectGithubRepo` for does not exist
+        locally, so there is nothing to remove.
+      - for "removed", `{:error, :no_project_github_repo}` means both the
+        `GithubAppInstallation` and the `GithubRepo` were found, but the
+        `ProjectGithubRepo` we're supposed to remove was not found.
+   """
+
+  alias CodeCorps.{
+    GithubAppInstallation,
+    GithubEvent,
+    GithubRepo,
+    GitHub.Event,
+    Project,
+    ProjectGithubRepo,
+    Repo
+  }
+
+  alias Ecto.Changeset
+
+
+  @doc """
+  Handles an "InstallationRepositories" GitHub Webhook event. The event could be
+  of subtype "added" or "removed" and is handled differently based on that.
+  """
+  @spec handle(GithubEvent.t, map) :: {:ok, GithubEvent.t}
+  def handle(%GithubEvent{action: "added"} = event, payload) do
+    event
+    |> Event.start_processing()
+    |> handle_added(payload)
+    |> Event.stop_processing(event)
+  end
+  def handle(%GithubEvent{action: "removed"} = event, payload) do
+    event
+    |> Event.start_processing()
+    |> handle_removed(payload)
+    |> Event.stop_processing(event)
+  end
+
+  @typep repo_added_outcome :: {:ok, ProjectGithubRepo.t} |
+                               {:error, :no_installation}
+
+  @spec handle_added({:ok, GithubEvent.t}, map) :: repo_added_outcome
+  defp handle_added({:ok, %GithubEvent{}}, %{} = %{"installation" => installation_attrs, "repositories_added" => [repo_attrs]}) do
+    case {installation_attrs |> find_installation(), repo_attrs |> find_repo()} do
+      {nil, _} ->
+        {:error, :no_installation}
+      {%GithubAppInstallation{project: %Project{} = project} = installation, nil} ->
+        with {:ok, %GithubRepo{} = github_repo} <- installation |> create_repo(repo_attrs) do
+          create_project_github_repo(project, github_repo)
+        end
+      {%GithubAppInstallation{project: %Project{} = project}, %GithubRepo{} = github_repo} ->
+        ensure_project_github_repo(project, github_repo)
+    end
+  end
+
+  @typep repo_removed_outcome :: {:ok, ProjectGithubRepo.t} |
+                                 {:error, :no_installation} |
+                                 {:error, :no_repo} |
+                                 {:error, :no_project_github_repo}
+
+  @spec handle_removed({:ok, GithubEvent.t}, map) :: repo_removed_outcome
+  defp handle_removed({:ok, %GithubEvent{}}, %{"installation" => installation_attrs, "repositories_removed" => [repo_attrs]}) do
+    case {installation_attrs |> find_installation(), repo_attrs |> find_repo()} do
+      {nil, _} ->
+        {:error, :no_installation}
+      {%GithubAppInstallation{project: %Project{}}, nil} ->
+        {:error, :no_repo}
+      {%GithubAppInstallation{project: %Project{} = project} = _installation, %GithubRepo{} = github_repo} ->
+        case project |> find_project_github_repo(github_repo) do
+          %ProjectGithubRepo{} = project_github_repo -> Repo.delete(project_github_repo)
+          nil -> {:error, :no_project_github_repo}
+        end
+    end
+  end
+
+  @spec find_installation(map) :: GithubAppInstallation.t | nil
+  defp find_installation(%{"id" => github_id}) do
+    GithubAppInstallation
+    |> Repo.get_by(github_id: github_id)
+    |> Repo.preload(:project)
+  end
+
+  @spec find_repo(map) :: GithubRepo.t | nil
+  defp find_repo(%{"id" => github_id}), do: GithubRepo |> Repo.get_by(github_id: github_id)
+
+  @spec find_project_github_repo(Project.t, GithubRepo.t) :: ProjectGithubRepo.t | nil
+  defp find_project_github_repo(%Project{id: project_id}, %GithubRepo{id: github_repo_id}) do
+    ProjectGithubRepo |> Repo.get_by(project_id: project_id, github_repo_id: github_repo_id)
+  end
+
+  @spec ensure_project_github_repo(Project.t, GithubRepo.t) :: {:ok, ProjectGithubRepo.t}
+  defp ensure_project_github_repo(%Project{} = project, %GithubRepo{} = github_repo) do
+    case project |> find_project_github_repo(github_repo) do
+      nil -> create_project_github_repo(project, github_repo)
+      %ProjectGithubRepo{} = project_github_repo -> {:ok, project_github_repo}
+    end
+  end
+
+  @spec create_repo(GithubAppInstallation.t, map) :: {:ok, GithubRepo.t}
+  defp create_repo(%GithubAppInstallation{} = installation, %{"name" => name, "id" => github_id}) do
+    %GithubRepo{}
+    |> Changeset.change(%{name: name, github_id: github_id})
+    |> Changeset.put_assoc(:github_app_installation, installation)
+    |> Repo.insert()
+  end
+
+  @spec create_project_github_repo(Project.t, GithubRepo.t) :: {:ok, ProjectGithubRepo.t}
+  defp create_project_github_repo(%Project{} = project, %GithubRepo{} = github_repo) do
+    %ProjectGithubRepo{}
+    |> Changeset.change(%{})
+    |> Changeset.put_assoc(:project, project)
+    |> Changeset.put_assoc(:github_repo, github_repo)
+    |> Repo.insert()
+  end
+end

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

@@ -1,4 +1,4 @@
-defmodule CodeCorps.GitHub.Events.IssueComment do
+defmodule CodeCorps.GitHub.Event.IssueComment do
   @moduledoc """
   In charge of dealing with "IssueComment" GitHub Webhook events
   """

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

@@ -1,4 +1,4 @@
-defmodule CodeCorps.GitHub.Events.Issues do
+defmodule CodeCorps.GitHub.Event.Issues do
   @moduledoc """
   In charge of dealing with "Issues" GitHub Webhook events
   """

lib/code_corps/github/webhook/handler.ex

@@ -5,10 +5,10 @@ defmodule CodeCorps.GitHub.Webhook.Handler do
 
   alias CodeCorps.{
     GithubEvent,
-    GitHub.Events.Installation,
-    GitHub.Events.InstallationRepositories,
-    GitHub.Events.IssueComment,
-    GitHub.Events.Issues,
+    GitHub.Event.Installation,
+    GitHub.Event.InstallationRepositories,
+    GitHub.Event.IssueComment,
+    GitHub.Event.Issues,
     GitHub.Webhook.EventSupport,
     Repo
   }

priv/repo/migrations/20170630140136_create_project_github_repo.exs

@@ -0,0 +1,14 @@
+defmodule CodeCorps.Repo.Migrations.CreateProjectGithubRepo do
+  use Ecto.Migration
+
+  def change do
+    create table(:project_github_repos) do
+      add :project_id, references(:projects, on_delete: :delete_all)
+      add :github_repo_id, references(:github_repos, on_delete: :delete_all)
+
+      timestamps()
+    end
+
+    create unique_index(:project_github_repos, [:project_id, :github_repo_id])
+  end
+end