Merge pull request #1225 from code-corps/1217-eliminate-duplicative-logic-in-github-sync-issue-github-issue

Deduplicate Sync.Issue.GithubIssue, clean up

Author: joshsmith

lib/code_corps/github/sync/issue/github_issue/github_issue.ex

@@ -1,7 +1,7 @@
 defmodule CodeCorps.GitHub.Sync.Issue.GithubIssue do
   @moduledoc ~S"""
-  In charge of finding a `CodeCorps.GithubIssue` to link with a
-  `CodeCorps.Issue` when processing a GitHub Issue payload.
+  In charge of finding or creating a `CodeCorps.GithubIssue` to link with a
+  `CodeCorps.Task` when processing a GitHub Issue payload.
 
   The only entry point is `create_or_update_issue/2`.
   """
@@ -17,93 +17,38 @@ defmodule CodeCorps.GitHub.Sync.Issue.GithubIssue do
   }
 
   alias Ecto.Changeset
-  alias Sync.User.GithubUser, as: GithubUserSyncer
 
   @typep linking_result :: {:ok, GithubIssue.t} | {:error, Changeset.t}
 
   @doc ~S"""
-  Finds or creates a `CodeCorps.GithubIssue` using the data in a GitHub Issue
-  payload.
+  Creates or updates a `CodeCorps.GithubIssue` from a github issue API payload.
 
-  The process is as follows:
-  - Search for the issue in our database with the payload data.
-   - If found, update it with payload data
-   - If not found, create it from payload data
+  The created record is associated to the provided `CodeCorps.GithubRepo` and, optionaly,
+  to a provided `CodeCorps.GithubPullRequest`.
 
-  `CodeCorps.GitHub.AdaptersIssue.to_issue/1` is used to adapt the payload data.
+  The created record is also associated with a matched `CodeCorps.GithubUser`, which is
+  created if necessary.
   """
-  @spec create_or_update_issue({GithubRepo.t, GithubPullRequest.t | nil}, map) :: linking_result
-  def create_or_update_issue({github_repo, github_pull_request}, %{"id" => github_issue_id} = attrs) do
-    params = to_params(attrs, github_repo, github_pull_request)
-    case Repo.get_by(GithubIssue, github_id: github_issue_id) do
-      nil -> create_issue(params)
-      %GithubIssue{} = issue -> update_issue(issue, params)
-    end
-  end
-
-  @doc ~S"""
-  Creates a `CodeCorps.GithubIssue` for the provided `CodeCorps.GithubRepo`
-  using specified attributes.
-
-  Links to existing `CodeCorps.GithubPullRequest` if matched by
-  `github_repo_id` and `number`.
-  """
-  @spec create_or_update_issue(GithubRepo.t, map) :: linking_result
-  def create_or_update_issue(%GithubRepo{} = repo, attrs) do
-    with {:ok, %GithubUser{} = github_user} <- GithubUserSyncer.create_or_update_github_user(attrs),
-         {:ok, %GithubIssue{} = github_issue} <- do_create_or_update_issue(repo, attrs, github_user)
-    do
-      {:ok, github_issue}
+  @spec create_or_update_issue(map, GithubRepo.t, GithubPullRequest.t | nil) :: linking_result
+  def create_or_update_issue(%{} = payload, %GithubRepo{} = github_repo, github_pull_request \\ nil) do
+    with {:ok, %GithubUser{} = github_user} <- Sync.User.GithubUser.create_or_update_github_user(payload) do
+      payload
+      |> find_or_init()
+      |> GithubIssue.changeset(payload |> Adapters.Issue.to_issue)
+      |> Changeset.put_assoc(:github_user, github_user)
+      |> Changeset.put_assoc(:github_repo, github_repo)
+      |> Changeset.put_assoc(:github_pull_request, github_pull_request)
+      |> Repo.insert_or_update
     else
       {:error, error} -> {:error, error}
     end
   end
 
-  defp do_create_or_update_issue(
-    %GithubRepo{id: repo_id} = repo,
-    %{"id" => github_id, "number" => number} = attrs,
-    %GithubUser{} = github_user) do
-
-    case Repo.get_by(GithubIssue, github_id: github_id) |> Repo.preload([:github_pull_request, :github_user]) do
-      nil ->
-        %GithubIssue{}
-        |> GithubIssue.create_changeset(attrs |> Adapters.Issue.to_issue)
-        |> Changeset.put_assoc(:github_pull_request, GithubPullRequest |> Repo.get_by(github_repo_id: repo_id, number: number))
-        |> Changeset.put_assoc(:github_repo, repo)
-        |> Changeset.put_assoc(:github_user, github_user)
-        |> Repo.insert
-      %GithubIssue{} = issue ->
-        issue
-        |> GithubIssue.update_changeset(attrs |> Adapters.Issue.to_issue)
-        |> Changeset.put_assoc(:github_pull_request, GithubPullRequest |> Repo.get_by(github_repo_id: repo_id, number: number))
-        |> Changeset.put_assoc(:github_user, github_user)
-        |> Repo.update
+  @spec find_or_init(map) :: GithubIssue.t
+  defp find_or_init(%{"id" => github_id}) do
+    case GithubIssue |> Repo.get_by(github_id: github_id) |> Repo.preload([:github_user, :github_repo, :github_pull_request]) do
+      nil -> %GithubIssue{}
+      %GithubIssue{} = github_issue -> github_issue
     end
   end
-
-  @spec create_issue(map) :: linking_result
-  defp create_issue(params) do
-    %GithubIssue{}
-    |> GithubIssue.create_changeset(params)
-    |> Repo.insert
-  end
-
-  @spec update_issue(GithubIssue.t, map) :: linking_result
-  defp update_issue(%GithubIssue{} = github_issue, params) do
-    github_issue
-    |> GithubIssue.update_changeset(params)
-    |> Repo.update
-  end
-
-  defp to_params(attrs, %GithubRepo{id: github_repo_id}, %GithubPullRequest{id: github_pull_request_id}) do
-    attrs
-    |> Adapters.Issue.to_issue()
-    |> Map.put(:github_repo_id, github_repo_id)
-    |> Map.put(:github_pull_request_id, github_pull_request_id)
-  end
-  defp to_params(attrs, %GithubRepo{id: github_repo_id}, _) do
-    attrs
-    |> Adapters.Issue.to_issue()
-    |> Map.put(:github_repo_id, github_repo_id)
-  end
 end

lib/code_corps/github/sync/issue/issue.ex

@@ -1,45 +1,38 @@
 defmodule CodeCorps.GitHub.Sync.Issue do
-  alias CodeCorps.{GitHub, GithubIssue, GithubPullRequest, GithubRepo}
-  alias GitHub.Sync.Issue.GithubIssue, as: IssueGithubIssueSyncer
-  alias GitHub.Sync.Issue.Task, as: IssueTaskSyncer
-  alias GitHub.Sync.User.RecordLinker, as: UserRecordLinker
+  alias CodeCorps.{
+    GithubPullRequest,
+    GithubRepo,
+    GitHub.Sync
+  }
   alias Ecto.Multi
 
   @doc ~S"""
-  Syncs a GitHub issue API payload with our data.
+  Performs sync of a github issue payload related to a repo and a pull request.
 
-  The process is as follows:
-
-  - match with `CodeCorps.User` using `CodeCorps.GitHub.Sync.User.RecordLinker`
-  - create or update the `CodeCorps.Task` for the `CodeCorps.Project` in the
-    matched `CodeCorps.GithubRepo`
-
-  If the sync succeeds, it will return an `:ok` tuple with the created or
-  updated task.
-
-  If the sync fails, it will return an `:error` tuple, where the second element
-  is the atom indicating a reason.
+  Performs work identical to `sync/2`, with the adition of associating the
+  resulting `CodeCorps.GithubIssue` with the specified `CodeCorps.GithubPullRequest`.
   """
-  @spec sync((map -> Multi.t), map) :: Multi.t
-  def sync(%{fetch_issue: issue} = changes, _payload) do
-    sync_multi(changes, issue)
-  end
-  def sync(changes, payload) do
-    sync_multi(changes, payload)
+  @spec sync(map, GithubRepo.t, GithubPullRequest.t) :: Multi.t
+  def sync(%{} = payload, %GithubRepo{} = github_repo, %GithubPullRequest{} = github_pull_request) do
+    Multi.new
+    |> Multi.run(:github_issue, fn _ -> payload |> Sync.Issue.GithubIssue.create_or_update_issue(github_repo, github_pull_request) end)
+    |> Multi.run(:issue_user, fn %{github_issue: github_issue} -> Sync.User.RecordLinker.link_to(github_issue, payload) end)
+    |> Multi.run(:task, fn %{github_issue: github_issue, issue_user: user} -> github_issue |> Sync.Issue.Task.sync_github_issue(user) end)
   end
 
-  @spec sync_multi(map, map) :: Multi.t
-  defp sync_multi(%{repo: github_repo, github_pull_request: github_pull_request}, payload) do
-    do_sync_multi({github_repo, github_pull_request}, payload)
-  end
-  defp sync_multi(%{repo: github_repo}, payload) do
-    do_sync_multi({github_repo, nil}, payload)
-  end
+  @doc ~S"""
+  Performs sync of a github issue payload related to a repo.
 
-  defp do_sync_multi({github_repo, github_pull_request}, payload) do
+  Creates or updates a `CodeCorps.GithubIssue`
+  - a `CodeCorps.GithubUser` is created or updated as part of the process and associated to `CodeCorps.GithubIssue`
+  - the record is associated to the `CodeCorps.GithubRepo`
+  - an 'unregistered' `CodeCorps.User` is created if no record with matching `:github_id` is found
+  """
+  @spec sync(map, GithubRepo.t) :: Multi.t
+  def sync(%{} = payload, %GithubRepo{} = github_repo) do
     Multi.new
-    |> Multi.run(:github_issue, fn _ -> IssueGithubIssueSyncer.create_or_update_issue({github_repo, github_pull_request}, payload) end)
-    |> Multi.run(:issue_user, fn %{github_issue: github_issue} -> UserRecordLinker.link_to(github_issue, payload) end)
-    |> Multi.run(:task, fn %{github_issue: github_issue, issue_user: user} -> github_issue |> IssueTaskSyncer.sync_github_issue(user) end)
+    |> Multi.run(:github_issue, fn _ -> payload |> Sync.Issue.GithubIssue.create_or_update_issue(github_repo) end)
+    |> Multi.run(:issue_user, fn %{github_issue: github_issue} -> Sync.User.RecordLinker.link_to(github_issue, payload) end)
+    |> Multi.run(:task, fn %{github_issue: github_issue, issue_user: user} -> github_issue |> Sync.Issue.Task.sync_github_issue(user) end)
   end
 end