ML-176.1: support search in collection api controller

backlog/tasks/ml-176.1 - API-accept-search-query-parameter-on-GET-api-v1-collection.md

@@ -1,10 +1,11 @@
 ---
 id: ML-176.1
 title: "API: accept search query parameter on GET /api/v1/collection"
-status: To Do
+status: Done
-assignee: []
+assignee:
+  - pi
 created_date: "2026-05-10 13:35"
-updated_date: "2026-05-10 13:41"
+updated_date: "2026-05-10 13:49"
 labels:
   - api
   - backend
@@ -44,12 +45,12 @@ The existing FTS5 search in `Records.Search` handles the query — no new search
 
 <!-- AC:BEGIN -->
 
-- [ ] #1 GET /api/v1/collection returns all records when q is absent (existing behavior unchanged)
+- [x] #1 GET /api/v1/collection returns all records when q is absent (existing behavior unchanged)
-- [ ] #2 GET /api/v1/collection?q=beatles returns only records matching FTS5 query for 'beatles'
+- [x] #2 GET /api/v1/collection?q=beatles returns only records matching FTS5 query for 'beatles'
-- [ ] #3 GET /api/v1/collection?q= returns all records (empty string treated as no query)
+- [x] #3 GET /api/v1/collection?q= returns all records (empty string treated as no query)
-- [ ] #4 GET /api/v1/collection?q=beatles&limit=5&offset=0 respects pagination with search
+- [x] #4 GET /api/v1/collection?q=beatles&limit=5&offset=0 respects pagination with search
-- [ ] #5 Response format matches existing schema: {total, limit, offset, records} with all record fields
+- [x] #5 Response format matches existing schema: {total, limit, offset, records} with all record fields
-- [ ] #6 Controller test covers: no q param, empty q, populated q, q with no matches, q with pagination
+- [x] #6 Controller test covers: no q param, empty q, populated q, q with no matches, q with pagination
 <!-- AC:END -->
 
 ## Implementation Plan
@@ -140,3 +141,52 @@ No cost impact. No external API calls, no additional storage, no compute changes
 No documentation updates needed. The API endpoint already exists in `router.ex`. The `q` parameter is additive and optional — existing API consumers are unaffected. If project API docs exist externally, note that `GET /api/v1/collection` now accepts `?q=<search term>` as an optional parameter, but this is self-documenting via the REST pattern.
 
 <!-- SECTION:PLAN:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+
+Implementation complete. Changes:
+
+1. **Controller** (`lib/music_library_web/controllers/collection_controller.ex`):
+   - Added `query = normalize_query(params["q"])` in `index/2`
+   - Pass `query` to `search_records_count/1` and `search_records/2` instead of hardcoded `""`
+   - Added private `normalize_query/1` helper: nil → "", "" → "", otherwise passthrough
+
+2. **Tests** (`test/music_library_web/controllers/collection_controller_test.exs`):
+   - 5 new tests added:
+     - `q` param matches → returns filtered results
+     - Empty `q` string → returns all records
+     - `q` with no matches → returns `total: 0`, empty `records`
+     - `q` with pagination → respects limit/offset on filtered results
+     - Response shape validation → confirms schema is unchanged
+
+3. **Full test suite**: 968 passed (0 failures), no regressions
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+
+## Summary
+
+Extended `GET /api/v1/collection` to accept an optional `q` query parameter for FTS5 full-text search.
+
+### Changes
+
+**Controller** (`lib/music_library_web/controllers/collection_controller.ex`):
+
+- `index/2` now reads `params["q"]` and normalizes nil/empty → `""` via a new private `normalize_query/1` helper
+- Passes the query string to the existing `Collection.search_records_count/1` and `Collection.search_records/2` pipeline (previously hardcoded `""`)
+
+**Tests** (`test/music_library_web/controllers/collection_controller_test.exs`):
+
+- 5 new tests: matching query, empty query string, no-match query, pagination with search, and response shape validation
+
+### Key points
+
+- No new search logic — reuses existing `Records.Search.build_search/3` which already handled empty queries as a no-op
+- Response format unchanged — `CollectionJSON.index/1` returns the same shape
+- No route/schema/migration changes required
+- Full suite: 968 passed, 0 regressions
+<!-- SECTION:FINAL_SUMMARY:END -->

lib/music_library_web/controllers/collection_controller.ex

@@ -28,12 +28,13 @@ defmodule MusicLibraryWeb.CollectionController do
   end
 
   def index(conn, params) do
+    query = normalize_query(params["q"])
     limit = parse_int(params["limit"], 20)
     offset = parse_int(params["offset"], 0)
 
-    total = Collection.search_records_count("")
+    total = Collection.search_records_count(query)
 
-    records = Collection.search_records("", limit: limit, offset: offset)
+    records = Collection.search_records(query, limit: limit, offset: offset)
 
     render(conn, :index, total: total, limit: limit, offset: offset, records: records)
   end
@@ -48,4 +49,8 @@ defmodule MusicLibraryWeb.CollectionController do
   end
 
   defp parse_int(_, default), do: default
+
+  defp normalize_query(nil), do: ""
+  defp normalize_query(""), do: ""
+  defp normalize_query(query), do: query
 end

test/music_library_web/controllers/collection_controller_test.exs

@@ -69,6 +69,81 @@ defmodule MusicLibraryWeb.CollectionControllerTest do
                "records" => [expected_record_json(record)]
              }
     end
+
+    test "filters records when q param matches", %{conn: conn} do
+      beatles = record_with_artist("The Beatles", %{title: "Abbey Road"})
+
+      conn =
+        conn
+        |> put_req_header("authorization", "Bearer #{api_token()}")
+        |> get(~p"/api/v1/collection?q=beatles")
+
+      assert json_response(conn, 200) == %{
+               "total" => 1,
+               "limit" => 20,
+               "offset" => 0,
+               "records" => [expected_record_json(beatles)]
+             }
+    end
+
+    test "returns all records when q is empty string", %{conn: conn, record: record} do
+      conn =
+        conn
+        |> put_req_header("authorization", "Bearer #{api_token()}")
+        |> get(~p"/api/v1/collection?q=")
+
+      assert json_response(conn, 200) == %{
+               "total" => 1,
+               "limit" => 20,
+               "offset" => 0,
+               "records" => [expected_record_json(record)]
+             }
+    end
+
+    test "returns empty results when q matches nothing", %{conn: conn} do
+      conn =
+        conn
+        |> put_req_header("authorization", "Bearer #{api_token()}")
+        |> get(~p"/api/v1/collection?q=nonexistentxyz")
+
+      assert json_response(conn, 200) == %{
+               "total" => 0,
+               "limit" => 20,
+               "offset" => 0,
+               "records" => []
+             }
+    end
+
+    test "respects pagination with search query", %{conn: conn} do
+      _a = record_with_artist("The Beatles", %{title: "Abbey Road"})
+      b = record_with_artist("The Beatles", %{title: "Revolver"})
+      _c = record_with_artist("The Beatles", %{title: "Sgt Pepper"})
+
+      conn =
+        conn
+        |> put_req_header("authorization", "Bearer #{api_token()}")
+        |> get(~p"/api/v1/collection?q=beatles&limit=1&offset=1")
+
+      assert json_response(conn, 200) == %{
+               "total" => 3,
+               "limit" => 1,
+               "offset" => 1,
+               "records" => [expected_record_json(b)]
+             }
+    end
+
+    test "response shape is unchanged when searching", %{conn: conn} do
+      beatles = record_with_artist("The Beatles", %{title: "Abbey Road"})
+
+      conn =
+        conn
+        |> put_req_header("authorization", "Bearer #{api_token()}")
+        |> get(~p"/api/v1/collection?q=beatles")
+
+      response = json_response(conn, 200)
+      assert %{"total" => 1, "limit" => 20, "offset" => 0, "records" => [record_json]} = response
+      assert record_json == expected_record_json(beatles)
+    end
   end
 
   describe "GET /api/v1/collection/on_this_day" do