Skip to content

Commit

Permalink
Merge pull request #1243 from code-corps/fix-issues
Browse files Browse the repository at this point in the history
Upgrade deps, add missing specs, fix some dialyzer issues
  • Loading branch information
joshsmith committed Nov 24, 2017
2 parents 0ad379b + ed4b5db commit f895eb9
Show file tree
Hide file tree
Showing 13 changed files with 90 additions and 111 deletions.
3 changes: 2 additions & 1 deletion lib/code_corps/emails/base_email.ex
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
defmodule CodeCorps.Emails.BaseEmail do
import Bamboo.Email
import Bamboo.Email, only: [from: 2, new_email: 0]

@spec create :: Bamboo.Email.t
def create do
new_email()
|> from("Code Corps<team@codecorps.org>")
Expand Down
9 changes: 6 additions & 3 deletions lib/code_corps/emails/forgot_password_email.ex
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
defmodule CodeCorps.Emails.ForgotPasswordEmail do
import Bamboo.Email
import Bamboo.Email, only: [to: 2]
import Bamboo.PostmarkHelper

alias CodeCorps.{Emails.BaseEmail, WebClient}
alias CodeCorps.{Emails.BaseEmail, User, WebClient}

def create(user, token) do
@spec create(User.t, String.t) :: Bamboo.Email.t
def create(%User{} = user, token) do
BaseEmail.create
|> to(user.email)
|> template(template_id(), %{link: link(token)})
end

@spec template_id :: String.t
defp template_id, do: Application.get_env(:code_corps, :postmark_forgot_password_template)

@spec link(String.t) :: String.t
defp link(token) do
WebClient.url()
|> URI.merge("password/reset?token=#{token}")
Expand Down
56 changes: 31 additions & 25 deletions lib/code_corps/emails/organization_invite_email.ex
Original file line number Diff line number Diff line change
@@ -1,33 +1,39 @@
defmodule CodeCorps.Emails.OrganizationInviteEmail do
import Bamboo.Email
import Bamboo.PostmarkHelper
import Bamboo.Email, only: [to: 2]
import Bamboo.PostmarkHelper

alias CodeCorps.{Emails.BaseEmail, OrganizationInvite, WebClient}
alias CodeCorps.{Emails.BaseEmail, OrganizationInvite, WebClient}

def create(%OrganizationInvite{} = invite) do
BaseEmail.create
|> to(invite.email)
|> template(template_id(), build_model(invite))
end
@spec create(OrganizationInvite.t) :: Bamboo.Email.t
def create(%OrganizationInvite{} = invite) do
BaseEmail.create
|> to(invite.email)
|> template(template_id(), build_model(invite))
end

defp build_model(%OrganizationInvite{} = invite) do
%{
organization_name: invite.organization_name,
invite_url: invite_url(invite.code, invite.organization_name),
subject: "Create your first project on Code Corps"
}
end
@spec build_model(OrganizationInvite.t) :: map
defp build_model(%OrganizationInvite{} = invite) do
%{
organization_name: invite.organization_name,
invite_url: invite_url(invite.code, invite.organization_name),
subject: "Create your first project on Code Corps"
}
end

defp invite_url(code, organization_name) do
WebClient.url()
|> URI.merge("/invites/organization" <> "?" <> set_params(code, organization_name))
|> URI.to_string
end
@spec invite_url(String.t, String.t) :: String.t
defp invite_url(code, organization_name) do
query_params = set_params(code, organization_name)
WebClient.url()
|> URI.merge("/invites/organization" <> "?" <> query_params)
|> URI.to_string
end

defp set_params(code, organization_name) do
%{code: code, organization_name: organization_name }
|> URI.encode_query
end
@spec set_params(String.t, String.t) :: binary
defp set_params(code, organization_name) do
%{code: code, organization_name: organization_name}
|> URI.encode_query
end

defp template_id, do: Application.get_env(:code_corps, :organization_invite_email_template)
@spec template_id :: String.t
defp template_id, do: Application.get_env(:code_corps, :organization_invite_email_template)
end
7 changes: 6 additions & 1 deletion lib/code_corps/emails/project_user_acceptance_email.ex
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
defmodule CodeCorps.Emails.ProjectUserAcceptanceEmail do
import Bamboo.Email
import Bamboo.Email, only: [to: 2]
import Bamboo.PostmarkHelper

alias CodeCorps.{Project, ProjectUser, Repo, User, WebClient}
alias CodeCorps.Emails.BaseEmail
alias CodeCorps.Presenters.ImagePresenter

@spec create(ProjectUser.t) :: Bamboo.Email.t
def create(%ProjectUser{project: project, user: user}) do
BaseEmail.create
|> to(user.email)
|> template(template_id(), build_model(project, user))
end

@spec build_model(Project.t, User.t) :: map
defp build_model(%Project{} = project, %User{} = user) do
%{
project_logo_url: ImagePresenter.large(project),
Expand All @@ -23,13 +25,16 @@ defmodule CodeCorps.Emails.ProjectUserAcceptanceEmail do
}
end

@spec preload(Project.t) :: Project.t
defp preload(%Project{} = project), do: project |> Repo.preload(:organization)

@spec url(Project.t) :: String.t
defp url(project) do
WebClient.url()
|> URI.merge(project.organization.slug <> "/" <> project.slug)
|> URI.to_string
end

@spec template_id :: String.t
defp template_id, do: Application.get_env(:code_corps, :postmark_project_acceptance_template)
end
20 changes: 15 additions & 5 deletions lib/code_corps/emails/receipt_email.ex
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
defmodule CodeCorps.Emails.ReceiptEmail do
import Bamboo.Email
import Bamboo.Email, only: [to: 2]
import Bamboo.PostmarkHelper

alias CodeCorps.Emails.BaseEmail
alias CodeCorps.{DonationGoal, Project, Repo, StripeConnectCharge, StripeConnectSubscription, WebClient}

@spec create(StripeConnectCharge.t, Stripe.Invoice.t) :: Bamboo.Email.t
def create(%StripeConnectCharge{} = charge, %Stripe.Invoice{} = invoice) do
with %StripeConnectCharge{} = charge <- preload(charge),
%Project{} = project <- get_project(invoice.subscription),
Expand All @@ -20,10 +21,10 @@ defmodule CodeCorps.Emails.ReceiptEmail do
end
end

defp preload(%StripeConnectCharge{} = charge) do
Repo.preload(charge, :user)
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
subscription.stripe_connect_plan.project
Expand All @@ -32,19 +33,22 @@ defmodule CodeCorps.Emails.ReceiptEmail do
end
end

@spec get_subscription(String.t) :: Subscription.t | nil
defp get_subscription(subscription_id_from_stripe) do
StripeConnectSubscription
|> Repo.get_by(id_from_stripe: subscription_id_from_stripe)
|> Repo.preload(stripe_connect_plan: [project: :organization])
end

@spec get_current_donation_goal(Project.t) :: DonationGoal.t | {:error, :donation_goal_not_found}
defp get_current_donation_goal(project) do
case Repo.get_by(DonationGoal, current: true, project_id: project.id) do
nil -> {:error, :donation_goal_not_found}
donation_goal -> {:ok, donation_goal}
end
end

@spec build_model(StripeConnectCharge.t, Project.t, DonationGoal.t) :: map
defp build_model(charge, project, current_donation_goal) do
%{
charge_amount: charge.amount |> format_amount(),
Expand All @@ -58,12 +62,15 @@ defmodule CodeCorps.Emails.ReceiptEmail do
}
end

@spec build_subject_line(Project.t) :: String.t
defp build_subject_line(project) do
"Your monthly donation to " <> project.title
end

@spec high_five_image_url :: String.t
defp high_five_image_url, do: Enum.random(high_five_image_urls())

@spec high_five_image_urls :: list(String.t)
defp high_five_image_urls, do: [
"https://d3pgew4wbk2vb1.cloudfront.net/emails/images/emoji-1f64c-1f3fb@2x.png",
"https://d3pgew4wbk2vb1.cloudfront.net/emails/images/emoji-1f64c-1f3fc@2x.png",
Expand All @@ -72,15 +79,18 @@ defmodule CodeCorps.Emails.ReceiptEmail do
"https://d3pgew4wbk2vb1.cloudfront.net/emails/images/emoji-1f64c-1f3ff@2x.png"
]

@spec format_amount(float) :: String.t
defp format_amount(amount) do
Money.to_string(Money.new(amount, :USD))
amount |> Money.new(:USD) |> Money.to_string()
end

@spec url(Project.t) :: String.t
defp url(project) do
WebClient.url()
|> URI.merge(project.organization.slug <> "/" <> project.slug)
|> URI.to_string
end

@spec template_id :: String.t
defp template_id, do: Application.get_env(:code_corps, :postmark_receipt_template)
end
7 changes: 0 additions & 7 deletions lib/code_corps/services/donation_goals_service.ex
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,6 @@ 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:
Expand Down
21 changes: 6 additions & 15 deletions lib/code_corps/services/user_service.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,34 +5,25 @@ defmodule CodeCorps.Services.UserService do
When operations happen on `CodeCorps.User`, we need to make sure changes
are propagated to related records, ex., `CodeCorps.StripePlatformCustomer` and
`CodeCorps.StripeConnectCustomer`
"""

alias CodeCorps.{Repo, StripeConnectCustomer, StripePlatformCustomer, User}
alias CodeCorps.StripeService.{StripeConnectCustomerService, StripePlatformCustomerService}
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
`CodeCorps.StripePlatformCustomer` and `CodeCorps.StripeConnectCustomer` records.
These related records inherit the email field from the user,
so they need to be kept in sync, both locally, and on the Stripe platform.
Returns one of
* `{:ok, %CodeCorps.User{}, nil, nil}`
* `{:ok, %CodeCorps.User{}, %CodeCorps.StripePlatformCustomer{}, nil}`
* `{:ok, %CodeCorps.User{}, %CodeCorps.StripePlatformCustomer{}, %CodeCorps.StripeConnectCustomer{}}`
* `{:error, %Ecto.Changeset{}}`
* `{:error, :unhandled}`
Returns one of:
- `{:ok, %CodeCorps.User{}, nil, nil}`
- `{:ok, %CodeCorps.User{}, %CodeCorps.StripePlatformCustomer{}, nil}`
- `{:ok, %CodeCorps.User{}, %CodeCorps.StripePlatformCustomer{}, %CodeCorps.StripeConnectCustomer{}}`
- `{:error, %Ecto.Changeset{}}`
- `{:error, :unhandled}`
"""
def update(%User{} = user, attributes) do
changeset = user |> User.update_changeset(attributes)
Expand Down
14 changes: 3 additions & 11 deletions lib/code_corps/stripe_service/adapters/stripe_platform_card.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ defmodule CodeCorps.StripeService.Adapters.StripePlatformCardAdapter do

@stripe_attributes [:brand, :customer, :cvc_check, :exp_month, :exp_year, :id, :last4, :name, :user_id]

@spec to_params(Stripe.Card.t, map) :: {:ok, map}
def to_params(%Stripe.Card{} = stripe_card, %{} = attributes) do
result =
stripe_card
Expand All @@ -18,19 +19,10 @@ defmodule CodeCorps.StripeService.Adapters.StripePlatformCardAdapter do

@non_stripe_attributes ["user_id"]

@spec add_non_stripe_attributes(map, map) :: map
defp add_non_stripe_attributes(%{} = params, %{} = attributes) do
attributes
|> get_non_stripe_attributes
|> add_to(params)
end

defp get_non_stripe_attributes(%{} = attributes) do
attributes
|> Map.take(@non_stripe_attributes)
end

defp add_to(%{} = attributes, %{} = params) do
params
|> Map.merge(attributes)
|> Map.merge(params)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,6 @@ 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.
Expand Down
7 changes: 0 additions & 7 deletions lib/code_corps/stripe_service/stripe_platform_card.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,6 @@ defmodule CodeCorps.StripeService.StripePlatformCardService 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

def create(%{"stripe_token" => stripe_token, "user_id" => user_id} = attributes) do
with %StripePlatformCustomer{} = customer <- StripePlatformCustomer |> CodeCorps.Repo.get_by(user_id: user_id),
{:ok, %Stripe.Card{} = card} <- @api.Card.create(%{customer: customer.id_from_stripe, source: stripe_token}),
Expand Down
7 changes: 0 additions & 7 deletions lib/code_corps/stripe_service/stripe_platform_customer.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,6 @@ defmodule CodeCorps.StripeService.StripePlatformCustomerService 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

def create(attributes) do
with stripe_attributes <- build_stripe_attributes(attributes),
{:ok, customer} <- @api.Customer.create(stripe_attributes),
Expand Down
12 changes: 6 additions & 6 deletions mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -49,26 +49,26 @@ defmodule CodeCorps.Mixfile do
{:phoenix_ecto, "~> 3.3.0"},
{:postgrex, ">= 0.0.0"},
{:phoenix_html, "~> 2.10.3"},
{:phoenix_live_reload, "~> 1.0.8", only: :dev},
{:gettext, "~> 0.12"},
{:phoenix_live_reload, "~> 1.1", only: :dev},
{:gettext, "~> 0.13"},
{:cowboy, "~> 1.0"},
{:benchfella, "~> 0.3.0", only: :dev},
{:bypass, "~> 0.8.1", only: :test},
{:cloudex, "~> 0.2.2"},
{:comeonin, "~> 3.1"},
{:comeonin, "~> 3.2"},
{:corsica, "~> 1.0"}, # CORS
{:credo, "~> 0.8", only: [:dev, :test]}, # Code style suggestions
{:earmark, "~> 1.2"}, # Markdown rendering
{:ecto_ordered, "0.2.0-beta1"},
{:ex_aws, "~> 1.0"}, # Amazon AWS
{:ex_aws, "~> 1.1"}, # Amazon AWS
{:excoveralls, "~> 0.7", only: :test}, # Test coverage
{:ex_doc, "~> 0.17", only: [:dev, :test]},
{:ex_machina, "~> 2.0", only: :test}, # test factories
{:guardian, "~> 0.14.5"}, # Authentication (JWT)
{:guardian, "~> 0.14"}, # Authentication (JWT)
{:hackney, ">= 1.4.4"},
{:httpoison, "~> 0.13"},
{:inch_ex, "~> 0.5", only: [:dev, :test]}, # Inch CI
{:inflex, "~> 1.8.1"},
{:inflex, "~> 1.9"},
{:ja_serializer, "~> 0.12"}, # JSON API
{:joken, "~> 1.5"}, # JWT encoding
{:money, "~> 1.2.1"},
Expand Down
Loading

0 comments on commit f895eb9

Please sign in to comment.