Refactor webhook handler to take only the payload

Author: joshsmith

lib/code_corps/github/event/handler.ex

@@ -3,14 +3,12 @@ defmodule CodeCorps.GitHub.Event.Handler do
   Default behavior for all GitHub webhook event handlers.
   """
 
-  alias CodeCorps.GithubEvent
-
   @doc ~S"""
   The only entry point a GitHub webhook event handler function should contain.
 
-  Receives a `CodeCorps.GithubEvent` record and a payload, returns an `:ok`
-  tuple if the process was successful, or an `:error` tuple, where the second
-  element is an atom, if it failed.
+  Receives the GitHub payload, returns an `:ok` tuple if the process was
+  successful, or an `:error` tuple, where the second element is an atom, if it
+  failed.
   """
-  @callback handle(GithubEvent.t, map) :: {:ok, any} | {:error, atom}
+  @callback handle(map) :: {:ok, any} | {:error, atom}
 end

lib/code_corps/github/event/installation.ex

@@ -8,7 +8,6 @@ defmodule CodeCorps.GitHub.Event.Installation do
 
   alias CodeCorps.{
     GithubAppInstallation,
-    GithubEvent,
     GitHub.Event.Installation,
     Repo,
     User
@@ -48,8 +47,8 @@ defmodule CodeCorps.GitHub.Event.Installation do
   If a step in the process failes, an `:error` tuple will be returned, where the
   second element is an atom indicating which step of the process failed.
   """
-  @spec handle(GithubEvent.t, map) :: outcome
-  def handle(%GithubEvent{}, payload) do
+  @spec handle(map) :: outcome
+  def handle(payload) do
     Multi.new
     |> Multi.run(:payload, fn _ -> payload |> validate_payload() end)
     |> Multi.run(:action, fn _ -> payload |> validate_action() end)

lib/code_corps/github/event/installation_repositories.ex

@@ -10,7 +10,6 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositories do
 
   alias CodeCorps.{
     GithubAppInstallation,
-    GithubEvent,
     GithubRepo,
     GitHub.Event.Common.ResultAggregator,
     GitHub.Event.InstallationRepositories,
@@ -43,8 +42,8 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositories do
         `CodeCorps.ProjectGithubRepo` records, they are deleted automatically,
         due to `on_delete: :delete_all` set at the database level.
   """
-  @spec handle(GithubEvent.t, map) :: outcome
-  def handle(%GithubEvent{}, payload) do
+  @spec handle(map) :: outcome
+  def handle(payload) do
     Multi.new
     |> Multi.run(:payload, fn _ -> payload |> validate_payload() end)
     |> Multi.run(:action, fn _ -> payload |> validate_action() end)

lib/code_corps/github/event/issue_comment.ex

@@ -8,7 +8,6 @@ defmodule CodeCorps.GitHub.Event.IssueComment do
 
   alias CodeCorps.{
     Comment,
-    GithubEvent,
     GitHub.Event.Common.RepoFinder,
     GitHub.Event.Issues,
     GitHub.Event.Issues.TaskSyncer,
@@ -50,8 +49,8 @@ defmodule CodeCorps.GitHub.Event.IssueComment do
   If it fails, it will instead return an `:error` tuple, where the second
   element is the atom indicating a reason.
   """
-  @spec handle(GithubEvent.t, map) :: outcome
-  def handle(%GithubEvent{}, payload) do
+  @spec handle(map) :: outcome
+  def handle(payload) do
     Multi.new
     |> Multi.run(:payload, fn _ -> payload |> validate_payload() end)
     |> Multi.run(:action, fn _ -> payload |> validate_action() end)

lib/code_corps/github/event/issues.ex

@@ -8,7 +8,6 @@ defmodule CodeCorps.GitHub.Event.Issues do
   @behaviour CodeCorps.GitHub.Event.Handler
 
   alias CodeCorps.{
-    GithubEvent,
     GitHub.Event.Common.RepoFinder,
     GitHub.Event.Issues.TaskSyncer,
     GitHub.Event.Issues.UserLinker,
@@ -45,8 +44,8 @@ defmodule CodeCorps.GitHub.Event.Issues do
   If it fails, it will instead return an `:error` tuple, where the second
   element is the atom indicating a reason.
   """
-  @spec handle(GithubEvent.t, map) :: outcome
-  def handle(%GithubEvent{}, %{} = payload) do
+  @spec handle(map) :: outcome
+  def handle(payload) do
     Multi.new
     |> Multi.run(:payload, fn _ -> payload |> validate_payload() end)
     |> Multi.run(:action, fn _ -> payload |> validate_action() end)

lib/code_corps/github/webhook/handler.ex

@@ -20,9 +20,9 @@ defmodule CodeCorps.GitHub.Webhook.Handler do
   def handle(type, id, payload) do
     with %{} = params <- build_params(type, id, payload),
          {:ok, %GithubEvent{} = event} <- params |> create_event(),
-         {:ok, %GithubEvent{status: "processing"} = processing_event} <- event |> Event.start_processing
+         {:ok, %GithubEvent{status: "processing"} = event} <- event |> Event.start_processing
     do
-      processing_event |> do_handle(payload) |> Event.stop_processing(event)
+      payload |> do_handle(type) |> Event.stop_processing(event)
     end
   end
 
@@ -47,8 +47,8 @@ defmodule CodeCorps.GitHub.Webhook.Handler do
     end
   end
 
-  defp do_handle(%GithubEvent{type: "installation"} = event, payload), do: Installation.handle(event, payload)
-  defp do_handle(%GithubEvent{type: "installation_repositories"} = event, payload), do: InstallationRepositories.handle(event, payload)
-  defp do_handle(%GithubEvent{type: "issue_comment"} = event, payload), do: IssueComment.handle(event, payload)
-  defp do_handle(%GithubEvent{type: "issues"} = event, payload), do: Issues.handle(event, payload)
+  defp do_handle(payload, "installation"), do: Installation.handle(payload)
+  defp do_handle(payload, "installation_repositories"), do: InstallationRepositories.handle(payload)
+  defp do_handle(payload, "issue_comment"), do: IssueComment.handle(payload)
+  defp do_handle(payload, "issues"), do: Issues.handle(payload)
 end

test/lib/code_corps/github/event/installation_repositories_test.exs

@@ -12,24 +12,22 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositoriesTest do
     Repo
   }
 
-  describe "handle/2" do
+  describe "handle/1" do
     @payload load_event_fixture("installation_repositories_added")
 
     test "marks event as errored if invalid action" do
       payload = @payload |> Map.put("action", "foo")
-      event = build(:github_event, action: "foo", type: "installation_repositories")
-      assert {:error, :unexpected_action} == InstallationRepositories.handle(event, payload)
+      assert {:error, :unexpected_action} == InstallationRepositories.handle(payload)
     end
 
     test "marks event as errored if invalid payload" do
-      event = build(:github_event, action: "added", type: "installation_repositories")
       payload = @payload |> Map.delete("action")
       assert {:error, :unexpected_payload} ==
-        InstallationRepositories.handle(event, payload)
+        InstallationRepositories.handle(payload)
     end
   end
 
-  describe "handle/2 for InstallationRepositories::added" do
+  describe "handle/1 for InstallationRepositories::added" do
     @payload load_event_fixture("installation_repositories_added")
 
     test "creates repos" do
@@ -40,8 +38,7 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositoriesTest do
 
       %{id: installation_id} = insert(:github_app_installation, github_id: installation_github_id)
 
-      event = build(:github_event, action: "added", type: "installation_repositories")
-      {:ok, [%GithubRepo{}, %GithubRepo{}]} = InstallationRepositories.handle(event, @payload)
+      {:ok, [%GithubRepo{}, %GithubRepo{}]} = InstallationRepositories.handle(@payload)
 
       github_repo_1 = Repo.get_by(GithubRepo, github_id: repo_1_payload["id"])
       assert github_repo_1
@@ -63,8 +60,7 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositoriesTest do
       installation = insert(:github_app_installation, github_id: installation_github_id)
       preinserted_repo = insert(:github_repo, github_app_installation: installation, github_id: repo_1_payload["id"])
 
-      event = build(:github_event, action: "added", type: "installation_repositories")
-      {:ok, [%GithubRepo{}, %GithubRepo{}]} = InstallationRepositories.handle(event, @payload)
+      {:ok, [%GithubRepo{}, %GithubRepo{}]} = InstallationRepositories.handle(@payload)
 
       github_repo_1 = Repo.get_by(GithubRepo, github_id: repo_1_payload["id"])
       assert github_repo_1.id == preinserted_repo.id
@@ -78,23 +74,20 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositoriesTest do
     end
 
     test "marks event as errored if invalid instalation payload" do
-      event = insert(:github_event, action: "added", type: "installation_repositories")
-      assert {:error, :unexpected_payload} == InstallationRepositories.handle(event, @payload |> Map.put("installation", "foo"))
+      assert {:error, :unexpected_payload} == InstallationRepositories.handle(@payload |> Map.put("installation", "foo"))
     end
 
     test "marks event as errored if invalid repo payload" do
-      event = insert(:github_event, action: "added", type: "installation_repositories")
       insert(:github_app_installation, github_id: @payload["installation"]["id"])
-      assert {:error, :unexpected_payload} == InstallationRepositories.handle(event, @payload |> Map.put("repositories_added", ["foo"]))
+      assert {:error, :unexpected_payload} == InstallationRepositories.handle(@payload |> Map.put("repositories_added", ["foo"]))
     end
 
     test "marks event as errored if no installation" do
-      event = insert(:github_event, action: "added", type: "installation_repositories")
-      assert {:error, :unmatched_installation} == InstallationRepositories.handle(event, @payload)
+      assert {:error, :unmatched_installation} == InstallationRepositories.handle(@payload)
     end
   end
 
-  describe "handle/2 for InstallationRepositories::removed" do
+  describe "handle/1 for InstallationRepositories::removed" do
     @payload load_event_fixture("installation_repositories_removed")
 
     test "deletes github repos and associated project github repos" do
@@ -108,8 +101,7 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositoriesTest do
       insert(:project_github_repo, project: project, github_repo: github_repo_1)
       insert(:github_repo, github_app_installation: installation, github_id: repo_2_payload["id"])
 
-      event = build(:github_event, action: "removed", type: "installation_repositories")
-      {:ok, [%GithubRepo{}, %GithubRepo{}]} = InstallationRepositories.handle(event, @payload)
+      {:ok, [%GithubRepo{}, %GithubRepo{}]} = InstallationRepositories.handle(@payload)
 
       assert Repo.aggregate(GithubRepo, :count, :id) == 0
       assert Repo.aggregate(ProjectGithubRepo, :count, :id) == 0
@@ -125,31 +117,27 @@ defmodule CodeCorps.GitHub.Event.InstallationRepositoriesTest do
       github_repo_1 = insert(:github_repo, github_app_installation: installation, github_id: repo_1_payload["id"])
       insert(:project_github_repo, project: project, github_repo: github_repo_1)
 
-      event = build(:github_event, action: "removed", type: "installation_repositories")
-      {:ok, [%GithubRepo{}]} = InstallationRepositories.handle(event, @payload)
+      {:ok, [%GithubRepo{}]} = InstallationRepositories.handle(@payload)
 
       assert Repo.aggregate(GithubRepo, :count, :id) == 0
       assert Repo.aggregate(ProjectGithubRepo, :count, :id) == 0
     end
 
     test "marks event as errored if invalid instalation payload" do
-      event = build(:github_event, action: "removed", type: "installation_repositories")
       payload = @payload |> Map.put("installation", "foo")
       assert {:error, :unexpected_payload} ==
-        InstallationRepositories.handle(event, payload)
+        InstallationRepositories.handle(payload)
     end
 
     test "marks event as errored if invalid repo payload" do
-      event = build(:github_event, action: "removed", type: "installation_repositories")
       insert(:github_app_installation, github_id: @payload["installation"]["id"])
       payload = @payload |> Map.put("repositories_removed", ["foo"])
       assert {:error, :unexpected_payload} ==
-        InstallationRepositories.handle(event, payload)
+        InstallationRepositories.handle(payload)
     end
 
     test "marks event as errored if no installation" do
-      event = build(:github_event, action: "added", type: "installation_repositories")
-      assert {:error, :unmatched_installation} == InstallationRepositories.handle(event, @payload)
+      assert {:error, :unmatched_installation} == InstallationRepositories.handle(@payload)
     end
   end
 end

test/lib/code_corps/github/event/installation_test.exs

@@ -44,56 +44,47 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do
   @bad_sender_payload @installation_created |> Map.put("sender", "foo")
   @bad_installation_payload @installation_created |> Map.put("installation", "foo")
 
-  describe "handle/2" do
+  describe "handle/1" do
     test "returns error if action of the event is wrong" do
-      event = build(:github_event, action: "foo", type: "installation")
       assert {:error, :unexpected_action} ==
-        Installation.handle(event, @bad_action_payload)
+        Installation.handle(@bad_action_payload)
     end
 
     test "returns error if payload is wrong" do
-      event = build(:github_event, action: "created", type: "installation")
-      assert {:error, :unexpected_payload} == Installation.handle(event, %{})
+      assert {:error, :unexpected_payload} == Installation.handle(%{})
     end
 
     test "returns error if user payload is wrong" do
-      event = build(:github_event, action: "created", type: "installation")
       assert {:error, :unexpected_payload} ==
-        Installation.handle(event, @bad_sender_payload)
+        Installation.handle(@bad_sender_payload)
     end
 
     test "returns error if installation payload is wrong" do
-      event = build(:github_event, action: "created", type: "installation")
       assert {:error, :unexpected_payload} ==
-        Installation.handle(event, @bad_installation_payload)
+        Installation.handle(@bad_installation_payload)
     end
 
     test "returns installation as errored if api error" do
-      event = build(:github_event, action: "created", type: "installation")
-
       with_mock_api(BadRepoRequest) do
         assert {:error, :github_api_error_on_syncing_repos}
-          = Installation.handle(event, @installation_created)
+          = Installation.handle(@installation_created)
       end
     end
 
     test "returns installation as errored if error creating repos" do
-      event = build(:github_event, action: "created", type: "installation")
-
       with_mock_api(InvalidRepoRequest) do
         assert {:error, :validation_error_on_syncing_existing_repos} ==
-          Installation.handle(event, @installation_created)
+          Installation.handle(@installation_created)
       end
     end
   end
 
-  describe "handle/2 for Installation::created" do
+  describe "handle/1 for Installation::created" do
     test "creates installation for unmatched user if no user, syncs repos" do
       payload = %{"installation" => %{"id" => installation_github_id}} = @installation_created
-      event = build(:github_event, action: "created", type: "installation")
 
       {:ok, %GithubAppInstallation{} = installation}
-        = Installation.handle(event, payload)
+        = Installation.handle(payload)
 
       assert installation.github_id == installation_github_id
       assert installation.origin == "github"
@@ -105,12 +96,11 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do
 
     test "creates installation if user matched but installation unmatched, syncs repos" do
       %{"sender" => %{"id" => user_github_id}} = payload = @installation_created
-      event = build(:github_event, action: "created", type: "installation")
 
       user = insert(:user, github_id: user_github_id)
 
       {:ok, %GithubAppInstallation{} = installation}
-        = Installation.handle(event, payload)
+        = Installation.handle(payload)
 
       assert installation.github_id == (payload |> get_in(["installation", "id"]))
       assert installation.origin == "github"
@@ -123,7 +113,6 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do
 
     test "updates installation, if both user and installation matched, syncs repos" do
       %{"sender" => %{"id" => user_github_id}, "installation" => %{"id" => installation_github_id}} = payload = @installation_created
-      event = build(:github_event, action: "created", type: "installation")
 
       user = insert(:user, github_id: user_github_id)
       insert(
@@ -134,7 +123,7 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do
       )
 
       {:ok, %GithubAppInstallation{} = installation}
-        = Installation.handle(event, payload)
+        = Installation.handle(payload)
 
       assert installation.origin == "codecorps"
       assert installation.state == "processed"
@@ -148,10 +137,9 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do
     test "updates installation if there is an installation, but no user, syncs repos" do
       %{"installation" => %{"id" => installation_github_id}, "sender" => %{"id" => sender_github_id}} = payload = @installation_created
       insert(:github_app_installation, github_id: installation_github_id)
-      event = build(:github_event, action: "created", type: "installation")
 
       {:ok, %GithubAppInstallation{} = installation}
-        = Installation.handle(event, payload)
+        = Installation.handle(payload)
 
       assert installation.origin == "codecorps"
       assert installation.state == "processed"