diff --git a/IMPLEMENTATION_COLLECTION_FIX.md b/IMPLEMENTATION_COLLECTION_FIX.md index 04d72e5..bccd13b 100644 --- a/IMPLEMENTATION_COLLECTION_FIX.md +++ b/IMPLEMENTATION_COLLECTION_FIX.md @@ -2,7 +2,12 @@ ## Overview -Fix the broken `/collections/:id` page (inline JS bug) and add library_id support to the search API. This uses a hybrid approach: minimal TypeScript for client-only features, HTMX-like patterns for CRUD operations. +Fix the broken `/collections/:id` page (inline JS bug) and add library_id support to the search API. This is a **full-stack task** involving: +- Backend: Search API enhancement, WebSocket permission fix +- Database: SQL query modification +- Frontend: TypeScript conversion, UI improvements + +Uses hybrid approach: minimal TypeScript for client-only features, HTMX-like patterns for CRUD operations, WebSocket for real-time sync. --- @@ -10,20 +15,221 @@ Fix the broken `/collections/:id` page (inline JS bug) and add library_id suppor | File | Changes | |------|---------| -| `templates/collections.templ` | Remove inline JS, add data attributes, add toggle UI | -| `web/src/collections.ts` | Add minimal TypeScript functions (~60 lines) | +| `internal/sync/websocket.go` | Add user-scoped broadcast method | +| `internal/handlers/collections.go` | Use user-scoped broadcasts | +| `internal/database/queries.sql` | Add library_id filter to search | | `internal/handlers/media.go` | Add library_id param to search handler | -| `internal/router/frontend.go` | (No changes needed - existing modals work) | +| `templates/collections.templ` | Remove inline JS, add data attributes, add toggle UI | +| `web/src/collections.ts` | Add TypeScript with WebSocket support | +| `internal/router/frontend.go` | Pass libraryID to template | +| `cmd/server/tests/collections_test.go` | Add integration tests | +| `docs/developer/api/search.md` | Document library_id parameter | +| `bruno/collections/search-with-library-filter.yml` | API test for new parameter | --- -## Step 1: Update Search API to Accept library_id +## Git Commit Strategy + +Use multiple logical commits: + +1. **feat(websocket): Add user-scoped broadcasting** + - `internal/sync/websocket.go` + - `internal/handlers/collections.go` + - Tests for user-scoped broadcasts + +2. **feat(database): Add library_id filter to search queries** + - `internal/database/queries.sql` + - Regenerate `internal/database/queries.sql.go` + - Verify params struct updated + +3. **feat(api): Add library_id parameter to search endpoint** + - `internal/handlers/media.go` + - Integration tests for library filtering + +4. **refactor(templates): Remove inline JS from collection detail** + - `templates/collections.templ` + - `internal/router/frontend.go` + - `web/src/collections.ts` + +5. **feat(frontend): Add library filter toggle UI** + - `templates/collections.templ` + - `web/src/collections.ts` + +6. **docs(api): Document library_id search parameter** + - `docs/developer/api/search.md` + - `bruno/collections/search-with-library-filter.yml` + +--- + +## Step 1: Add User-Scoped Broadcasting (WebSocket Fix) + +### File: `internal/sync/websocket.go` + +**Add after line 94** (after existing `Broadcast` method): + +```go +// BroadcastToUser sends a message to all connections for a specific user +func (m *ConnectionManager) BroadcastToUser(userID string, msg BroadcastMessage) { + m.mu.RLock() + defer m.mu.RUnlock() + + for _, conn := range m.connections { + if conn.UserID == userID { + select { + case conn.Send <- msg: + default: + // Channel full, skip this connection + log.Printf("WebSocket: Channel full for %s, skipping broadcast", conn.DeviceName) + } + } + } +} +``` + +**Why**: Current `Broadcast()` sends to ALL users (security issue). User-scoped broadcasts ensure collection updates only go to that user's devices. + +--- + +## Step 2: Update Collections Handler to Use User-Scoped Broadcasts + +### File: `internal/handlers/collections.go` + +**Find all instances of** `h.connManager.Broadcast` **and replace with user-scoped**: + +**Line 333** (AddBooks): +```go +if addedCount > 0 && h.connManager != nil { + h.connManager.BroadcastToUser(userUUID.String(), wsync.BroadcastMessage{ + Type: "collection_updated", + Timestamp: time.Now().Format(time.RFC3339), + Data: map[string]interface{}{ + "collection_id": collectionID.String(), + "action": "books_added", + "book_ids": addedBookIDs, + "count": addedCount, + }, + }) +} +``` + +**Line 401** (RemoveBook): +```go +if h.connManager != nil { + h.connManager.BroadcastToUser(userUUID.String(), wsync.BroadcastMessage{ + Type: "collection_updated", + Timestamp: time.Now().Format(time.RFC3339), + Data: map[string]interface{}{ + "collection_id": collectionID.String(), + "action": "book_removed", + "book_id": bookID.String(), + }, + }) +} +``` + +**Line 831** (BulkRemoveBooks): +```go +if removedCount > 0 && h.connManager != nil { + h.connManager.BroadcastToUser(userUUID.String(), wsync.BroadcastMessage{ + Type: "collection_updated", + Timestamp: time.Now().Format(time.RFC3339), + Data: map[string]interface{}{ + "collection_id": collectionID.String(), + "action": "books_bulk_removed", + "book_ids": removedBookIDs, + "count": removedCount, + }, + }) +} +``` + +**Why**: Ensures collection updates only broadcast to the user who made the change, not all connected users. + +--- + +## Step 3: Add library_id Filter to SQL Query + +### File: `internal/database/queries.sql` + +**Find the `SearchMediaItems` query** (around line 393): + +**Current:** +```sql +-- name: SearchMediaItems :many +SELECT mi.*, l.name as library_name, lt.name as library_type_name +FROM media_items mi +JOIN libraries l ON mi.library_id = l.id +JOIN library_types lt ON l.library_type_id = lt.id +LEFT JOIN library_visibility lv ON l.id = lv.library_id AND lv.user_id = sqlc.narg('user_id') +WHERE COALESCE(lv.is_visible, true) = true + AND ( + mi.title ILIKE sqlc.narg('search_pattern') OR + mi.author ILIKE sqlc.narg('search_pattern') OR + mi.series ILIKE sqlc.narg('search_pattern') OR + sqlc.narg('search_pattern') = ANY(mi.tags_search) OR + sqlc.narg('search_pattern') = ANY(mi.contributors_search) + ) +ORDER BY ... +``` + +**Add library_id parameter and filter**: + +```sql +-- name: SearchMediaItems :many +SELECT mi.*, l.name as library_name, lt.name as library_type_name +FROM media_items mi +JOIN libraries l ON mi.library_id = l.id +JOIN library_types lt ON l.library_type_id = lt.id +LEFT JOIN library_visibility lv ON l.id = lv.library_id AND lv.user_id = sqlc.narg('user_id') +WHERE COALESCE(lv.is_visible, true) = true + AND ($5::uuid IS NULL OR mi.library_id = $5::uuid) -- library_id filter + AND ( + mi.title ILIKE sqlc.narg('search_pattern') OR + mi.author ILIKE sqlc.narg('search_pattern') OR + mi.series ILIKE sqlc.narg('search_pattern') OR + sqlc.narg('search_pattern') = ANY(mi.tags_search) OR + sqlc.narg('search_pattern') = ANY(mi.contributors_search) + ) +ORDER BY ... +``` + +**Also update `SearchMediaItemsFuzzy`** (around line 418) with the same filter: +```sql +WHERE COALESCE(lv.is_visible, true) = true + AND ($5::uuid IS NULL OR mi.library_id = $5::uuid) -- library_id filter + AND ( + word_similarity(sqlc.narg('search_query'), mi.title) > 0.3 OR + ... + ) +``` + +**Regenerate database code:** +```bash +cd internal/database +sqlc generate +``` + +**Verify** `internal/database/queries.sql.go` now has: +```go +type SearchMediaItemsParams struct { + UserID pgtype.UUID `db:"user_id" json:"user_id"` + SearchPattern pgtype.Text `db:"search_pattern" json:"search_pattern"` + Offset pgtype.Int4 `db:"offset" json:"offset"` + Limit pgtype.Int4 `db:"limit" json:"limit"` + LibraryID pgtype.UUID `db:"library_id" json:"library_id"` // NEW +} +``` + +--- + +## Step 4: Update Search API to Accept library_id ### File: `internal/handlers/media.go` -**Find** (around line 1418-1442): +**Find** (around line 1418-1472): + +**Current:** ```go -// SearchMediaItems handles GET /api/media-items/search func (mh *MediaHandler) SearchMediaItems(c echo.Context) error { query := c.QueryParam("q") userID := c.Get("user_id").(string) @@ -48,11 +254,26 @@ func (mh *MediaHandler) SearchMediaItems(c echo.Context) error { Limit: pgtype.Int4{Int32: limit, Valid: true}, Offset: pgtype.Int4{Int32: offset, Valid: true}, }) + + if err != nil && err != pgx.ErrNoRows { + return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()}) + } + + if len(partialResults) > 0 { + return c.JSON(http.StatusOK, partialResults) + } + + fuzzyResults, err := mh.db.SearchMediaItemsFuzzy(c.Request().Context(), database.SearchMediaItemsFuzzyParams{ + SearchQuery: pgtype.Text{String: query, Valid: true}, + UserID: pgtype.UUID{Bytes: userUUID, Valid: true}, + Limit: pgtype.Int4{Int32: limit, Valid: true}, + Offset: pgtype.Int4{Int32: offset, Valid: true}, + }) + // ... rest of function ``` -**Replace with**: +**Replace with:** ```go -// SearchMediaItems handles GET /api/media-items/search func (mh *MediaHandler) SearchMediaItems(c echo.Context) error { query := c.QueryParam("q") userID := c.Get("user_id").(string) @@ -68,13 +289,13 @@ func (mh *MediaHandler) SearchMediaItems(c echo.Context) error { } // Validate library_id if provided - var libUUID *uuid.UUID + var libUUID pgtype.UUID if libraryID != "" { lib, err := uuid.Parse(libraryID) if err != nil { return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid library_id"}) } - libUUID = &lib + libUUID = pgtype.UUID{Bytes: lib, Valid: true} } limit := int32(50) @@ -83,81 +304,62 @@ func (mh *MediaHandler) SearchMediaItems(c echo.Context) error { searchPattern := "%" + query + "%" // Build params - conditionally add library_id filter - params := database.SearchMediaItemsParams{ + partialParams := database.SearchMediaItemsParams{ SearchPattern: pgtype.Text{String: searchPattern, Valid: true}, UserID: pgtype.UUID{Bytes: userUUID, Valid: true}, Limit: pgtype.Int4{Int32: limit, Valid: true}, Offset: pgtype.Int4{Int32: offset, Valid: true}, + LibraryID: libUUID, // May be invalid (empty) } - // If library_id provided, add to query params - if libUUID != nil { - params.LibraryID = pgtype.UUID{Bytes: *libUUID, Valid: true} + partialResults, err := mh.db.SearchMediaItems(c.Request().Context(), partialParams) + + if err != nil && err != pgx.ErrNoRows { + return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()}) } - partialResults, err := mh.db.SearchMediaItems(c.Request().Context(), params) + if len(partialResults) > 0 { + return c.JSON(http.StatusOK, partialResults) + } + + // Fuzzy search also gets library_id filter + fuzzyParams := database.SearchMediaItemsFuzzyParams{ + SearchQuery: pgtype.Text{String: query, Valid: true}, + UserID: pgtype.UUID{Bytes: userUUID, Valid: true}, + Limit: pgtype.Int4{Int32: limit, Valid: true}, + Offset: pgtype.Int4{Int32: offset, Valid: true}, + LibraryID: libUUID, // May be invalid (empty) + } + + fuzzyResults, err := mh.db.SearchMediaItemsFuzzy(c.Request().Context(), fuzzyParams) + + if err != nil && err != pgx.ErrNoRows { + return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()}) + } + + if len(fuzzyResults) == 0 { + return c.JSON(http.StatusNotFound, map[string]interface{}{ + "error": "no results found", + "query": query, + "results": []interface{}{}, + }) + } + + return c.JSON(http.StatusOK, fuzzyResults) +} ``` -**Note**: You need to check if `SearchMediaItemsParams` in `database.SearchMediaItemsParams` already has a `LibraryID` field. If not, you'll need to add it to the SQL query. +**Why**: Both partial and fuzzy searches respect library filter for consistent UX. --- -## Step 2: Add library_id to SQL Query (if needed) - -### File: `internal/database/queries.sql` - -**Find** the `SearchMediaItems` query: -```sql --- name: SearchMediaItems :many -SELECT ... -FROM media_items mi -WHERE ... -``` - -**Add** library_id filter (if not present): -```sql --- name: SearchMediaItems :many -SELECT mi.id, mi.user_id, mi.title, mi.author, mi.cover_image_path, mi.library_id, ... -FROM media_items mi -WHERE ... - AND ($1::uuid IS NULL OR mi.library_id = $1::uuid) -- Add this filter -``` - -**Update** the `SearchMediaItemsParams` struct in `queries.sql.go` if needed to include LibraryID. - ---- - -## Step 3: Remove Inline JS from Template +## Step 5: Remove Inline JS from Template ### File: `templates/collections.templ` -**Find** the inline script block (lines 255-520): -```go - -``` - -**Replace with** a hidden data element (add after the `` tag or where appropriate): +**Add after the `` tag** (after line 116): ```go
``` -**Note**: You need to pass `libraryID` to the template. Check the handler in `frontend.go` that renders `CollectionDetail` and add `libraryID` to the template data. +**Also update the CollectionDetail function signature** (around line 105): + +**Current:** +```go +func CollectionDetail(user User, collection CollectionData, books []handlers.BookInfo) templ.Component { +``` + +**Replace with:** +```go +func CollectionDetail(user User, collection CollectionData, books []handlers.BookInfo, libraryID string) templ.Component { +``` --- -## Step 4: Add Toggle UI to Add Books Modal +## Step 6: Add Toggle UI to Add Books Modal ### File: `templates/collections.templ` -**Find** the Add Books modal (around line 157): +**Find the Add Books modal content** (around line 228): + +**Current:** ```go - +