From 22093b8ca4ed1a1f8224833dd11baf1da86fd77a Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Sun, 21 Jan 2018 19:43:16 +0100 Subject: [PATCH] Fix some warnings Signed-off-by: Thomas Citharel fix error Signed-off-by: Thomas Citharel Fix tests and warnings Move process_geom/2 to addresses where it belongs Create a special error view for invalid requests Signed-off-by: Thomas Citharel credo fix Signed-off-by: Thomas Citharel --- lib/eventos/addresses/addresses.ex | 36 ++++++++++++++++ .../controllers/address_controller.ex | 41 +++++++------------ .../controllers/event_controller.ex | 10 ++--- .../controllers/event_request_controller.ex | 2 +- .../controllers/fallback_controller.ex | 6 +++ lib/eventos_web/router.ex | 2 +- lib/eventos_web/views/error_view.ex | 4 ++ test/eventos/addresses/addresses_test.exs | 10 +++++ 8 files changed, 77 insertions(+), 34 deletions(-) diff --git a/lib/eventos/addresses/addresses.ex b/lib/eventos/addresses/addresses.ex index 84c119e6c..0caa488dd 100644 --- a/lib/eventos/addresses/addresses.ex +++ b/lib/eventos/addresses/addresses.ex @@ -8,6 +8,10 @@ defmodule Eventos.Addresses do alias Eventos.Addresses.Address + import Logger + + @geom_types [:point] + @doc """ Returns the list of addresses. @@ -101,4 +105,36 @@ defmodule Eventos.Addresses do def change_address(%Address{} = address) do Address.changeset(address, %{}) end + + @doc """ + Processes raw geo data informations and return a `Geo` geometry which can be one of `Geo.Point`. + """ + def process_geom(%{"type" => type_input, "data" => data}) do + type = + if !is_atom(type_input) && type_input != nil do + try do + String.to_existing_atom(type_input) + rescue + e in ArgumentError -> + Logger.error("#{type_input} is not an existing atom : #{inspect e}") + nil + end + else + type_input + end + + if Enum.member?(@geom_types, type) do + case type do + :point -> + {:ok, %Geo.Point{coordinates: {data["latitude"], data["longitude"]}, srid: 4326}} + end + else + {:error, nil} + end + end + + @doc false + def process_geom(nil) do + {:error, nil} + end end diff --git a/lib/eventos_web/controllers/address_controller.ex b/lib/eventos_web/controllers/address_controller.ex index 395795000..51c12762a 100644 --- a/lib/eventos_web/controllers/address_controller.ex +++ b/lib/eventos_web/controllers/address_controller.ex @@ -16,43 +16,30 @@ defmodule EventosWeb.AddressController do end def create(conn, %{"address" => address_params}) do - address_params = %{address_params | "geom" => process_geom(address_params["geom"])} - with {:ok, %Address{} = address} <- Addresses.create_address(address_params) do - conn - |> put_status(:created) - |> put_resp_header("location", address_path(conn, :show, address)) - |> render("show.json", address: address) + with {:ok, geom} <- Addresses.process_geom(address_params["geom"]) do + address_params = %{address_params | "geom" => geom} + with {:ok, %Address{} = address} <- Addresses.create_address(address_params) do + conn + |> put_status(:created) + |> put_resp_header("location", address_path(conn, :show, address)) + |> render("show.json", address: address) + end end end - def process_geom(%{"type" => type, "data" => data}) do - types = [:point] - unless is_atom(type) do - type = String.to_existing_atom(type) - end - case type do - :point -> - %Geo.Point{coordinates: {data["latitude"], data["longitude"]}, srid: 4326} - nil -> - nil - end - end - - def process_geom(nil) do - nil - end - def show(conn, %{"id" => id}) do address = Addresses.get_address!(id) render(conn, "show.json", address: address) end def update(conn, %{"id" => id, "address" => address_params}) do - address = Addresses.get_address!(id) - address_params = %{address_params | "geom" => process_geom(address_params["geom"])} + with {:ok, geom} <- Addresses.process_geom(address_params["geom"]) do + address = Addresses.get_address!(id) + address_params = %{address_params | "geom" => geom} - with {:ok, %Address{} = address} <- Addresses.update_address(address, address_params) do - render(conn, "show.json", address: address) + with {:ok, %Address{} = address} <- Addresses.update_address(address, address_params) do + render(conn, "show.json", address: address) + end end end diff --git a/lib/eventos_web/controllers/event_controller.ex b/lib/eventos_web/controllers/event_controller.ex index 37c791c5e..1da9ba84a 100644 --- a/lib/eventos_web/controllers/event_controller.ex +++ b/lib/eventos_web/controllers/event_controller.ex @@ -7,6 +7,7 @@ defmodule EventosWeb.EventController do alias Eventos.Events alias Eventos.Events.Event alias Eventos.Export.ICalendar + alias Eventos.Addresses action_fallback EventosWeb.FallbackController @@ -26,12 +27,11 @@ defmodule EventosWeb.EventController do end defp process_address(address) do - geom = EventosWeb.AddressController.process_geom(address["geom"]) - case geom do - nil -> - address - _ -> + case Addresses.process_geom(address["geom"]) do + {:ok, geom} -> %{address | "geom" => geom} + _ -> + address end end diff --git a/lib/eventos_web/controllers/event_request_controller.ex b/lib/eventos_web/controllers/event_request_controller.ex index ce541d14b..b8d30fb24 100644 --- a/lib/eventos_web/controllers/event_request_controller.ex +++ b/lib/eventos_web/controllers/event_request_controller.ex @@ -5,7 +5,7 @@ defmodule EventosWeb.EventRequestController do use EventosWeb, :controller alias Eventos.Events - alias Eventos.Events.{Event, Request} + alias Eventos.Events.Request action_fallback EventosWeb.FallbackController diff --git a/lib/eventos_web/controllers/fallback_controller.ex b/lib/eventos_web/controllers/fallback_controller.ex index e7b949f99..863db7029 100644 --- a/lib/eventos_web/controllers/fallback_controller.ex +++ b/lib/eventos_web/controllers/fallback_controller.ex @@ -12,6 +12,12 @@ defmodule EventosWeb.FallbackController do |> render(EventosWeb.ChangesetView, "error.json", changeset: changeset) end + def call(conn, {:error, nil}) do + conn + |> put_status(:unprocessable_entity) + |> render(EventosWeb.ErrorView, "invalid_request.json") + end + def call(conn, {:error, :not_found}) do conn |> put_status(:not_found) diff --git a/lib/eventos_web/router.ex b/lib/eventos_web/router.ex index d46559860..40e40dcda 100644 --- a/lib/eventos_web/router.ex +++ b/lib/eventos_web/router.ex @@ -49,7 +49,7 @@ defmodule EventosWeb.Router do resources "/accounts", AccountController, except: [:new, :edit] resources "/events", EventController post "/events/:id/request", EventRequestController, :create_for_event - resources "/participant", ParticipantController + resources "/participants", ParticipantController resources "/requests", EventRequestController resources "/groups", GroupController, except: [:index, :show] post "/groups/:id/request", GroupRequestController, :create_for_group diff --git a/lib/eventos_web/views/error_view.ex b/lib/eventos_web/views/error_view.ex index cc43cda19..7e8c57e00 100644 --- a/lib/eventos_web/views/error_view.ex +++ b/lib/eventos_web/views/error_view.ex @@ -8,6 +8,10 @@ defmodule EventosWeb.ErrorView do "Page not found" end + def render("invalid_request.json", _assigns) do + %{errors: "Invalid request"} + end + def render("500.html", _assigns) do "Internal server error" end diff --git a/test/eventos/addresses/addresses_test.exs b/test/eventos/addresses/addresses_test.exs index 9bf30850d..74833e9d1 100644 --- a/test/eventos/addresses/addresses_test.exs +++ b/test/eventos/addresses/addresses_test.exs @@ -73,5 +73,15 @@ defmodule Eventos.AddressesTest do address = address_fixture() assert %Ecto.Changeset{} = Addresses.change_address(address) end + + test "process_geom/2 with valid data returns a Point element" do + attrs = %{"type" => "point", "data" => %{"latitude" => 10, "longitude" => -10}} + assert {:ok, %Geo.Point{}} = Addresses.process_geom(attrs) + end + + test "process_geom/2 with invalid data returns nil" do + attrs = %{"type" => "linfdfsfe", "data" => %{"latitude" => 10, "longitude" => -10}} + assert {:error, nil} = Addresses.process_geom(attrs) + end end end