Refactor and add logic for upload size check

config/config.exs

@@ -32,8 +32,9 @@ config :guardian, Guardian,
   secret_key: System.get_env("GUARDIAN_SECRET_KEY"),
   serializer: Phamello.GuardianSerializer
 
-config :phamello, Phamello.PictureUploader,
-  storage_path: System.get_env("IMAGE_STORAGE_FOLDER")
+config :phamello, Phamello.Picture,
+  storage_path: System.get_env("IMAGE_STORAGE_FOLDER"),
+  max_file_size: 5_000_000
 
 config :phamello, :s3_client,
   aws_access_key_id: System.get_env("AWS_ACCESS_KEY_ID"),

config/dev.exs

@@ -14,7 +14,6 @@ config :phamello, Phamello.Endpoint,
   watchers: [node: ["node_modules/brunch/bin/brunch", "watch", "--stdin",
                     cd: Path.expand("../", __DIR__)]]
 
-
 # Watch static and templates for browser reloading.
 config :phamello, Phamello.Endpoint,
   live_reload: [

config/test.exs

@@ -18,7 +18,8 @@ config :phamello, Phamello.Repo,
   hostname: "localhost",
   pool: Ecto.Adapters.SQL.Sandbox
 
-config :phamello, Phamello.PictureUploader,
-  storage_path: "fixture/storage"
+config :phamello, Phamello.Picture,
+  storage_path: "fixture/storage",
+  max_file_size: 200_000
 
 import_config "test.secret.exs"

test/controllers/picture_controller_test.exs

@@ -6,12 +6,12 @@ defmodule Phamello.PictureControllerTest do
   @invalid_attrs %{}
 
   setup do
-    user = factory(:user)
+    user = factory(:unsaved_user) |> Repo.insert!
 
     {:ok, %{
       picture_map: factory(:picture_map),
-      conn: guardian_login(user)
-        |> guardian_impersonate(user),
+      big_picture_map: factory(:picture_map, size: :big),
+      conn: guardian_login(user),
       user: user
     }}
   end
@@ -40,41 +40,57 @@ defmodule Phamello.PictureControllerTest do
     assert html_response(conn, 200) =~ "New picture"
   end
 
-  test "shows chosen resource", %{conn: conn} do
-    picture = Repo.insert! %Picture{}
+  test "does not create resource and renders errors when image size is too big", %{conn: conn, big_picture_map: picture_map} do
+    conn = post conn, picture_path(conn, :create), picture: picture_map
+    assert html_response(conn, 200) =~ "New picture"
+  end
+
+  test "shows chosen resource", %{conn: conn, user: user, picture_map: picture_map} do
+    picture = build_assoc(user, :pictures)
+    |> Picture.create_changeset(picture_map)
+    |> Repo.insert!
+
     conn = get conn, picture_path(conn, :show, picture)
     assert html_response(conn, 200) =~ "Show picture"
   end
 
-  test "renders page not found when id is nonexistent", %{conn: conn} do
+  test "renders 404 when showing resource not belonging to user", %{conn: conn, picture_map: picture_map} do
+    user = factory(:user) |> Repo.insert!
+
+    picture = build_assoc(user, :pictures)
+    |> Picture.create_changeset(picture_map)
+    |> Repo.insert!
+
     assert_error_sent 404, fn ->
-      get conn, picture_path(conn, :show, -1)
+      get conn, picture_path(conn, :show, picture)
     end
   end
 
-  test "renders form for editing chosen resource", %{conn: conn} do
-    picture = Repo.insert! %Picture{}
-    conn = get conn, picture_path(conn, :edit, picture)
-    assert html_response(conn, 200) =~ "Edit picture"
-  end
-
-  test "updates chosen resource and redirects when data is valid", %{conn: conn, picture_map: picture_map} do
-    picture = Repo.insert! struct(Picture, picture_map)
-    conn = put conn, picture_path(conn, :update, picture), picture: picture_map
-    assert redirected_to(conn) == picture_path(conn, :show, picture)
-    assert Repo.get_by(Picture, Map.take(picture, [:name, :description]))
+  test "renders page not found when id is nonexistent", %{conn: conn} do
+    assert_error_sent 404, fn ->
+      get conn, picture_path(conn, :show, -1)
+    end
   end
 
-  test "does not update chosen resource and renders errors when data is invalid", %{conn: conn} do
-    picture = Repo.insert! %Picture{}
-    conn = put conn, picture_path(conn, :update, picture), picture: @invalid_attrs
-    assert html_response(conn, 200) =~ "Edit picture"
-  end
+  test "deletes chosen resource", %{conn: conn, user: user, picture_map: picture_map} do
+    picture = build_assoc(user, :pictures)
+    |> Picture.create_changeset(picture_map)
+    |> Repo.insert!
 
-  test "deletes chosen resource", %{conn: conn} do
-    picture = Repo.insert! %Picture{}
     conn = delete conn, picture_path(conn, :delete, picture)
     assert redirected_to(conn) == picture_path(conn, :index)
     refute Repo.get(Picture, picture.id)
   end
+
+  test "renders 404 when deleting resource not belonging to user", %{conn: conn, picture_map: picture_map} do
+    user = factory(:user) |> Repo.insert!
+
+    picture = build_assoc(user, :pictures)
+    |> Picture.create_changeset(picture_map)
+    |> Repo.insert!
+
+    assert_error_sent 404, fn ->
+      delete conn, picture_path(conn, :delete, picture)
+    end
+  end
 end

test/models/picture_test.exs

@@ -12,12 +12,12 @@ defmodule Phamello.PictureTest do
   end
 
   test "changeset with valid attributes", %{picture_map: picture_map} do
-    changeset = Picture.changeset(%Picture{}, picture_map)
+    changeset = Picture.insert_changeset(%Picture{}, picture_map)
     assert changeset.valid?
   end
 
   test "changeset with invalid attributes" do
-    changeset = Picture.changeset(%Picture{}, @invalid_attrs)
+    changeset = Picture.insert_changeset(%Picture{}, @invalid_attrs)
     refute changeset.valid?
   end
 end

test/support/factory.ex

@@ -1,33 +1,42 @@
 defmodule Phamello.Factory do
   alias Phamello.User
 
-  def factory(:user) do
+  def factory(resource), do: factory(resource, [])
+
+  def factory(:user, []) do
     %User{
       id: 1,
       username: "tomfulp",
       github_id: 123456
     }
   end
 
-  def factory(:unsaved_user) do
+  def factory(:unsaved_user, []) do
     %User{
       username: "ginger",
       github_id: 987654
     }
   end
 
-  def factory(:picture_map) do
+  def factory(:picture_map, opts) do
     %{
       name: "McGyver",
       description: "Great show",
-      image: factory(:image)
+      image: factory(:image, opts)
     }
   end
 
-  def factory(:image) do
+  def factory(:image, []) do
     %Plug.Upload{
       path: "fixture/images/face.jpg",
       filename: "face.jpg"
     }
   end
+
+  def factory(:image, [size: :big]) do
+    %Plug.Upload{
+      path: "fixture/images/big.jpg",
+      filename: "big.jpg"
+    }
+  end
 end

web/controllers/picture_controller.ex

@@ -2,23 +2,24 @@ defmodule Phamello.PictureController do
   use Phamello.Web, :controller
   use Guardian.Phoenix.Controller
   alias Phamello.{Picture, PictureUploader, PictureWorker}
+  import Ecto
 
   plug Guardian.Plug.EnsureAuthenticated,
       handler: __MODULE__
 
   def index(conn, _params, user, _claims) do
-    pictures = Repo.all(Picture)
+    pictures = Repo.all(assoc(user, :pictures))
     render(conn, "index.html", pictures: pictures)
   end
 
-  def new(conn, _params, user, _claims) do
-    changeset = Picture.create_changeset(%Picture{})
+  def new(conn, _params, _user, _claims) do
+    changeset = Picture.insert_changeset(%Picture{})
     render(conn, "new.html", changeset: changeset)
   end
 
   def create(conn, %{"picture" => picture_params}, user, _claims) do
-    changeset = Picture.create_changeset(%Picture{user: user}, picture_params)
-    changeset = changeset |> PictureUploader.with_local_storage
+    changeset = build_assoc(user, :pictures)
+    |> Picture.create_changeset(picture_params)
 
     case Repo.insert(changeset) do
       {:ok, picture} ->
@@ -33,35 +34,12 @@ defmodule Phamello.PictureController do
   end
 
   def show(conn, %{"id" => id}, user, _claims) do
-    picture = Repo.get!(Picture, id)
+    picture = Repo.get!(assoc(user, :pictures), id)
     render(conn, "show.html", picture: picture)
   end
 
-  def edit(conn, %{"id" => id}, user, _claims) do
-    picture = Repo.get!(Picture, id)
-    changeset = Picture.update_changeset(picture)
-    render(conn, "edit.html", picture: picture, changeset: changeset)
-  end
-
-  def update(conn, %{"id" => id, "picture" => picture_params}, user, _claims) do
-    picture = Repo.get!(Picture, id)
-    changeset = Picture.update_changeset(picture, picture_params)
-
-    case Repo.update(changeset) do
-      {:ok, picture} ->
-        conn
-        |> put_flash(:info, "Picture updated successfully.")
-        |> redirect(to: picture_path(conn, :show, picture))
-      {:error, changeset} ->
-        render(conn, "edit.html", picture: picture, changeset: changeset)
-    end
-  end
-
   def delete(conn, %{"id" => id}, user, _claims) do
-    picture = Repo.get!(Picture, id)
-
-    # Here we use delete! (with a bang) because we expect
-    # it to always work (and if it does not, it will raise).
+    picture = Repo.get!(assoc(user, :pictures), id)
     Repo.delete!(picture)
 
     conn