Merge pull request #1016 from code-corps/981-add-unique-constraint-to-user-github-id

Add unique constraints for GitHub users

Author: joshsmith

lib/code_corps/accounts/changesets.ex

@@ -3,6 +3,8 @@ defmodule CodeCorps.Accounts.Changesets do
   Changesets for Code Corps accounts.
   """
 
+  import CodeCorpsWeb.Gettext
+
   alias CodeCorps.GitHub.Adapters
   alias CodeCorps.Helpers.RandomIconColor
   alias Ecto.Changeset
@@ -15,9 +17,10 @@ defmodule CodeCorps.Accounts.Changesets do
     struct
     |> Changeset.change(params |> Adapters.User.from_github_user())
     |> Changeset.put_change(:sign_up_context, "github")
-    |> Changeset.unique_constraint(:email)
     |> Changeset.validate_inclusion(:type, ["bot", "user"])
     |> RandomIconColor.generate_icon_color(:default_color)
+    |> Changeset.unique_constraint(:email)
+    |> unique_github_constraint()
   end
 
   @doc ~S"""
@@ -28,8 +31,9 @@ defmodule CodeCorps.Accounts.Changesets do
     struct
     |> Changeset.cast(params, [:github_auth_token, :github_avatar_url, :github_id, :github_username, :type])
     |> ensure_email_without_overwriting(params)
-    |> Changeset.unique_constraint(:email)
     |> Changeset.validate_required([:github_auth_token, :github_avatar_url, :github_id, :github_username, :type])
+    |> Changeset.unique_constraint(:email)
+    |> unique_github_constraint()
   end
 
   @spec ensure_email_without_overwriting(Changeset.t, map) :: Changeset.t
@@ -40,4 +44,9 @@ defmodule CodeCorps.Accounts.Changesets do
     end
   end
   defp ensure_email_without_overwriting(%Changeset{} = changeset, _params), do: changeset
+
+  defp unique_github_constraint(struct) do
+    struct
+    |> Changeset.unique_constraint(:github_id, message: dgettext("errors", "account is already connected to someone else"))
+  end
 end

lib/code_corps/github/user.ex

@@ -33,13 +33,15 @@ defmodule CodeCorps.GitHub.User do
     do
        user |> do_connect(user_payload, access_token)
     else
+      {:ok, %{"error" => _} = error} -> handle_oauth_error(error)
       {:error, error} -> {:error, error}
     end
   end
 
   @spec do_connect(User.t, map, String.t) :: {:ok, User.t} | {:error, Changeset.t}
   defp do_connect(%User{} = user, %{} = user_payload, access_token)
     when is_binary(access_token) do
+
     Accounts.update_from_github_oauth(user, user_payload, access_token)
   end
 
@@ -53,4 +55,8 @@ defmodule CodeCorps.GitHub.User do
       {:error, error} -> {:error, error}
     end
   end
+
+  defp handle_oauth_error(%{"error_description" => message, "error_uri" => documentation_url}) do
+    {:error, GitHub.APIError.new({401, %{"message" => message, "documentation_url" => documentation_url}})}
+  end
 end

lib/code_corps_web/controllers/fallback_controller.ex

@@ -18,7 +18,7 @@ defmodule CodeCorpsWeb.FallbackController do
   def call(%Conn{} = conn, {:error, %Changeset{} = changeset}) do
     conn
     |> put_status(:unprocessable_entity)
-    |> render(:errors, data: changeset)
+    |> render(CodeCorpsWeb.ChangesetView, "422.json", changeset: changeset)
   end
   def call(%Conn{} = conn, {:error, :not_authorized}) do
     conn
@@ -41,4 +41,10 @@ defmodule CodeCorpsWeb.FallbackController do
     |> put_status(500)
     |> render(CodeCorpsWeb.ErrorView, "500.json", message: "An unknown error occurred with Stripe's API.")
   end
+  def call(%Conn{} = conn, {:error, %CodeCorps.GitHub.APIError{message: message}}) do
+    Logger.info message
+    conn
+    |> put_status(500)
+    |> render(CodeCorpsWeb.ErrorView, "github-error.json", message: message)
+  end
 end

lib/code_corps_web/controllers/user_controller.ex

@@ -49,11 +49,6 @@ defmodule CodeCorpsWeb.UserController do
     with {:ok, user} <- GitHub.User.connect(current_user, code, state)
     do
       conn |> render("show.json-api", data: user)
-    else
-      {:error, _error} ->
-        conn
-        |> put_status(:internal_server_error)
-        |> render(CodeCorpsWeb.ErrorView, "500.json-api")
     end
   end
 

lib/code_corps_web/templates/layout/app.html.eex

@@ -1,19 +1,23 @@
 defmodule CodeCorpsWeb.ChangesetView do
   use CodeCorpsWeb, :view
+  use JaSerializer.PhoenixView
+
+  import CodeCorpsWeb.Gettext
 
   alias Ecto.Changeset
+  alias JaSerializer.Formatter.Utils
 
   @doc """
   Traverses and translates changeset errors.
 
   See `Ecto.Changeset.traverse_errors/2` and
   `CodeCorpsWeb.ErrorHelpers.translate_error/1` for more details.
   """
-  def translate_errors(changeset) do
+  def translate_errors(%Ecto.Changeset{} = changeset) do
     errors =
       changeset
       |> Changeset.traverse_errors(&translate_error/1)
-      |> format_errors
+      |> format_errors()
     errors
   end
 
@@ -32,18 +36,30 @@ defmodule CodeCorpsWeb.ChangesetView do
 
   def create_error(attribute, message) do
     %{
-      id: "VALIDATION_ERROR",
+      detail: format_detail(attribute, message),
+      title: message,
       source: %{
         pointer: "data/attributes/#{attribute}"
       },
-      detail: message,
-      status: 422
+      status: "422"
     }
   end
 
-  def render("error.json-api", %{changeset: changeset}) do
+  def render("422.json", %{changeset: changeset}) do
     # When encoded, the changeset returns its errors
     # as a JSON object. So we just pass it forward.
-    %{errors: translate_errors(changeset)}
+    %{
+      errors: translate_errors(changeset),
+      jsonapi: %{
+        version: "1.0"
+      }
+    }
   end
+
+  defp format_detail(attribute, message) do
+    "#{attribute |> Utils.humanize |> translate_attribute} #{message}"
+  end
+
+  defp translate_attribute("Github"), do: dgettext("errors", "Github")
+  defp translate_attribute(attribute), do: attribute
 end

lib/code_corps_web/templates/page/index.html.eex

@@ -2,29 +2,29 @@ defmodule CodeCorpsWeb.ErrorView do
   use CodeCorpsWeb, :view
   use JaSerializer.PhoenixView
 
-  def render("stripe-400.json-api", _assigns) do
+  def render("404.json-api", _assigns) do
     %{
-      id: "INVALID_GRANT",
-      title: "This authorization code has already been used. All tokens issued with this code have been revoked.",
-      status: 400
+      title: "404 Not Found",
+      detail: "404 Not Found",
+      status: "404"
     }
     |> JaSerializer.ErrorSerializer.format
   end
 
-  def render("404.json-api", _assigns) do
+  def render("500.json-api", _assigns) do
     %{
-      id: "NOT_FOUND",
-      title: "404 Resource not found",
-      status: 404
+      title: "500 Internal Server Error",
+      detail: "500 Internal Server Error",
+      status: "500"
     }
     |> JaSerializer.ErrorSerializer.format
   end
 
-  def render("500.json-api", _assigns) do
+  def render("github-error.json", %{message: message}) do
     %{
-      id: "INTERNAL_SERVER_ERROR",
-      title: "500 Internal server error",
-      status: 500
+      title: "GitHub API error",
+      detail: message,
+      status: "500"
     }
     |> JaSerializer.ErrorSerializer.format
   end

lib/code_corps_web/views/changeset_view.ex

@@ -0,0 +1,11 @@
+## This file is a PO Template file.
+##
+## `msgid`s here are often extracted from source code.
+## Add new translations manually only if they're dynamic
+## translations that can't be statically extracted.
+##
+## Run `mix gettext.extract` to bring this file up to
+## date. Leave `msgstr`s empty as changing them here as no
+## effect: edit them in PO (`.po`) files instead.
+msgid ""
+msgstr ""

lib/code_corps_web/views/error_view.ex

@@ -0,0 +1,11 @@
+## `msgid`s in this file come from POT (.pot) files.
+##
+## Do not add, change, or remove `msgid`s manually here as
+## they're tied to the ones in the corresponding POT file
+## (with the same domain).
+##
+## Use `mix gettext.extract --merge` or `mix gettext.merge`
+## to merge POT files into PO files.
+msgid ""
+msgstr ""
+"Language: en\n"