Merge pull request #677 from code-corps/add-dialyzer

Added dialyzer locally (not on CI)

Author: joshsmith

circle.yml

@@ -14,8 +14,11 @@ dependencies:
 test:
   override:
     - |
-      if [ ${CIRCLE_PR_USERNAME} ]; then MIX_ENV=test mix test; else MIX_ENV=test mix coveralls.circle --include requires_env; fi
-
+      if [ ${CIRCLE_PR_USERNAME} ]; then
+        MIX_ENV=test mix test;
+      else
+        MIX_ENV=test mix coveralls.circle --include requires_env;
+      fi
   post:
     - mix inch.report
 

lib/code_corps/emails/receipt_email.ex

@@ -13,7 +13,7 @@ defmodule CodeCorps.Emails.ReceiptEmail do
     do
       BaseEmail.create
       |> to(charge.user.email)
-      |> template(template_id, template_model)
+      |> template(template_id(), template_model)
     else
       nil -> {:error, :project_not_found}
       other -> other
@@ -49,7 +49,7 @@ defmodule CodeCorps.Emails.ReceiptEmail do
     %{
       charge_amount: charge.amount |> format_amount(),
       charge_statement_descriptor: charge.statement_descriptor,
-      high_five_image_url: high_five_image_url,
+      high_five_image_url: high_five_image_url(),
       project_current_donation_goal_description: current_donation_goal.description,
       project_title: project.title,
       project_url: project |> url(),
@@ -62,7 +62,7 @@ defmodule CodeCorps.Emails.ReceiptEmail do
     "Your monthly donation to " <> project.title
   end
 
-  defp high_five_image_url, do: Enum.random(high_five_image_urls)
+  defp high_five_image_url, do: Enum.random(high_five_image_urls())
 
   defp high_five_image_urls, do: [
     "https://d3pgew4wbk2vb1.cloudfront.net/emails/images/[email protected]",

lib/code_corps/helpers/policy.ex

@@ -46,7 +46,7 @@ defmodule CodeCorps.Helpers.Policy do
 
   Returns `:string`
   """
-  @spec get_role(nil | OrganizationMembership.t | Changeset.t) :: String.t
+  @spec get_role(nil | OrganizationMembership.t | Changeset.t) :: String.t | nil
   def get_role(nil), do: nil
   def get_role(%OrganizationMembership{role: role}), do: role
   def get_role(%Changeset{} = changeset), do: changeset |> Changeset.get_field(:role)

lib/code_corps/repo.ex

@@ -1,4 +1,6 @@
 defmodule CodeCorps.Repo do
   use Ecto.Repo, otp_app: :code_corps
   use Scrivener, page_size: 10
+
+   @dialyzer {:nowarn_function, rollback: 1}
 end

lib/code_corps/services/donation_goals.ex

@@ -15,13 +15,21 @@ defmodule CodeCorps.Services.DonationGoalsService do
   alias CodeCorps.{DonationGoal, Project, Repo}
   alias Ecto.Multi
 
+  # Prevents warning for calling `Repo.transaction(multi)`.
+  # The warning was caused with how the function is internally
+  # implemented, so there's no way around it
+  # As we update Ecto, we should check if this is still necessary.
+  # Last check was Ecto 2.1.3
+  @dialyzer :no_opaque
+
   @doc """
   Creates the `CodeCorps.DonationGoal` by wrapping the following in a
   transaction:
 
   - Inserting the donation goal
   - Updating the sibling goals with `update_related_goals/1`
   """
+  @spec create(map) :: tuple
   def create(attributes) do
     changeset =
       %DonationGoal{}

lib/code_corps/services/markdown_renderer.ex

@@ -1,25 +1,27 @@
 defmodule CodeCorps.Services.MarkdownRendererService do
+  @moduledoc """
+  Used to render provided markdown into html using an external renderer package.
+  """
+
+  alias Ecto.Changeset
+
+  @spec render_markdown_to_html(Changeset.t, atom, atom) :: Changeset.t
+  def render_markdown_to_html(%Changeset{valid?: false} = changeset, _, _), do: changeset
   def render_markdown_to_html(changeset, source_field, destination_field) do
-    case changeset do
-      %Ecto.Changeset{valid?: false} ->
-        changeset
-      %Ecto.Changeset{changes: %{^source_field => _}} ->
-        changeset
-        |> do_render_markdown_to_html(source_field, destination_field)
-      _ ->
-        changeset
+    case Changeset.get_change(changeset, source_field) do
+      nil -> changeset
+      markdown -> markdown |> convert_into_html() |> put_into(changeset, destination_field)
     end
   end
-  defp do_render_markdown_to_html(changeset, source_field, destination_field) do
-    markdown =
-      changeset
-      |> Ecto.Changeset.get_change(source_field)
 
-    {html, _errors} =
-      markdown
-      |> Earmark.as_html()
+  @spec convert_into_html(String.t) :: String.t
+  defp convert_into_html(markdown) do
+    # `Earmark.as_html` will return `{html, errors}`
+    markdown |> Earmark.as_html() |> Tuple.to_list() |> List.first()
+  end
 
-    changeset
-    |> Ecto.Changeset.put_change(destination_field, html)
+  @spec put_into(String.t, Changeset.t, atom) :: Changeset.t
+  defp put_into(html, changeset, destination_field) do
+    Ecto.Changeset.put_change(changeset, destination_field, html)
   end
 end

lib/code_corps/services/user_service.ex

@@ -10,8 +10,14 @@ defmodule CodeCorps.Services.UserService do
 
   alias CodeCorps.{Repo, StripeConnectCustomer, StripePlatformCustomer, User}
   alias CodeCorps.StripeService.{StripeConnectCustomerService, StripePlatformCustomerService}
-  alias Ecto.Changeset
-  alias Ecto.Multi
+  alias Ecto.{Changeset, Multi}
+
+  # Prevents warning for calling `Repo.transaction(multi)`.
+  # The warning was caused with how the function is internally
+  # implemented, so there's no way around it
+  # As we update Ecto, we should check if this is still necessary.
+  # Last check was Ecto 2.1.3
+  @dialyzer :no_opaque
 
   @doc """
   Updates a `CodeCorps.User` record and, if necessary, associated

lib/code_corps/stripe_service/stripe_connect_account_service.ex

@@ -8,6 +8,13 @@ defmodule CodeCorps.StripeService.StripeConnectAccountService do
 
   @api Application.get_env(:code_corps, :stripe)
 
+  # Prevents warning for calling `Repo.transaction(multi)`.
+  # The warning was caused with how the function is internally
+  # implemented, so there's no way around it
+  # As we update Ecto, we should check if this is still necessary.
+  # Last check was Ecto 2.1.3
+  @dialyzer :no_opaque
+
   @doc """
   Used to create a remote `Stripe.Account` record as well as an associated local
   `StripeConnectAccount` record.

lib/code_corps/stripe_service/stripe_connect_external_account_service.ex

@@ -1,9 +1,12 @@
 defmodule CodeCorps.StripeService.StripeConnectExternalAccountService do
+  @moduledoc """
+  Used to perform actions on a `StripeConnectExternalAccount` record while
+  propagating to and from the associated `Stripe.ExternalAccount` record.
+  """
   alias CodeCorps.{Repo, StripeConnectAccount, StripeExternalAccount}
   alias CodeCorps.StripeService.Adapters.StripeExternalAccountAdapter
 
-  @api Application.get_env(:code_corps, :stripe)
-
+  @spec create(Stripe.ExternalAccount.t, StripeConnectAccount.t) :: {:ok, StripeExternalAccount.t}
   def create(%Stripe.ExternalAccount{} = external_account, %StripeConnectAccount{} = connect_account) do
     with  {:ok, params} <- StripeExternalAccountAdapter.to_params(external_account, connect_account) do
       %StripeExternalAccount{} |> StripeExternalAccount.changeset(params) |> Repo.insert

lib/code_corps/stripe_service/stripe_connect_plan.ex

@@ -1,4 +1,10 @@
 defmodule CodeCorps.StripeService.StripeConnectPlanService do
+  @moduledoc """
+  Used to perform actions on `StripeConnectPlan` records
+  while at the same time propagating to and from associated `Stripe.Plan`
+  records.
+  """
+
   alias CodeCorps.{Project, Repo, StripeConnectPlan}
   alias CodeCorps.StripeService.Adapters.StripeConnectPlanAdapter
   alias CodeCorps.StripeService.Validators.ProjectCanEnableDonations
@@ -8,17 +14,15 @@ defmodule CodeCorps.StripeService.StripeConnectPlanService do
   @doc """
   Creates a new `Stripe.Plan` record on Stripe API, as well as an associated local
   `StripeConnectPlan` record
-
-  # Possible return values
-
-  - `{:ok, %StripeConnectPlan{}}` - the created record.
-  - `{:error, %Ecto.Changeset{}}` - the record was not created due to validation issues.
-  - `{:error, :project_not_ready}` - the associated project does not meed the prerequisites for creating a plan.
-  - `{:error, %Stripe.APIErrorResponse{}}` - there was a problem with the stripe request
-  - `{:error, :not_found}` - one of the associated records was not found
   """
+  @spec create(map) :: {:ok, StripeConnectPlan.t} |
+                       {:error, Ecto.Changeset.t} |
+                       {:error, Stripe.APIErrorResponse.t} |
+                       {:error, :project_not_ready} |
+                       {:error, :not_found}
   def create(%{"project_id" => project_id} = attributes) do
-    with {:ok, %Project{} = project} <- get_project(project_id) |> ProjectCanEnableDonations.validate,
+    with {:ok, %Project{} = project} <- get_project(project_id),
+         {:ok, %Project{}} <- ProjectCanEnableDonations.validate(project),
          %{} = create_attributes <- get_create_attributes(project_id),
          connect_account_id <- project.organization.stripe_connect_account.id_from_stripe,
          {:ok, plan} <- @api.Plan.create(create_attributes, connect_account: connect_account_id),
@@ -28,12 +32,10 @@ defmodule CodeCorps.StripeService.StripeConnectPlanService do
       |> StripeConnectPlan.create_changeset(params)
       |> Repo.insert
     else
-      nil -> {:error, :not_found}
       failure -> failure
     end
   end
 
-  @spec get_create_attributes(binary) :: map
   defp get_create_attributes(project_id) do
     %{
       amount: 1, # in cents
@@ -46,8 +48,11 @@ defmodule CodeCorps.StripeService.StripeConnectPlanService do
   end
 
   defp get_project(project_id) do
-    Project
-    |> Repo.get(project_id)
-    |> Repo.preload([:donation_goals, {:organization, :stripe_connect_account}, :stripe_connect_plan])
+    preloads = [:donation_goals, {:organization, :stripe_connect_account}, :stripe_connect_plan]
+
+    case Project |> Repo.get(project_id) |> Repo.preload(preloads) do
+      nil -> {:error, :not_found}
+      record -> {:ok, record}
+    end
   end
 end