From 4aaa7b3c3d5aa724eece4737dd86fed9681795dd Mon Sep 17 00:00:00 2001 From: Josh Smith Date: Fri, 24 Nov 2017 15:40:16 -0800 Subject: [PATCH] Fix more dialyzer errors with GitHub sync/services and with emails --- lib/code_corps/comment/service.ex | 54 +++++++++------- lib/code_corps/emails/receipt_email.ex | 7 +-- .../comment/github_comment/github_comment.ex | 19 +++--- .../sync/issue/github_issue/github_issue.ex | 8 +-- .../github/sync/user/record_linker.ex | 10 +-- lib/code_corps/task/service.ex | 61 +++++++++++-------- 6 files changed, 88 insertions(+), 71 deletions(-) diff --git a/lib/code_corps/comment/service.ex b/lib/code_corps/comment/service.ex index c5021eb2e..48d05eecc 100644 --- a/lib/code_corps/comment/service.ex +++ b/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,10 +60,13 @@ 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 @@ -66,19 +74,20 @@ defmodule CodeCorps.Comment.Service do 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 diff --git a/lib/code_corps/emails/receipt_email.ex b/lib/code_corps/emails/receipt_email.ex index 05d9e0d31..d632da918 100644 --- a/lib/code_corps/emails/receipt_email.ex +++ b/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 diff --git a/lib/code_corps/github/sync/comment/github_comment/github_comment.ex b/lib/code_corps/github/sync/comment/github_comment/github_comment.ex index a045b90b7..0b11cfdd5 100644 --- a/lib/code_corps/github/sync/comment/github_comment/github_comment.ex +++ b/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,11 +48,10 @@ 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} @@ -62,7 +59,7 @@ defmodule CodeCorps.GitHub.Sync.Comment.GithubComment do 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) diff --git a/lib/code_corps/github/sync/issue/github_issue/github_issue.ex b/lib/code_corps/github/sync/issue/github_issue/github_issue.ex index 1f37b42a3..a29620239 100644 --- a/lib/code_corps/github/sync/issue/github_issue/github_issue.ex +++ b/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 diff --git a/lib/code_corps/github/sync/user/record_linker.ex b/lib/code_corps/github/sync/user/record_linker.ex index 714f05712..d9cc6b53f 100644 --- a/lib/code_corps/github/sync/user/record_linker.ex +++ b/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() diff --git a/lib/code_corps/task/service.ex b/lib/code_corps/task/service.ex index 62c6328b5..271155b89 100644 --- a/lib/code_corps/task/service.ex +++ b/lib/code_corps/task/service.ex @@ -3,11 +3,26 @@ 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 """ @@ -15,7 +30,10 @@ defmodule CodeCorps.Task.Service do 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