Merge pull request #1247 from code-corps/fix-more-dialyzer-errors

Fix more dialyzer errors with GitHub sync/services and with emails

Author: joshsmith

lib/code_corps/comment/service.ex

@@ -7,16 +7,19 @@ defmodule CodeCorps.Comment.Service do
   alias CodeCorps.{
     Comment,
     GitHub,
+    GitHub.Sync,
     GithubComment,
     GithubIssue,
     Task,
     Repo
   }
-  alias CodeCorps.GitHub.Sync.Comment.GithubComment, as: GithubCommentSyncer
   alias Ecto.{Changeset, Multi}
 
   require Logger
 
+  @type record_result ::
+          {:ok, Comment.t} | {:error, Changeset.t} | {:error, GitHub.api_error_struct()}
+
   # :user, :github_issue and :github_repo are required for connecting to github
   # :project and :organization are required in order to add a header to the
   # github comment body when the user themselves are not connected to github,
@@ -42,11 +45,13 @@ defmodule CodeCorps.Comment.Service do
   """
   @spec create(map) :: {:ok, Comment.t} | {:error, Changeset.t}
   def create(%{} = attributes) do
-    Multi.new
+    Multi.new()
     |> Multi.insert(:comment, %Comment{} |> Comment.create_changeset(attributes))
-    |> Multi.run(:preload, fn %{comment: %Comment{} = comment} -> {:ok, comment |> Repo.preload(@preloads)} end)
-    |> Multi.run(:github, (fn %{preload: %Comment{} = comment} -> comment |> create_on_github() end))
-    |> Repo.transaction
+    |> Multi.run(:preload, fn %{comment: %Comment{} = comment} ->
+         {:ok, comment |> Repo.preload(@preloads)}
+       end)
+    |> Multi.run(:github, fn %{preload: %Comment{} = comment} -> comment |> create_on_github() end)
+    |> Repo.transaction()
     |> marshall_result
   end
 
@@ -55,30 +60,34 @@ defmodule CodeCorps.Comment.Service do
   """
   @spec update(Comment.t, map) :: {:ok, Comment.t} | {:error, Changeset.t}
   def update(%Comment{} = comment, %{} = attributes) do
-    Multi.new
+    Multi.new()
     |> Multi.update(:comment, comment |> Comment.update_changeset(attributes))
-    |> Multi.run(:github, (fn %{comment: %Comment{} = comment} -> comment |> update_on_github() end))
-    |> Repo.transaction
+    |> Multi.run(:preload, fn %{comment: %Comment{} = comment} ->
+         {:ok, comment |> Repo.preload(@preloads)}
+       end)
+    |> Multi.run(:github, fn %{preload: %Comment{} = comment} -> comment |> update_on_github() end)
+    |> Repo.transaction()
     |> marshall_result()
   end
 
   @spec marshall_result(tuple) :: {:ok, Comment.t} | {:error, Changeset.t} | {:error, :github}
   defp marshall_result({:ok, %{github: %Comment{} = comment}}), do: {:ok, comment}
   defp marshall_result({:error, :comment, %Changeset{} = changeset, _steps}), do: {:error, changeset}
   defp marshall_result({:error, :github, result, _steps}) do
-    Logger.info "An error occurred when creating/updating the comment with the GitHub API"
-    Logger.info "#{inspect result}"
+    Logger.info("An error occurred when creating/updating the comment with the GitHub API")
+    Logger.info("#{inspect(result)}")
     {:error, :github}
   end
 
-  @spec create_on_github(Comment.t) :: {:ok, Comment.t} :: {:error, GitHub.api_error_struct}
+  @spec create_on_github(Comment.t) :: record_result
   defp create_on_github(%Comment{task: %Task{github_issue_id: nil}} = comment), do: {:ok, comment}
   defp create_on_github(%Comment{task: %Task{github_issue: github_issue}} = comment) do
-    with {:ok, payload} <- comment |> GitHub.API.Comment.create,
-         {:ok, %GithubComment{} = github_comment} <- GithubCommentSyncer.create_or_update_comment(github_issue, payload)do
-      comment |> link_with_github_changeset(github_comment) |> Repo.update
+    with {:ok, payload} <- comment |> GitHub.API.Comment.create(),
+         {:ok, %GithubComment{} = github_comment} <-
+           Sync.Comment.GithubComment.create_or_update_comment(github_issue, payload) do
+      comment |> link_with_github_changeset(github_comment) |> Repo.update()
     else
-      {:error, github_error} -> {:error, github_error}
+      {:error, error} -> {:error, error}
     end
   end
 
@@ -87,16 +96,17 @@ defmodule CodeCorps.Comment.Service do
     comment |> Changeset.change(%{github_comment: github_comment})
   end
 
-  @spec update_on_github(Comment.t) :: {:ok, Comment.t} :: {:error, GitHub.api_error_struct}
+  @spec update_on_github(Comment.t) :: record_result
   defp update_on_github(%Comment{github_comment_id: nil} = comment), do: {:ok, comment}
-  defp update_on_github(%Comment{} = comment) do
-    with %Comment{task: %Task{github_issue: %GithubIssue{} = github_issue}} = comment <- comment |> Repo.preload(@preloads),
-         {:ok, payload} <- comment |> GitHub.API.Comment.update,
-         {:ok, %GithubComment{}} <- GithubCommentSyncer.create_or_update_comment(github_issue, payload) do
-
+  defp update_on_github(
+         %Comment{task: %Task{github_issue: %GithubIssue{} = github_issue}} = comment
+       ) do
+    with {:ok, payload} <- comment |> GitHub.API.Comment.update(),
+         {:ok, %GithubComment{}} <-
+           Sync.Comment.GithubComment.create_or_update_comment(github_issue, payload) do
       {:ok, comment}
     else
-      {:error, github_error} -> {:error, github_error}
+      {:error, error} -> {:error, error}
     end
   end
 end

lib/code_corps/emails/receipt_email.ex

@@ -7,7 +7,7 @@ defmodule CodeCorps.Emails.ReceiptEmail do
 
   @spec create(StripeConnectCharge.t, Stripe.Invoice.t) :: Bamboo.Email.t
   def create(%StripeConnectCharge{} = charge, %Stripe.Invoice{} = invoice) do
-    with %StripeConnectCharge{} = charge <- preload(charge),
+    with %StripeConnectCharge{} = charge <- Repo.preload(charge, :user),
          %Project{} = project <- get_project(invoice.subscription),
          {:ok, %DonationGoal{} = current_donation_goal} <- get_current_donation_goal(project),
          template_model <- build_model(charge, project, current_donation_goal)
@@ -21,9 +21,6 @@ defmodule CodeCorps.Emails.ReceiptEmail do
     end
   end
 
-  @spec preload(Project.t) :: Project.t
-  defp preload(%StripeConnectCharge{} = charge), do: Repo.preload(charge, :user)
-
   @spec get_project(String.t) :: Project.t | {:error, :subscription_not_found}
   defp get_project(subscription_id_from_stripe) do
     with %StripeConnectSubscription{} = subscription <- get_subscription(subscription_id_from_stripe) do
@@ -79,7 +76,7 @@ defmodule CodeCorps.Emails.ReceiptEmail do
     "https://d3pgew4wbk2vb1.cloudfront.net/emails/images/emoji-1f64c-1f3ff@2x.png"
   ]
 
-  @spec format_amount(float) :: String.t
+  @spec format_amount(integer) :: binary
   defp format_amount(amount) do
     amount |> Money.new(:USD) |> Money.to_string()
   end

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

@@ -15,11 +15,9 @@ defmodule CodeCorps.GitHub.Sync.Comment.GithubComment do
     GithubUser,
     Repo
   }
-
   alias Ecto.Changeset
-  alias Sync.User.GithubUser, as: GithubUserSyncer
 
-  @typep linking_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
@@ -34,7 +32,7 @@ defmodule CodeCorps.GitHub.Sync.Comment.GithubComment do
   `CodeCorps.GitHub.Adapters.Comment.to_github_comment/1` is used to adapt the
   payload data.
   """
-  @spec create_or_update_comment(GithubIssue.t, map) :: linking_result
+  @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
@@ -50,19 +48,18 @@ defmodule CodeCorps.GitHub.Sync.Comment.GithubComment do
   The comment is matched with an existing GithubIssue record using the
   `issue_url` property of the payload.
   """
-  @spec create_or_update_comment(GithubRepo.t, map) :: linking_result
+  @spec create_or_update_comment(GithubRepo.t, map) :: result
   def create_or_update_comment(%GithubRepo{} = github_repo, %{"id" => _, "issue_url" => _} = attrs) do
-    with {:ok, %GithubUser{} = github_user} <- GithubUserSyncer.create_or_update_github_user(attrs),
-         {:ok, %GithubComment{} = github_comment} <- do_create_or_update_comment(github_repo, attrs, github_user)
-    do
+    with {:ok, %GithubUser{} = github_user} <- Sync.User.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}
     else
       {:error, error} -> {:error, error}
     end
   end
 
   defp do_create_or_update_comment(
-  %GithubRepo{} = github_repo,
+    %GithubRepo{} = github_repo,
     %{"id" => github_id, "issue_url" => issue_url} = attrs,
     %GithubUser{} = github_user) do
 
@@ -100,14 +97,14 @@ defmodule CodeCorps.GitHub.Sync.Comment.GithubComment do
     end
   end
 
-  @spec create_comment(map) :: linking_result
+  @spec create_comment(map) :: result
   defp create_comment(params) do
     %GithubComment{}
     |> GithubComment.create_changeset(params)
     |> Repo.insert
   end
 
-  @spec update_comment(GithubComment.t, map) :: linking_result
+  @spec update_comment(GithubComment.t, map) :: result
   defp update_comment(%GithubComment{} = github_comment, %{} = params) do
     github_comment
     |> GithubComment.update_changeset(params)

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

@@ -18,7 +18,7 @@ defmodule CodeCorps.GitHub.Sync.Issue.GithubIssue do
 
   alias Ecto.Changeset
 
-  @typep linking_result :: {:ok, GithubIssue.t} | {:error, Changeset.t}
+  @type result :: {:ok, GithubIssue.t} | {:error, Changeset.t}
 
   @doc ~S"""
   Creates or updates a `CodeCorps.GithubIssue` from a github issue API payload.
@@ -29,16 +29,16 @@ defmodule CodeCorps.GitHub.Sync.Issue.GithubIssue do
   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) :: linking_result
+  @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
     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)
+      |> 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
+      |> Repo.insert_or_update()
     else
       {:error, error} -> {:error, error}
     end

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

@@ -18,9 +18,9 @@ defmodule CodeCorps.GitHub.Sync.User.RecordLinker do
     User
   }
 
-  @typep linking_result :: {:ok, User.t} |
-                           {:error, Ecto.Changeset.t} |
-                           {:error, :multiple_users}
+  @type result :: {:ok, User.t} |
+                  {:error, Ecto.Changeset.t} |
+                  {:error, :multiple_users}
 
   @doc ~S"""
   Finds or creates a user using information in the resource and the GitHub API
@@ -38,7 +38,7 @@ 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) :: linking_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)
 
@@ -65,7 +65,7 @@ defmodule CodeCorps.GitHub.Sync.User.RecordLinker do
     query |> Repo.all
   end
 
-  @spec marshall_response(list, map) :: linking_result
+  @spec marshall_response(list, map) :: result
   defp marshall_response([%User{} = single_user], %{}), do: {:ok, single_user}
   defp marshall_response([], %{} = user_attrs) do
     user_attrs |> find_or_create_disassociated_user()

lib/code_corps/task/service.ex

@@ -3,19 +3,37 @@ defmodule CodeCorps.Task.Service do
   Handles special CRUD operations for `CodeCorps.Task`.
   """
 
-  alias CodeCorps.{GitHub, GithubIssue, Repo, Task}
+  alias CodeCorps.{GitHub, GitHub.Sync, GithubIssue, Repo, Task}
   alias Ecto.{Changeset, Multi}
 
   require Logger
 
+  # :user, :github_issue and :github_repo are required for connecting to github
+  # :project and :organization are required in order to add a header to the
+  # github issue body when the user themselves are not connected to github, but
+  # the task is
+  #
+  # Right now, all of these preloads are loaded at once. If there are
+  # performance issues, we can split them up according the the information
+  # provided here.
+  @preloads [
+    :github_issue,
+    [github_repo: :github_app_installation],
+    [project: :organization],
+    :user
+  ]
+
   @doc ~S"""
   Performs all actions involved in creating a task on a project
   """
   @spec create(map) :: {:ok, Task.t} | {:error, Changeset.t} | {:error, :github}
   def create(%{} = attributes) do
     Multi.new
     |> Multi.insert(:task, %Task{} |> Task.create_changeset(attributes))
-    |> Multi.run(:github, (fn %{task: %Task{} = task} -> task |> create_on_github() end))
+    |> Multi.run(:preload, fn %{task: %Task{} = task} ->
+         {:ok, task |> Repo.preload(@preloads)}
+       end)
+    |> Multi.run(:github, (fn %{preload: %Task{} = task} -> task |> create_on_github() end))
     |> Repo.transaction
     |> marshall_result()
   end
@@ -24,7 +42,10 @@ defmodule CodeCorps.Task.Service do
   def update(%Task{} = task, %{} = attributes) do
     Multi.new
     |> Multi.update(:task, task |> Task.update_changeset(attributes))
-    |> Multi.run(:github, (fn %{task: %Task{} = task} -> task |> update_on_github() end))
+    |> Multi.run(:preload, fn %{task: %Task{} = task} ->
+         {:ok, task |> Repo.preload(@preloads)}
+       end)
+    |> Multi.run(:github, (fn %{preload: %Task{} = task} -> task |> update_on_github() end))
     |> Repo.transaction
     |> marshall_result()
   end
@@ -39,25 +60,16 @@ defmodule CodeCorps.Task.Service do
     {:error, :github}
   end
 
-  # :user, :github_issue and :github_repo are required for connecting to github
-  # :project and :organization are required in order to add a header to the
-  # github issue body when the user themselves are not connected to github, but
-  # the task is
-  #
-  # Right now, all of these preloads are loaded at once. If there are
-  # performance issues, we can split them up according the the information
-  # provided here.
-  @preloads [:github_issue, [github_repo: :github_app_installation], :user, [project: :organization]]
-
-  @spec create_on_github(Task.t) :: {:ok, Task.t} :: {:error, GitHub.api_error_struct}
+  @spec create_on_github(Task.t) :: {:ok, Task.t} | {:error, Changeset.t} | {:error, GitHub.api_error_struct}
   defp create_on_github(%Task{github_repo_id: nil} = task) do
     # Don't create: no GitHub repo was selected
     {:ok, task}
   end
-  defp create_on_github(%Task{github_repo: _} = task) do
-    with %Task{github_repo: github_repo} = task <- task |> Repo.preload(@preloads),
-         {:ok, payload} <- GitHub.API.Issue.create(task),
-         {:ok, %GithubIssue{} = github_issue} <- payload |> GitHub.Sync.Issue.GithubIssue.create_or_update_issue(github_repo) do
+  defp create_on_github(%Task{github_repo: github_repo} = task) do
+    with {:ok, payload} <- GitHub.API.Issue.create(task),
+         {:ok, %GithubIssue{} = github_issue} <-
+           payload
+           |> Sync.Issue.GithubIssue.create_or_update_issue(github_repo) do
       task |> link_with_github_changeset(github_issue) |> Repo.update
     else
       {:error, error} -> {:error, error}
@@ -69,16 +81,17 @@ defmodule CodeCorps.Task.Service do
     task |> Changeset.change(%{github_issue: github_issue})
   end
 
-  @spec update_on_github(Task.t) :: {:ok, Task.t} :: {:error, GitHub.api_error_struct}
+  @spec update_on_github(Task.t) :: {:ok, Task.t} | {:error, Changeset.t} | {:error, GitHub.api_error_struct}
   defp update_on_github(%Task{github_repo_id: nil, github_issue_id: nil} = task), do: {:ok, task}
   defp update_on_github(%Task{github_repo_id: _, github_issue_id: nil} = task), do: task |> create_on_github()
-  defp update_on_github(%Task{github_repo_id: _} = task) do
-    with %Task{github_repo: github_repo} = task <- task |> Repo.preload(@preloads),
-         {:ok, payload} <- GitHub.API.Issue.update(task),
-         {:ok, %GithubIssue{}} <- payload |> GitHub.Sync.Issue.GithubIssue.create_or_update_issue(github_repo) do
+  defp update_on_github(%Task{github_repo: github_repo} = task) do
+    with {:ok, payload} <- GitHub.API.Issue.update(task),
+         {:ok, %GithubIssue{}} <-
+           payload
+           |> Sync.Issue.GithubIssue.create_or_update_issue(github_repo) do
       {:ok, Task |> Repo.get(task.id)}
     else
-      {:error, github_error} -> {:error, github_error}
+      {:error, error} -> {:error, error}
     end
   end
 end