Clean up GitHub Sync with some minor documentation and spec changes

Author: joshsmith

lib/code_corps/github/sync/comment/comment/changeset.ex

@@ -16,7 +16,6 @@ defmodule CodeCorps.GitHub.Sync.Comment.Comment.Changeset do
   alias Ecto.Changeset
 
   @create_attrs ~w(created_at markdown modified_at)a
-  @update_attrs ~w(markdown modified_at)a
 
   @doc ~S"""
   Constructs a changeset for creating a `CodeCorps.Comment` when syncing from a
@@ -39,6 +38,8 @@ defmodule CodeCorps.GitHub.Sync.Comment.Comment.Changeset do
     |> Changeset.validate_required([:markdown, :body])
   end
 
+  @update_attrs ~w(markdown modified_at)a
+
   @doc ~S"""
   Constructs a changeset for updating a `CodeCorps.Comment` when syncing from a
   GitHub API Comment payload.

lib/code_corps/github/sync/comment/comment/comment.ex

@@ -67,7 +67,7 @@ defmodule CodeCorps.GitHub.Sync.Comment.Comment do
   `CodeCorps.GithubRepo`.
 
   For each `CodeCorps.GithubComment` record that relates to the
-  `CodeCorps.GithubRepo` for a given`CodeCorps.GithubRepo`:
+  `CodeCorps.GithubRepo` for a given `CodeCorps.GithubRepo`:
 
   - Find the related `CodeCorps.Task` record
   - Create or update the related `CodeCorps.Comment` record
@@ -91,7 +91,7 @@ defmodule CodeCorps.GitHub.Sync.Comment.Comment do
     |> ResultAggregator.aggregate()
   end
 
-  # can this return a nil?
+  # TODO: can this return a nil?
   @spec find_task(GithubComment.t(), GithubRepo.t()) :: Task.t()
   defp find_task(
     %GithubComment{github_issue: %GithubIssue{id: github_issue_id}},

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

@@ -21,13 +21,13 @@ defmodule CodeCorps.GitHub.Sync.Issue.GithubIssue do
   @type result :: {:ok, GithubIssue.t} | {:error, Changeset.t}
 
   @doc ~S"""
-  Creates or updates a `CodeCorps.GithubIssue` from a github issue API payload.
+  Creates or updates a `CodeCorps.GithubIssue` from a GitHub issue API payload.
 
-  The created record is associated to the provided `CodeCorps.GithubRepo` and, optionaly,
-  to a provided `CodeCorps.GithubPullRequest`.
+  The created record is associated to the provided `CodeCorps.GithubRepo` and,
+  optionally, to a provided `CodeCorps.GithubPullRequest`.
 
-  The created record is also associated with a matched `CodeCorps.GithubUser`, which is
-  created if necessary.
+  The created record is also associated with a matched `CodeCorps.GithubUser`,
+  which is created if necessary.
   """
   @spec create_or_update_issue(map, GithubRepo.t, GithubPullRequest.t | nil) :: result
   def create_or_update_issue(%{} = payload, %GithubRepo{} = github_repo, github_pull_request \\ nil) do

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

@@ -7,7 +7,7 @@ defmodule CodeCorps.GitHub.Sync.Issue do
   alias Ecto.Multi
 
   @doc ~S"""
-  Performs sync of a github issue payload related to a repo and a pull request.
+  Performs sync of a GitHub issue payload related to a repo and a pull request.
 
   Performs work identical to `sync/2`, with the adition of associating the
   resulting `CodeCorps.GithubIssue` with the specified `CodeCorps.GithubPullRequest`.

lib/code_corps/github/sync/sync.ex

@@ -1,7 +1,9 @@
 defmodule CodeCorps.GitHub.Sync do
   @moduledoc """
-  Syncs events received from the GitHub API and also syncs entire GitHub
-  repositories.
+  GitHub syncing functions for:
+
+  - events received from the GitHub API
+  - entire GitHub repositories
   """
 
   alias CodeCorps.{
@@ -11,29 +13,12 @@ defmodule CodeCorps.GitHub.Sync do
     GitHub.Sync.Utils.Finder,
     GitHub.Utils.ResultAggregator,
     GithubAppInstallation,
-    GithubPullRequest,
     GithubRepo,
     Repo,
     Task
   }
-
   alias Ecto.{Changeset, Multi}
 
-  @type outcome :: {:ok, list(Comment.t())}
-                 | {:ok, GithubPullRequest.t()}
-                 | {:error, :repo_not_found, %{}}
-                 | {:error, :fetching_issue}
-                 | {:error, :fetching_pull_request}
-                 | {:error, :multiple_comment_users_match}
-                 | {:error, :validating_github_pull_request}
-                 | {:error, :validating_github_comment}
-                 | {:error, :validating_user}
-                 | {:error, :validating_task}
-                 | {:error, :validating_comment}
-                 | {:error, :unexpected_transaction_outcome}
-
-
-
   @type issue_event_outcome ::
     {:ok, Task.t()} |
     {:error, :repo_not_found, map} |
@@ -102,8 +87,6 @@ defmodule CodeCorps.GitHub.Sync do
     {:error, :fetching_pull_request, struct} |
     {:error, :validating_github_pull_request, Changeset.t()}
 
-
-
   @doc ~S"""
   Syncs a GitHub IssueComment event.
 
@@ -241,7 +224,7 @@ defmodule CodeCorps.GitHub.Sync do
       {:error, :repo, :unmatched_repository, _steps} ->
         {:error, :repo_not_found, %{}}
 
-      {:error, :github_issue, %Ecto.Changeset{} = changeset, _steps} ->
+      {:error, :github_issue, %Changeset{} = changeset, _steps} ->
         {:error, :validating_github_issue, changeset}
 
       {:error, :issue_user, %Changeset{} = changeset, _steps} ->
@@ -336,10 +319,10 @@ defmodule CodeCorps.GitHub.Sync do
   @doc ~S"""
   Syncs a GitHub InstallationRepositories event.
 
-  - For the "removed" action
-    - Deletes all  `CodeCorps.GithubRepo` records matched with the payload
-  - For the "added" action
-    -
+  - For the "removed" action:
+    - Deletes all `CodeCorps.GithubRepo` records matched with the payload
+  - For the "added" action:
+    - Adds all `CodeCorps.GithubRepo` records matching data from the payload
 
   [https://developer.github.com/v3/activity/events/types/#installationrepositoriesevent](https://developer.github.com/v3/activity/events/types/#installationrepositoriesevent)
   """
@@ -446,7 +429,7 @@ defmodule CodeCorps.GitHub.Sync do
   end
 
   @doc ~S"""
-  Syncs a `GithubRepo` with Code Corps.
+  Syncs a `GithubRepo`.
 
   Fetches and syncs records from the GitHub API for a given repository, marking
   progress of the sync state along the way.
@@ -508,6 +491,14 @@ defmodule CodeCorps.GitHub.Sync do
     end
   end
 
+  @spec mark_repo(GithubRepo.t(), String.t(), map) ::
+    {:ok, GithubRepo.t()} | {:error, Changeset.t()}
+  defp mark_repo(%GithubRepo{} = repo, sync_state, params \\ %{}) do
+    repo
+    |> GithubRepo.update_sync_changeset(params |> Map.put(:sync_state, sync_state))
+    |> Repo.update
+  end
+
   @spec pair_issues_payloads_with_prs(list, list) :: list(tuple)
   defp pair_issues_payloads_with_prs(issue_payloads, pull_requests) do
     issue_payloads |> Enum.map(fn %{"number" => number} = issue_payload ->
@@ -532,12 +523,4 @@ defmodule CodeCorps.GitHub.Sync do
   @spec sync_step(tuple, atom) :: tuple
   defp sync_step({:ok, _} = result, _step), do: result
   defp sync_step({:error, _}, step), do: {:error, step}
-
-  @spec mark_repo(GithubRepo.t(), String.t(), map) ::
-    {:ok, GithubRepo.t()} | {:error, Changeset.t()}
-  defp mark_repo(%GithubRepo{} = repo, sync_state, params \\ %{}) do
-    repo
-    |> GithubRepo.update_sync_changeset(params |> Map.put(:sync_state, sync_state))
-    |> Repo.update
-  end
 end