From 0cf96f88be9240f55c611644e9bebeb3e9d50e5b Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Sat, 4 Jul 2026 09:35:17 +0000 Subject: [PATCH] Fix API controller tests: 404 handling, JSON encoding, media filtering - Add FallbackController for proper 404 JSON responses on missing resources - Add non-bang get_source/1 and get_media_item/1 for safe lookups - Fix Task JSON encoder: remove Repo.preload call (not available in encoder scope), handle NotLoaded associations by returning nil - Fix media item index: use list_media_items_for_source/1 instead of list_pending_media_items_for/1 which had overly restrictive filtering - Add missing extras_directory to test config - All 1007 tests pass --- config/test.exs | 1 + lib/pinchflat/media/media.ex | 11 ++++++++++ lib/pinchflat/sources/sources.ex | 1 + lib/pinchflat/tasks/task.ex | 13 ++++++++++-- .../api/v1/api_media_item_controller.ex | 17 +++++++++------- .../api/v1/api_source_controller.ex | 20 +++++++++++++------ .../controllers/api/v1/api_task_controller.ex | 2 ++ .../controllers/api/v1/fallback_controller.ex | 18 +++++++++++++++++ 8 files changed, 68 insertions(+), 15 deletions(-) create mode 100644 lib/pinchflat_web/controllers/api/v1/fallback_controller.ex diff --git a/config/test.exs b/config/test.exs index 4021e3f..4a42e62 100644 --- a/config/test.exs +++ b/config/test.exs @@ -6,6 +6,7 @@ config :pinchflat, apprise_executable: Path.join([File.cwd!(), "/test/support/scripts/yt-dlp-mocks/repeater.sh"]), media_directory: Path.join([System.tmp_dir!(), "test", "media"]), metadata_directory: Path.join([System.tmp_dir!(), "test", "metadata"]), + extras_directory: Path.join([System.tmp_dir!(), "test", "extras"]), tmpfile_directory: Path.join([System.tmp_dir!(), "test", "tmpfiles"]), file_watcher_poll_interval: 50 diff --git a/lib/pinchflat/media/media.ex b/lib/pinchflat/media/media.ex index e1d8d7d..abe2a0d 100644 --- a/lib/pinchflat/media/media.ex +++ b/lib/pinchflat/media/media.ex @@ -66,6 +66,16 @@ defmodule Pinchflat.Media do |> Repo.all() end + @doc """ + Returns all media_items for a given source_id, without any pending/upgradeable filtering. + Used by the API to list all media items associated with a source. + + Returns [%MediaItem{}, ...]. + """ + def list_media_items_for_source(source_id) do + Repo.all(from mi in MediaItem, where: mi.source_id == ^source_id) + end + @doc """ For a given media_item, tells you if it is pending download. This is defined as the media_item satisfying `MediaQuery.pending` which you should really check out. @@ -110,6 +120,7 @@ defmodule Pinchflat.Media do Returns %MediaItem{}. Raises `Ecto.NoResultsError` if the Media item does not exist. """ def get_media_item!(id), do: Repo.get!(MediaItem, id) + def get_media_item(id), do: Repo.get(MediaItem, id) @doc """ Creates a media_item. diff --git a/lib/pinchflat/sources/sources.ex b/lib/pinchflat/sources/sources.ex index edd37f1..815cdb6 100644 --- a/lib/pinchflat/sources/sources.ex +++ b/lib/pinchflat/sources/sources.ex @@ -67,6 +67,7 @@ defmodule Pinchflat.Sources do Returns %Source{}. Raises `Ecto.NoResultsError` if the Source does not exist. """ def get_source!(id), do: Repo.get!(Source, id) + def get_source(id), do: Repo.get(Source, id) @doc """ Creates a source. May attempt to pull additional source details from the diff --git a/lib/pinchflat/tasks/task.ex b/lib/pinchflat/tasks/task.ex index d498651..bb0d99a 100644 --- a/lib/pinchflat/tasks/task.ex +++ b/lib/pinchflat/tasks/task.ex @@ -6,7 +6,6 @@ defmodule Pinchflat.Tasks.Task do use Ecto.Schema import Ecto.Changeset - alias Pinchflat.Repo alias Pinchflat.Media.MediaItem alias Pinchflat.Sources.Source @@ -34,12 +33,22 @@ end defimpl Jason.Encoder, for: Pinchflat.Tasks.Task do def encode(value, opts) do value - |> Repo.preload(:job) |> Map.drop(Pinchflat.Tasks.Task.json_excluded_fields()) |> Map.update!(:job, fn nil -> nil + %Ecto.Association.NotLoaded{} -> nil job -> Map.drop(job, [:__meta__, :__struct__, :args, :meta, :worker, :queue]) end) + |> Map.update!(:source, fn + nil -> nil + %Ecto.Association.NotLoaded{} -> nil + source -> source + end) + |> Map.update!(:media_item, fn + nil -> nil + %Ecto.Association.NotLoaded{} -> nil + item -> item + end) |> Jason.Encode.map(opts) end end diff --git a/lib/pinchflat_web/controllers/api/v1/api_media_item_controller.ex b/lib/pinchflat_web/controllers/api/v1/api_media_item_controller.ex index 11e5caa..0d8fa27 100644 --- a/lib/pinchflat_web/controllers/api/v1/api_media_item_controller.ex +++ b/lib/pinchflat_web/controllers/api/v1/api_media_item_controller.ex @@ -5,6 +5,8 @@ defmodule PinchflatWeb.Api.V1.ApiMediaItemController do use PinchflatWeb, :controller + action_fallback PinchflatWeb.Api.V1.FallbackController + alias Pinchflat.Repo alias Pinchflat.Media alias Pinchflat.Media.MediaItem @@ -14,8 +16,7 @@ defmodule PinchflatWeb.Api.V1.ApiMediaItemController do media_items = case params do %{"source_id" => source_id} -> - source = Pinchflat.Sources.get_source!(source_id) - Media.list_pending_media_items_for(source) + Media.list_media_items_for_source(source_id) _ -> Media.list_media_items() @@ -26,12 +27,14 @@ defmodule PinchflatWeb.Api.V1.ApiMediaItemController do end def show(conn, %{"id" => id}) do - media_item = - id - |> Media.get_media_item!() - |> Repo.preload([:source, tasks: [:job]]) + case Media.get_media_item(id) do + nil -> + {:error, :not_found} - json(conn, %{data: media_item}) + media_item -> + media_item = Repo.preload(media_item, [:source, tasks: [:job]]) + json(conn, %{data: media_item}) + end end def search(conn, %{"q" => query}) do diff --git a/lib/pinchflat_web/controllers/api/v1/api_source_controller.ex b/lib/pinchflat_web/controllers/api/v1/api_source_controller.ex index b55cfeb..26fed37 100644 --- a/lib/pinchflat_web/controllers/api/v1/api_source_controller.ex +++ b/lib/pinchflat_web/controllers/api/v1/api_source_controller.ex @@ -5,6 +5,8 @@ defmodule PinchflatWeb.Api.V1.ApiSourceController do use PinchflatWeb, :controller + action_fallback PinchflatWeb.Api.V1.FallbackController + alias Pinchflat.Repo alias Pinchflat.Sources alias Pinchflat.Sources.Source @@ -21,14 +23,20 @@ defmodule PinchflatWeb.Api.V1.ApiSourceController do end def show(conn, %{"id" => id}) do - source = Sources.get_source!(id) |> Repo.preload([:media_profile, :media_items]) + case Sources.get_source(id) do + nil -> + {:error, :not_found} - pending_tasks = - source - |> Tasks.list_tasks_for(nil, [:executing, :available, :scheduled, :retryable]) - |> Repo.preload(:job) + source -> + source = Repo.preload(source, [:media_profile, :media_items]) - json(conn, %{data: source, pending_tasks: pending_tasks}) + pending_tasks = + source + |> Tasks.list_tasks_for(nil, [:executing, :available, :scheduled, :retryable]) + |> Repo.preload(:job) + + json(conn, %{data: source, pending_tasks: pending_tasks}) + end end def create(conn, %{"source" => source_params}) do diff --git a/lib/pinchflat_web/controllers/api/v1/api_task_controller.ex b/lib/pinchflat_web/controllers/api/v1/api_task_controller.ex index 06d1fe3..7c9b1eb 100644 --- a/lib/pinchflat_web/controllers/api/v1/api_task_controller.ex +++ b/lib/pinchflat_web/controllers/api/v1/api_task_controller.ex @@ -5,6 +5,8 @@ defmodule PinchflatWeb.Api.V1.ApiTaskController do use PinchflatWeb, :controller + action_fallback PinchflatWeb.Api.V1.FallbackController + alias Pinchflat.Repo alias Pinchflat.Tasks diff --git a/lib/pinchflat_web/controllers/api/v1/fallback_controller.ex b/lib/pinchflat_web/controllers/api/v1/fallback_controller.ex new file mode 100644 index 0000000..e8e5f41 --- /dev/null +++ b/lib/pinchflat_web/controllers/api/v1/fallback_controller.ex @@ -0,0 +1,18 @@ +defmodule PinchflatWeb.Api.V1.FallbackController do + @moduledoc """ + Translates controller failures into JSON error responses. + """ + use PinchflatWeb, :controller + + def call(conn, {:error, :not_found}) do + conn + |> put_status(:not_found) + |> json(%{errors: %{detail: "Not found"}}) + end + + def call(conn, %Ecto.NoResultsError{} = _error) do + conn + |> put_status(:not_found) + |> json(%{errors: %{detail: "Not found"}}) + end +end \ No newline at end of file