fix: Adding id validation on update tag. Adding tests. #256

lib/app/tag.ex

@@ -5,7 +5,7 @@ defmodule App.Tag do
   alias App.{Item, ItemTag, Repo}
   alias __MODULE__
 
-  @derive {Jason.Encoder, only: [:text, :person_id, :color]}
+  @derive {Jason.Encoder, only: [:id, :text, :person_id, :color]}
   schema "tags" do
     field :color, :string
     field :person_id, :integer
@@ -79,6 +79,8 @@ defmodule App.Tag do
 
   def get_tag!(id), do: Repo.get!(Tag, id)
 
+  def get_tag(id), do: Repo.get(Tag, id)
+
   def list_person_tags(person_id) do
     Tag
     |> where(person_id: ^person_id)

lib/app_web/controllers/api/tag_controller.ex

@@ -7,7 +7,7 @@ defmodule AppWeb.API.TagController do
     case Integer.parse(id) do
       # ID is an integer
       {id, _float} ->
-        case Tag.get_tag!(id) do
+        case Tag.get_tag(id) do
           nil ->
             errors = %{
               code: 404,
@@ -40,8 +40,6 @@ defmodule AppWeb.API.TagController do
       color: Map.get(params, "color", App.Color.random())
     }
 
-    dbg(attrs)
-
     case Tag.create_tag(attrs) do
       # Successfully creates tag
       {:ok, tag} ->
@@ -62,21 +60,32 @@ defmodule AppWeb.API.TagController do
   def update(conn, params) do
     id = Map.get(params, "id")
 
-    tag = Tag.get_tag!(id)
+    # Get tag with the ID
+    case Tag.get_tag(id) do
+      nil ->
+        errors = %{
+          code: 404,
+          message: "No tag found with the given \'id\'."
+        }
 
-    case Tag.update_tag(tag, params) do
-      # Successfully updates tag
-      {:ok, tag} ->
-        json(conn, tag)
+        json(conn |> put_status(404), errors)
 
-      # Error creating tag
-      {:error, %Ecto.Changeset{} = changeset} ->
-        errors = make_changeset_errors_readable(changeset)
+      # If tag is found, try to update it
+      tag ->
+        case Tag.update_tag(tag, params) do
+          # Successfully updates tag
+          {:ok, tag} ->
+            json(conn, tag)
 
-        json(
-          conn |> put_status(400),
-          errors
-        )
+          # Error creating tag
+          {:error, %Ecto.Changeset{} = changeset} ->
+            errors = make_changeset_errors_readable(changeset)
+
+            json(
+              conn |> put_status(400),
+              errors
+            )
+        end
     end
   end
 

lib/app_web/router.ex

@@ -34,7 +34,7 @@ defmodule AppWeb.Router do
     resources "/tags", TagController, except: [:show]
   end
 
-  scope "/api", AppWeb do
+  scope "/api", AppWeb, as: :api do
     pipe_through [:api, :authOptional]
 
     resources "/items", API.ItemController, only: [:create, :update, :show]

test/app_web/controllers/api/item_controller_test.exs

@@ -9,28 +9,28 @@ defmodule AppWeb.API.ItemControllerTest do
   describe "show" do
     test "specific item", %{conn: conn} do
       {:ok, %{model: item, version: _version}} = Item.create_item(@create_attrs)
-      conn = get(conn, Routes.item_path(conn, :show, item.id))
+      conn = get(conn, Routes.api_item_path(conn, :show, item.id))
 
       assert conn.status == 200
       assert json_response(conn, 200)["id"] == item.id
       assert json_response(conn, 200)["text"] == item.text
     end
 
     test "not found item", %{conn: conn} do
-      conn = get(conn, Routes.item_path(conn, :show, -1))
+      conn = get(conn, Routes.api_item_path(conn, :show, -1))
 
       assert conn.status == 404
     end
 
     test "invalid id (not being an integer)", %{conn: conn} do
-      conn = get(conn, Routes.item_path(conn, :show, "invalid"))
+      conn = get(conn, Routes.api_item_path(conn, :show, "invalid"))
       assert conn.status == 400
     end
   end
 
   describe "create" do
     test "a valid item", %{conn: conn} do
-      conn = post(conn, Routes.item_path(conn, :create, @create_attrs))
+      conn = post(conn, Routes.api_item_path(conn, :create, @create_attrs))
 
       assert conn.status == 200
       assert json_response(conn, 200)["text"] == Map.get(@create_attrs, "text")
@@ -43,7 +43,7 @@ defmodule AppWeb.API.ItemControllerTest do
     end
 
     test "an invalid item", %{conn: conn} do
-      conn = post(conn, Routes.item_path(conn, :create, @invalid_attrs))
+      conn = post(conn, Routes.api_item_path(conn, :create, @invalid_attrs))
 
       assert conn.status == 400
       assert length(json_response(conn, 400)["errors"]["text"]) > 0
@@ -53,15 +53,15 @@ defmodule AppWeb.API.ItemControllerTest do
   describe "update" do
     test "item with valid attributes", %{conn: conn} do
       {:ok, %{model: item, version: _version}} = Item.create_item(@create_attrs)
-      conn = put(conn, Routes.item_path(conn, :update, item.id, @update_attrs))
+      conn = put(conn, Routes.api_item_path(conn, :update, item.id, @update_attrs))
 
       assert conn.status == 200
       assert json_response(conn, 200)["text"] == Map.get(@update_attrs, :text)
     end
 
     test "item with invalid attributes", %{conn: conn} do
       {:ok, %{model: item, version: _version}} = Item.create_item(@create_attrs)
-      conn = put(conn, Routes.item_path(conn, :update, item.id, @invalid_attrs))
+      conn = put(conn, Routes.api_item_path(conn, :update, item.id, @invalid_attrs))
 
       assert conn.status == 400
       assert length(json_response(conn, 400)["errors"]["text"]) > 0

test/app_web/controllers/api/tag_controller_test.exs

@@ -0,0 +1,86 @@
+defmodule AppWeb.API.TagControllerTest do
+  use AppWeb.ConnCase
+  alias App.Tag
+
+  @create_attrs %{person_id: 42, color: "#FFFFFF", text: "some text"}
+  @update_attrs %{person_id: 43, color: "#DDDDDD", text: "some updated text"}
+  @invalid_attrs %{person_id: nil, color: nil, text: nil}
+  @update_invalid_color %{color: "invalid"}
+
+  describe "show" do
+    test "specific tag", %{conn: conn} do
+      {:ok, tag} = Tag.create_tag(@create_attrs)
+      conn = get(conn, Routes.api_tag_path(conn, :show, tag.id))
+
+      assert conn.status == 200
+      assert json_response(conn, 200)["id"] == tag.id
+      assert json_response(conn, 200)["text"] == tag.text
+    end
+
+    test "not found tag", %{conn: conn} do
+      conn = get(conn, Routes.api_tag_path(conn, :show, -1))
+
+      assert conn.status == 404
+    end
+
+    test "invalid id (not being an integer)", %{conn: conn} do
+      conn = get(conn, Routes.api_tag_path(conn, :show, "invalid"))
+      assert conn.status == 400
+    end
+  end
+
+  describe "create" do
+    test "a valid tag", %{conn: conn} do
+      conn = post(conn, Routes.api_tag_path(conn, :create, @create_attrs))
+
+      assert conn.status == 200
+      assert json_response(conn, 200)["text"] == Map.get(@create_attrs, "text")
+
+      assert json_response(conn, 200)["color"] ==
+               Map.get(@create_attrs, "color")
+
+      assert json_response(conn, 200)["person_id"] ==
+               Map.get(@create_attrs, "person_id")
+    end
+
+    test "an invalid tag", %{conn: conn} do
+      conn = post(conn, Routes.api_tag_path(conn, :create, @invalid_attrs))
+
+      assert conn.status == 400
+      assert length(json_response(conn, 400)["errors"]["text"]) > 0
+    end
+  end
+
+  describe "update" do
+    test "tag with valid attributes", %{conn: conn} do
+      {:ok, tag} = Tag.create_tag(@create_attrs)
+      conn = put(conn, Routes.api_tag_path(conn, :update, tag.id, @update_attrs))
+
+      assert conn.status == 200
+      assert json_response(conn, 200)["text"] == Map.get(@update_attrs, :text)
+    end
+
+    test "tag with invalid attributes", %{conn: conn} do
+      {:ok, tag} = Tag.create_tag(@create_attrs)
+      conn = put(conn, Routes.api_tag_path(conn, :update, tag.id, @invalid_attrs))
+
+      assert conn.status == 400
+      assert length(json_response(conn, 400)["errors"]["text"]) > 0
+    end
+
+    test "tag that doesn't exist", %{conn: conn} do
+      {:ok, tag} = Tag.create_tag(@create_attrs)
+      conn = put(conn, Routes.api_tag_path(conn, :update, -1, @update_attrs))
+
+      assert conn.status == 404
+    end
+
+    test "a tag with invalid color", %{conn: conn} do
+      {:ok, tag} = Tag.create_tag(@create_attrs)
+      conn = put(conn, Routes.api_tag_path(conn, :update, tag.id, @update_invalid_color))
+
+      assert conn.status == 400
+      assert length(json_response(conn, 400)["errors"]["color"]) > 0
+    end
+  end
+end

test/app_web/controllers/api/timer_controller_test.exs

@@ -14,7 +14,7 @@ defmodule AppWeb.API.TimerControllerTest do
       # Create item and timer
       {item, timer} = item_and_timer_fixture()
 
-      conn = get(conn, Routes.timer_path(conn, :index, item.id))
+      conn = get(conn, Routes.api_timer_path(conn, :index, item.id))
 
       assert conn.status == 200
       assert length(json_response(conn, 200)) == 1
@@ -26,7 +26,7 @@ defmodule AppWeb.API.TimerControllerTest do
       # Create item and timer
       {item, timer} = item_and_timer_fixture()
 
-      conn = get(conn, Routes.timer_path(conn, :show, item.id, timer.id))
+      conn = get(conn, Routes.api_timer_path(conn, :show, item.id, timer.id))
 
       assert conn.status == 200
       assert json_response(conn, 200)["id"] == timer.id
@@ -37,7 +37,7 @@ defmodule AppWeb.API.TimerControllerTest do
       {:ok, %{model: item, version: _version}} =
         Item.create_item(@create_item_attrs)
 
-      conn = get(conn, Routes.timer_path(conn, :show, item.id, -1))
+      conn = get(conn, Routes.api_timer_path(conn, :show, item.id, -1))
 
       assert conn.status == 404
     end
@@ -47,7 +47,7 @@ defmodule AppWeb.API.TimerControllerTest do
       {:ok, %{model: item, version: _version}} =
         Item.create_item(@create_item_attrs)
 
-      conn = get(conn, Routes.timer_path(conn, :show, item.id, "invalid"))
+      conn = get(conn, Routes.api_timer_path(conn, :show, item.id, "invalid"))
       assert conn.status == 400
     end
   end
@@ -60,7 +60,7 @@ defmodule AppWeb.API.TimerControllerTest do
 
       # Create timer
       conn =
-        post(conn, Routes.timer_path(conn, :create, item.id, @create_attrs))
+        post(conn, Routes.api_timer_path(conn, :create, item.id, @create_attrs))
 
       assert conn.status == 200
 
@@ -74,7 +74,7 @@ defmodule AppWeb.API.TimerControllerTest do
         Item.create_item(@create_item_attrs)
 
       conn =
-        post(conn, Routes.timer_path(conn, :create, item.id, @invalid_attrs))
+        post(conn, Routes.api_timer_path(conn, :create, item.id, @invalid_attrs))
 
       assert conn.status == 400
       assert length(json_response(conn, 400)["errors"]["start"]) > 0
@@ -89,7 +89,7 @@ defmodule AppWeb.API.TimerControllerTest do
       conn =
         put(
           conn,
-          Routes.timer_path(conn, :update, item.id, timer.id, @update_attrs)
+          Routes.api_timer_path(conn, :update, item.id, timer.id, @update_attrs)
         )
 
       assert conn.status == 200
@@ -103,7 +103,7 @@ defmodule AppWeb.API.TimerControllerTest do
       conn =
         put(
           conn,
-          Routes.timer_path(conn, :update, item.id, timer.id, @invalid_attrs)
+          Routes.api_timer_path(conn, :update, item.id, timer.id, @invalid_attrs)
         )
 
       assert conn.status == 400