Test outcomes for IssueComment event on pull request.

Rewrite Sync.IssueComment

Author: begedin

lib/code_corps/github/sync/github_comment/github_comment.ex

@@ -17,27 +17,35 @@ defmodule CodeCorps.GitHub.Sync.GithubComment do
   }
   alias Ecto.Changeset
 
-  @type result :: {:ok, GithubComment.t} | {:error, Changeset.t}
+  @type result :: {:ok, GithubComment.t()} | {:error, Changeset.t()}
 
   @doc ~S"""
   Finds or creates a `CodeCorps.GithubComment` using the data in a GitHub
   IssueComment payload.
 
   The process is as follows:
 
-  - Search for the issue in our database with the payload data.
+  - Search for the comment in our database with the payload data.
     - If found, update it with payload data
     - If not found, create it from payload data
 
   `CodeCorps.GitHub.Adapters.Comment.to_github_comment/1` is used to adapt the
   payload data.
   """
   @spec create_or_update_comment(GithubIssue.t, map) :: result
-  def create_or_update_comment(%GithubIssue{} = github_issue, %{} = %{"id" => github_comment_id} = attrs) do
-    params = to_params(attrs, github_issue)
-    case Repo.get_by(GithubComment, github_id: github_comment_id) do
-      nil -> create_comment(params)
-      %GithubComment{} = github_comment -> github_comment |> update_comment(params)
+  def create_or_update_comment(%GithubIssue{} = github_issue, %{} = attrs) do
+    with {:ok, %GithubUser{} = github_user} <- Sync.GithubUser.create_or_update_github_user(attrs),
+         params <- attrs |> Adapters.Comment.to_github_comment()
+    do
+       case attrs |> find_comment() do
+         nil ->
+           params |> create_comment(github_issue |> find_repo(), github_issue, github_user)
+
+         %GithubComment{} = github_comment ->
+           github_comment |> update_comment(params)
+       end
+    else
+      {:error, error} -> {:error, error}
     end
   end
 
@@ -49,36 +57,53 @@ defmodule CodeCorps.GitHub.Sync.GithubComment do
   `issue_url` property of the payload.
   """
   @spec create_or_update_comment(GithubRepo.t, map) :: result
-  def create_or_update_comment(%GithubRepo{} = github_repo, %{"id" => _, "issue_url" => _} = attrs) do
+  def create_or_update_comment(%GithubRepo{} = github_repo, %{} = attrs) do
     with {:ok, %GithubUser{} = github_user} <- Sync.GithubUser.create_or_update_github_user(attrs),
-         {:ok, %GithubComment{} = github_comment} <- do_create_or_update_comment(github_repo, attrs, github_user) do
-      {:ok, github_comment}
+         params <- attrs |> Adapters.Comment.to_github_comment()
+    do
+      case attrs |> find_comment() do
+        nil ->
+          params
+          |> create_comment(github_repo, attrs |> find_issue(), github_user)
+
+        %GithubComment{} = github_comment ->
+          github_comment |> update_comment(params)
+      end
     else
       {:error, error} -> {:error, error}
     end
   end
 
-  defp do_create_or_update_comment(
-    %GithubRepo{} = github_repo,
-    %{"id" => github_id, "issue_url" => issue_url} = attrs,
-    %GithubUser{} = github_user) do
-
-    case Repo.get_by(GithubComment, github_id: github_id) |> Repo.preload([:github_issue, :github_repo, :github_user]) do
-      nil ->
-        %GithubComment{}
-        |> GithubComment.create_changeset(attrs |> Adapters.Comment.to_github_comment)
-        |> Changeset.put_assoc(:github_issue, GithubIssue |> Repo.get_by(url: issue_url))
-        |> Changeset.put_assoc(:github_repo, github_repo)
-        |> Changeset.put_assoc(:github_user, github_user)
-        |> Repo.insert
-      %GithubComment{} = github_comment ->
-        github_comment
-        |> GithubComment.update_changeset(attrs |> Adapters.Comment.to_github_comment)
-        |> Changeset.put_assoc(:github_issue, GithubIssue |> Repo.get_by(url: issue_url))
-        |> Changeset.put_assoc(:github_repo, github_repo)
-        |> Changeset.put_assoc(:github_user, github_user)
-        |> Repo.update
-    end
+
+  @spec find_comment(map) :: GithubComment.t() | nil
+  defp find_comment(%{"id" => github_id}) do
+    GithubComment |> Repo.get_by(github_id: github_id)
+  end
+
+  @spec find_issue(map) :: GithubIssue.t() | nil
+  defp find_issue(%{"issue_url" => issue_url}) do
+    GithubIssue |> Repo.get_by(url: issue_url)
+  end
+
+  @spec find_repo(GithubIssue.t()) :: GithubRepo.t() | nil
+  defp find_repo(%GithubIssue{github_repo_id: github_repo_id}) do
+    GithubRepo |> Repo.get(github_repo_id)
+  end
+
+  @spec create_comment(map, GithubRepo.t() | nil, GithubIssue.t() | nil, GithubUser.t() | nil) :: result()
+  defp create_comment(%{} = params, github_repo, github_issue, github_user) do
+    %GithubComment{}
+    |> GithubComment.create_changeset(params)
+    |> Changeset.put_assoc(:github_issue, github_issue)
+    |> Changeset.put_assoc(:github_repo, github_repo)
+    |> Changeset.put_assoc(:github_user, github_user)
+    |> Changeset.validate_required([:github_issue, :github_repo, :github_user])
+    |> Repo.insert()
+  end
+
+  @spec update_comment(GitHubComment.t(), map) :: result()
+  defp update_comment(%GithubComment{} = github_comment, %{} = params) do
+    github_comment |> GithubComment.update_changeset(params) |> Repo.update()
   end
 
   @doc ~S"""
@@ -88,33 +113,12 @@ defmodule CodeCorps.GitHub.Sync.GithubComment do
   Returns the deleted `CodeCorps.GithubComment` record or an empty
   `CodeCorps.GithubComment` record if no such record existed.
   """
-  @spec delete(String.t) :: {:ok, GithubComment.t}
+  @spec delete(String.t) :: {:ok, GithubComment.t()}
   def delete(github_id) do
     comment = Repo.get_by(GithubComment, github_id: github_id)
     case comment do
       nil -> {:ok, %GithubComment{}}
       _ -> Repo.delete(comment, returning: true)
     end
   end
-
-  @spec create_comment(map) :: result
-  defp create_comment(params) do
-    %GithubComment{}
-    |> GithubComment.create_changeset(params)
-    |> Repo.insert
-  end
-
-  @spec update_comment(GithubComment.t, map) :: result
-  defp update_comment(%GithubComment{} = github_comment, %{} = params) do
-    github_comment
-    |> GithubComment.update_changeset(params)
-    |> Repo.update
-  end
-
-  @spec to_params(map, GithubIssue.t) :: map
-  defp to_params(attrs, %GithubIssue{id: github_issue_id}) do
-    attrs
-    |> Adapters.Comment.to_github_comment()
-    |> Map.put(:github_issue_id, github_issue_id)
-  end
 end

lib/code_corps/github/sync/sync.ex

@@ -13,7 +13,9 @@ defmodule CodeCorps.GitHub.Sync do
     GitHub.Sync.Utils.Finder,
     GitHub.Utils.ResultAggregator,
     GithubAppInstallation,
+    GithubComment,
     GithubIssue,
+    GithubPullRequest,
     GithubRepo,
     GithubUser,
     Repo,
@@ -26,7 +28,7 @@ defmodule CodeCorps.GitHub.Sync do
     {:error, :repo_not_found} |
     {:error, :validating_github_issue, Changeset.t()} |
     {:error, :validating_user, Changeset.t()} |
-    {:error, :multiple_issue_users_match} |
+    {:error, :multiple_task_users_match} |
     {:error, :validating_task, Changeset.t()} |
     {:error, :unexpected_transaction_outcome, any}
 
@@ -47,10 +49,10 @@ defmodule CodeCorps.GitHub.Sync do
         issue_payload
         |> Sync.GithubIssue.create_or_update_issue(github_repo)
       end)
-      |> Multi.run(:issue_user, fn %{github_issue: github_issue} ->
+      |> Multi.run(:task_user, fn %{github_issue: github_issue} ->
         github_issue |> Sync.User.RecordLinker.link_to(issue_payload)
       end)
-      |> Multi.run(:task, fn %{github_issue: github_issue, issue_user: user} ->
+      |> Multi.run(:task, fn %{github_issue: github_issue, task_user: user} ->
         github_issue |> Sync.Task.sync_github_issue(user)
       end)
 
@@ -66,11 +68,11 @@ defmodule CodeCorps.GitHub.Sync do
       {:error, :github_issue, %Changeset{data: %GithubUser{}} = changeset, _steps} ->
         {:error, :validating_github_user, changeset}
 
-      {:error, :issue_user, %Changeset{} = changeset, _steps} ->
+      {:error, :task_user, %Changeset{} = changeset, _steps} ->
         {:error, :validating_user, changeset}
 
-      {:error, :issue_user, :multiple_users, _steps} ->
-        {:error, :multiple_issue_users_match}
+      {:error, :task_user, :multiple_users, _steps} ->
+        {:error, :multiple_task_users_match}
 
       {:error, :task, %Changeset{} = changeset, _steps} ->
         {:error, :validating_task, changeset}
@@ -86,19 +88,22 @@ defmodule CodeCorps.GitHub.Sync do
     {:ok, Comment.t()} |
     {:error, :repo_not_found} |
     {:error, :validating_github_issue, Changeset.t()} |
-    {:error, :validating_user, Changeset.t()} |
-    {:error, :multiple_issue_users_match} |
+    {:error, :validating_github_user_on_github_issue, Changeset.t()} |
+    {:error, :validating_task_user, Changeset.t()} |
+    {:error, :multiple_task_users_match} |
     {:error, :validating_task, Changeset.t()} |
     {:error, :validating_github_comment, Changeset.t()} |
-    {:error, :validating_user, Changeset.t()} |
+    {:error, :validating_github_user_on_github_comment, Changeset.t()} |
+    {:error, :validating_comment_user, Changeset.t()} |
     {:error, :multiple_comment_users_match} |
     {:error, :validating_comment, Changeset.t()} |
     {:error, :unexpected_transaction_outcome, any}
 
   @type pull_request_comment_outcome ::
     issue_comment_outcome() |
     {:error, :fetching_pull_request, struct} |
-    {:error, :validating_github_pull_request, Changeset.t()}
+    {:error, :validating_github_pull_request, Changeset.t()} |
+    {:error, :validating_github_user_on_github_pull_request, Changeset.t()}
 
   @type issue_comment_event_outcome ::
     comment_deleted_outcome() |
@@ -151,10 +156,10 @@ defmodule CodeCorps.GitHub.Sync do
         issue_payload
         |> Sync.GithubIssue.create_or_update_issue(github_repo, github_pull_request)
       end)
-      |> Multi.run(:issue_user, fn %{github_issue: github_issue} ->
+      |> Multi.run(:task_user, fn %{github_issue: github_issue} ->
         github_issue |> Sync.User.RecordLinker.link_to(issue_payload)
       end)
-      |> Multi.run(:task, fn %{github_issue: github_issue, issue_user: user} ->
+      |> Multi.run(:task, fn %{github_issue: github_issue, task_user: user} ->
         github_issue |> Sync.Task.sync_github_issue(user)
       end)
       |> Multi.run(:github_comment, fn %{github_issue: github_issue} ->
@@ -177,26 +182,35 @@ defmodule CodeCorps.GitHub.Sync do
       {:error, :fetch_pull_request, error, _steps} ->
         {:error, :fetching_pull_request, error}
 
-      {:error, :github_pull_request, %Ecto.Changeset{} = changeset, _steps} ->
+      {:error, :github_pull_request, %Changeset{data: %GithubPullRequest{}} = changeset, _steps} ->
         {:error, :validating_github_pull_request, changeset}
 
-      {:error, :github_issue, %Ecto.Changeset{} = changeset, _steps} ->
+      {:error, :github_pull_request, %Changeset{data: %GithubUser{}} = changeset, _steps} ->
+        {:error, :validating_github_user_on_github_pull_request, changeset}
+
+      {:error, :github_issue, %Changeset{data: %GithubIssue{}} = changeset, _steps} ->
         {:error, :validating_github_issue, changeset}
 
-      {:error, :issue_user, %Changeset{} = changeset, _steps} ->
-        {:error, :validating_user, changeset}
+      {:error, :github_issue, %Changeset{data: %GithubUser{}} = changeset, _steps} ->
+        {:error, :validating_github_user_on_github_issue, changeset}
 
-      {:error, :issue_user, :multiple_users, _steps} ->
-        {:error, :multiple_issue_users_match}
+      {:error, :task_user, %Changeset{} = changeset, _steps} ->
+        {:error, :validating_task_user, changeset}
+
+      {:error, :task_user, :multiple_users, _steps} ->
+        {:error, :multiple_task_users_match}
 
       {:error, :task, %Changeset{} = changeset, _steps} ->
         {:error, :validating_task, changeset}
 
-      {:error, :github_comment, %Changeset{} = changeset, _steps} ->
+      {:error, :github_comment, %Changeset{data: %GithubComment{}} = changeset, _steps} ->
         {:error, :validating_github_comment, changeset}
 
+      {:error, :github_comment, %Changeset{data: %GithubUser{}} = changeset, _steps} ->
+        {:error, :validating_github_user_on_github_comment, changeset}
+
       {:error, :comment_user, %Changeset{} = changeset, _steps} ->
-        {:error, :validating_user, changeset}
+        {:error, :validating_comment_user, changeset}
 
       {:error, :comment_user, :multiple_users, _steps} ->
         {:error, :multiple_comment_users_match}
@@ -215,10 +229,10 @@ defmodule CodeCorps.GitHub.Sync do
       |> Multi.run(:github_issue, fn %{repo: github_repo} ->
         issue_payload |> Sync.GithubIssue.create_or_update_issue(github_repo)
       end)
-      |> Multi.run(:issue_user, fn %{github_issue: github_issue} ->
+      |> Multi.run(:task_user, fn %{github_issue: github_issue} ->
         github_issue |> Sync.User.RecordLinker.link_to(issue_payload)
       end)
-      |> Multi.run(:task, fn %{github_issue: github_issue, issue_user: user} ->
+      |> Multi.run(:task, fn %{github_issue: github_issue, task_user: user} ->
         github_issue |> Sync.Task.sync_github_issue(user)
       end)
       |> Multi.run(:github_comment, fn %{github_issue: github_issue} ->
@@ -238,14 +252,17 @@ defmodule CodeCorps.GitHub.Sync do
       {:error, :repo, :unmatched_repository, _steps} ->
         {:error, :repo_not_found}
 
-      {:error, :github_issue, %Changeset{} = changeset, _steps} ->
+      {:error, :github_issue, %Changeset{data: %GithubIssue{}} = changeset, _steps} ->
         {:error, :validating_github_issue, changeset}
 
-      {:error, :issue_user, %Changeset{} = changeset, _steps} ->
-        {:error, :validating_user, changeset}
+      {:error, :github_issue, %Changeset{data: %GithubUser{}} = changeset, _steps} ->
+        {:error, :validating_github_user_on_github_issue, changeset}
 
-      {:error, :issue_user, :multiple_users, _steps} ->
-        {:error, :multiple_issue_users_match}
+      {:error, :task_user, %Changeset{} = changeset, _steps} ->
+        {:error, :validating_task_user, changeset}
+
+      {:error, :task_user, :multiple_users, _steps} ->
+        {:error, :multiple_task_users_match}
 
       {:error, :task, %Changeset{} = changeset, _steps} ->
         {:error, :validating_task, changeset}
@@ -254,7 +271,7 @@ defmodule CodeCorps.GitHub.Sync do
         {:error, :validating_github_comment, changeset}
 
       {:error, :comment_user, %Changeset{} = changeset, _steps} ->
-        {:error, :validating_user, changeset}
+        {:error, :validating_comment_user, changeset}
 
       {:error, :comment_user, :multiple_users, _steps} ->
         {:error, :multiple_comment_users_match}

lib/code_corps/github/sync/user/record_linker.ex

@@ -38,31 +38,29 @@ defmodule CodeCorps.GitHub.Sync.User.RecordLinker do
     - If there are multiple matching users, this is an unexpected scenario and
       should error out.
   """
-  @spec link_to(GithubComment.t | GithubIssue.t, map) :: result
+  @spec link_to(GithubComment.t() | GithubIssue.t(), map) :: result
   def link_to(%GithubComment{} = comment, %{"user" => user}), do: do_link_to(comment, user)
   def link_to(%GithubIssue{} = issue, %{"user" => user}), do: do_link_to(issue, user)
 
   defp do_link_to(record, user_attrs) do
-    record
-    |> match_users
-    |> marshall_response(user_attrs)
+    record |> match_users() |> marshall_response(user_attrs)
   end
 
-  @spec match_users(GithubComment.t | GithubIssue.t) :: list(User.t)
+  @spec match_users(GithubComment.t() | GithubIssue.t()) :: list(User.t())
   defp match_users(%GithubComment{github_id: github_id}) do
     query = from u in User,
       distinct: u.id,
       join: c in Comment, on: u.id == c.user_id,
       join: gc in GithubComment, on: gc.id == c.github_comment_id, where: gc.github_id == ^github_id
 
-    query |> Repo.all
+    query |> Repo.all()
   end
   defp match_users(%GithubIssue{id: github_issue_id}) do
     query = from u in User,
       distinct: u.id,
       join: t in Task, on: u.id == t.user_id, where: t.github_issue_id == ^github_issue_id
 
-    query |> Repo.all
+    query |> Repo.all()
   end
 
   @spec marshall_response(list, map) :: result
@@ -75,7 +73,7 @@ defmodule CodeCorps.GitHub.Sync.User.RecordLinker do
   @spec find_or_create_disassociated_user(map) :: {:ok, User.t}
   def find_or_create_disassociated_user(%{"id" => github_id} = attrs) do
     case User |> Repo.get_by(github_id: github_id) do
-      nil -> attrs |> Accounts.create_from_github
+      nil -> attrs |> Accounts.create_from_github()
       %User{} = user -> {:ok, user}
     end
   end