Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue/101 REST API: Chapter Management #152

Merged
merged 23 commits into from
Jul 18, 2019
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
9e5a494
add(api/chapters): create
eteubert Jul 4, 2019
81ecff6
add(api/chapters): update
eteubert Jul 4, 2019
a09deca
add(api/chapters): delete
eteubert Jul 4, 2019
462d8c1
refactor: contorller only accesses Directory.Editor
eteubert Jul 4, 2019
fcbbe1c
Merge branch 'master' into issue/101-api-chapter-management
eteubert Jul 4, 2019
d8d80a8
refactor: use :rest_controller
eteubert Jul 4, 2019
8223a3a
refactor: extract assign_audio/2 plug
eteubert Jul 4, 2019
07b8446
make chapter start time required
eteubert Jul 4, 2019
e3bd205
doc: add chapter api doc
eteubert Jul 4, 2019
6aa3564
refactor: shorten halt call
eteubert Jul 4, 2019
fb51cfa
add fixme comment
eteubert Jul 12, 2019
7291e7b
Merge branch 'master' into issue/101-api-chapter-management
eteubert Jul 17, 2019
25b1adf
make `[:audio_id, :start]` chapter primary key
eteubert Jul 17, 2019
8d9a93e
remove "set chapters" mutation
eteubert Jul 18, 2019
825e549
consolidate list_chapters/3 resolver
eteubert Jul 18, 2019
031fa87
rest: include audio_id in chapter
eteubert Jul 18, 2019
a205db1
add fixme comment for chapter image
eteubert Jul 18, 2019
e584565
delete previous chapter image on update
eteubert Jul 18, 2019
1b650a5
delete chapter deletes chapter image
eteubert Jul 18, 2019
6f3c769
rest: add missing `show` action
eteubert Jul 18, 2019
00d716d
refactor: use :assign_chapter plug
eteubert Jul 18, 2019
1b5e786
permissions: check directly against audio
eteubert Jul 18, 2019
e0b367f
refactor: introduce `apply_action_fallback`
eteubert Jul 18, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .iex.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 44 additions & 0 deletions guides/rest_api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -354,3 +360,41 @@ POST /api/rest/v1/audio_file
```
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 |
| ------------------- | --------- | -------------------------------------------------------------- |
| `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/:start
```

### Update

```
PATCH /api/rest/v1/audios/:audio_id/chapters/:start
```

### Delete

```
DELETE /api/rest/v1/audios/:audio_id/chapters/:start
```
5 changes: 5 additions & 0 deletions lib/radiator/audio_meta/audio_meta.ex
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ defmodule Radiator.AudioMeta do
|> Repo.all()
end

def delete_chapter(chapter = %Chapter{}) do
delete_chapter_image(chapter)
Repo.delete(chapter)
end

def delete_chapters(%Audio{} = audio) do
audio = Repo.preload(audio, :chapters)

Expand Down
14 changes: 12 additions & 2 deletions lib/radiator/audio_meta/chapter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -25,9 +26,18 @@ defmodule Radiator.AudioMeta.Chapter do
:title,
: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.
"""
Expand Down
40 changes: 40 additions & 0 deletions lib/radiator/directory/editor.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down Expand Up @@ -442,6 +444,44 @@ defmodule Radiator.Directory.Editor do
episode
end

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

chapter = %Chapter{} ->
if has_permission(actor, audio, :readonly) do
{:ok, chapter}
else
@not_authorized_match
end
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, %Audio{id: chapter.audio_id}, :edit) do
AudioMeta.update_chapter(chapter, attrs)
eteubert marked this conversation as resolved.
Show resolved Hide resolved
else
@not_authorized_match
end
end

def delete_chapter(actor = %Auth.User{}, chapter = %Chapter{}) do
if has_permission(actor, %Audio{id: chapter.audio_id}, :own) do
AudioMeta.delete_chapter(chapter)
eteubert marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
1 change: 1 addition & 0 deletions lib/radiator/directory/editor/permission.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
1 change: 1 addition & 0 deletions lib/radiator/media/audio_file.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/radiator/media/chapter_image.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ defmodule Radiator.Media.ChapterImage do
end

def storage_dir(_version, {_file, chapter}) do
"chapter/#{chapter.id}"
"chapter/#{chapter.audio_id}"
end
end
74 changes: 74 additions & 0 deletions lib/radiator_web/controllers/api/chapters_controller.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
defmodule RadiatorWeb.Api.ChaptersController do
use RadiatorWeb, :rest_controller

alias Radiator.Directory.Editor

plug :assign_audio when action in [:show, :create, :update, :delete]
plug :assign_chapter when action in [:show, :update, :delete]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah - i see what you did there. Interesting pattern.


def show(conn, _params) do
render(conn, "show.json")
end

def create(conn, %{"chapter" => chapter_params}) do
with user <- current_user(conn),
{: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, conn.assigns[:audio].id, chapter.start)
)
|> assign(:chapter, chapter)
|> render("show.json")
end
end

def update(conn, %{"chapter" => chapter_params}) do
with user <- current_user(conn),
{:ok, chapter} <- Editor.update_chapter(user, conn.assigns[:chapter], chapter_params) do
conn
|> assign(:chapter, chapter)
|> render("show.json")
end
end

eteubert marked this conversation as resolved.
Show resolved Hide resolved
def delete(conn, _) do
with user <- current_user(conn),
{:ok, _} <- Editor.delete_chapter(user, conn.assigns[:chapter]) do
send_delete_resp(conn)
else
@not_found_match -> send_delete_resp(conn)
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, _} ->
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned before, this is probably better done using a raise and catch the exception the way the original not found is done

conn
|> RadiatorWeb.Api.FallbackController.call(error)
|> 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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, shouldn't we somehow just refer to the assigned fallbackcontroller? e.g. something like

apply(@phoenix_fallback, :call, [error])

? - in the middle of some other change now, can't verify if that is the way this should work. but the falback seems to be in the phoenix_fallback attribute

|> halt()
end
end
end
17 changes: 0 additions & 17 deletions lib/radiator_web/graphql/admin/resolvers/editor.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -316,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
Expand Down
2 changes: 1 addition & 1 deletion lib/radiator_web/graphql/admin/types.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions lib/radiator_web/graphql/schema.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 6 additions & 1 deletion lib/radiator_web/router.ex
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,12 @@ 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,
param: "start",
only: [:show, :create, :update, :delete]
end

# todo: pluralize
resources "/audio_file", AudioFileController, only: [:show, :create]
Expand Down
25 changes: 25 additions & 0 deletions lib/radiator_web/views/api/chapters_view.ex
Original file line number Diff line number Diff line change
@@ -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.start)
})
|> Document.add_properties(%{
audio_id: chapter.audio_id,
start: chapter.start,
title: chapter.title,
link: chapter.link,
image: Chapter.image_url(chapter)
})
end
end
8 changes: 4 additions & 4 deletions priv/repo/migrations/0600_create_chapters.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading