From 9e5a49404f60cb5088b730a924cecc26b7a873ea Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 4 Jul 2019 10:10:24 +0200
Subject: [PATCH 01/21] add(api/chapters): create

---
 .../controllers/api/chapters_controller.ex    | 23 +++++++++++
 lib/radiator_web/router.ex                    |  5 ++-
 lib/radiator_web/views/api/chapters_view.ex   | 25 +++++++++++
 .../api/chapters_controller_test.exs          | 41 +++++++++++++++++++
 4 files changed, 93 insertions(+), 1 deletion(-)
 create mode 100644 lib/radiator_web/controllers/api/chapters_controller.ex
 create mode 100644 lib/radiator_web/views/api/chapters_view.ex
 create mode 100644 test/radiator_web/controllers/api/chapters_controller_test.exs

diff --git a/lib/radiator_web/controllers/api/chapters_controller.ex b/lib/radiator_web/controllers/api/chapters_controller.ex
new file mode 100644
index 00000000..ef4fedb0
--- /dev/null
+++ b/lib/radiator_web/controllers/api/chapters_controller.ex
@@ -0,0 +1,23 @@
+defmodule RadiatorWeb.Api.ChaptersController do
+  use RadiatorWeb, :controller
+
+  alias Radiator.AudioMeta
+  alias Radiator.Directory.Editor
+
+  action_fallback(RadiatorWeb.Api.FallbackController)
+
+  def create(conn, %{"audio_id" => audio_id, "chapter" => chapter_params}) do
+    with {:ok, audio} <- Editor.get_audio(current_user(conn), audio_id),
+         {:ok, chapter} <- AudioMeta.create_chapter(audio, chapter_params) do
+      conn
+      |> put_status(:created)
+      |> put_resp_header(
+        "location",
+        Routes.api_audio_chapters_path(conn, :show, audio.id, chapter)
+      )
+      |> assign(:audio, audio)
+      |> assign(:chapter, chapter)
+      |> render("show.json")
+    end
+  end
+end
diff --git a/lib/radiator_web/router.ex b/lib/radiator_web/router.ex
index 85117c46..fdcb661c 100644
--- a/lib/radiator_web/router.ex
+++ b/lib/radiator_web/router.ex
@@ -101,7 +101,10 @@ defmodule RadiatorWeb.Router do
     end
 
     resources "/episodes", EpisodeController, only: [:show, :create, :update, :delete]
-    resources "/audios", AudioController, only: [:show, :create, :update, :delete]
+
+    resources "/audios", AudioController, only: [:show, :create, :update, :delete] do
+      resources "/chapters", ChaptersController, only: [:show, :create, :update, :delete]
+    end
 
     # todo: pluralize
     resources "/audio_file", AudioFileController, only: [:show, :create]
diff --git a/lib/radiator_web/views/api/chapters_view.ex b/lib/radiator_web/views/api/chapters_view.ex
new file mode 100644
index 00000000..b8dc0b7f
--- /dev/null
+++ b/lib/radiator_web/views/api/chapters_view.ex
@@ -0,0 +1,25 @@
+defmodule RadiatorWeb.Api.ChaptersView do
+  use RadiatorWeb, :view
+
+  alias HAL.{Document, Link}
+  alias Radiator.AudioMeta.Chapter
+
+  def render("show.json", assigns) do
+    render(__MODULE__, "chapter.json", assigns)
+  end
+
+  def render("chapter.json", %{conn: conn, chapter: chapter, audio: audio}) do
+    %Document{}
+    |> Document.add_link(%Link{
+      rel: "self",
+      href: Routes.api_audio_chapters_path(conn, :show, audio.id, chapter.id)
+    })
+    |> Document.add_properties(%{
+      id: chapter.id,
+      title: chapter.title,
+      start: chapter.start,
+      link: chapter.link,
+      image: Chapter.image_url(chapter)
+    })
+  end
+end
diff --git a/test/radiator_web/controllers/api/chapters_controller_test.exs b/test/radiator_web/controllers/api/chapters_controller_test.exs
new file mode 100644
index 00000000..a03c4411
--- /dev/null
+++ b/test/radiator_web/controllers/api/chapters_controller_test.exs
@@ -0,0 +1,41 @@
+defmodule RadiatorWeb.Api.ChaptersControllerTest do
+  use RadiatorWeb.ConnCase
+
+  import Radiator.Factory
+
+  def setup_user_and_conn(%{conn: conn}) do
+    user = Radiator.TestEntries.user()
+
+    [
+      conn: Radiator.TestEntries.put_current_user(conn, user),
+      user: user
+    ]
+  end
+
+  setup :setup_user_and_conn
+
+  describe "create chapter" do
+    test "renders chapter when data is valid", %{conn: conn, user: user} do
+      audio = insert(:audio) |> owned_by(user)
+
+      conn =
+        post(conn, Routes.api_audio_chapters_path(conn, :create, audio.id),
+          chapter: %{title: "example", start: 123_000, link: "http://example.com"}
+        )
+
+      assert %{"title" => "example", "start" => 123_000, "link" => "http://example.com"} =
+               json_response(conn, 201)
+    end
+
+    test "renders error when permissions are invalid", %{conn: conn} do
+      audio = insert(:audio)
+
+      conn =
+        post(conn, Routes.api_audio_chapters_path(conn, :create, audio.id),
+          chapter: %{title: "example"}
+        )
+
+      assert json_response(conn, 401)
+    end
+  end
+end

From 81ecff602fef2904ae2cad801e18655fa87b3e77 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 4 Jul 2019 10:41:39 +0200
Subject: [PATCH 02/21] add(api/chapters): update

---
 lib/radiator/audio_meta/audio_meta.ex         |  8 ++++++
 .../controllers/api/chapters_controller.ex    | 11 ++++++++
 .../api/chapters_controller_test.exs          | 27 +++++++++++++++++++
 test/support/factory.ex                       |  8 ++++++
 4 files changed, 54 insertions(+)

diff --git a/lib/radiator/audio_meta/audio_meta.ex b/lib/radiator/audio_meta/audio_meta.ex
index 604abe92..14322bce 100644
--- a/lib/radiator/audio_meta/audio_meta.ex
+++ b/lib/radiator/audio_meta/audio_meta.ex
@@ -39,6 +39,14 @@ defmodule Radiator.AudioMeta do
     |> Repo.all()
   end
 
+  def find_chapter(%Audio{} = audio, id) do
+    Repo.get_by(Chapter, audio_id: audio.id, id: id)
+    |> case do
+      nil -> {:error, :not_found}
+      chapter -> {:ok, chapter}
+    end
+  end
+
   def delete_chapters(%Audio{} = audio) do
     audio = Repo.preload(audio, :chapters)
 
diff --git a/lib/radiator_web/controllers/api/chapters_controller.ex b/lib/radiator_web/controllers/api/chapters_controller.ex
index ef4fedb0..02607fd8 100644
--- a/lib/radiator_web/controllers/api/chapters_controller.ex
+++ b/lib/radiator_web/controllers/api/chapters_controller.ex
@@ -20,4 +20,15 @@ defmodule RadiatorWeb.Api.ChaptersController do
       |> render("show.json")
     end
   end
+
+  def update(conn, %{"audio_id" => audio_id, "id" => id, "chapter" => chapter_params}) do
+    with {:ok, audio} <- Editor.get_audio(current_user(conn), audio_id),
+         {:ok, chapter} <- AudioMeta.find_chapter(audio, id),
+         {:ok, chapter} <- AudioMeta.update_chapter(chapter, chapter_params) do
+      conn
+      |> assign(:audio, audio)
+      |> assign(:chapter, chapter)
+      |> render("show.json")
+    end
+  end
 end
diff --git a/test/radiator_web/controllers/api/chapters_controller_test.exs b/test/radiator_web/controllers/api/chapters_controller_test.exs
index a03c4411..a80563f5 100644
--- a/test/radiator_web/controllers/api/chapters_controller_test.exs
+++ b/test/radiator_web/controllers/api/chapters_controller_test.exs
@@ -38,4 +38,31 @@ defmodule RadiatorWeb.Api.ChaptersControllerTest do
       assert json_response(conn, 401)
     end
   end
+
+  describe "update chapter" do
+    test "updates and renders chapter", %{conn: conn, user: user} do
+      audio = insert(:audio) |> owned_by(user)
+      chapter = insert(:chapter, audio: audio)
+
+      conn =
+        put(conn, Routes.api_audio_chapters_path(conn, :update, audio.id, chapter.id), %{
+          chapter: %{title: "new"}
+        })
+
+      assert %{"title" => "new"} = json_response(conn, 200)
+    end
+
+    test "renders error when accessing foreign chapter", %{conn: conn, user: user} do
+      audio = insert(:audio) |> owned_by(user)
+      _chapter = insert(:chapter, audio: audio)
+      other_chapter = insert(:chapter, audio: build(:audio))
+
+      conn =
+        put(conn, Routes.api_audio_chapters_path(conn, :update, audio.id, other_chapter.id), %{
+          chapter: %{title: "new"}
+        })
+
+      assert json_response(conn, 404)
+    end
+  end
 end
diff --git a/test/support/factory.ex b/test/support/factory.ex
index a284d667..414dd7fb 100644
--- a/test/support/factory.ex
+++ b/test/support/factory.ex
@@ -146,6 +146,14 @@ defmodule Radiator.Factory do
     }
   end
 
+  def chapter_factory do
+    %Radiator.AudioMeta.Chapter{
+      start: 0,
+      title: sequence(:title, &"chapter #{&1}"),
+      link: sequence(:link, &"http://example.com/#{&1}")
+    }
+  end
+
   # @deprecated, use audio_file_factory
   def enclosure_factory do
     %Radiator.Media.AudioFile{

From a09decae13f28d9208c99d5a696ca5e795c54284 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 4 Jul 2019 11:09:12 +0200
Subject: [PATCH 03/21] add(api/chapters): delete

---
 lib/radiator/audio_meta/audio_meta.ex         |  4 ++++
 lib/radiator/directory/editor.ex              | 24 +++++++++++++++++++
 lib/radiator/directory/editor/permission.ex   | 11 +++++++++
 .../controllers/api/chapters_controller.ex    |  8 +++++++
 .../api/chapters_controller_test.exs          |  9 +++++++
 test/support/factory.ex                       |  3 ++-
 6 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/lib/radiator/audio_meta/audio_meta.ex b/lib/radiator/audio_meta/audio_meta.ex
index 14322bce..2070b4e6 100644
--- a/lib/radiator/audio_meta/audio_meta.ex
+++ b/lib/radiator/audio_meta/audio_meta.ex
@@ -47,6 +47,10 @@ defmodule Radiator.AudioMeta do
     end
   end
 
+  def delete_chapter(chapter = %Chapter{}) do
+    Repo.delete(chapter)
+  end
+
   def delete_chapters(%Audio{} = audio) do
     audio = Repo.preload(audio, :chapters)
 
diff --git a/lib/radiator/directory/editor.ex b/lib/radiator/directory/editor.ex
index aefb7379..92ec90a4 100644
--- a/lib/radiator/directory/editor.ex
+++ b/lib/radiator/directory/editor.ex
@@ -13,6 +13,8 @@ defmodule Radiator.Directory.Editor do
 
   alias Radiator.Support
   alias Radiator.Repo
+  alias Radiator.AudioMeta
+  alias Radiator.AudioMeta.Chapter
   alias Radiator.Directory
   alias Radiator.Directory.{Network, Podcast, Episode, Editor, Audio, Collaborator}
 
@@ -421,6 +423,28 @@ defmodule Radiator.Directory.Editor do
     episode
   end
 
+  def get_chapter(actor = %Auth.User{}, id) do
+    case Repo.get(Chapter, id) do
+      nil ->
+        @not_found_match
+
+      chapter = %Chapter{} ->
+        if has_permission(actor, chapter, :readonly) do
+          {:ok, chapter}
+        else
+          @not_authorized_match
+        end
+    end
+  end
+
+  def delete_chapter(actor = %Auth.User{}, chapter = %Chapter{}) do
+    if has_permission(actor, chapter, :own) do
+      AudioMeta.delete_chapter(chapter)
+    else
+      @not_authorized_match
+    end
+  end
+
   @spec get_audio(Auth.User.t(), pos_integer()) ::
           {:ok, Audio.t()} | {:error, :not_authorized | :not_found}
   def get_audio(actor = %Auth.User{}, id) do
diff --git a/lib/radiator/directory/editor/permission.ex b/lib/radiator/directory/editor/permission.ex
index b423ce9b..4437cf6c 100644
--- a/lib/radiator/directory/editor/permission.ex
+++ b/lib/radiator/directory/editor/permission.ex
@@ -4,6 +4,7 @@ defmodule Radiator.Directory.Editor.Permission do
   use Radiator.Constants
 
   alias Radiator.Repo
+  alias Radiator.AudioMeta.Chapter
   alias Radiator.Auth
   alias Radiator.Perm.Permission
   alias Radiator.Directory.{Network, Podcast, Episode, Audio}
@@ -25,6 +26,9 @@ defmodule Radiator.Directory.Editor.Permission do
   def get_permission(user = %Auth.User{}, subject = %Audio{}),
     do: do_get_permission(user, subject)
 
+  def get_permission(_user = %Auth.User{}, _subject = %Chapter{}),
+    do: nil
+
   defp do_get_permission(user, subject) do
     case fetch_permission(user, subject) do
       nil -> nil
@@ -83,6 +87,13 @@ defmodule Radiator.Directory.Editor.Permission do
     |> List.wrap()
   end
 
+  defp parents(subject = %Chapter{}) do
+    subject
+    |> Ecto.assoc(:audio)
+    |> Repo.one!()
+    |> List.wrap()
+  end
+
   defp parents(_) do
     []
   end
diff --git a/lib/radiator_web/controllers/api/chapters_controller.ex b/lib/radiator_web/controllers/api/chapters_controller.ex
index 02607fd8..12fd4c21 100644
--- a/lib/radiator_web/controllers/api/chapters_controller.ex
+++ b/lib/radiator_web/controllers/api/chapters_controller.ex
@@ -31,4 +31,12 @@ defmodule RadiatorWeb.Api.ChaptersController do
       |> render("show.json")
     end
   end
+
+  def delete(conn, %{"id" => id}) do
+    with user <- current_user(conn),
+         {:ok, chapter} <- Editor.get_chapter(user, id),
+         {:ok, _} <- Editor.delete_chapter(user, chapter) do
+      send_resp(conn, 204, "")
+    end
+  end
 end
diff --git a/test/radiator_web/controllers/api/chapters_controller_test.exs b/test/radiator_web/controllers/api/chapters_controller_test.exs
index a80563f5..2af1976a 100644
--- a/test/radiator_web/controllers/api/chapters_controller_test.exs
+++ b/test/radiator_web/controllers/api/chapters_controller_test.exs
@@ -65,4 +65,13 @@ defmodule RadiatorWeb.Api.ChaptersControllerTest do
       assert json_response(conn, 404)
     end
   end
+
+  test "delete chapter", %{conn: conn, user: user} do
+    audio = insert(:audio) |> owned_by(user)
+    chapter = insert(:chapter, audio: audio)
+
+    conn = delete(conn, Routes.api_audio_chapters_path(conn, :delete, audio.id, chapter.id))
+
+    assert response(conn, 204)
+  end
 end
diff --git a/test/support/factory.ex b/test/support/factory.ex
index 414dd7fb..f736ce1b 100644
--- a/test/support/factory.ex
+++ b/test/support/factory.ex
@@ -150,7 +150,8 @@ defmodule Radiator.Factory do
     %Radiator.AudioMeta.Chapter{
       start: 0,
       title: sequence(:title, &"chapter #{&1}"),
-      link: sequence(:link, &"http://example.com/#{&1}")
+      link: sequence(:link, &"http://example.com/#{&1}"),
+      audio: build(:audio)
     }
   end
 

From 462d8c19f7fb306bd5dde231fbb53d5527a054d3 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 4 Jul 2019 11:19:15 +0200
Subject: [PATCH 04/21] refactor: contorller only accesses Directory.Editor

---
 lib/radiator/audio_meta/audio_meta.ex            |  8 --------
 lib/radiator/directory/editor.ex                 | 16 ++++++++++++++++
 .../controllers/api/chapters_controller.ex       | 13 +++++++------
 .../controllers/api/chapters_controller_test.exs |  2 +-
 4 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/lib/radiator/audio_meta/audio_meta.ex b/lib/radiator/audio_meta/audio_meta.ex
index 2070b4e6..1bacce80 100644
--- a/lib/radiator/audio_meta/audio_meta.ex
+++ b/lib/radiator/audio_meta/audio_meta.ex
@@ -39,14 +39,6 @@ defmodule Radiator.AudioMeta do
     |> Repo.all()
   end
 
-  def find_chapter(%Audio{} = audio, id) do
-    Repo.get_by(Chapter, audio_id: audio.id, id: id)
-    |> case do
-      nil -> {:error, :not_found}
-      chapter -> {:ok, chapter}
-    end
-  end
-
   def delete_chapter(chapter = %Chapter{}) do
     Repo.delete(chapter)
   end
diff --git a/lib/radiator/directory/editor.ex b/lib/radiator/directory/editor.ex
index 92ec90a4..4b34ed0a 100644
--- a/lib/radiator/directory/editor.ex
+++ b/lib/radiator/directory/editor.ex
@@ -437,6 +437,22 @@ defmodule Radiator.Directory.Editor do
     end
   end
 
+  def create_chapter(actor = %Auth.User{}, audio, attrs) do
+    if has_permission(actor, audio, :edit) do
+      AudioMeta.create_chapter(audio, attrs)
+    else
+      @not_authorized_match
+    end
+  end
+
+  def update_chapter(actor = %Auth.User{}, chapter = %Chapter{}, attrs) do
+    if has_permission(actor, chapter, :edit) do
+      AudioMeta.update_chapter(chapter, attrs)
+    else
+      @not_authorized_match
+    end
+  end
+
   def delete_chapter(actor = %Auth.User{}, chapter = %Chapter{}) do
     if has_permission(actor, chapter, :own) do
       AudioMeta.delete_chapter(chapter)
diff --git a/lib/radiator_web/controllers/api/chapters_controller.ex b/lib/radiator_web/controllers/api/chapters_controller.ex
index 12fd4c21..83c13114 100644
--- a/lib/radiator_web/controllers/api/chapters_controller.ex
+++ b/lib/radiator_web/controllers/api/chapters_controller.ex
@@ -1,14 +1,14 @@
 defmodule RadiatorWeb.Api.ChaptersController do
   use RadiatorWeb, :controller
 
-  alias Radiator.AudioMeta
   alias Radiator.Directory.Editor
 
   action_fallback(RadiatorWeb.Api.FallbackController)
 
   def create(conn, %{"audio_id" => audio_id, "chapter" => chapter_params}) do
-    with {:ok, audio} <- Editor.get_audio(current_user(conn), audio_id),
-         {:ok, chapter} <- AudioMeta.create_chapter(audio, chapter_params) do
+    with user <- current_user(conn),
+         {:ok, audio} <- Editor.get_audio(user, audio_id),
+         {:ok, chapter} <- Editor.create_chapter(user, audio, chapter_params) do
       conn
       |> put_status(:created)
       |> put_resp_header(
@@ -22,9 +22,10 @@ defmodule RadiatorWeb.Api.ChaptersController do
   end
 
   def update(conn, %{"audio_id" => audio_id, "id" => id, "chapter" => chapter_params}) do
-    with {:ok, audio} <- Editor.get_audio(current_user(conn), audio_id),
-         {:ok, chapter} <- AudioMeta.find_chapter(audio, id),
-         {:ok, chapter} <- AudioMeta.update_chapter(chapter, chapter_params) do
+    with user <- current_user(conn),
+         {:ok, audio} <- Editor.get_audio(user, audio_id),
+         {:ok, chapter} <- Editor.get_chapter(user, id),
+         {:ok, chapter} <- Editor.update_chapter(user, chapter, chapter_params) do
       conn
       |> assign(:audio, audio)
       |> assign(:chapter, chapter)
diff --git a/test/radiator_web/controllers/api/chapters_controller_test.exs b/test/radiator_web/controllers/api/chapters_controller_test.exs
index 2af1976a..9efb4947 100644
--- a/test/radiator_web/controllers/api/chapters_controller_test.exs
+++ b/test/radiator_web/controllers/api/chapters_controller_test.exs
@@ -62,7 +62,7 @@ defmodule RadiatorWeb.Api.ChaptersControllerTest do
           chapter: %{title: "new"}
         })
 
-      assert json_response(conn, 404)
+      assert json_response(conn, 401)
     end
   end
 

From d8d80a8c3b9c5a7b8e004390169dee2d6407c032 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 4 Jul 2019 11:24:24 +0200
Subject: [PATCH 05/21] refactor: use :rest_controller

---
 lib/radiator_web/controllers/api/chapters_controller.ex | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/radiator_web/controllers/api/chapters_controller.ex b/lib/radiator_web/controllers/api/chapters_controller.ex
index 83c13114..48302f67 100644
--- a/lib/radiator_web/controllers/api/chapters_controller.ex
+++ b/lib/radiator_web/controllers/api/chapters_controller.ex
@@ -1,10 +1,8 @@
 defmodule RadiatorWeb.Api.ChaptersController do
-  use RadiatorWeb, :controller
+  use RadiatorWeb, :rest_controller
 
   alias Radiator.Directory.Editor
 
-  action_fallback(RadiatorWeb.Api.FallbackController)
-
   def create(conn, %{"audio_id" => audio_id, "chapter" => chapter_params}) do
     with user <- current_user(conn),
          {:ok, audio} <- Editor.get_audio(user, audio_id),
@@ -37,7 +35,10 @@ defmodule RadiatorWeb.Api.ChaptersController do
     with user <- current_user(conn),
          {:ok, chapter} <- Editor.get_chapter(user, id),
          {:ok, _} <- Editor.delete_chapter(user, chapter) do
-      send_resp(conn, 204, "")
+      send_delete_resp(conn)
+    else
+      @not_found_match -> send_delete_resp(conn)
+      error -> error
     end
   end
 end

From 8223a3a0b95b991ec70f8354c120e604d41ae11e Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 4 Jul 2019 11:39:19 +0200
Subject: [PATCH 06/21] refactor: extract assign_audio/2 plug

---
 .../controllers/api/chapters_controller.ex    | 30 ++++++++++++++-----
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/lib/radiator_web/controllers/api/chapters_controller.ex b/lib/radiator_web/controllers/api/chapters_controller.ex
index 48302f67..544ee99f 100644
--- a/lib/radiator_web/controllers/api/chapters_controller.ex
+++ b/lib/radiator_web/controllers/api/chapters_controller.ex
@@ -3,29 +3,27 @@ defmodule RadiatorWeb.Api.ChaptersController do
 
   alias Radiator.Directory.Editor
 
-  def create(conn, %{"audio_id" => audio_id, "chapter" => chapter_params}) do
+  plug :assign_audio when action in [:create, :update]
+
+  def create(conn, %{"chapter" => chapter_params}) do
     with user <- current_user(conn),
-         {:ok, audio} <- Editor.get_audio(user, audio_id),
-         {:ok, chapter} <- Editor.create_chapter(user, audio, chapter_params) do
+         {:ok, chapter} <- Editor.create_chapter(user, conn.assigns[:audio], chapter_params) do
       conn
       |> put_status(:created)
       |> put_resp_header(
         "location",
-        Routes.api_audio_chapters_path(conn, :show, audio.id, chapter)
+        Routes.api_audio_chapters_path(conn, :show, conn.assigns[:audio].id, chapter)
       )
-      |> assign(:audio, audio)
       |> assign(:chapter, chapter)
       |> render("show.json")
     end
   end
 
-  def update(conn, %{"audio_id" => audio_id, "id" => id, "chapter" => chapter_params}) do
+  def update(conn, %{"id" => id, "chapter" => chapter_params}) do
     with user <- current_user(conn),
-         {:ok, audio} <- Editor.get_audio(user, audio_id),
          {:ok, chapter} <- Editor.get_chapter(user, id),
          {:ok, chapter} <- Editor.update_chapter(user, chapter, chapter_params) do
       conn
-      |> assign(:audio, audio)
       |> assign(:chapter, chapter)
       |> render("show.json")
     end
@@ -41,4 +39,20 @@ defmodule RadiatorWeb.Api.ChaptersController do
       error -> error
     end
   end
+
+  defp assign_audio(conn, _) do
+    conn
+    |> current_user()
+    |> Editor.get_audio(conn.params["audio_id"])
+    |> case do
+      {:ok, audio} ->
+        conn
+        |> assign(:audio, audio)
+
+      error = {:error, _} ->
+        conn
+        |> RadiatorWeb.Api.FallbackController.call(error)
+        |> Plug.Conn.halt()
+    end
+  end
 end

From 07b844647bf0c7ec8e923c313c5115751e17690a Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 4 Jul 2019 13:09:09 +0200
Subject: [PATCH 07/21] make chapter start time required

---
 lib/radiator/audio_meta/chapter.ex | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/radiator/audio_meta/chapter.ex b/lib/radiator/audio_meta/chapter.ex
index 3223b70d..3c2774d1 100644
--- a/lib/radiator/audio_meta/chapter.ex
+++ b/lib/radiator/audio_meta/chapter.ex
@@ -25,6 +25,7 @@ defmodule Radiator.AudioMeta.Chapter do
       :title,
       :link
     ])
+    |> validate_required([:start])
     |> cast_attachments(attrs, [:image], allow_paths: true, allow_urls: true)
   end
 

From e3bd2059da108756c692ed74826114744f313a31 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 4 Jul 2019 13:09:23 +0200
Subject: [PATCH 08/21] doc: add chapter api doc

---
 guides/rest_api.md | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/guides/rest_api.md b/guides/rest_api.md
index 84ae0cd0..747e10f6 100644
--- a/guides/rest_api.md
+++ b/guides/rest_api.md
@@ -47,6 +47,12 @@
   - [Parameters for Create](#Parameters-for-Create)
   - [Create](#Create-6)
   - [Read](#Read-6)
+- [Audio Chapters](#Audio-Chapters)
+  - [Parameters for Create & Update](#Parameters-for-Create--Update-6)
+  - [Create](#Create-7)
+  - [Read](#Read-7)
+  - [Update](#Update-6)
+  - [Delete](#Delete-6)
 
 ## API Usage
 
@@ -350,3 +356,39 @@ POST /api/rest/v1/audio_file
 ```
 GET /api/rest/v1/audio_file/:id
 ```
+
+## Audio Chapters
+
+### Parameters for Create & Update
+
+| Name                | Type      | Description                                                    |
+| ------------------- | --------- | -------------------------------------------------------------- |
+| `chapter[audio_id]` | `integer` | **Required.** Chapter is attached to Audio object of given ID. |
+| `chapter[start]`    | `integer` | **Required.** chapter start time in milliseconds               |
+| `chapter[title]`    | `string`  | chapter title                                                  |
+| `chapter[link]`     | `string`  | chapter link                                                   |
+| `chapter[file]`     | `image`   | chapter image                                                  |
+
+### Create
+
+```
+POST /api/rest/v1/audios/:audio_id/chapters
+```
+
+### Read
+
+```
+GET /api/rest/v1/audios/:audio_id/chapters/:id
+```
+
+### Update
+
+```
+PATCH /api/rest/v1/audios/:audio_id/chapters/:id
+```
+
+### Delete
+
+```
+DELETE /api/rest/v1/audios/:audio_id/chapters/:id
+```

From 6aa35647b2077ee907ca7e2cfa8f1b6bd92ac9c4 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 4 Jul 2019 13:15:27 +0200
Subject: [PATCH 09/21] refactor: shorten halt call

---
 lib/radiator_web/controllers/api/chapters_controller.ex | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/radiator_web/controllers/api/chapters_controller.ex b/lib/radiator_web/controllers/api/chapters_controller.ex
index 544ee99f..b57b6b1e 100644
--- a/lib/radiator_web/controllers/api/chapters_controller.ex
+++ b/lib/radiator_web/controllers/api/chapters_controller.ex
@@ -52,7 +52,7 @@ defmodule RadiatorWeb.Api.ChaptersController do
       error = {:error, _} ->
         conn
         |> RadiatorWeb.Api.FallbackController.call(error)
-        |> Plug.Conn.halt()
+        |> halt()
     end
   end
 end

From fb51cfade7587ed007ed08f8c16d09257cb89ce8 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Fri, 12 Jul 2019 11:00:15 +0200
Subject: [PATCH 10/21] add fixme comment

---
 lib/radiator/media/audio_file.ex | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/radiator/media/audio_file.ex b/lib/radiator/media/audio_file.ex
index b8df52ac..c77cb8d3 100644
--- a/lib/radiator/media/audio_file.ex
+++ b/lib/radiator/media/audio_file.ex
@@ -31,6 +31,7 @@ defmodule Radiator.Media.AudioFile do
     # todo: determine byte length and mime type on file change, don't take them as attrs
   end
 
+  # fixme: auidofile.upload geht bei dom nicht mehr
   # not sure if this is the best way
   # via https://elixirforum.com/t/ecto-validating-belongs-to-association-is-not-nil/2665/13
   defp cast_or_constraint_assoc(changeset, name) do

From 25b1adf800b2b4e17cba4e8baa80957e9f8250b0 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Wed, 17 Jul 2019 15:24:35 +0200
Subject: [PATCH 11/21] make `[:audio_id, :start]` chapter primary key

---
 .iex.exs                                        |  2 ++
 guides/rest_api.md                              |  8 +++++---
 lib/radiator/audio_meta/chapter.ex              |  5 +++--
 lib/radiator/directory/editor.ex                |  4 ++--
 lib/radiator/media/chapter_image.ex             |  2 +-
 .../controllers/api/chapters_controller.ex      | 12 ++++++------
 lib/radiator_web/router.ex                      |  4 +++-
 lib/radiator_web/views/api/chapters_view.ex     |  3 +--
 priv/repo/migrations/0600_create_chapters.exs   |  8 ++++----
 .../api/chapters_controller_test.exs            | 17 ++---------------
 test/support/factory.ex                         |  2 +-
 11 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/.iex.exs b/.iex.exs
index 1ec54101..ac713985 100644
--- a/.iex.exs
+++ b/.iex.exs
@@ -38,6 +38,8 @@ alias Radiator.Directory.{Audio, Episode, Podcast, Network, Editor}
 alias Radiator.Media
 alias Radiator.Media.AudioFile
 
+alias Radiator.AudioMeta.Chapter
+
 alias Radiator.Feed.Builder
 
 alias Radiator.Auth
diff --git a/guides/rest_api.md b/guides/rest_api.md
index 142135ac..065db480 100644
--- a/guides/rest_api.md
+++ b/guides/rest_api.md
@@ -363,6 +363,8 @@ GET /api/rest/v1/audio_file/:id
 
 ## Audio Chapters
 
+> ⚠️ A chapter is uniquely identified by its start time and the associated audio. There can only be exactly one chapter per audio with a given start time.
+
 ### Parameters for Create & Update
 
 | Name                | Type      | Description                                                    |
@@ -382,17 +384,17 @@ POST /api/rest/v1/audios/:audio_id/chapters
 ### Read
 
 ```
-GET /api/rest/v1/audios/:audio_id/chapters/:id
+GET /api/rest/v1/audios/:audio_id/chapters/:start
 ```
 
 ### Update
 
 ```
-PATCH /api/rest/v1/audios/:audio_id/chapters/:id
+PATCH /api/rest/v1/audios/:audio_id/chapters/:start
 ```
 
 ### Delete
 
 ```
-DELETE /api/rest/v1/audios/:audio_id/chapters/:id
+DELETE /api/rest/v1/audios/:audio_id/chapters/:start
 ```
diff --git a/lib/radiator/audio_meta/chapter.ex b/lib/radiator/audio_meta/chapter.ex
index 3c2774d1..36ea51e0 100644
--- a/lib/radiator/audio_meta/chapter.ex
+++ b/lib/radiator/audio_meta/chapter.ex
@@ -8,13 +8,14 @@ defmodule Radiator.AudioMeta.Chapter do
   alias Radiator.Directory.Audio
   alias Radiator.Media
 
+  @primary_key false
   schema "chapters" do
-    field :start, :integer
+    field :start, :integer, primary_key: true
     field :title, :string
     field :link, :string
     field :image, Media.ChapterImage.Type
 
-    belongs_to :audio, Audio
+    belongs_to :audio, Audio, primary_key: true
   end
 
   @doc false
diff --git a/lib/radiator/directory/editor.ex b/lib/radiator/directory/editor.ex
index cfbf0b1d..43731e2f 100644
--- a/lib/radiator/directory/editor.ex
+++ b/lib/radiator/directory/editor.ex
@@ -444,8 +444,8 @@ defmodule Radiator.Directory.Editor do
     episode
   end
 
-  def get_chapter(actor = %Auth.User{}, id) do
-    case Repo.get(Chapter, id) do
+  def get_chapter(actor = %Auth.User{}, audio = %Audio{}, start) do
+    case Repo.get_by(Chapter, audio_id: audio.id, start: start) do
       nil ->
         @not_found_match
 
diff --git a/lib/radiator/media/chapter_image.ex b/lib/radiator/media/chapter_image.ex
index 05f3f0c3..ddd761a0 100644
--- a/lib/radiator/media/chapter_image.ex
+++ b/lib/radiator/media/chapter_image.ex
@@ -9,6 +9,6 @@ defmodule Radiator.Media.ChapterImage do
   end
 
   def storage_dir(_version, {_file, chapter}) do
-    "chapter/#{chapter.id}"
+    "chapter/#{chapter.audio_id}_#{chapter.start}"
   end
 end
diff --git a/lib/radiator_web/controllers/api/chapters_controller.ex b/lib/radiator_web/controllers/api/chapters_controller.ex
index b57b6b1e..b78d897b 100644
--- a/lib/radiator_web/controllers/api/chapters_controller.ex
+++ b/lib/radiator_web/controllers/api/chapters_controller.ex
@@ -3,7 +3,7 @@ defmodule RadiatorWeb.Api.ChaptersController do
 
   alias Radiator.Directory.Editor
 
-  plug :assign_audio when action in [:create, :update]
+  plug :assign_audio when action in [:create, :update, :delete]
 
   def create(conn, %{"chapter" => chapter_params}) do
     with user <- current_user(conn),
@@ -12,16 +12,16 @@ defmodule RadiatorWeb.Api.ChaptersController do
       |> put_status(:created)
       |> put_resp_header(
         "location",
-        Routes.api_audio_chapters_path(conn, :show, conn.assigns[:audio].id, chapter)
+        Routes.api_audio_chapters_path(conn, :show, conn.assigns[:audio].id, chapter.start)
       )
       |> assign(:chapter, chapter)
       |> render("show.json")
     end
   end
 
-  def update(conn, %{"id" => id, "chapter" => chapter_params}) do
+  def update(conn, %{"start" => start, "chapter" => chapter_params}) do
     with user <- current_user(conn),
-         {:ok, chapter} <- Editor.get_chapter(user, id),
+         {:ok, chapter} <- Editor.get_chapter(user, conn.assigns[:audio], start),
          {:ok, chapter} <- Editor.update_chapter(user, chapter, chapter_params) do
       conn
       |> assign(:chapter, chapter)
@@ -29,9 +29,9 @@ defmodule RadiatorWeb.Api.ChaptersController do
     end
   end
 
-  def delete(conn, %{"id" => id}) do
+  def delete(conn, %{"start" => start}) do
     with user <- current_user(conn),
-         {:ok, chapter} <- Editor.get_chapter(user, id),
+         {:ok, chapter} <- Editor.get_chapter(user, conn.assigns[:audio], start),
          {:ok, _} <- Editor.delete_chapter(user, chapter) do
       send_delete_resp(conn)
     else
diff --git a/lib/radiator_web/router.ex b/lib/radiator_web/router.ex
index fdcb661c..76f5c94c 100644
--- a/lib/radiator_web/router.ex
+++ b/lib/radiator_web/router.ex
@@ -103,7 +103,9 @@ defmodule RadiatorWeb.Router do
     resources "/episodes", EpisodeController, only: [:show, :create, :update, :delete]
 
     resources "/audios", AudioController, only: [:show, :create, :update, :delete] do
-      resources "/chapters", ChaptersController, only: [:show, :create, :update, :delete]
+      resources "/chapters", ChaptersController,
+        param: "start",
+        only: [:show, :create, :update, :delete]
     end
 
     # todo: pluralize
diff --git a/lib/radiator_web/views/api/chapters_view.ex b/lib/radiator_web/views/api/chapters_view.ex
index b8dc0b7f..13ed3da3 100644
--- a/lib/radiator_web/views/api/chapters_view.ex
+++ b/lib/radiator_web/views/api/chapters_view.ex
@@ -12,10 +12,9 @@ defmodule RadiatorWeb.Api.ChaptersView do
     %Document{}
     |> Document.add_link(%Link{
       rel: "self",
-      href: Routes.api_audio_chapters_path(conn, :show, audio.id, chapter.id)
+      href: Routes.api_audio_chapters_path(conn, :show, audio.id, chapter.start)
     })
     |> Document.add_properties(%{
-      id: chapter.id,
       title: chapter.title,
       start: chapter.start,
       link: chapter.link,
diff --git a/priv/repo/migrations/0600_create_chapters.exs b/priv/repo/migrations/0600_create_chapters.exs
index 8014a882..dd7a2244 100644
--- a/priv/repo/migrations/0600_create_chapters.exs
+++ b/priv/repo/migrations/0600_create_chapters.exs
@@ -2,13 +2,13 @@ defmodule Radiator.Repo.Migrations.CreateChapters do
   use Ecto.Migration
 
   def change do
-    create table(:chapters) do
-      add :start, :integer
+    create table(:chapters, primary_key: false) do
+      add :audio_id, references(:audios, on_delete: :delete_all), primary_key: true
+
+      add :start, :integer, primary_key: true
       add :title, :text
       add :link, :text
       add :image, :string
-
-      add :audio_id, references(:audios, on_delete: :delete_all)
     end
   end
 end
diff --git a/test/radiator_web/controllers/api/chapters_controller_test.exs b/test/radiator_web/controllers/api/chapters_controller_test.exs
index 9efb4947..f4521bd3 100644
--- a/test/radiator_web/controllers/api/chapters_controller_test.exs
+++ b/test/radiator_web/controllers/api/chapters_controller_test.exs
@@ -45,32 +45,19 @@ defmodule RadiatorWeb.Api.ChaptersControllerTest do
       chapter = insert(:chapter, audio: audio)
 
       conn =
-        put(conn, Routes.api_audio_chapters_path(conn, :update, audio.id, chapter.id), %{
+        put(conn, Routes.api_audio_chapters_path(conn, :update, audio.id, chapter.start), %{
           chapter: %{title: "new"}
         })
 
       assert %{"title" => "new"} = json_response(conn, 200)
     end
-
-    test "renders error when accessing foreign chapter", %{conn: conn, user: user} do
-      audio = insert(:audio) |> owned_by(user)
-      _chapter = insert(:chapter, audio: audio)
-      other_chapter = insert(:chapter, audio: build(:audio))
-
-      conn =
-        put(conn, Routes.api_audio_chapters_path(conn, :update, audio.id, other_chapter.id), %{
-          chapter: %{title: "new"}
-        })
-
-      assert json_response(conn, 401)
-    end
   end
 
   test "delete chapter", %{conn: conn, user: user} do
     audio = insert(:audio) |> owned_by(user)
     chapter = insert(:chapter, audio: audio)
 
-    conn = delete(conn, Routes.api_audio_chapters_path(conn, :delete, audio.id, chapter.id))
+    conn = delete(conn, Routes.api_audio_chapters_path(conn, :delete, audio.id, chapter.start))
 
     assert response(conn, 204)
   end
diff --git a/test/support/factory.ex b/test/support/factory.ex
index f736ce1b..65757473 100644
--- a/test/support/factory.ex
+++ b/test/support/factory.ex
@@ -148,7 +148,7 @@ defmodule Radiator.Factory do
 
   def chapter_factory do
     %Radiator.AudioMeta.Chapter{
-      start: 0,
+      start: sequence(:start, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]),
       title: sequence(:title, &"chapter #{&1}"),
       link: sequence(:link, &"http://example.com/#{&1}"),
       audio: build(:audio)

From 8d9a93e6ec6eea8846b2c8455d9c91a79a7483a1 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 18 Jul 2019 10:43:24 +0200
Subject: [PATCH 12/21] remove "set chapters" mutation

---
 .../graphql/admin/resolvers/editor.ex         | 10 ----
 lib/radiator_web/graphql/schema.ex            | 10 ----
 .../admin/schema/mutation/episodes_test.exs   | 51 -------------------
 3 files changed, 71 deletions(-)

diff --git a/lib/radiator_web/graphql/admin/resolvers/editor.ex b/lib/radiator_web/graphql/admin/resolvers/editor.ex
index 0d69ed2d..8fc2a393 100644
--- a/lib/radiator_web/graphql/admin/resolvers/editor.ex
+++ b/lib/radiator_web/graphql/admin/resolvers/editor.ex
@@ -284,16 +284,6 @@ defmodule RadiatorWeb.GraphQL.Admin.Resolvers.Editor do
     {:ok, AudioMeta.list_chapters(audio)}
   end
 
-  def set_episode_chapters(_parent, %{id: id, chapters: chapters, type: type}, %{
-        context: %{current_user: user}
-      }) do
-    with_audio user, id do
-      fn audio ->
-        AudioMeta.set_chapters(audio, chapters, String.to_existing_atom(type))
-      end
-    end
-  end
-
   def get_audio_files(audio = %Audio{}, _args, %{context: %{current_user: user}}) do
     {:ok, Editor.list_audio_files(user, audio)}
   end
diff --git a/lib/radiator_web/graphql/schema.ex b/lib/radiator_web/graphql/schema.ex
index d6f919e0..e9e1485c 100644
--- a/lib/radiator_web/graphql/schema.ex
+++ b/lib/radiator_web/graphql/schema.ex
@@ -311,15 +311,5 @@ defmodule RadiatorWeb.GraphQL.Schema do
       middleware Middleware.RequireAuthentication
       resolve &Admin.Resolvers.Editor.delete_episode/3
     end
-
-    @desc "Set chapters for an episode"
-    field :set_chapters, type: :audio do
-      arg :id, non_null(:id)
-      arg :chapters, non_null(:string)
-      arg :type, non_null(:string)
-
-      middleware Middleware.RequireAuthentication
-      resolve &Admin.Resolvers.Editor.set_episode_chapters/3
-    end
   end
 end
diff --git a/test/radiator_web/graphql/admin/schema/mutation/episodes_test.exs b/test/radiator_web/graphql/admin/schema/mutation/episodes_test.exs
index 01ef0c5e..7daa0634 100644
--- a/test/radiator_web/graphql/admin/schema/mutation/episodes_test.exs
+++ b/test/radiator_web/graphql/admin/schema/mutation/episodes_test.exs
@@ -340,55 +340,4 @@ defmodule RadiatorWeb.GraphQL.Schema.Mutation.EpisodesTest do
 
     assert %{"errors" => [%{"message" => "Entity not found"}]} = json_response(conn, 200)
   end
-
-  @set_chapters_query """
-  mutation ($id: ID!, $chapters: String!, $type: String!) {
-    setChapters(id: $id, chapters: $chapters, type: $type) {
-      chapters {
-        start
-        title
-        link
-      }
-    }
-  }
-  """
-
-  test "setChapters sets chapters for audio", %{conn: conn, user: user} do
-    audio = insert(:audio) |> owned_by(user)
-
-    chapters = ~S"""
-    00:00:01.234 Intro <http://example.com>
-    00:12:34.000 About us
-    01:02:03.000 Later
-    """
-
-    conn =
-      post conn, "/api/graphql",
-        query: @set_chapters_query,
-        variables: %{"chapters" => chapters, "id" => audio.id, "type" => "mp4chaps"}
-
-    assert %{
-             "data" => %{
-               "setChapters" => %{
-                 "chapters" => [
-                   %{
-                     "start" => 1234,
-                     "title" => "Intro",
-                     "link" => "http://example.com"
-                   },
-                   %{
-                     "start" => 754_000,
-                     "title" => "About us",
-                     "link" => nil
-                   },
-                   %{
-                     "start" => 3_723_000,
-                     "title" => "Later",
-                     "link" => nil
-                   }
-                 ]
-               }
-             }
-           } = json_response(conn, 200)
-  end
 end

From 825e549df5f0f01a300e9e95cd4d681e842367a5 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 18 Jul 2019 10:43:43 +0200
Subject: [PATCH 13/21] consolidate list_chapters/3 resolver

---
 lib/radiator_web/graphql/admin/resolvers/editor.ex | 7 -------
 lib/radiator_web/graphql/admin/types.ex            | 2 +-
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/lib/radiator_web/graphql/admin/resolvers/editor.ex b/lib/radiator_web/graphql/admin/resolvers/editor.ex
index 8fc2a393..94a0b368 100644
--- a/lib/radiator_web/graphql/admin/resolvers/editor.ex
+++ b/lib/radiator_web/graphql/admin/resolvers/editor.ex
@@ -306,13 +306,6 @@ defmodule RadiatorWeb.GraphQL.Admin.Resolvers.Editor do
     end
   end
 
-  def get_chapters(%Audio{} = audio, _, _) do
-    chapter_query = Radiator.AudioMeta.Chapter.ordered_query()
-    audio = Radiator.Repo.preload(audio, chapters: chapter_query)
-
-    {:ok, audio.chapters}
-  end
-
   def get_duration(%Episode{audio: audio}, _, _) do
     {:ok, audio.duration}
   end
diff --git a/lib/radiator_web/graphql/admin/types.ex b/lib/radiator_web/graphql/admin/types.ex
index 169896f0..20fb0dab 100644
--- a/lib/radiator_web/graphql/admin/types.ex
+++ b/lib/radiator_web/graphql/admin/types.ex
@@ -173,7 +173,7 @@ defmodule RadiatorWeb.GraphQL.Admin.Types do
       arg :order, type: :sort_order, default_value: :asc
 
       # resolve dataloader(Radiator.AudioMeta, :chapters)
-      resolve &Resolvers.Editor.get_chapters/3
+      resolve &Resolvers.Editor.list_chapters/3
     end
 
     field :episodes, list_of(:episode) do

From 031fa87fd084cb964c9448f77d7ad89ff989403e Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 18 Jul 2019 10:47:05 +0200
Subject: [PATCH 14/21] rest: include audio_id in chapter

---
 lib/radiator_web/views/api/chapters_view.ex | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/radiator_web/views/api/chapters_view.ex b/lib/radiator_web/views/api/chapters_view.ex
index 13ed3da3..b55baf19 100644
--- a/lib/radiator_web/views/api/chapters_view.ex
+++ b/lib/radiator_web/views/api/chapters_view.ex
@@ -15,8 +15,9 @@ defmodule RadiatorWeb.Api.ChaptersView do
       href: Routes.api_audio_chapters_path(conn, :show, audio.id, chapter.start)
     })
     |> Document.add_properties(%{
-      title: chapter.title,
+      audio_id: chapter.audio_id,
       start: chapter.start,
+      title: chapter.title,
       link: chapter.link,
       image: Chapter.image_url(chapter)
     })

From a205db160c719192baae9ff9444e7a1f5eb724a3 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 18 Jul 2019 10:51:54 +0200
Subject: [PATCH 15/21] add fixme comment for chapter image

---
 lib/radiator/media/chapter_image.ex | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/radiator/media/chapter_image.ex b/lib/radiator/media/chapter_image.ex
index ddd761a0..e4f3fc47 100644
--- a/lib/radiator/media/chapter_image.ex
+++ b/lib/radiator/media/chapter_image.ex
@@ -8,6 +8,8 @@ defmodule Radiator.Media.ChapterImage do
     "#{basename}_#{version}"
   end
 
+  # fixme: when the start is changed after the image is uploaded the connection to the image is probably lost?
+  # reintroduce a stable internal id just for this, or find another solution
   def storage_dir(_version, {_file, chapter}) do
     "chapter/#{chapter.audio_id}_#{chapter.start}"
   end

From e584565829c326f1cfecf55cf3fe87166cf55ec1 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 18 Jul 2019 11:36:08 +0200
Subject: [PATCH 16/21] delete previous chapter image on update

---
 lib/radiator/audio_meta/chapter.ex  | 8 ++++++++
 lib/radiator/media/chapter_image.ex | 4 +---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/lib/radiator/audio_meta/chapter.ex b/lib/radiator/audio_meta/chapter.ex
index 36ea51e0..9f16f6c9 100644
--- a/lib/radiator/audio_meta/chapter.ex
+++ b/lib/radiator/audio_meta/chapter.ex
@@ -27,9 +27,17 @@ defmodule Radiator.AudioMeta.Chapter do
       :link
     ])
     |> validate_required([:start])
+    |> maybe_delete_existing_attachments(attrs)
     |> cast_attachments(attrs, [:image], allow_paths: true, allow_urls: true)
   end
 
+  def maybe_delete_existing_attachments(changeset, %{image: _image}) do
+    Radiator.AudioMeta.delete_chapter_image(changeset.data)
+    changeset
+  end
+
+  def maybe_delete_existing_attachments(changeset, _attrs), do: changeset
+
   @doc """
   Convenience accessor for image URL.
   """
diff --git a/lib/radiator/media/chapter_image.ex b/lib/radiator/media/chapter_image.ex
index e4f3fc47..f77827a6 100644
--- a/lib/radiator/media/chapter_image.ex
+++ b/lib/radiator/media/chapter_image.ex
@@ -8,9 +8,7 @@ defmodule Radiator.Media.ChapterImage do
     "#{basename}_#{version}"
   end
 
-  # fixme: when the start is changed after the image is uploaded the connection to the image is probably lost?
-  # reintroduce a stable internal id just for this, or find another solution
   def storage_dir(_version, {_file, chapter}) do
-    "chapter/#{chapter.audio_id}_#{chapter.start}"
+    "chapter/#{chapter.audio_id}"
   end
 end

From 1b650a55b85027a9e594c94f0f1ddad3d76d2580 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 18 Jul 2019 11:39:32 +0200
Subject: [PATCH 17/21] delete chapter deletes chapter image

---
 lib/radiator/audio_meta/audio_meta.ex | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/radiator/audio_meta/audio_meta.ex b/lib/radiator/audio_meta/audio_meta.ex
index 915353a4..8e672d8d 100644
--- a/lib/radiator/audio_meta/audio_meta.ex
+++ b/lib/radiator/audio_meta/audio_meta.ex
@@ -40,6 +40,7 @@ defmodule Radiator.AudioMeta do
   end
 
   def delete_chapter(chapter = %Chapter{}) do
+    delete_chapter_image(chapter)
     Repo.delete(chapter)
   end
 

From 6f3c769d4e3f6fe8bf5a44efdebb216b0eeb6918 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 18 Jul 2019 12:18:10 +0200
Subject: [PATCH 18/21] rest: add missing `show` action

---
 .../controllers/api/chapters_controller.ex    | 20 ++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/lib/radiator_web/controllers/api/chapters_controller.ex b/lib/radiator_web/controllers/api/chapters_controller.ex
index b78d897b..93a7ae07 100644
--- a/lib/radiator_web/controllers/api/chapters_controller.ex
+++ b/lib/radiator_web/controllers/api/chapters_controller.ex
@@ -3,7 +3,12 @@ defmodule RadiatorWeb.Api.ChaptersController do
 
   alias Radiator.Directory.Editor
 
-  plug :assign_audio when action in [:create, :update, :delete]
+  plug :assign_audio when action in [:show, :create, :update, :delete]
+  plug :assign_chapter when action in [:show]
+
+  def show(conn, _params) do
+    render(conn, "show.json")
+  end
 
   def create(conn, %{"chapter" => chapter_params}) do
     with user <- current_user(conn),
@@ -55,4 +60,17 @@ defmodule RadiatorWeb.Api.ChaptersController do
         |> halt()
     end
   end
+
+  defp assign_chapter(conn, _) do
+    with {:ok, chapter} <-
+           Editor.get_chapter(current_user(conn), conn.assigns[:audio], conn.params["start"]) do
+      conn
+      |> assign(:chapter, chapter)
+    else
+      error = {:error, _} ->
+        conn
+        |> RadiatorWeb.Api.FallbackController.call(error)
+        |> halt()
+    end
+  end
 end

From 00d716dba6fd5b7ab104bac58c4e5068c988c5da Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 18 Jul 2019 12:20:29 +0200
Subject: [PATCH 19/21] refactor: use :assign_chapter plug

---
 .../controllers/api/chapters_controller.ex           | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/lib/radiator_web/controllers/api/chapters_controller.ex b/lib/radiator_web/controllers/api/chapters_controller.ex
index 93a7ae07..b68cff87 100644
--- a/lib/radiator_web/controllers/api/chapters_controller.ex
+++ b/lib/radiator_web/controllers/api/chapters_controller.ex
@@ -4,7 +4,7 @@ defmodule RadiatorWeb.Api.ChaptersController do
   alias Radiator.Directory.Editor
 
   plug :assign_audio when action in [:show, :create, :update, :delete]
-  plug :assign_chapter when action in [:show]
+  plug :assign_chapter when action in [:show, :update, :delete]
 
   def show(conn, _params) do
     render(conn, "show.json")
@@ -24,20 +24,18 @@ defmodule RadiatorWeb.Api.ChaptersController do
     end
   end
 
-  def update(conn, %{"start" => start, "chapter" => chapter_params}) do
+  def update(conn, %{"chapter" => chapter_params}) do
     with user <- current_user(conn),
-         {:ok, chapter} <- Editor.get_chapter(user, conn.assigns[:audio], start),
-         {:ok, chapter} <- Editor.update_chapter(user, chapter, chapter_params) do
+         {:ok, chapter} <- Editor.update_chapter(user, conn.assigns[:chapter], chapter_params) do
       conn
       |> assign(:chapter, chapter)
       |> render("show.json")
     end
   end
 
-  def delete(conn, %{"start" => start}) do
+  def delete(conn, _) do
     with user <- current_user(conn),
-         {:ok, chapter} <- Editor.get_chapter(user, conn.assigns[:audio], start),
-         {:ok, _} <- Editor.delete_chapter(user, chapter) do
+         {:ok, _} <- Editor.delete_chapter(user, conn.assigns[:chapter]) do
       send_delete_resp(conn)
     else
       @not_found_match -> send_delete_resp(conn)

From 1b5e78618ac3454b92981369c541de724916ef7e Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 18 Jul 2019 12:28:36 +0200
Subject: [PATCH 20/21] permissions: check directly against audio

---
 lib/radiator/directory/editor.ex            |  6 +++---
 lib/radiator/directory/editor/permission.ex | 10 ----------
 2 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/lib/radiator/directory/editor.ex b/lib/radiator/directory/editor.ex
index 43731e2f..312814a1 100644
--- a/lib/radiator/directory/editor.ex
+++ b/lib/radiator/directory/editor.ex
@@ -450,7 +450,7 @@ defmodule Radiator.Directory.Editor do
         @not_found_match
 
       chapter = %Chapter{} ->
-        if has_permission(actor, chapter, :readonly) do
+        if has_permission(actor, audio, :readonly) do
           {:ok, chapter}
         else
           @not_authorized_match
@@ -467,7 +467,7 @@ defmodule Radiator.Directory.Editor do
   end
 
   def update_chapter(actor = %Auth.User{}, chapter = %Chapter{}, attrs) do
-    if has_permission(actor, chapter, :edit) do
+    if has_permission(actor, %Audio{id: chapter.audio_id}, :edit) do
       AudioMeta.update_chapter(chapter, attrs)
     else
       @not_authorized_match
@@ -475,7 +475,7 @@ defmodule Radiator.Directory.Editor do
   end
 
   def delete_chapter(actor = %Auth.User{}, chapter = %Chapter{}) do
-    if has_permission(actor, chapter, :own) do
+    if has_permission(actor, %Audio{id: chapter.audio_id}, :own) do
       AudioMeta.delete_chapter(chapter)
     else
       @not_authorized_match
diff --git a/lib/radiator/directory/editor/permission.ex b/lib/radiator/directory/editor/permission.ex
index 4437cf6c..b5e24cc9 100644
--- a/lib/radiator/directory/editor/permission.ex
+++ b/lib/radiator/directory/editor/permission.ex
@@ -26,9 +26,6 @@ defmodule Radiator.Directory.Editor.Permission do
   def get_permission(user = %Auth.User{}, subject = %Audio{}),
     do: do_get_permission(user, subject)
 
-  def get_permission(_user = %Auth.User{}, _subject = %Chapter{}),
-    do: nil
-
   defp do_get_permission(user, subject) do
     case fetch_permission(user, subject) do
       nil -> nil
@@ -87,13 +84,6 @@ defmodule Radiator.Directory.Editor.Permission do
     |> List.wrap()
   end
 
-  defp parents(subject = %Chapter{}) do
-    subject
-    |> Ecto.assoc(:audio)
-    |> Repo.one!()
-    |> List.wrap()
-  end
-
   defp parents(_) do
     []
   end

From e0b367f10cb160f99482e290b7875d6edeb198e8 Mon Sep 17 00:00:00 2001
From: Eric Teubert <eric@ericteubert.de>
Date: Thu, 18 Jul 2019 16:10:02 +0200
Subject: [PATCH 21/21] refactor: introduce `apply_action_fallback`

---
 .../controllers/api/chapters_controller.ex    | 35 ++++++++++---------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/lib/radiator_web/controllers/api/chapters_controller.ex b/lib/radiator_web/controllers/api/chapters_controller.ex
index b68cff87..6bf957f0 100644
--- a/lib/radiator_web/controllers/api/chapters_controller.ex
+++ b/lib/radiator_web/controllers/api/chapters_controller.ex
@@ -44,31 +44,32 @@ defmodule RadiatorWeb.Api.ChaptersController do
   end
 
   defp assign_audio(conn, _) do
-    conn
-    |> current_user()
-    |> Editor.get_audio(conn.params["audio_id"])
-    |> case do
-      {:ok, audio} ->
-        conn
-        |> assign(:audio, audio)
-
-      error = {:error, _} ->
-        conn
-        |> RadiatorWeb.Api.FallbackController.call(error)
-        |> halt()
+    with {:ok, audio} <-
+           conn
+           |> current_user()
+           |> Editor.get_audio(conn.params["audio_id"]) do
+      conn
+      |> assign(:audio, audio)
+    else
+      response -> apply_action_fallback(conn, response)
     end
   end
 
   defp assign_chapter(conn, _) do
     with {:ok, chapter} <-
-           Editor.get_chapter(current_user(conn), conn.assigns[:audio], conn.params["start"]) do
+           conn
+           |> current_user()
+           |> Editor.get_chapter(conn.assigns[:audio], conn.params["start"]) do
       conn
       |> assign(:chapter, chapter)
     else
-      error = {:error, _} ->
-        conn
-        |> RadiatorWeb.Api.FallbackController.call(error)
-        |> halt()
+      response -> apply_action_fallback(conn, response)
+    end
+  end
+
+  defp apply_action_fallback(conn, response) do
+    case @phoenix_fallback do
+      {:module, module} -> apply(module, :call, [conn, response]) |> halt()
     end
   end
 end