Switched other handlers to using ResultAggregator

Added tests to improve coverage

Author: begedin

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

@@ -1,6 +1,7 @@
 defmodule CodeCorps.GitHub.Event.IssueComment.CommentSyncer do
   alias CodeCorps.{
     Comment,
+    GitHub.Event.Common.ResultAggregator,
     GitHub.Event.IssueComment.ChangesetBuilder,
     Task,
     User,
@@ -9,17 +10,19 @@ defmodule CodeCorps.GitHub.Event.IssueComment.CommentSyncer do
 
   alias Ecto.Changeset
 
+  @type outcome :: {:ok, list(Comment.t)} |
+                   {:error, {list(Comment.t), list(Changeset.t)}}
+
   @doc """
   When provided a list of `Task`s, a `User` and a GitHub API payload, for each
   `Comment` associated to those `Task`s it creates or updates a `Comment`
   associated to the specified `User`.
   """
-  @spec sync_all(list(Task.t), User.t, map) :: {:ok, list(Comment.t)}
+  @spec sync_all(list(Task.t), User.t, map) :: outcome
   def sync_all(tasks, %User{} = user, %{} = payload) do
     tasks
     |> Enum.map(&sync(&1, user, payload))
-    |> collect()
-    |> summarize()
+    |> ResultAggregator.aggregate
   end
 
   @spec sync(Task.t, User.t, map) :: {:ok, Comment.t} | {:error, Changeset.t}
@@ -41,18 +44,4 @@ defmodule CodeCorps.GitHub.Event.IssueComment.CommentSyncer do
   @spec commit(Changeset.t) :: {:ok, Comment.t} | {:error, Changeset.t}
   defp commit(%Changeset{data: %Comment{id: nil}} = changeset), do: changeset |> Repo.insert
   defp commit(%Changeset{} = changeset), do: changeset |> Repo.update
-
-  @spec collect(list, list, list) :: tuple
-  defp collect(results, comments \\ [], errors \\ [])
-  defp collect([{:ok, %Comment{} = comment} | tail], comments, errors) do
-    collect(tail, comments ++ [comment], errors)
-  end
-  defp collect([{:error, %Changeset{} = changeset} | tail], comments, errors) do
-    collect(tail, comments, errors ++ [changeset])
-  end
-  defp collect([], comments, errors), do: {comments, errors}
-
-  @spec summarize(tuple) :: tuple
-  defp summarize({comments, []}), do: {:ok, comments}
-  defp summarize({comments, errors}), do: {:error, {comments, errors}}
 end

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

@@ -1,6 +1,7 @@
 defmodule CodeCorps.GitHub.Event.Issues.TaskSyncer do
   alias CodeCorps.{
     GithubRepo,
+    GitHub.Event.Common.ResultAggregator,
     GitHub.Event.Issues.ChangesetBuilder,
     ProjectGithubRepo,
     Task,
@@ -10,6 +11,9 @@ defmodule CodeCorps.GitHub.Event.Issues.TaskSyncer do
 
   alias Ecto.Changeset
 
+  @type outcome :: {:ok, list(Task.t)} |
+                   {:error, {list(Task.t), list(Changeset.t)}}
+
   @doc """
   When provided a `GithubRepo`, a `User` and a GitHub API payload, for each
   `Project` associated to that `GithubRepo` via a `ProjectGithubRepo`, it
@@ -19,8 +23,7 @@ defmodule CodeCorps.GitHub.Event.Issues.TaskSyncer do
   def sync_all(%GithubRepo{project_github_repos: project_github_repos}, %User{} = user, %{} = payload) do
     project_github_repos
     |> Enum.map(&sync(&1, user, payload))
-    |> collect()
-    |> summarize()
+    |> ResultAggregator.aggregate
   end
 
   @spec sync(ProjectGithubRepo.t, User.t, map) :: {:ok, ProjectGithubRepo.t} | {:error, Changeset.t}
@@ -51,18 +54,4 @@ defmodule CodeCorps.GitHub.Event.Issues.TaskSyncer do
   @spec commit(Changeset.t) :: {:ok, Task.t} | {:error, Changeset.t}
   defp commit(%Changeset{data: %Task{id: nil}} = changeset), do: changeset |> Repo.insert
   defp commit(%Changeset{} = changeset), do: changeset |> Repo.update
-
-  @spec collect(list, list, list) :: tuple
-  defp collect(results, tasks \\ [], errors \\ [])
-  defp collect([{:ok, %Task{} = task} | tail], tasks, errors) do
-    collect(tail, tasks ++ [task], errors)
-  end
-  defp collect([{:error, %Changeset{} = changeset} | tail], tasks, errors) do
-    collect(tail, tasks, errors ++ [changeset])
-  end
-  defp collect([], tasks, errors), do: {tasks, errors}
-
-  @spec summarize(tuple) :: tuple
-  defp summarize({tasks, []}), do: {:ok, tasks}
-  defp summarize({tasks, errors}), do: {:error, {tasks, errors}}
 end

test/lib/code_corps/github/event/common/result_aggregator_test.exs

@@ -0,0 +1,57 @@
+defmodule CodeCorps.GitHub.Event.Common.ResultAggregatorTest do
+  use CodeCorps.DbAccessCase
+
+  alias CodeCorps.{
+    Comment,
+    Task,
+    GithubRepo,
+    GitHub.Event.Common.ResultAggregator
+  }
+
+  alias Ecto.Changeset
+
+
+  describe "aggregate/1" do
+    test "aggregates Task results correctly" do
+      record = %Task{}
+      good = {:ok, record}
+      changeset = %Changeset{}
+      bad = {:error, changeset}
+
+      assert [] |> ResultAggregator.aggregate == {:ok, []}
+      assert [good] |> ResultAggregator.aggregate == {:ok, [record]}
+      assert [good, good] |> ResultAggregator.aggregate == {:ok, [record, record]}
+      assert [good, bad] |> ResultAggregator.aggregate == {:error, {[record], [changeset]}}
+      assert [bad] |> ResultAggregator.aggregate == {:error, {[], [changeset]}}
+      assert [bad, bad] |> ResultAggregator.aggregate == {:error, {[], [changeset, changeset]}}
+    end
+
+    test "aggregates Comment results correctly" do
+      record = %Comment{}
+      good = {:ok, record}
+      changeset = %Changeset{}
+      bad = {:error, changeset}
+
+      assert [] |> ResultAggregator.aggregate == {:ok, []}
+      assert [good] |> ResultAggregator.aggregate == {:ok, [record]}
+      assert [good, good] |> ResultAggregator.aggregate == {:ok, [record, record]}
+      assert [good, bad] |> ResultAggregator.aggregate == {:error, {[record], [changeset]}}
+      assert [bad] |> ResultAggregator.aggregate == {:error, {[], [changeset]}}
+      assert [bad, bad] |> ResultAggregator.aggregate == {:error, {[], [changeset, changeset]}}
+    end
+
+    test "aggregates GithubRepo results correctly" do
+      record = %GithubRepo{}
+      good = {:ok, record}
+      changeset = %Changeset{}
+      bad = {:error, changeset}
+
+      assert [] |> ResultAggregator.aggregate == {:ok, []}
+      assert [good] |> ResultAggregator.aggregate == {:ok, [record]}
+      assert [good, good] |> ResultAggregator.aggregate == {:ok, [record, record]}
+      assert [good, bad] |> ResultAggregator.aggregate == {:error, {[record], [changeset]}}
+      assert [bad] |> ResultAggregator.aggregate == {:error, {[], [changeset]}}
+      assert [bad, bad] |> ResultAggregator.aggregate == {:error, {[], [changeset, changeset]}}
+    end
+  end
+end

test/lib/code_corps/github/event/installation/repos_test.exs

@@ -18,6 +18,23 @@ defmodule CodeCorps.GitHub.Event.Installation.ReposTest do
   @installation_repositories load_endpoint_fixture("installation_repositories")
   @app_github_id 2
 
+  defmodule InvalidRepoRequest do
+    def request(:get, "https://api.github.com/installation/repositories", _, _, _) do
+      good_payload = "installation_repositories" |> load_endpoint_fixture
+      %{"repositories" => [repo_1, repo_2]} = good_payload
+
+      bad_repo_1 = repo_1 |> Map.put("name", nil)
+
+      bad_payload =
+        good_payload |> Map.put("repositories", [bad_repo_1, repo_2])
+
+      {:ok, bad_payload}
+    end
+    def request(method, endpoint, headers, body, options) do
+      CodeCorps.GitHub.SuccessAPI.request(method, endpoint, headers, body, options)
+    end
+  end
+
   describe "process/1" do
     test "syncs repos by performing a diff using payload as master list" do
       installation = insert(:github_app_installation, github_id: @app_github_id, state: "initiated_on_code_corps")
@@ -48,5 +65,19 @@ defmodule CodeCorps.GitHub.Event.Installation.ReposTest do
       # ensure no other repos have been created
       assert GithubRepo |> Repo.aggregate(:count, :id) == 2
     end
+
+    test "returned multi fails if there are repo validation erorrs" do
+      installation = insert(:github_app_installation, github_id: @app_github_id, state: "initiated_on_code_corps")
+      with_mock_api(InvalidRepoRequest) do
+        {:error, :synced_repos, {repos, changesets}, _steps} =
+          installation
+          |> Repo.preload(:github_repos)
+          |> Repos.process()
+          |> Repo.transaction
+
+        assert repos |> Enum.count == 1
+        assert changesets |> Enum.count == 1
+      end
+    end
   end
 end

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

@@ -24,11 +24,15 @@ defmodule CodeCorps.GitHub.Event.InstallationTest do
 
   defmodule InvalidRepoRequest do
     def request(:get, "https://api.github.com/installation/repositories", _, _, _) do
-      payload =
-        "installation_repositories"
-        |> load_endpoint_fixture
-        |> Map.put("repositories", [%{}])
-      {:ok, payload}
+      good_payload = "installation_repositories" |> load_endpoint_fixture
+      %{"repositories" => [repo_1, repo_2]} = good_payload
+
+      bad_repo_1 = repo_1 |> Map.put("name", nil)
+
+      bad_payload =
+        good_payload |> Map.put("repositories", [bad_repo_1, repo_2])
+
+      {:ok, bad_payload}
     end
     def request(method, endpoint, headers, body, options) do
       CodeCorps.GitHub.SuccessAPI.request(method, endpoint, headers, body, options)